#451 fedora-review warns about rpmautospec processing and .spec being different
Opened 2 years ago by zbyszek. Modified 2 years ago

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.

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.

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? :)

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.

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).

Log in to comment on this ticket.

Metadata