[Bug 2293636] New: Review Request: golang-github-nvidia-nvml - Go Bindings for the NVIDIA Management Library (NVML)
https://bugzilla.redhat.com/show_bug.cgi?id=2293636
Bug ID: 2293636 Summary: Review Request: golang-github-nvidia-nvml - Go Bindings for the NVIDIA Management Library (NVML) Product: Fedora Version: rawhide Hardware: All OS: Linux Status: NEW Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: debarshir@redhat.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: https://rishi.fedorapeople.org/golang-github-nvidia-nvml.spec SRPM URL: https://rishi.fedorapeople.org/golang-github-nvidia-nvml-0-1.20240620gitv0.1...
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
https://bugzilla.redhat.com/show_bug.cgi?id=2293636
--- Comment #1 from Debarshi Ray debarshir@redhat.com --- 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.
https://bugzilla.redhat.com/show_bug.cgi?id=2293636
--- Comment #2 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/7646718 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
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.
https://bugzilla.redhat.com/show_bug.cgi?id=2293636
Mikel Olasagasti Uranga mikel@olasagasti.info changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |mikel@olasagasti.info Status|NEW |ASSIGNED Assignee|nobody@fedoraproject.org |mikel@olasagasti.info
--- Comment #3 from Mikel Olasagasti Uranga mikel@olasagasti.info ---
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?
https://bugzilla.redhat.com/show_bug.cgi?id=2293636
--- Comment #4 from Debarshi Ray debarshir@redhat.com --- (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
https://bugzilla.redhat.com/show_bug.cgi?id=2293636
--- Comment #5 from Debarshi Ray debarshir@redhat.com --- 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@fedoraproject.org GIT_COMMITTER_EMAIL=rishi@fedoraproject.org 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...
https://bugzilla.redhat.com/show_bug.cgi?id=2293636
--- Comment #6 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Created attachment 2038185 --> https://bugzilla.redhat.com/attachment.cgi?id=2038185&action=edit The .spec file difference from Copr build 7646718 to 7662267
https://bugzilla.redhat.com/show_bug.cgi?id=2293636
--- Comment #7 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/7662267 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
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.
https://bugzilla.redhat.com/show_bug.cgi?id=2293636
--- Comment #8 from Debarshi Ray debarshir@redhat.com --- (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.
https://bugzilla.redhat.com/show_bug.cgi?id=2293636
--- Comment #9 from Debarshi Ray debarshir@redhat.com --- (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/
https://bugzilla.redhat.com/show_bug.cgi?id=2293636
Debarshi Ray debarshir@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |2294109
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=2294109 [Bug 2294109] Review Request: golang-github-nvidia-nvlib - A collection of useful Go libraries for use with NVIDIA GPU management tools
package-review@lists.fedoraproject.org
-
bugzilla＠redhat.com