[Bug 2176241] New: Review Request: golang-github-zcalusic-sysinfo - Sysinfo is a Go library providing Linux OS / kernel / hardware system information

https://bugzilla.redhat.com/show_bug.cgi?id=2176241

Bug ID: 2176241
           Summary: Review Request: golang-github-zcalusic-sysinfo -
                    Sysinfo is a Go library providing Linux OS / kernel /
                    hardware system information
           Product: Fedora
           Version: rawhide
          Hardware: All
                OS: Linux
            Status: NEW
         Component: Package Review
          Severity: medium
          Priority: medium
          Assignee: [e-mail]
          Reporter: [e-mail]
        QA Contact: [e-mail]
                CC: [e-mail]
  Target Milestone: ---
    Classification: Fedora

Spec URL:
https://download.copr.fedorainfracloud.org/results/linkdupont/reviews/fedora...
SRPM URL:
https://download.copr.fedorainfracloud.org/results/linkdupont/reviews/fedora...
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...

[Bug 2176241] Review Request: golang-github-zcalusic-sysinfo - Sysinfo is a Go library providing Linux OS / kernel / hardware system information

https://bugzilla.redhat.com/show_bug.cgi?id=2176241

Link Dupont  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |2176342

Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=2176342
[Bug 2176342] Review Request: yggdrasil-worker-package-manager - Package
manager worker for yggdrasil

[Bug 2176241] Review Request: golang-github-zcalusic-sysinfo - Sysinfo is a Go library providing Linux OS / kernel / hardware system information

https://bugzilla.redhat.com/show_bug.cgi?id=2176241

Mikel Olasagasti Uranga  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|[e-mail]    |[e-mail]
           Doc Type|---                         |If docs needed, set a value
                 CC|                            |[e-mail]
             Status|NEW                         |ASSIGNED
              Flags|                            |fedora-review?

--- Comment #1 from Mikel Olasagasti Uranga  ---
> ## 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  - 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.

[Bug 2176241] Review Request: golang-github-zcalusic-sysinfo - Sysinfo is a Go library providing Linux OS / kernel / hardware system information

https://bugzilla.redhat.com/show_bug.cgi?id=2176241

--- Comment #2 from Link Dupont  ---
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

[Bug 2176241] Review Request: golang-github-zcalusic-sysinfo - Sysinfo is a Go library providing Linux OS / kernel / hardware system information

https://bugzilla.redhat.com/show_bug.cgi?id=2176241

--- Comment #3 from Mikel Olasagasti Uranga  ---
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

[Bug 2176241] Review Request: golang-github-zcalusic-sysinfo - Sysinfo is a Go library providing Linux OS / kernel / hardware system information

https://bugzilla.redhat.com/show_bug.cgi?id=2176241

--- Comment #4 from Link Dupont  ---
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.f...

[Bug 2176241] Review Request: golang-github-zcalusic-sysinfo - Sysinfo is a Go library providing Linux OS / kernel / hardware system information

https://bugzilla.redhat.com/show_bug.cgi?id=2176241

--- Comment #5 from Jakub Kadlčík  ---
Created attachment 1949195
  --> https://bugzilla.redhat.com/attachment.cgi?id=1949195&action=edit
The .spec file difference from Copr build 5603636 to 5617056

[Bug 2176241] Review Request: golang-github-zcalusic-sysinfo - Sysinfo is a Go library providing Linux OS / kernel / hardware system information

https://bugzilla.redhat.com/show_bug.cgi?id=2176241

--- Comment #6 from Link Dupont  ---
Spec URL:
https://linkdupont.fedorapeople.org/reviews/golang-github-zcalusic-sysinfo.s...
SRPM URL:
https://linkdupont.fedorapeople.org/reviews/golang-github-zcalusic-sysinfo-0...

[Bug 2176241] Review Request: golang-github-zcalusic-sysinfo - Sysinfo is a Go library providing Linux OS / kernel / hardware system information

