-
-
Notifications
You must be signed in to change notification settings - Fork 14.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cudaPackages: .pc files should not contain /usr paths #224119
Comments
Hellol! I was looking for good first bug issues and tried this. Describe the bug:The pkg-config files for the Steps to reproduce the bug:Build and install the NIXPKGS_ALLOW_UNFREE=1 nix build --impure nixpkgs#cudaPackages.cuda_cudart Check the contents of the cat result/pkg-config/cudart-11.7.pc
cudaroot=/usr/local/cuda-11.7
libdir=${cudaroot}/targets/x86_64-linux/lib
includedir=${cudaroot}/targets/x86_64-linux/include
Name: cudart
Description: CUDA Runtime Library
Version: 11.7
Libs: -L${libdir} -lcudart
Cflags: -I${includedir} Observe that the cudaroot path is set to Expected behaviorThe paths in the Let's begin:Fork the git clone https://github.com/dooygoy/nixpkgs.git
Cloning into 'nixpkgs'...
remote: Enumerating objects: 3783643, done.
remote: Counting objects: 100% (458/458), done.
remote: Compressing objects: 100% (172/172), done.
remote: Total 3783643 (delta 332), reused 390 (delta 286), pack-reused 3783185
Receiving objects: 100% (3783643/3783643), 3.11 GiB | 6.33 MiB/s, done.
Resolving deltas: 100% (2576941/2576941), done.
Updating files: 100% (34603/34603), done.
cd nixpkgs/ Search for the package: grep -r --include='*.nix' -l 'cuda_cudart' .
./pkgs/applications/science/math/mathematica/generic.nix
./pkgs/development/libraries/science/math/tiny-cuda-nn/default.nix
./pkgs/development/libraries/science/math/nccl/default.nix
./pkgs/development/libraries/science/math/faiss/default.nix
./pkgs/development/libraries/science/math/magma/generic.nix
./pkgs/development/libraries/opencv/4.x.nix
./pkgs/development/libraries/nvidia-thrust/default.nix
./pkgs/development/python-modules/torchvision/default.nix
./pkgs/development/python-modules/openai-triton/default.nix
./pkgs/development/python-modules/bitsandbytes/default.nix I see there is the https://github.com/NixOS/nixpkgs/blob/nixos-22.11/pkgs/development/compilers/cudatoolkit/ There is also the # TODO: choose whether to install static/dynamic libs
installPhase = ''
runHook preInstall
rm LICENSE
mkdir -p $out
mv * $out
runHook postInstall
''; How should I go from here? I am not sure how to continue for now or whether I am looking at the correct file: |
@dooygoy hi, and thank you for taking interest in this issue! In the hindsight, I see I didn't include all of the inputs in the description, sorry about that. I also should warn you in advance that although I added the good-first-bug label, merging a PR to close this issue might take a little time and a bit of experimenting. The individual steps should be very simple though! I won't assume how familiar you are with nixpkgs, so I'll include details that may possibly be (probably are) redundant. A note on impact: hypothetically, we could just delete the pkg-config files, because most of our downstream packages seem to be using cmake, meson, or bazel for builds, and they either rely on proper mechanisms like Also, do not worry if you decide to spend your time elsewhere: I think it's good to have these notes written up anyway You guessed right, most of our CUDA-related expressions live in pkgs/development/compilers/cudatoolkit, and the actual
This should be enough to get started Footnotes
|
Thank you very much for such a long description and thank you for including the details that may possibly be redundant (which are not!). Since this is my very first package issue and I wish to blog about it too (and I am pretty excited about it too, yay!) I will document it in detail including my analysis which may also go into much detail. So let's begin:
Analysis: I see downstream packages and upstream is mentioned and I wasn't sure about this terminology. Upstream refers to the original software projects, while downstream refers to the packages in the Nix package repository (nixpkgs) that depend on those upstream projects. When a bug is fixed or a new version is released upstream, it's usually the responsibility of the downstream package maintainers to update their packages to incorporate those changes. I see this quote "hard-code paths like /opt/something/cuda," is referring to the practice of hardcoding absolute paths in the build process or configuration files which looks to me as impure with the regard to how Nix packages are made. Why is that? Hardcoding paths can cause issues because it makes assumptions about the file system layout and the location of dependencies, which may not hold true for every user or system. Nix reproducible builds actually solve this problem, in this case by making these paths relative instead of absolute.
I open the linked JSON file published by nvidia and I see the relative paths which seem to be important to make the package functional. In the Nix expression, the relative_path is used along with the fetchurl function to download the appropriate source file for building the package. This part caught my attention that shows the relative paths for each platform. For example for the Linux it shows: "linux-x86_64": {
"relative_path": "cuda_cudart/linux-x86_64/cuda_cudart-linux-x86_64-11.7.60-archive.tar.xz",
"sha256": "1c079add60a107f6dd9e72a0cc9cde03eb9d833506f355c22b9177c47a977552",
"md5": "1ef515eb31691f2c43fb0de1443893a3",
"size": "854744"
}, But let's read further instructions.
I am familiar with sed but have never actually used it and I am wondering at the same time if there is a more Nix-way to solve this too. It seems the suggested solution is to patch the existing installPhase = ''
runHook preInstall
rm LICENSE
mkdir -p $out
mv * $out
# Patch the .pc files to replace /usr/local/cuda-XX.Y with $out
sed -i 's|^cudaroot=.*$|cudaroot=${placeholder "out"}|' $out/pkg-config/*.pc
runHook postInstall
''; What is Now is there a more Nix way to solve this? Is there a Nix function that would do this operation instead of postPatch = ''
substituteInPlace lib/zoneinfo.rs \
--replace "/usr/share/zoneinfo" "${tzdata}/share/zoneinfo"
''; How could we apply this function in our case instead of postInstall = ''
substituteInPlace $out/pkg-config/cudart-11.7.pc \
--replace "cudaroot=/usr/local/cuda-11.7" "cudaroot=${placeholder "out"}"
''; But another issue is that unlike sed, UPDATE: Is the
OK, let's look at the
{ lib
, stdenv
, backendStdenv
, fetchurl
, autoPatchelfHook
, autoAddOpenGLRunpathHook
, validatePkgConfig
}:
nativeBuildInputs = [
autoPatchelfHook
# This hook will make sure libcuda can be found
# in typically /lib/opengl-driver by adding that
# directory to the rpath of all ELF binaries.
# Check e.g. with `patchelf --print-rpath path/to/my/binary
autoAddOpenGLRunpathHook
validatePkgConfig
]; How is the hook defined in validatePkgConfig = makeSetupHook
{ name = "validate-pkg-config"; propagatedBuildInputs = [ findutils pkg-config ]; }
../build-support/setup-hooks/validate-pkg-config.sh; Now I read test derivations and CI are mentioned too which sounds cool and definetly as something useful and functional to have when building packages. Tests are super important, but I am not familiar with how these things work
We must somehow create a kind of That sounds super interesting!
OK so we move on and leave this for later. I am not familiar with
In order to ensure that the hooks don't break packages that don't ship any .pc files, how can we conditionally apply the hooks based on the presence of .pc files in the package? Upon further investigation I see that one way to achieve this is by using the
Oh, let's try this right now! mkShell { packages = [ pkg-config ]; buildInputs = with cudaPackages; [ cuda_cudart cuda_nvrtc cuda_nvtx libcublas libcufft ]; }
bash: syntax error near unexpected token `}' Oh, we got an error! Was there a typo? Is something missing? I am instructed to enter nix shell and it seems to me I didn't actually start the nix shell. Maybe I need to pull this command after that: nix-shell -E 'with import <nixpkgs> {}; mkShell { buildInputs = with cudaPackages; [ pkg-config cuda_cudart cuda_nvrtc cuda_nvtx libcublas libcufft ]; }'
error: Package ‘cuda_cudart-11.7.60’ in /nix/store/79fr5k4kky1p59y865ivnaxxq23y0nxg-source/pkgs/development/compilers/cudatoolkit/redist/build-cuda-redist-package.nix:55 has an unfree license (‘unfree’), refusing to evaluate.
a) To temporarily allow unfree packages, you can use an environment variable
for a single invocation of the nix tools.
$ export NIXPKGS_ALLOW_UNFREE=1
Note: For `nix shell`, `nix build`, `nix develop` or any other Nix 2.4+
(Flake) command, `--impure` must be passed in order to read this
environment variable.
b) For `nixos-rebuild` you can set
{ nixpkgs.config.allowUnfree = true; }
in configuration.nix to override this.
Alternatively you can configure a predicate to allow specific packages:
{ nixpkgs.config.allowUnfreePredicate = pkg: builtins.elem (lib.getName pkg) [
"cuda_cudart"
];
}
c) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
{ allowUnfree = true; }
to ~/.config/nixpkgs/config.nix.
(use '--show-trace' to show detailed location information) Another error! This tells us we need to export NIXPKGS_ALLOW_UNFREE=1
~ via 🐍 v3.10.10
❯ nix-shell -E 'with import <nixpkgs> {}; mkShell { buildInputs = with cudaPackages; [ pkg-config cuda_cudart cuda_nvrtc cuda_nvtx libcublas libcufft ]; }'
these 8 derivations will be built:
/nix/store/53qvvhwj1mqdssx6lnpgymdpvv15jbxh-cuda_nvrtc-linux-x86_64-11.7.50-archive.tar.xz.drv
/nix/store/4zk729lbyn8381ijj1ff1vh8wg03mv41-cuda_nvrtc-11.7.50.drv
/nix/store/6ciynn4hdamj6ihym3icyh37ndybancc-libcufft-linux-x86_64-10.7.2.50-archive.tar.xz.drv
/nix/store/maz1aq725zxk8wdp38b7nlfcx5lq45qv-libcublas-linux-x86_64-11.10.1.25-archive.tar.xz.drv
/nix/store/7lvyc15c2f3qqjhwpxfgq0hc5w2wfic8-libcublas-11.10.1.25.drv
/nix/store/hfr7kk26wqfbqs89qdi61n1f0wlza0c6-libcufft-10.7.2.50.drv
/nix/store/sfccgfg4f0ry6d44grwa5zshxzryqy6j-cuda_nvtx-linux-x86_64-11.7.50-archive.tar.xz.drv
/nix/store/xwqbkra4rhg02nadk8v792f93w51pi73-cuda_nvtx-11.7.50.drv
building '/nix/store/53qvvhwj1mqdssx6lnpgymdpvv15jbxh-cuda_nvrtc-linux-x86_64-11.7.50-archive.tar.xz.drv'...
building '/nix/store/sfccgfg4f0ry6d44grwa5zshxzryqy6j-cuda_nvtx-linux-x86_64-11.7.50-archive.tar.xz.drv'...
building '/nix/store/maz1aq725zxk8wdp38b7nlfcx5lq45qv-libcublas-linux-x86_64-11.10.1.25-archive.tar.xz.drv'...
building '/nix/store/6ciynn4hdamj6ihym3icyh37ndybancc-libcufft-linux-x86_64-10.7.2.50-archive.tar.xz.drv'...
... And now we are back in business! There is a huge output running so let's see.. I wonder if it's necessary to copy the whole output here? Now it is unpacking a source archive into the pkg-config --list-all
~ via 🐍 v3.10.1 via impure (nix-shell) We get nothing which means it is somehow not finding it in nix store? I thought while in nix shell everything is magically available. Well, let's try to find it? echo $cuda_cudart Hmm, still nothing. It seems that the environment variables are not set when entering a nix-shell. Instead, let's find the location of the find /nix/store -name "*.pc" | grep cuda
/nix/store/vqqdcl22a5cynh9m7jxnmmhi4ylyy47r-cuda_nvtx-11.7.50/pkg-config/nvToolsExt-11.7.pc
/nix/store/4qz7zqrxd1anyhhi0fyix8dn4hqbifps-cuda_cudart-11.7.60/pkg-config/cudart-11.7.pc
/nix/store/4qz7zqrxd1anyhhi0fyix8dn4hqbifps-cuda_cudart-11.7.60/pkg-config/cuda-11.7.pc
/nix/store/vz5jd0y7wlj94cj4k3abcb94njz3f52d-cuda_nvrtc-11.7.50/pkg-config/nvrtc-11.7.pc Oh and what do we do now? We must find a way to extract these directory paths and add them to our environment. export PKG_CONFIG_PATH="/nix/store/vqqdcl22a5cynh9m7jxnmmhi4ylyy47r-cuda_nvtx-11.7.50/pkg-config:/nix/store/4qz7zqrxd1anyhhi0fyix8dn4hqbifps-cuda_cudart-11.7.60/pkg-config:/nix/store/vz5jd0y7wlj94cj4k3abcb94njz3f52d-cuda_nvrtc-11.7.50/pkg-config" Now when we try to run the Let's verify that the echo $PKG_CONFIG_PATH
/nix/store/vqqdcl22a5cynh9m7jxnmmhi4ylyy47r-cuda_nvtx-11.7.50/pkg-config:/nix/store/4qz7zqrxd1anyhhi0fyix8dn4hqbifps-cuda_cudart-11.7.60/pkg-config:/nix/store/vz5jd0y7wlj94cj4k3abcb94njz3f52d-cuda_nvrtc-11.7.50/pkg-config Let's make sure the ls -l /nix/store/vqqdcl22a5cynh9m7jxnmmhi4ylyy47r-cuda_nvtx-11.7.50/pkg-config
ls -l /nix/store/4qz7zqrxd1anyhhi0fyix8dn4hqbifps-cuda_cudart-11.7.60/pkg-config
ls -l /nix/store/vz5jd0y7wlj94cj4k3abcb94njz3f52d-cuda_nvrtc-11.7.50/pkg-config
total 4
-r--r--r-- 2 root root 249 sij 1 1970 nvToolsExt-11.7.pc
total 8
-r--r--r-- 2 root root 234 sij 1 1970 cuda-11.7.pc
-r--r--r-- 2 root root 239 sij 1 1970 cudart-11.7.pc
total 4
-r--r--r-- 2 root root 259 sij 1 1970 nvrtc-11.7.pc But still,
Oh, then can we make a small ConclusionOverall this is a wonderful experience and thank you very much for insightful advices and directions. I would very much like to continue helping with this. I will study the steps made and try to learn more about it. Among others the nix shell issue. Let me know how to continue from here too <3 |
Ah, I guess we need to move You can do this with
Yes, and these
Yes, and in fact a lot of nixpkgs is (managed) "imperative". A way to think about this is that Bash is the effect system for the purely-functional Nix. Of course, we do prefer declarative solutions
Note that
Yes, and note that this also only works because
The file itself is specific to our cuda packages, but the mechanisms are common in nixpkgs. One is cuda_cudart = prev.cuda_cudart.overrideAttrs (oldAttrs: {
meta = oldAttrs.meta // {
pkgConfigModules = [ "cuda" "cudart" ];
};
}); Here, Extra info that you can totally live without. There are, in fact, two mechanisms at play here, and the second has to do with where
Yes, that would be the reasonable next step once we get
Aha, maybe I should have pointed out that this is somewhat specific to how we handle cudatoolkit, which is unlike "normal" Nix packages: we don't have access to sources, so we download pre-built binaries from nvidia and ensure that they can be used within nixpkgs. We do so in a semi-automated manner by re-using these jsons in the nix expression |
Thinking of it, maybe we could wrap this one up quickly and switch to some issue where you could see the practical benefit of your work 🙃 E.g. #224533 shouldn't be hard to do, but you would be able to measure the impact in gigabytes of saved storage when including something like pytorch in Nix-built docker images I think you can already open a draft PR with the |
I hope it is OK I am writing all the steps which are obvious for you and thank you for the patience and guidance. As you can tell I am a beginner so I know if you see some weird steps that I am making you will let me know I begin like this: cd nixpkgs Create and switch to a new branch for changes: git checkout -b fix-cuda-pkg-config
Great tip! nix edit nixpkgs#cudaPackages.cuda_cudart Now the fix for the missing
Here's the breakdown as I understand:
# Patch the .pc files to replace /usr/local/cuda-XX.Y with $out
sed -i 's|^cudaroot=.*$|cudaroot=${placeholder "out"}|' $out/lib/pkg-config/*.pc
Mhmm this makes sense. Does this also mean, and I'm just thinking out loud, that since
What does this mean actually? Are you refering to a specific with import ./. { ... };
mkShell {
packages = [ pkg-config ];
buildInputs = [ zlib ];
} I do have some reading to do to better understand all this and the difference between pure and impure OK, this is all super interesting, and i am doing it in spare time so I will keep digging at this. Regarding the second issue you referenced I investigated it a bit and will work on it tonight after which I will write about it too. |
Or just
So, by "nixpkgs" I refer to any check out of the nixpkgs git repo. When you write
Your path will likely be different, but the structure will be the same and it'll match that of https://github.com/NixOS/nixpkgs/
Now, after
A required argument of As a result, you can just use
We use both in practice |
Yes, what I meant is that you can test pkg-config like so:
|
Github PR web interface is rather convenient to discuss such details! |
Oh I think I understand now what you mean! |
Describe the bug
We need to patch cuda's pkg-config files
Steps To Reproduce
Steps to reproduce the behavior:
❯ NIXPKGS_ALLOW_UNFREE=1 nix build --impure nixpkgs#cudaPackages.cuda_cudart
❯ cat result/pkg-config/cudart-11.7.pc
cudaroot=/usr/local/cuda-11.7
Expected behavior
Paths relative to outputs
Notify maintainers
@NixOS/cuda-maintainers
The text was updated successfully, but these errors were encountered: