Fedora Account System
Red Hat Associate
Red Hat Customer
Spec URL: https://rishi.fedorapeople.org/golang-github-nvidia-nvml.spec SRPM URL: https://rishi.fedorapeople.org/golang-github-nvidia-nvml-0-1.20240620gitv0.12.4.0.fc38.src.rpm Description: Go Bindings for the NVIDIA Management Library (NVML). This package contains the source code needed for building packages that reference the following Go import paths: – github.com/NVIDIA/go-nvml Fedora Account System Username: rishi
I chose to disable the tests by default because a few of them require the presence of the proprietary NVIDIA driver. This is my first attempt at packaging a Go module. Here are some doubts that I have about it. The Git tags and versions used by upstream (eg., v0.12.4-0) are not valid RPM versions because they contain a dash. That's why I chose to define '%tag' and left the %version as 0. The generated Provides don't have 'golang(github.com/NVIDIA/go-nvml)'. I hope that's alright because the top-level source directory doesn't have any Go sources. So, users of this module need to import the sub-paths.
Copr build: https://copr.fedorainfracloud.org/coprs/build/7646718 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2293636-golang-github-nvidia-nvml/fedora-rawhide-x86_64/07646718-golang-github-nvidia-nvml/fedora-review/review.txt Found issues: - No gcc, gcc-c++ or clang found in BuildRequires Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/ Please know that there can be false-positives. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
> I chose to disable the tests by default because a few of them require the presence of the proprietary NVIDIA driver. Can add this as a comment before the bcond_with? > The Git tags and versions used by upstream (eg., v0.12.4-0) are not valid RPM versions because they contain a dash. That's why I chose to define '%tag' and left the %version as 0. What a strange way to version the package. One option would be to use: Version: 0.12.4.0 %global tag v0.12.4-0 WDYT? You'll need to generate the package with `go2rpm -q github.com/NVIDIA/go-nvml -t v0.12.4-0` and modify the Version entry. > The generated Provides don't have 'golang(github.com/NVIDIA/go-nvml)'. I hope that's alright because the top-level source directory doesn't have any Go sources. So, users of this module need to import the sub-paths. It's correct and not an issue. If you check the example in the README file you'll see they're not using top-level dir, but the whole path as expected: https://github.com/NVIDIA/go-nvml/blob/main/README.md?plain=1#L59 On top of these changes, would it be possible to update to latest go2rpm (1.12.0) package?
(In reply to Debarshi Ray from comment #1) > The Git tags and versions used by upstream (eg., v0.12.4-0) are not valid > RPM versions because they contain a dash. That's why I chose to define > '%tag' and left the %version as 0. I filed an upstream request about not using a hyphen or dash in the Git tag: https://github.com/NVIDIA/go-nvml/issues/126
Thanks for the review! (In reply to Mikel Olasagasti Uranga from comment #3) > > I chose to disable the tests by default because a few of them require the presence of the proprietary NVIDIA driver. > > Can add this as a comment before the bcond_with? Sure. Added. > > The Git tags and versions used by upstream (eg., v0.12.4-0) are not valid RPM versions because they contain a dash. That's why I chose to define '%tag' and left the %version as 0. > > What a strange way to version the package. One option would be to use: > > Version: 0.12.4.0 > %global tag v0.12.4-0 > > WDYT? You'll need to generate the package with `go2rpm -q > github.com/NVIDIA/go-nvml -t v0.12.4-0` and modify the Version entry. Okay! That definitely makes the RPM's version a lot saner. Thanks for the tip. :) > > The generated Provides don't have 'golang(github.com/NVIDIA/go-nvml)'. I hope that's alright because the top-level source directory doesn't have any Go sources. So, users of this module need to import the sub-paths. > > It's correct and not an issue. If you check the example in the README file > you'll see they're not using top-level dir, but the whole path as expected: > https://github.com/NVIDIA/go-nvml/blob/main/README.md?plain=1#L59 Okay. > On top of these changes, would it be possible to update to latest go2rpm > (1.12.0) package? For some reason, running go2rpm inside my Fedora 40 Toolbx container is erroring out with a Python traceback that seems to be due to 'git config user.name' finishing with exit code 1, even though I have: $ env | grep GIT GIT_COMMITTER_NAME=Debarshi Ray GIT_AUTHOR_EMAIL=rishi GIT_COMMITTER_EMAIL=rishi GIT_AUTHOR_NAME=Debarshi Ray I will dig into it. Meanwhile, I have uploaded the other changes: Spec URL: https://rishi.fedorapeople.org/golang-github-nvidia-nvml.spec SRPM URL: https://rishi.fedorapeople.org/golang-github-nvidia-nvml-0.12.4.0-1.fc38.src.rpm
Created attachment 2038185 [details] The .spec file difference from Copr build 7646718 to 7662267
Copr build: https://copr.fedorainfracloud.org/coprs/build/7662267 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2293636-golang-github-nvidia-nvml/fedora-rawhide-x86_64/07662267-golang-github-nvidia-nvml/fedora-review/review.txt Found issues: - No gcc, gcc-c++ or clang found in BuildRequires Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/ Please know that there can be false-positives. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
(In reply to Debarshi Ray from comment #1) > I chose to disable the tests by default because a few of them require the > presence of the proprietary NVIDIA driver. I tried the tests with the proprietary NVIDIA driver version 550.78 installed. They failed in the exact same way as they do without it. So, I expanded the comment to clarify it with more details.
(In reply to Mikel Olasagasti Uranga from comment #3) > On top of these changes, would it be possible to update to latest go2rpm > (1.12.0) package? Done. I hope I didn't mess it up when hand-editing the *.spec file. Is %{goname} deprecated? I am asking because go2rpm doesn't use it. I see that the packaging guidelines still use it: https://docs.fedoraproject.org/en-US/packaging-guidelines/Golang/
> Is %{goname} deprecated? I am asking because go2rpm doesn't use it. I see that the packaging guidelines still use it: https://docs.fedoraproject.org/en-US/packaging-guidelines/Golang/ Documentation needs some love. The template was changed here: https://gitlab.com/fedora/sigs/go/go2rpm/-/commit/8b6977dbeca1a7e54921bfe08136520a039b279f
This package was generated using go2rpm, which simplifies the review. Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated - [x] The latest version is packaged or packaging an earlier version is justified. - [x] The License tag reflects the package contents and uses the correct identifiers. - [x] The package builds successfully in mock. - [x] Package is installable (checked by fedora-review). - [x] There are no relevant rpmlint errors. - [x] The package runs tests in %check. - [x] `%goipath` is set correctly. - [-] The package's binaries don't conflict with binaries already in the distribution. (Some Go projects include utility binaries with very generic names) - [-] There are no `%{_bindir}/*` wildcards in %files. (go2rpm includes these by default) - [x] The package does not use `%gometa -f` if it has dependents that still build for %ix86. - [x] The package complies with the Golang and general Packaging Guidelines. Package approved! On import, don't forget to do the following: - [ ] Add the package to release-monitoring.org - [ ] Give go-sig privileges (at least commit) on the package - [ ] Close the review bug by referencing its ID in the rpm changelog and the Bodhi ticket. - [ ] Consider configuring Packit service to help with maintenance Thanks!
(In reply to Mikel Olasagasti Uranga from comment #10) > > Is %{goname} deprecated? I am asking because go2rpm > doesn't use it. I see that the packaging guidelines > still use it: > https://docs.fedoraproject.org/en-US/packaging-guidelines/Golang/ > > Documentation needs some love. > > The template was changed here: > > https://gitlab.com/fedora/sigs/go/go2rpm/-/commit/ > 8b6977dbeca1a7e54921bfe08136520a039b279f I see, thanks for the reference. The commit message says: "We now hardcode the Name field to the value of goname," ... and the changes replace %{goname} with the name unconditionally: Name: %{goname} Name: {{ name }} So, are we not supposed to use %{goname} in *.spec files anymore? go-rpm-macros doesn't seem to have deprecated it, but I saw this commit: https://pagure.io/go-rpm-macros/c/39aa1bbc6ad33a3b603d2f2a16ab186c84c2342a ... which looks related, but it's not clear to me how. :)
The Pagure repository was created at https://src.fedoraproject.org/rpms/golang-github-nvidia-nvml
(In reply to Mikel Olasagasti Uranga from comment #11) > > [...] > > Package approved! On import, don't forget to do the following: Thanks, Mikel. > - [ ] Add the package to release-monitoring.org > - [ ] Give go-sig privileges (at least commit) on the package > - [ ] Close the review bug by referencing its ID in the rpm changelog and > the Bodhi ticket. > - [ ] Consider configuring Packit service to help with maintenance Okay, I will do those. Requested Git repository: https://pagure.io/releng/fedora-scm-requests/issue/63279 Requested Git branches for f40 and f39: https://pagure.io/releng/fedora-scm-requests/issue/63280 https://pagure.io/releng/fedora-scm-requests/issue/63281
(In reply to Debarshi Ray from comment #12) > (In reply to Mikel Olasagasti Uranga from comment #10) > > > Is %{goname} deprecated? I am asking because go2rpm > > doesn't use it. I see that the packaging guidelines > > still use it: > > https://docs.fedoraproject.org/en-US/packaging-guidelines/Golang/ > > > > Documentation needs some love. > > > > The template was changed here: > > > > https://gitlab.com/fedora/sigs/go/go2rpm/-/commit/ > > 8b6977dbeca1a7e54921bfe08136520a039b279f > > I see, thanks for the reference. > > The commit message says: > "We now hardcode the Name field to the value of goname," > > ... and the changes replace %{goname} with the name unconditionally: > Name: %{goname} > Name: {{ name }} > > So, are we not supposed to use %{goname} in *.spec files anymore? Correct, no need to use goname, name works just fine. > go-rpm-macros doesn't seem to have deprecated it, but I saw this commit: > https://pagure.io/go-rpm-macros/c/39aa1bbc6ad33a3b603d2f2a16ab186c84c2342a > > ... which looks related, but it's not clear to me how. :) Not a macro expert myself, so not sure if something else should be changed. You can try to ask in https://lists.fedoraproject.org/archives/list/golang@lists.fedoraproject.org/ or Fedora Golang room in Matrix.
FEDORA-2024-390b5c7d9a (golang-github-nvidia-nvml-0.12.4.0-1.fc40) has been submitted as an update to Fedora 40. https://bodhi.fedoraproject.org/updates/FEDORA-2024-390b5c7d9a
FEDORA-2024-d33b6e25fc (golang-github-nvidia-nvml-0.12.4.0-1.fc39) has been submitted as an update to Fedora 39. https://bodhi.fedoraproject.org/updates/FEDORA-2024-d33b6e25fc
Added https://github.com/NVIDIA/go-nvml to Anitya: https://release-monitoring.org/project/373214/ ... and enabled monitoring in Fedora: https://src.fedoraproject.org/rpms/golang-github-nvidia-nvml
FEDORA-2024-390b5c7d9a has been pushed to the Fedora 40 testing repository. Soon you'll be able to install the update with the following command: `sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2024-390b5c7d9a \*` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2024-390b5c7d9a See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-2024-d33b6e25fc has been pushed to the Fedora 39 testing repository. Soon you'll be able to install the update with the following command: `sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2024-d33b6e25fc \*` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2024-d33b6e25fc See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-2024-390b5c7d9a (golang-github-nvidia-nvml-0.12.4.0-1.fc40) has been pushed to the Fedora 40 stable repository. If problem still persists, please make note of it in this bug report.
FEDORA-2024-d33b6e25fc (golang-github-nvidia-nvml-0.12.4.0-1.fc39) has been pushed to the Fedora 39 stable repository. If problem still persists, please make note of it in this bug report.