https://bugzilla.redhat.com/show_bug.cgi?id=2176241

--- Comment #7 from Jakub Kadlčík  ---
Created attachment 1949381
  --> https://bugzilla.redhat.com/attachment.cgi?id=1949381&action=edit
The .spec file difference from Copr build 5617056 to 5618866

[Bug 2176241] Review Request: golang-github-zcalusic-sysinfo - Sysinfo is a Go library providing Linux OS / kernel / hardware system information

https://bugzilla.redhat.com/show_bug.cgi?id=2176241

--- Comment #8 from Mikel Olasagasti Uranga  ---
> %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.

[Bug 2176241] Review Request: golang-github-zcalusic-sysinfo - Sysinfo is a Go library providing Linux OS / kernel / hardware system information

https://bugzilla.redhat.com/show_bug.cgi?id=2176241

--- Comment #9 from Link Dupont  ---
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.s...
SRPM URL:
https://linkdupont.fedorapeople.org/reviews/golang-github-zcalusic-sysinfo-0...

[Bug 2176241] Review Request: golang-github-zcalusic-sysinfo - Sysinfo is a Go library providing Linux OS / kernel / hardware system information

https://bugzilla.redhat.com/show_bug.cgi?id=2176241

--- Comment #10 from Jakub Kadlčík  ---
Created attachment 1950258
  --> https://bugzilla.redhat.com/attachment.cgi?id=1950258&action=edit
The .spec file difference from Copr build 5618866 to 5633033

[Bug 2176241] Review Request: golang-github-zcalusic-sysinfo - Sysinfo is a Go library providing Linux OS / kernel / hardware system information

https://bugzilla.redhat.com/show_bug.cgi?id=2176241

