Fedora Account System
Red Hat Associate
Red Hat Customer
Spec URL: https://jsteffan.fedorapeople.org/nvidia-container-toolkit/nvidia-container-toolkit.spec SRPM URL: https://jsteffan.fedorapeople.org/nvidia-container-toolkit/nvidia-container-toolkit-1.19.0-1.fc45.src.rpm Description: Build and run containers leveraging NVIDIA GPUs. Fedora Account System Username: jsteffan
Copr build: https://copr.fedorainfracloud.org/coprs/build/10250149 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2450007-nvidia-container-toolkit/fedora-rawhide-x86_64/10250149-nvidia-container-toolkit/fedora-review/review.txt Please take a look if any issues were found. --- 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.
- Add explicit BuildRequires on make > %make_build - Why custom __golang_extldflags are required? Add a comment > %global __golang_extldflags -Wl,-z,lazy -Wl,--export-dynamic - Current template for the %prep part does the following: > %prep > %goprep -p1 > tar -xf %{S:1} While you're doing: > %goprep -A > %setup -q -T -D -a1 %{forgesetupargs} > tar xf %{SOURCE3} > %autopatch -p1 Feels a pre 1.19 go2rpm template was used and later reapplied somehow. It's purely cosmetic request, but if you could normalize it would be better.
[fedora-review-service-build]
Copr build: https://copr.fedorainfracloud.org/coprs/build/10304665 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2450007-nvidia-container-toolkit/fedora-rawhide-x86_64/10304665-nvidia-container-toolkit/fedora-review/review.txt Please take a look if any issues were found. --- 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.
- Checking upstream Makefile, NVIDIA sets to: > EXTLDFLAGS = -Wl,--export-dynamic -Wl,--unresolved-symbols=ignore-in-object-files -Wl,-z,lazy https://github.com/NVIDIA/nvidia-container-toolkit/blob/main/Makefile#L63 - Version ldflags are not set: > -X $(CLI_VERSION_PACKAGE).gitCommit=$(GIT_COMMIT) -X $(CLI_VERSION_PACKAGE).version=$(CLI_VERSION)" Export the following after "%global gomodulesmode GO111MODULE=on" > export GO_LDFLAGS="-X github.com/NVIDIA/nvidia-container-toolkit/internal/info.version=%{version} \ > -X github.com/NVIDIA/nvidia-container-toolkit/internal/info.gitCommit=%{commit}" The problem is that gitCommit/commit would need to be modified on each version update. You can have something else written there like "unkonwn" or "Fedora". Check how is printed with the upstream version and if it has to have any known format.
Changelog: - Update __golang_extldflags to match upstream, link to upstream - Set info.version and info.gitCommit - Add make BR Spec URL: https://jsteffan.fedorapeople.org/nvidia-container-toolkit/nvidia-container-toolkit.spec SRPM URL: https://jsteffan.fedorapeople.org/nvidia-container-toolkit/nvidia-container-toolkit-1.19.0-1.fc45.src.rpm Why I'm doing the awkwardly looking %prep is because of an odd mix between forge, go macros, what I need to patch, and the added sources. I'll experiment a bit to see where this is breaking down.
Created attachment 2136591 [details] The .spec file difference from Copr build 10304665 to 10311924
Copr build: https://copr.fedorainfracloud.org/coprs/build/10311924 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2450007-nvidia-container-toolkit/fedora-rawhide-x86_64/10311924-nvidia-container-toolkit/fedora-review/review.txt Please take a look if any issues were found. --- 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.
Changelog: - Remove patch, instead copy in staged required binary to expected location: https://github.com/NVIDIA/nvidia-container-toolkit/issues/1758 - Update %prep to be more modern Spec URL: https://jsteffan.fedorapeople.org/nvidia-container-toolkit/nvidia-container-toolkit.spec SRPM URL: https://jsteffan.fedorapeople.org/nvidia-container-toolkit/nvidia-container-toolkit-1.19.0-1.fc45.src.rpm
Created attachment 2136942 [details] The .spec file difference from Copr build 10311924 to 10317718
Copr build: https://copr.fedorainfracloud.org/coprs/build/10317718 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2450007-nvidia-container-toolkit/fedora-rawhide-x86_64/10317718-nvidia-container-toolkit/fedora-review/review.txt Please take a look if any issues were found. --- 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 Mikel Olasagasti Uranga from comment #5) > - Checking upstream Makefile, NVIDIA sets to: > > > EXTLDFLAGS = -Wl,--export-dynamic -Wl,--unresolved-symbols=ignore-in-object-files -Wl,-z,lazy > > https://github.com/NVIDIA/nvidia-container-toolkit/blob/main/Makefile#L63 > rpmlint is also complaining a lot about symbols. What resolves these symbols ?
Source3: https://github.com/NVIDIA/dgx-selinux/archive/%{dgx_selinux_commit}/dgx-selinux-%{dgx_selinux_commit}.tar.gz From the readme https://github.com/NVIDIA/dgx-selinux/blob/master/README.md This is expected to run on bare metal, not just containers. I think this needs its own package. And it can follow the selinux packaging policy https://fedoraproject.org/wiki/PackagingDrafts/SELinux_Independent_Policy
(In reply to Tom.Rix from comment #13) > (In reply to Mikel Olasagasti Uranga from comment #5) > > - Checking upstream Makefile, NVIDIA sets to: > > > > > EXTLDFLAGS = -Wl,--export-dynamic -Wl,--unresolved-symbols=ignore-in-object-files -Wl,-z,lazy > > > > https://github.com/NVIDIA/nvidia-container-toolkit/blob/main/Makefile#L63 > > > rpmlint is also complaining a lot about symbols. What resolves these > symbols ? They are resolved at runtime by the linker: Library location (pkg/lookup/library.go): A First() combinator tries three strategies in order, returning the first hit: 1. Direct dlopen attempt 2. Predefined filesystem search paths 3. Parse /etc/ld.so.cache ld.so.cache parsing (internal/ldcache/ldcache.go): The binary cache is mmap-ed directly. Each entry2 struct has a Flags field with ELF architecture bits (0x0300 = x86_64, 0x0a00 = aarch64, etc.) used to filter 32-bit vs 64-bit libs. No ldconfig subprocess — raw struct parsing. dlopen/dlsym (vendor/github.com/NVIDIA/go-nvml/pkg/dl/dl.go): CGO wrapper around libc dlopen/dlsym. Flags: RTLD_LAZY | RTLD_GLOBAL | RTLD_NODELETE. Every call is wrapped with runtime.LockOSThread() for thread safety. Symlink chain resolution (pkg/lookup/symlinks/symlink.go): Once a library path is found, the full symlink chain is walked (e.g., libcuda.so → libcuda.so.1 → libcuda.so.550.1.0) and all aliases are collected — so containers get every soname variant. Versioned symbol negotiation (internal/nvsandboxutils/lib.go): After dlopen, updateVersionedSymbols() probes for _v2/_v3 suffixed symbols via dlsym. If found, the function pointer is upgraded — forward compatibility without ABI breaks. Summary: locate via ldcache mmap → dlopen → dlsym with version probing → expose full symlink chain.
(In reply to Tom.Rix from comment #14) > Source3: > https://github.com/NVIDIA/dgx-selinux/archive/%{dgx_selinux_commit}/dgx- > selinux-%{dgx_selinux_commit}.tar.gz > > From the readme > https://github.com/NVIDIA/dgx-selinux/blob/master/README.md > This is expected to run on bare metal, not just containers. > > I think this needs its own package. > And it can follow the selinux packaging policy > https://fedoraproject.org/wiki/PackagingDrafts/SELinux_Independent_Policy Agreed. I'll drop it and add a new policy package for inclusion.
Changelog: - Drop the additional SELinux policy, keep the boolean management Spec URL: https://jsteffan.fedorapeople.org/nvidia-container-toolkit/nvidia-container-toolkit.spec SRPM URL: https://jsteffan.fedorapeople.org/nvidia-container-toolkit/nvidia-container-toolkit-1.19.0-1.fc45.src.rpm
Copr build: https://copr.fedorainfracloud.org/coprs/build/10452587 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2450007-nvidia-container-toolkit/fedora-rawhide-x86_64/10452587-nvidia-container-toolkit/fedora-review/review.txt Please take a look if any issues were found. --- 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.
Changelog: - Fix the SELinux changes (oops don't remove the %files section :)) Spec URL: https://jsteffan.fedorapeople.org/nvidia-container-toolkit/nvidia-container-toolkit.spec SRPM URL: https://jsteffan.fedorapeople.org/nvidia-container-toolkit/nvidia-container-toolkit-1.19.0-1.fc45.src.rpm
Created attachment 2141077 [details] The .spec file difference from Copr build 10452587 to 10456018
Copr build: https://copr.fedorainfracloud.org/coprs/build/10456018 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2450007-nvidia-container-toolkit/fedora-rawhide-x86_64/10456018-nvidia-container-toolkit/fedora-review/review.txt Please take a look if any issues were found. --- 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.
> > Symlink chain resolution (pkg/lookup/symlinks/symlink.go): Once a library > path is found, the full > symlink chain is walked (e.g., libcuda.so → libcuda.so.1 → > libcuda.so.550.1.0) and all aliases are > collected — so containers get every soname variant. > cuda is of course not open source. What are the legal issues for this method ? I am hoping you can point to a past ruling saying it is ok.
(In reply to Tom.Rix from comment #22) > > > > Symlink chain resolution (pkg/lookup/symlinks/symlink.go): Once a library > > path is found, the full > > symlink chain is walked (e.g., libcuda.so → libcuda.so.1 → > > libcuda.so.550.1.0) and all aliases are > > collected — so containers get every soname variant. > > > > cuda is of course not open source. > What are the legal issues for this method ? > I am hoping you can point to a past ruling saying it is ok. Dynamically linking is the licensing demarcation point. Everything shipped in this package is licensed with an approved license and should be fine for Fedora. If the relevant DSOs are available in the probed locations, they are exposed inside the container. If not, nothing happens and the CDI configuration is unable to be assembled.
For the approval, well it's already approved as it's already been included in Fedora: https://src.fedoraproject.org/rpms/golang-github-nvidia-container-toolkit
Which reminded me :) Changelog: - Add Obsoletes for https://src.fedoraproject.org/rpms/golang-github-nvidia-container-toolkit < 1.18 Spec URL: https://jsteffan.fedorapeople.org/nvidia-container-toolkit/nvidia-container-toolkit.spec SRPM URL: https://jsteffan.fedorapeople.org/nvidia-container-toolkit/nvidia-container-toolkit-1.19.0-1.fc45.src.rpm
Created attachment 2141321 [details] The .spec file difference from Copr build 10456018 to 10461429
Copr build: https://copr.fedorainfracloud.org/coprs/build/10461429 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2450007-nvidia-container-toolkit/fedora-rawhide-x86_64/10461429-nvidia-container-toolkit/fedora-review/review.txt Please take a look if any issues were found. --- 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 Jonathan Steffan from comment #24) > For the approval, well it's already approved as it's already been included > in Fedora: > https://src.fedoraproject.org/rpms/golang-github-nvidia-container-toolkit Why are we doing a package review then ?
(In reply to Tom.Rix from comment #28) > Why are we doing a package review then ? It's a new package name and a new spec, because we are changing what the package ships. In addition, this new package uses vendoring.
I am going to recommend the cuda part to be removed. For licensing I was specifically looking for an nvidia and cuda references that would allow wrapping of cuda in fedora. If we do this and zluda or similar hook it up their own cuda wrapper, will fedora or its users be sued ?
I'm not sure I follow. If we don't think this content is permissive in Fedora, I need to file a ticket to get it approved? It would be silly to block this code IMHO.
Changelog: - Update to 1.19.1 Spec URL: https://jsteffan.fedorapeople.org/nvidia-container-toolkit/nvidia-container-toolkit.spec SRPM URL: https://jsteffan.fedorapeople.org/nvidia-container-toolkit/nvidia-container-toolkit-1.19.1-1.fc45.src.rpm
Created attachment 2142275 [details] The .spec file difference from Copr build 10461429 to 10501619
Copr build: https://copr.fedorainfracloud.org/coprs/build/10501619 (failed) Build log: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2450007-nvidia-container-toolkit/fedora-rawhide-x86_64/10501619-nvidia-container-toolkit/builder-live.log.gz Please make sure the package builds successfully at least for Fedora Rawhide. - If the build failed for unrelated reasons (e.g. temporary network unavailability), please ignore it. - If the build failed because of missing BuildRequires, please make sure they are listed in the "Depends On" field --- 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.
Changelog: - Fix test determinism https://github.com/NVIDIA/nvidia-container-toolkit/pull/1846 Spec URL: https://jsteffan.fedorapeople.org/nvidia-container-toolkit/nvidia-container-toolkit.spec SRPM URL: https://jsteffan.fedorapeople.org/nvidia-container-toolkit/nvidia-container-toolkit-1.19.1-1.fc45.src.rpm
Created attachment 2142420 [details] The .spec file difference from Copr build 10501619 to 10504408
Copr build: https://copr.fedorainfracloud.org/coprs/build/10504408 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2450007-nvidia-container-toolkit/fedora-rawhide-x86_64/10504408-nvidia-container-toolkit/fedora-review/review.txt Please take a look if any issues were found. --- 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.