https://bugzilla.redhat.com/show_bug.cgi?id=2120418 --- Comment #4 from nsella@xxxxxxxxxx --- Hi Kalev, thanks for your answer. Here are my thoughts. I mostly agree on everything with you now. Only the BuildRequires bugs me a little. See below. > There are just no man pages upstream :) Someone needs to write them first before we can package them up. Right. > Looks like rpmlint needs updating here, because we have new guidelines in Fedora that went live a few weeks ago that mandate the use of SPDX license IDs for new packages. See https://lists.fedoraproject.org/archives/list/devel@xxxxxxxxxxxxxxxxxxxxxxx/thread/6GKXXE7AMBB76IBRMCTU3L57GLCCZL55/#5M6UBNLEX4LHITP74TAIDAG3DDYJL2HR Didn't know about it. I was just surprised by the warning and wanted to bring this up. I guess licenses are OK then. > I had the same question which name to prefer, so I reached out to the upstream author and discussed this before submitting the package and he suggested d-spy. In fact, in response to this the upstream tarball (or rather, the next upcoming tarball) has been renamed to d-spy as well, see https://gitlab.gnome.org/GNOME/d-spy/-/commit/b0765bc7557ffbbf0bf9b6741b393adc2778cb06 I see, that is great. > No, I don't think this is right. Upstream is licensing the shared library and the app differently, so that the app is under the GPL license (%license COPYING is included in the subpackage that ships the main app), but the shared library is under LGPL (%license COPYING.lgplv3 for the -libs subpackage). It's deliberately different licenses for different subpackages and it's split along the licensing split so that each subpackage has a different license. I see your point, it is my mistake on interpreting the licenses then. > Looks like I've gotten the -devel subpackage license wrong though, let me fix that (it should be the same as -libs). Great, thank you. > No, first this is upstream decision how to name it. I don't think we as a downstream should start changing the desktop or appstream IDs. OK, I agree on this and just wanted to point out the documentation. I said it is a gray zone specifically for the strong *must* that is written in the packaging guidelines. I was sure that you/upstream had a good answer for it, which, in fact, you had. Thanks for the clarification. > I find it clearer when we require the specific tool that we use in the spec file, which makes it self-documenting. Otherwise we'd have to have something like: > > # for desktop-file-validate in %check > BuildRequires: desktop-file-utils > > But I agree that writing /usr/bin in there is a bit hairy and it's bothering me as well. So I'm not super convinced which way to prefer :) I would still prefer not to have /usr/bin in BuildRequires. I counted how many spec files are using the package or /usr/bin. $ grep -r "/usr/bin/desktop-file-validate" | wc -l 38 $ grep -r "desktop-file-utils" | wc -l 2176 $ grep -r "/usr/bin/appstream-util" | wc -l 78 $ grep -r "libappstream-glib" | wc -l 929 Not saying this is an argument on how to do things the right way, but assuming packagers are doing mostly the right thing, the numbers would point me in the direction. > desktop-file-install is a tool that both installs and validates and allows editing a desktop file when installing it. In this case, it's upstream build system that installs it so we only need to validate it. OK, got it. > In this case the packaging is a bit more complicated because both the GUI app and shared libraries come from the same source package. The GUI app is in the d-spy binary package and the libraries are in the d-spy-libs binary package. > > When the packaging guidelines say that -devel should require the base package using a fully versioned dependency, what they really mean is that the -devel package can't solely rely on automatic soname deps to pull in the base library package, but instead it needs to tighten the deps by using a fully versioned dependency. So in this case, -devel provides headers and other bits that are needed for linking with the libraries in -libs, so we need to make sure that -devel has a fully versioned dependency on -libs (which is does). OK, I got a good explanation about it, looks good then. > I managed to disagree with almost all the suggestions but I really appreciate the time you took to carefully think about this package! Thank you! That was exactly what I wanted, :) I needed clarification fron you. > Let me just quickly update the -devel subpackage license. Great. > Spec URL: https://kalev.fedorapeople.org/d-spy.spec > SRPM URL: https://kalev.fedorapeople.org/d-spy-1.2.1-2.fc37.src.rpm > > * Thu Aug 25 2022 Kalev Lember <klember@xxxxxxxxxx> - 1.2.1-2 > - Correct -devel subpackage license to match -libs (rhbz#2120418) I will do a second check of the spec file just to be sure, but otherwise everything LGTM. Please, let me know what do you think of BuildRequires after my comment. I think the request could be approved after we agree on a solution. Thank you once again, Nicola -- 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 https://bugzilla.redhat.com/show_bug.cgi?id=2120418 _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
