Created attachment 577456 [details] lib3mf-1.8.1.ebuild I set out to knock together an ebuild for the new OpenSCAD release (not bothering to see if one was already in Portage first), and in the process of doing that, found that it needed lib3mf...so I put together an ebuild for that. It's in my overlay (https://gitlab.com/salfter/portage), and will be uploaded here shortly.
Hi Scott, in reply to bug #686044, where you're not on the CC list, I'm going to tell you here. Good work, that you put together an ebuild for lib3mf. The other two packages mentioned in the openscad ebuild don't actually need an ebuild, there's one in the tree for both AFAIK, they just need to be supported by the ebuild. I'm already at it.
Might I ask a few questions on the ebuild? - Why do you use a git commit instead of the released version? They have a v1.8.1 archive on their github release page. - Have you tried whether it will build if you use the dev-cpp/gtest instead of pulling google test framework from github?
(In reply to Bernd from comment #2) > - Why do you use a git commit instead of the released version? They have a > v1.8.1 archive on their github release page[?] It's cribbed from another ebuild that's set up that way to track a project with no releases. I suspect a release tarball could be substituted. > - Have you tried whether it will build if you use the dev-cpp/gtest instead > of pulling google test framework from github? googletest is a submodule in the lib3mf Git repo. Building without it in-tree causes a configuration error...or it did until I patched out references to the in-tree copy. Revised files will be uploaded next.
Created attachment 577522 [details] revised ebuild
Created attachment 577524 [details, diff] system gtest patch
Nice Scott, have you tested the ebuild? Would you like to proxy maintain this package? And, do you have any ideas how the ebuild could be further enhanced?
(In reply to Bernd from comment #6) > Nice Scott, have you tested the ebuild? Just ran a few tests from within OpenSCAD. It really doesn't like non-manifold objects; you'll get a zero-byte export. (I suppose this encourages you to fix your models. :-) ) As long as you don't get non-manifold warnings, it looks like it exports properly; I exported a part for one of my printers from OpenSCAD on Gentoo to 3MF and previewed it in Windows 10's 3D viewer. > Would you like to proxy maintain this package? And, do you have any ideas > how the ebuild could be further enhanced? What's involved in being a proxy maintainer? There are several ebuilds in my overlay for which I monitor the project RSS feeds and update the ebuilds when new releases are available. (I just added lib3mf's GitHub repo to my feed reader.) If this gets packages out of my overlay and into the main Portage tree, that'd be a good thing. As for enhancements, I've not dug too deeply yet, but it looks like lib3mf uses bundled versions of zlib and libzip. The top-level CMakeLists.txt suggests that passing a couple of options to cmake might force it to use the system-provided libraries instead.
Created attachment 577658 [details] revised ebuild...uses system zlib & libzip
(In reply to Scott Alfter from comment #7) > > Would you like to proxy maintain this package? And, do you have any ideas > > how the ebuild could be further enhanced? > > What's involved in being a proxy maintainer? There are several ebuilds in > my overlay for which I monitor the project RSS feeds and update the ebuilds > when new releases are available. (I just added lib3mf's GitHub repo to my > feed reader.) If this gets packages out of my overlay and into the main > Portage tree, that'd be a good thing. > Most basically that is what it's about. You will be responsible for the ebuild in the first place. Bugs reported in b.g.o will be directed to you. If you have some git skills, some knowledge on one or more build systems and possibly basic coding skills to be able to patch a build yourself, you IMO have good basics to get you started. If you like this idea, take a look at https://wiki.gentoo.org/wiki/Project:Proxy_Maintainers and the resources linked there. This will give you an idea what will be expected from you if you take the step. > As for enhancements, I've not dug too deeply yet, but it looks like lib3mf > uses bundled versions of zlib and libzip. The top-level CMakeLists.txt > suggests that passing a couple of options to cmake might force it to use the > system-provided libraries instead. Yes, good watch. But if an ebuild uses system provided packages, it needs to ensure the packages are pulled in if they are not present. Runtime dependencies go into RDEPEND, build time dependencies in DEPEND and BDEPEND. Your ebuild works on your machine, because zip and libzip are widely used and probably installed on many machines. But if you would try it on a freshly installed gentoo it will most likely fail, due to missing libzip and possibly missing zip. The $(usex test...) line will also not work as expected. You can install app-doc/pms or take a lookt at https://wiki.gentoo.org/wiki/Package_Manager_Specification for the syntax of usex. Your current construct will still use bundled zip and libzip, if USE="test" is used. The options for libzip and zip are not dependent on whether test USE flag is set or not. Try to use the latest EAPI if possible, which currently is EAPI=7. Using app-portage/repoman can help you a lot with basic QA checks and writing good quality ebuilds, as expected by the devs.
Created attachment 577716 [details] more fixes...test USE flag should be properly handled
Scott, the -DLIB3MF_TEST looks good, but leave the LIBZIP and ZLIB defines both at OFF. We always want to use the system provided libraries, no matter if test USE flag is set or not. And consider sorting the lines in the mycmakeargs array strictly alphabetical. The devs who review the PR usually expect it like this, and it's easier to find a specific line in more complex ebuilds. In this case it's just swapping the LIBZIP and ZLIB lines. Have you tested the building with USE="test". On my side it failed the other day, using dev-cpp/gtest, though I need to take another test to verify and take a look if it can be fixed easily. You may consider removing the test USE flag at all for now, if it's not working, or add a RESTRICT="test" line. If you want it to stay and src_test is working, you need to add a line RESTRICT="!test? ( test )" somewhere after IUSE. This will ensure that src_test is only run, if FEATURES="test" and USE="test" are set.
(In reply to Bernd from comment #11) > Scott, the -DLIB3MF_TEST looks good, but leave the LIBZIP and ZLIB defines > both at OFF. We always want to use the system provided libraries, no matter > if test USE flag is set or not. I got link errors with the test code if the bundled libzip and zlib were disabled. I suspect it's missing -lzip and -lz (?) somewhere in the build system...not entirely sure where to look for that as I'm not too familiar with cmake. > And consider sorting the lines in the > mycmakeargs array strictly alphabetical. The devs who review the PR usually > expect it like this, and it's easier to find a specific line in more complex > ebuilds. In this case it's just swapping the LIBZIP and ZLIB lines. That'd be easy enough to fix. > Have you tested the building with USE="test". On my side it failed the other > day, using dev-cpp/gtest, though I need to take another test to verify and > take a look if it can be fixed easily. > You may consider removing the test USE flag at all for now... See the above...if I can't get test linking straightened out, I might end up doing this. I'll see if I can get this stuff figured out this evening. If not, it'll probably be Monday before I can look at it...good thing Monday's a holiday. :)
(In reply to Scott Alfter from comment #12) > (In reply to Bernd from comment #11) > > Scott, the -DLIB3MF_TEST looks good, but leave the LIBZIP and ZLIB defines > > both at OFF. We always want to use the system provided libraries, no matter > > if test USE flag is set or not. > > I got link errors with the test code if the bundled libzip and zlib were > disabled. I suspect it's missing -lzip and -lz (?) somewhere in the build > system...not entirely sure where to look for that as I'm not too familiar > with cmake. > I too got these. They're because upstream didn't link against those two libs, which they actually should if the included versions are not used. If you guard the linker command according to whether the included or external libs are used and add both libs in the latter case, you can solve this issue. It pops up a second time on the CPP unittest and can be solved alike. If you need help with this, don't hesitate to ping me. > I'll see if I can get this stuff figured out this evening. If not, it'll > probably be Monday before I can look at it...good thing Monday's a holiday. > :) No need to hurry! Take your time, to understand what's going on. Some ideas on fine-tuning the ebuild. The dependencies are usually also to be sorted alphabetically. And think again about your dependencies. You currently have all three deps in DEPEND. Is this correct? The package has some documentation, of which the README.md gets installed by default. You might consider installing the CONTRIBUTING.md and the PDF file as well. Search for the DOCS tag in the PMS for how this can be done.
Created attachment 577884 [details] updated ebuild
Created attachment 577886 [details, diff] system libs patch (gtest, zlib, libzip)
Created attachment 577890 [details] possibly the last revision needed?
(In reply to Bernd from comment #13) > (In reply to Scott Alfter from comment #12) > > (In reply to Bernd from comment #11) > > > Scott, the -DLIB3MF_TEST looks good, but leave the LIBZIP and ZLIB defines > > > both at OFF... > > > > I got link errors with the test code if the bundled libzip and zlib were > > disabled... > > I too got these. They're because upstream didn't link against those two > libs, which they actually should if the included versions are not used. If > you guard the linker command according to whether the included or external > libs are used and add both libs in the latter case, you can solve this issue. > > It pops up a second time on the CPP unittest and can be solved alike. Additional patches to the cmake build files have the unit tests building against the system-provided zlib and libzip. This allows them to be used all the time, so the only remaining conditional is whether to build the tests. I've built it with test both enabled and disabled, each time without error. > Some ideas on fine-tuning the ebuild. The dependencies are usually also to > be sorted alphabetically. And think again about your dependencies. You > currently have all three deps in DEPEND. Is this correct? The tests build against libraries and headers provided by the dependencies, so if I'm reading this right: https://flameeyes.blog/2008/05/15/when-you-should-use-rdepend-and-when-you-should-use-depend/ they belong in DEPEND, not RDEPEND. Since they are only pulled in if built with USE=test, though, perhaps they should be conditional. > The package has some documentation, of which the README.md gets installed by > default. You might consider installing the CONTRIBUTING.md and the PDF file > as well. Search for the DOCS tag in the PMS for how this can be done. I think I might have everything sorted out in the latest attachments.
Think about the following: If the included zlib and libzip are used, they compile the files of those sub-packages and link them within the library (see Source/CMakeLists.txt). This probably means they're needed at runtime. If this is true, two implications come out of this. First, libzip and zlib need to go into RDEPEND, and second in your patch you have to change the link command of the library instead of the test executables. In my tests, the C test exe did link without issues, after linking the library with libz and libzip, but the C++ test exe still needs patching, for some reasons I haven't looked into. You could also guard the link command with something like if (USE_INCLUDED_LIBZIP AND USE_INCLUDED_ZLIB) original target_link_libraries else() target_link_libraries( add z and zip ) endif() Such a patch could be reported upstream, as they seem to have missed this in their makefile. You also don't need the src_install function. It's not wrong, but it's not really needed. You can put the three documents in a DOCS=() bash array somewhere before you declare the functions. This makes the ebuild cleaner and easier to read.
Ping. Are you still interested in working at this?
The bug has been closed via the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=d63003e9ac333d54b8cb4b80d0550e2de0016468 commit d63003e9ac333d54b8cb4b80d0550e2de0016468 Author: Bernd Waibel <waebbl@gmail.com> AuthorDate: 2019-05-24 07:04:08 +0000 Commit: Andreas Sturmlechner <asturm@gentoo.org> CommitDate: 2020-04-11 14:17:26 +0000 media-libs/lib3mf: new package Thanks to Scott Alfter who initially wrote the ebuild. Reported-by: Scott Alfter <scott@alfter.us> Closes: https://bugs.gentoo.org/686476 Package-Manager: Portage-2.3.96-r1, Repoman-2.3.22 Signed-off-by: Bernd Waibel <waebbl@gmail.com> Closes: https://github.com/gentoo/gentoo/pull/15269 Closes: https://github.com/gentoo/gentoo/pull/12097 Signed-off-by: Andreas Sturmlechner <asturm@gentoo.org> media-libs/lib3mf/Manifest | 1 + ...ntoo-specific-avoid-pre-stripping-library.patch | 27 ++++++++++ ...-1.8.1-0002-Add-library-link-dependencies.patch | 59 ++++++++++++++++++++++ ....8.1-0003-Change-installation-include-dir.patch | 44 ++++++++++++++++ ...4-Gentoo-specific-Remove-gtest-source-dir.patch | 35 +++++++++++++ media-libs/lib3mf/lib3mf-1.8.1.ebuild | 48 ++++++++++++++++++ media-libs/lib3mf/metadata.xml | 21 ++++++++ 7 files changed, 235 insertions(+)