--- Comment #11 from Mikel Olasagasti Uranga  ---
> %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.`.

[Bug 2176241] Review Request: golang-github-zcalusic-sysinfo - Sysinfo is a Go library providing Linux OS / kernel / hardware system information

https://bugzilla.redhat.com/show_bug.cgi?id=2176241

--- Comment #12 from Link Dupont  ---
(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.s...
SRPM URL:
https://linkdupont.fedorapeople.org/reviews/golang-github-zcalusic-sysinfo-0...

I also did a new build in COPR:
https://download.copr.fedorainfracloud.org/results/linkdupont/reviews/fedora...

along with running a fedora-review:
https://download.copr.fedorainfracloud.org/results/linkdupont/reviews/fedora...

[Bug 2176241] Review Request: golang-github-zcalusic-sysinfo - Sysinfo is a Go library providing Linux OS / kernel / hardware system information

https://bugzilla.redhat.com/show_bug.cgi?id=2176241

--- Comment #13 from Jakub Kadlčík  ---
Created attachment 1950652
  --> https://bugzilla.redhat.com/attachment.cgi?id=1950652&action=edit
The .spec file difference from Copr build 5633033 to 5641547

[Bug 2176241] Review Request: golang-github-zcalusic-sysinfo - Sysinfo is a Go library providing Linux OS / kernel / hardware system information

https://bugzilla.redhat.com/show_bug.cgi?id=2176241

--- Comment #14 from Mikel Olasagasti Uranga  ---
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  - 0.9.5-1
+- Intitial package

[Bug 2176241] Review Request: golang-github-zcalusic-sysinfo - Sysinfo is a Go library providing Linux OS / kernel / hardware system information

https://bugzilla.redhat.com/show_bug.cgi?id=2176241

--- Comment #15 from Link Dupont  ---
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?

[Bug 2176241] Review Request: golang-github-zcalusic-sysinfo - Sysinfo is a Go library providing Linux OS / kernel / hardware system information

https://bugzilla.redhat.com/show_bug.cgi?id=2176241

--- Comment #16 from Mikel Olasagasti Uranga  ---
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.s...
and the spec file inside the srpm
https://linkdupont.fedorapeople.org/reviews/golang-github-zcalusic-sysinfo-0...
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.

[Bug 2176241] Review Request: golang-github-zcalusic-sysinfo - Sysinfo is a Go library providing Linux OS / kernel / hardware system information

https://bugzilla.redhat.com/show_bug.cgi?id=2176241

--- Comment #17 from Link Dupont  ---
"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...
SRPM URL:
https://download.copr.fedorainfracloud.org/results/linkdupont/reviews/fedora...

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

[Bug 2176241] Review Request: golang-github-zcalusic-sysinfo - Sysinfo is a Go library providing Linux OS / kernel / hardware system information

https://bugzilla.redhat.com/show_bug.cgi?id=2176241

--- Comment #18 from Mikel Olasagasti Uranga  ---
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  - 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.

[Bug 2176241] Review Request: golang-github-zcalusic-sysinfo - Sysinfo is a Go library providing Linux OS / kernel / hardware system information

https://bugzilla.redhat.com/show_bug.cgi?id=2176241

--- Comment #19 from Link Dupont  ---
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  - 0.9.5-1
+- Intitial package

1: https://pagure.io/GoSIG/go2rpm/c/9aeb391410bd6dd9baddcaa8bfa305549ea7448a
2: https://copr.fedorainfracloud.org/coprs/linkdupont/reviews/build/5641494/

[Bug 2176241] Review Request: golang-github-zcalusic-sysinfo - Sysinfo is a Go library providing Linux OS / kernel / hardware system information

https://bugzilla.redhat.com/show_bug.cgi?id=2176241

Link Dupont  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |needinfo?(mikel@olasagasti.
                   |                            |info)

--- Comment #20 from Link Dupont  ---
Mikel, do you have any questions about this review? Was my explanation in
comment #19 helpful?

[Bug 2176241] Review Request: golang-github-zcalusic-sysinfo - Sysinfo is a Go library providing Linux OS / kernel / hardware system information

https://bugzilla.redhat.com/show_bug.cgi?id=2176241

Mikel Olasagasti Uranga  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|needinfo?(mikel@olasagasti. |
                   |info)                       |

--- Comment #21 from Mikel Olasagasti Uranga  ---
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...

The spec is fine, will approve it now.

[Bug 2176241] Review Request: golang-github-zcalusic-sysinfo - Sysinfo is a Go library providing Linux OS / kernel / hardware system information

https://bugzilla.redhat.com/show_bug.cgi?id=2176241

Mikel Olasagasti Uranga  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|fedora-review?              |fedora-review+

--- Comment #22 from Mikel Olasagasti Uranga  ---
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.

[Bug 2176241] Review Request: golang-github-zcalusic-sysinfo - Sysinfo is a Go library providing Linux OS / kernel / hardware system information

https://bugzilla.redhat.com/show_bug.cgi?id=2176241

--- Comment #23 from Fedora Admin user for bugzilla script actions  ---
The Pagure repository was created at
https://src.fedoraproject.org/rpms/golang-github-zcalusic-sysinfo

[Bug 2176241] Review Request: golang-github-zcalusic-sysinfo - Sysinfo is a Go library providing Linux OS / kernel / hardware system information

https://bugzilla.redhat.com/show_bug.cgi?id=2176241

Fedora Update System  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |MODIFIED

--- Comment #24 from Fedora Update System  ---
FEDORA-2023-ae7b7587df has been submitted as an update to Fedora 39.
https://bodhi.fedoraproject.org/updates/FEDORA-2023-ae7b7587df

[Bug 2176241] Review Request: golang-github-zcalusic-sysinfo - Sysinfo is a Go library providing Linux OS / kernel / hardware system information

https://bugzilla.redhat.com/show_bug.cgi?id=2176241

Fedora Update System  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |ERRATA
             Status|MODIFIED                    |CLOSED
        Last Closed|                            |2023-04-03 15:55:20

--- Comment #25 from Fedora Update System  ---
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.