https://bugzilla.redhat.com/show_bug.cgi?id=1650987 --- Comment #6 from Fabio Valentini <decathorpe@xxxxxxxxx> --- (In reply to Alain V. from comment #5) > Thanks a lot Fabio for this detailed feed-back. I will use your numbering to > answer. That was the idea. ;) I find it makes it a lot easier to structure suggestions and answers. > 1) Originally, I named it genht.spec, and developed the spec file as you > suggested (it was easier :). > But when I submitted to the main developer, he said Debian was already > packaging the lib as "libgenht1", under > https://packages.debian.org/sid/amd64/libgenht1 > and he prefers the package name to be "libgenht". > I have the genht.spec file. Who will have the last word ? Well, you have the last word, as the maintainer of this package. I also know that debian package naming has different heuristics than fedora's. Applying the heuristics from the fedora Naming Guidelines [0] would lead to "genht" name (since it's what the upstream project is called, and it's what the tarball is named). But since the upstream developer would prefer libgenht, that's usually respected, so I would stick to "libgenht". In fedora, the "upstream name" is per convention usually defined as "srcname", if it is different than %{name} (like this: "%global srcname genht"). > 2a, 2b) I should investigate and implement your proposal... You just need to add the line "%set_build_flags" as the first line after "%build". It sets all required environment variables. After doing that, you only have to check if "make" respects those variables. If not, you will have to patch the Makefile to do so. Replacing "make" with "%make_build" and "make install" with "%make_install" is also easy. > 2c) I create a .so symlink from the .spec file. The ln-s command is in > between pushd/popd directives. That's not the issue. The library's ELF metadata is incomplete, because the Makefile is missing those linker flags. I'd notify the upstream developer about that. The missing entry in the LDFLAGS would probably look like this: "-Wl,-soname,$(LIBSO)" > 2d) Again, originally I did not have this -static subpackage. But the main > developer wanted to distribute the archive lib. too. > He plans to use this library for other applications. > I think, by proper packaging the other applications, we can avoid this > static lib. Correct ? Yes. Usually the static library isn't needed. So for now I'd drop the -static sub-package. If it *really* will be needed in the future, you can always add it back. > 2e) The sole binary is a "trivial" example of how to use the lib. Not needed > in the distro. Good. Just checking :) > I am used to work with the main developer. Never, ever he would accept > meson, because it is Python. He is C89 to the bare. Well, that doesn't make things easier for you as a packager ... > 3) > You suggest to use the "genht" text iso a macro. For a "final" spec file > (close to "approved" state), I agree. I think I will simplify that. What I meant to say here was: You don't need to replace every occurrence of "genht" with a macro. For example, it doesn't really help in summaries and descriptions, but makes the .spec file less readable for a human. > About comments, I prefer to keep them, but did not realize this "section" > subtle thing. I will simplify and move them around. Alright, that's your decision. And yeah, that's probably a really subtle difference. It's good to know that the %prep, %build, %install, %check, etc. sections are just parsed as "sh" scripts, though. > 3d) Will change the comment, as I think the name "libgenht" will be kept. You're right. > 3e) > %dir %{_includedir}/%{generic_name} > is to own the directory, while > %{_includedir}/%{generic_name}/* > describes all the files in the dir. It seems not equivalent to me ? But it is equivalent. Both - %{_includedir}/%{srcname} - %{_includedir}/%{srcname}/ indicate the same thing: This package owns this directory and everything below it (although the first option makes it less clear that it's a directory). I prefer the second option. On the other hand, %dir %{_includedir}/%{srcname} indicates that this package only owns the "empty" directory, and nothing below it (which can be useful for empty directories!). That's why you have to add the second line %{_includedir}/%{srcname}/* for the package to own the actual directory contents, too. > > Thank you again for your time. > As soon as I can, I will experiment those changes and update my COPR. I'll > then post a comment here, linking to the new proposal. Sure! I hope my comments are helpful. I know that starting with packaging can be a bit intimidating, and has a steep learning curve. Don't hesitate to ask if you have more questions. [0]: https://docs.fedoraproject.org/en-US/packaging-guidelines/Naming/ -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx
