Spec URL: https://download.copr.fedorainfracloud.org/results/linkdupont/reviews/fedora-rawhide-x86_64/05603629-golang-github-zcalusic-sysinfo/golang-github-zcalusic-sysinfo.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/linkdupont/reviews/fedora-rawhide-x86_64/05603629-golang-github-zcalusic-sysinfo/golang-github-zcalusic-sysinfo-0.9.5-1.fc39.src.rpm Description: Sysinfo is a Go library providing Linux OS / kernel / hardware system information Fedora Account System Username: linkdupont Fedora review: https://download.copr.fedorainfracloud.org/results/linkdupont/reviews/fedora-rawhide-x86_64/05603629-golang-github-zcalusic-sysinfo/fedora-review/review.txt
> ## START: Set by rpmautospec > ## (rpmautospec version 0.3.5) > ## RPMAUTOSPEC: autorelease, autochangelog > %define autorelease(e:s:pb:n) %{?-p:0.}%{lua: > release_number = 1; > base_release_number = tonumber(rpm.expand("%{?-b*}%{!?-b:1}")); > print(release_number + base_release_number - 1); > }%{?-e:.%{-e*}}%{?-s:.%{-s*}}%{!?-n:%{?dist}} > ## END: Set by rpmautospec Is this block required nowadays? > %global golicenses LICENSE You're missing cpuid/LICENSE, you can try with something like: %global golicenses LICENSE cpuid-LICENSE (...) %install %gopkginstall mv cpuid/LICENSE cpuid-LICENSE (...) %license LICENSE cpuid-LICENSE > %changelog > * Tue Mar 07 2023 Link Dupont <link> - 0.9.5-1 > - Intitial package I suggest to use %autochangelog to make it easier to maintain the spec in the future, but not a hard requirement.
Wow, I didn't notice that the spec produced by a build does not contain the autochangelog bits. I did use autochangelog. Here's the spec I used to produce the srpm. https://linkdupont.fedorapeople.org/golang-github-zcalusic-sysinfo.spec
Please, upload an updated srpm created with the new spec. > golang-github-zcalusic-sysinfo.x86_64: E: summary-too-long Sysinfo is a Go library providing Linux OS / kernel / hardware system information Reduce the Summary
Spec URL: https://linkdupont.fedorapeople.org/golang-github-zcalusic-sysinfo.spec SRPM URL: https://linkdupont.fedorapeople.org/golang-github-zcalusic-sysinfo-0.9.5-2.fc39.src.rpm
Created attachment 1949195 [details] The .spec file difference from Copr build 5603636 to 5617056
Spec URL: https://linkdupont.fedorapeople.org/reviews/golang-github-zcalusic-sysinfo.spec SRPM URL: https://linkdupont.fedorapeople.org/reviews/golang-github-zcalusic-sysinfo-0.9.5-1.fc39.src.rpm
Created attachment 1949381 [details] The .spec file difference from Copr build 5617056 to 5618866
> %license LICENSE cpiud-LICENSE You've a typo in cpiud and thus fails to build. Checking upstream package, I see this spec builds and installs cmd/sysinfo binary. I understand that it's a test cmd to verify the library, but I'm not sure if it has any benefit for users. If it's not a requirement then I'll rather go with the package being a devel package only, and these parts could be removed: > (...) > %build > for cmd in cmd/* ; do > %gobuild -o %{gobuilddir}/bin/$(basename $cmd) %{goipath}/$cmd > done > (...) > install -m 0755 -vd %{buildroot}%{_bindir} > install -m 0755 -vp %{gobuilddir}/bin/* %{buildroot}%{_bindir}/ > (...) > %files > %license LICENSE cpiud-LICENSE > %doc README.md > %{_bindir}/* But this line would be required: > %global debug_package %{nil} You can check package `golang-github-cloudflare-circl` as example.
Hm. Good point. It does look like the binary is mostly a demo utility. I removed it, and fixed the typo in the %license line. Updated the spec and SRPM at the same URL: Spec URL: https://linkdupont.fedorapeople.org/reviews/golang-github-zcalusic-sysinfo.spec SRPM URL: https://linkdupont.fedorapeople.org/reviews/golang-github-zcalusic-sysinfo-0.9.5-1.fc39.src.rpm
Created attachment 1950258 [details] The .spec file difference from Copr build 5618866 to 5633033
> %files > %license LICENSE cpuid-LICENSE > %doc README.md This part is not required as there is binary installed, %gopkgfiles will take care of it. You should regenerate the srpm each time you change the spec file. Otherwise fedora-review reports `[!]: Spec file according to URL is the same as in SRPM.`.
(In reply to Mikel Olasagasti Uranga from comment #11) > > %files > > %license LICENSE cpuid-LICENSE > > %doc README.md > > This part is not required as there is binary installed, %gopkgfiles will > take care of it. Removed. > You should regenerate the srpm each time you change the spec file. Otherwise > fedora-review reports `[!]: Spec file according to URL is the same as in > SRPM.`. I thought I did. Perhaps I didn't do it this latest iteration. The spec file I'm uploading contains the %autorelease macro, while the SRPM is the one produced by the command 'fedpkg --release rawhide srpm'. Perhaps the spec file inside that SRPM contains the expanded %autochangelog and %autorelease macro contents. At any rate, here are new spec and SRPMs: Spec URL: https://linkdupont.fedorapeople.org/reviews/golang-github-zcalusic-sysinfo.spec SRPM URL: https://linkdupont.fedorapeople.org/reviews/golang-github-zcalusic-sysinfo-0.9.5-1.fc39.src.rpm I also did a new build in COPR: https://download.copr.fedorainfracloud.org/results/linkdupont/reviews/fedora-rawhide-x86_64/05641494-golang-github-zcalusic-sysinfo/ along with running a fedora-review: https://download.copr.fedorainfracloud.org/results/linkdupont/reviews/fedora-rawhide-x86_64/05641494-golang-github-zcalusic-sysinfo/fedora-review/review.txt
Created attachment 1950652 [details] The .spec file difference from Copr build 5633033 to 5641547
If I download the "SRPM URL" .src.rpm file, using link in comment #12, I still see the rpmautospec block at the top and the changelog. fedora-review also reports the issue: Diff spec file in url and in SRPM --------------------------------- --- /tmp/2176241-golang-github-zcalusic-sysinfo/srpm/golang-github-zcalusic-sysinfo.spec 2023-03-14 16:53:58.632705568 +0100 +++ /tmp/2176241-golang-github-zcalusic-sysinfo/srpm-unpacked/golang-github-zcalusic-sysinfo.spec 2023-03-14 16:34:25.000000000 +0100 @@ -1,2 +1,12 @@ +## START: Set by rpmautospec +## (rpmautospec version 0.3.5) +## RPMAUTOSPEC: autorelease, autochangelog +%define autorelease(e:s:pb:n) %{?-p:0.}%{lua: + release_number = 1; + base_release_number = tonumber(rpm.expand("%{?-b*}%{!?-b:1}")); + print(release_number + base_release_number - 1); +}%{?-e:.%{-e*}}%{?-s:.%{-s*}}%{!?-n:%{?dist}} +## END: Set by rpmautospec + # Generated by go2rpm 1.9.0 %bcond_without check @@ -50,3 +60,4 @@ %changelog -%autochangelog +* Tue Mar 14 2023 Link Dupont <link> - 0.9.5-1 +- Intitial package
That's expected. The upstream RPM contains the %autorelease and %autochangelog macros. Those macros need to be applied to the spec file when the SRPM is built, in order to include the changelog. The spec file from the contents of an SRPM can't contain the %autochangelog macro, for example, because it doesn't have a git history to read from. So there will always be a mismatch between the upstream spec file (containing the %autorelease and %autochangelog macros) and the spec file included in the SRPM. Would you prefer to review the upstream spec (that will eventually be committed to dist-git) or the generated spec file that includes the expanded %autorelease and %autochangelog macros?
What I understand is that during review process we upload a spec file and srpm file generated from that spec. I don't undestand what "upstream spec" is in this scenario. If I diff the spec file https://linkdupont.fedorapeople.org/reviews/golang-github-zcalusic-sysinfo.spec and the spec file inside the srpm https://linkdupont.fedorapeople.org/reviews/golang-github-zcalusic-sysinfo-0.9.5-1.fc39.src.rpm I can see they're different. The spec inside the srpm as includes the processed changelog but also the "rpmautospec 0.3.5" block that should not exist. If you use the `fedpkg import package.srpm` to import the package to dist-git, it will contain the wrong spec. Please, regenerate the srpm using the spec file and upload both files. But if my explanation is not correct, please, do point me to the guide part that explains this.
"Upstream spec" is the spec file that will eventually get committed to dist-git. You are correct that `fedpkg import package.srpm` would not import the correct spec file. In this case, I do no plan to import the srpm directly. I will be importing the spec file and sources manually. To appease fedora-review, the following Spec and SRPM should work: Spec URL: https://download.copr.fedorainfracloud.org/results/linkdupont/reviews/fedora-rawhide-x86_64/05641494-golang-github-zcalusic-sysinfo/golang-github-zcalusic-sysinfo.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/linkdupont/reviews/fedora-rawhide-x86_64/05641494-golang-github-zcalusic-sysinfo/golang-github-zcalusic-sysinfo-0.9.5-1.fc39.src.rpm A couple of resources on rpmautospec that might help explain things: - https://fedoraproject.org/wiki/Changes/Rpmautospec_by_Default - https://docs.pagure.org/fedora-infra.rpmautospec/index.html
I'm sorry, but I don't understand what you want to achieve. The spec doesn't need this part: > ## START: Set by rpmautospec > ## (rpmautospec version 0.3.5) > ## RPMAUTOSPEC: autorelease, autochangelog > %define autorelease(e:s:pb:n) %{?-p:0.}%{lua: > release_number = 1; > base_release_number = tonumber(rpm.expand("%{?-b*}%{!?-b:1}")); > print(release_number + base_release_number - 1); > }%{?-e:.%{-e*}}%{?-s:.%{-s*}}%{!?-n:%{?dist}} > ## END: Set by rpmautospec I guess this is an old requirement that is not needed anymore. Where are you getting this block from? And why are you using it? > * Tue Mar 14 2023 Link Dupont <link> - 0.9.5-1 > - Intitial package Why do you add an entry to changelog if you plan to use %autochangelog ? Both things are not done by default by go2rpm so I guess you're adding them after or you're post-processing the spec somehow, but it's not required.
I want to avoid manually maintaining the %changelog section and the %release value. To do so, and to follow the current Fedora packaging guidelines, I use the %autorelease and %autochangelog macros in my spec files. When I generated this spec file, I ran `go2rpm github.com/zcalusic/sysinfo`. The resulting spec file does contain both the %autorelease and %autochangelog macros; they have been enabled by default[1] since go2rpm 1.7.0. From the spec file that was generated by go2rpm, I then ran `fedpkg --release rawhide srpm` to generate an SRPM. I then uploaded the SRPM to COPR to be built[2]. The spec file that exists in the SRPM differs from the spec file the SRPM was built from in two important ways: 1) A locally defined macro called autorelease is inserted into the top of the spec file. 2) The "%autochangelog" line at the bottom is replaced with a proper changelog Neither of these operations are performed by me; this is the work of rpmautospec during the 'fedpkg srpm' operation. This is a necessary side effect of using %autorelease and %autochangelog to maintain %release and %changelog automatically. In order for a given SRPM to have a static %release value, a local definition of %autorelease has to be injected into the spec file so that the %autorelease macro *always* returns the same fixed number, otherwise each time the SRPM is built, the release would be incremented. Similary, in order for a given SRPM to have a static changelog, the changelog must be constructed from the VCS at the time the SRPM is created and inserted into the SRPM, otherwise each time the SRPM is built, the changelog would be empty (since there is not VCS metadata within the SRPM archive to extract changelog entries from). I believe you can recreate this and observe these changes for yourself by running the following: rpm -q go2rpm go2rpm github.com/zcalusic/sysinfo fedpkg --release rawhide srpm rpm2cpio golang-github-zcalusic-sysinfo-0.9.5-1.fc39.src.rpm | cpio --extract --verbose --make-directories --directory=EXTRACT diff -u golang-github-zcalusic-sysinfo.spec EXTRACT/golang-github-zcalusic-sysinfo.spec You should see a diff that looks like: --- golang-github-zcalusic-sysinfo.spec 2023-03-16 11:10:16.231983088 -0400 +++ EXTRACT/golang-github-zcalusic-sysinfo.spec 2023-03-16 11:12:26.901514002 -0400 @@ -1,3 +1,13 @@ +## START: Set by rpmautospec +## (rpmautospec version 0.3.5) +## RPMAUTOSPEC: autorelease, autochangelog +%define autorelease(e:s:pb:n) %{?-p:0.}%{lua: + release_number = 1; + base_release_number = tonumber(rpm.expand("%{?-b*}%{!?-b:1}")); + print(release_number + base_release_number - 1); +}%{?-e:.%{-e*}}%{?-s:.%{-s*}}%{!?-n:%{?dist}} +## END: Set by rpmautospec + # Generated by go2rpm 1.9.0 %bcond_without check %global debug_package %{nil} @@ -49,4 +59,5 @@ %gopkgfiles %changelog -%autochangelog +* Tue Mar 14 2023 Link Dupont <link> - 0.9.5-1 +- Intitial package 1: https://pagure.io/GoSIG/go2rpm/c/9aeb391410bd6dd9baddcaa8bfa305549ea7448a 2: https://copr.fedorainfracloud.org/coprs/linkdupont/reviews/build/5641494/
Mikel, do you have any questions about this review? Was my explanation in comment #19 helpful?
Sorry Link, I've been busy and I couldn't update you earlier. Thanks for detailing how you create the srpm file, now I understand why the spec file differs from the one you want to use. I use `rpmbuild -bs foo.spec` to create the srpm and then upload the result and the original spec. The spec and the spec inside the sprm files are the same in that case. All the reviews I did previously didn't had the sections we were discussing about, so that was confusing me. My reviewed list: https://bugzilla.redhat.com/buglist.cgi?classification=Fedora&columnlist=product%2Ccomponent%2Cassigned_to%2Cbug_status%2Cshort_desc%2Cchangeddate%2Cbug_severity&component=Package+Review&email1=mikel%40olasagasti.info&emailassigned_to1=1&emailtype1=substring&f1=flagtypes.name&o1=substring&order=status%2C+last_change_time%2C+assigned_to%2C+id%2C+&product=Fedora&query_format=advanced&resolution=---&resolution=CURRENTRELEASE&resolution=RAWHIDE&resolution=ERRATA&resolution=NEXTRELEASE&resolution=CANTFIX&v1=fedora-review The spec is fine, will approve it now.
I am not going to go through the whole fedora-review template, as this package uses go2rpm. - [x] The specfile is sane. - [x] License is correct - [x] Builds successfully in mock - [x] Package is installable (checked by fedora-review) - [x] No relevant rpmlint errors - [x] %check section passes - [x] The latest version is packaged - [x] `%goipath` is set correctly - [-] Binaries don't conflict with binaries already in the distribution - [x] The package complies with the Packaging Guidelines. Package approved! On import, don't forget to do the following: - [ ] Add package to release-monitoring.org - [ ] Give go-sig privileges on package - [ ] Close the review bug by referencing it in the rpm changelog and the Bodhi ticket.
The Pagure repository was created at https://src.fedoraproject.org/rpms/golang-github-zcalusic-sysinfo
FEDORA-2023-ae7b7587df has been submitted as an update to Fedora 39. https://bodhi.fedoraproject.org/updates/FEDORA-2023-ae7b7587df
FEDORA-2023-ae7b7587df has been pushed to the Fedora 39 stable repository. If problem still persists, please make note of it in this bug report.