From https://pagure.io/fedora-infra/rpmautospec/issue/238:
FedoraReview#417 fixed the complaint about missing disttag. But using https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=2103475 as an example, with fedora-review-0.8.0-1.fc35.noarch:
Generic: [!]: Spec file according to URL is the same as in SRPM. Note: Spec file as given by url is not the same as in SRPM (see attached diff). See: (this test has no URL)
Diff spec file in url and in SRPM --------------------------------- --- /var/tmp/2103477-rust-normpath/srpm/rust-normpath.spec 2022-08-07 10:19:58.561126206 +0000 +++ /var/tmp/2103477-rust-normpath/srpm-unpacked/rust-normpath.spec 2022-07-04 07:58:47.0 00000000 +0000 @@ -1,2 +1,11 @@ +## START: Set by rpmautospec +## (rpmautospec version 0.2.6) +%define autorelease(e:s:pb:) %{?-p:0.}%{lua: + release_number = 2; + base_release_number = tonumber(rpm.expand("%{?-b*}%{!?-b:1}")); + print(release_number + base_release_number - 1); +}%{?-e:.%{-e*}}%{?-s:.%{-s*}}%{?dist} +## END: Set by rpmautospec + # Generated by rust2rpm 21 %bcond_without check @@ -100,3 +109,7 @@ %changelog -%autochangelog +* Mon Jul 04 2022 Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> 0.3.2-2 +- Add Provides:bundled() for the bundled code + +* Sun Jul 03 2022 Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> 0.3.2-1 +- First version
Essentially it warns about the processing that rpmautospec does.
This is a problem when people submit SRPM files that contain a spec file that was already mangled by rpmautospec, for example, if you run "fedpkg srpm" instead of "rpmbuild -bs *.spec".
I think fedora-review should warn in this case. Because otherwise people might just "fedpkg import" those SRPM files as well, resulting in broken packages.
(BTW, This is not a hypothetical, I've seen this happen multiple times already.)
So… of course everything is complicated… But I think that in the short term: - fedora-review should recognize this case and strip away the changelog part and the rpmautospec header and redo the comparison. The diff that it does now is very confusing to users. - fedpkg import must refuse to import rpms after rpmautospec postprocessing.
fedora-review
fedpkg import
The second item will actually resolve the issue with erroneous imports and it's not something for fedora-review to care about.
In the longer term: - IMO rpmbuild -bs *spec is something that we shouldn't advertise. There's a few reasons for this: it's a complicated command that one would normally never use in normal workflows. And I think it should be a basic tenet of the review workflow that you're doing things as close to the post-approval workflow as possible. We shouldn't require new packagers to work a new set of arcane commands just for the review process. Secondly, rpmbuild is stupid because it requires the manual setting of --define '_sourcedir .' --define '_builddir .' to behave like fedpkg. For most packages this doesn't make a difference, but for some it does (e.g. systemd), and, again, I don't like advertising a command which works most of the time but not always when we have a more modern replacement. - fedpkg import could be enhanced to work with rpmautospec packages.
rpmbuild -bs *spec
rpmbuild
--define '_sourcedir .' --define '_builddir .'
fedpkg
I don't like advertising a command which works most of the time but not always when we have a more modern replacement.
It's not a "modern replacement" though. It's a glorified wrapper around the exact same rpmbuild -bs command that sets _sourcedir and _builddir like you described. On the other hand, fedpkg also suffers from the "works fine in most cases, fails in subtle or spectacular ways in others. Does that mean we shouldn't advertise either tool? :)
rpmbuild -bs
_sourcedir
_builddir
This isn't the best place to discuss the future of our packaging tools. Let's talk about the reported issue. I hope you don't disagree with the fixes described above.
This is a problem when people submit SRPM files that contain a spec file that was already mangled by rpmautospec, for example, if you run "fedpkg srpm" instead of "rpmbuild -bs *.spec". I think fedora-review should warn in this case. Because otherwise people might just "fedpkg import" those SRPM files as well, resulting in broken packages.
Woah, is this documented somewhere? I assumed those to be equivalent, too.
I think in general we could be more transparent about what fedpkg command really does (in terms of git or rpmbuild commands and such), and that's a different ticket, of course.
fedpkg command
But documenting that pitfall for new packages would aleviate the reported problem already. One way to "document" this could be to teach FedoraReview to still warn about the diff but report a possible cause (or even detect rpmautospec processing and suggest a solution).
This makes fedpkg import refuse such rpms: https://pagure.io/rpkg/pull-request/628.
Log in to comment on this ticket.