Skip to content
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

cmake/setup-hook.sh: Don't skip build-RPATH #108496

Merged
merged 3 commits into from Jul 11, 2022
Merged

Conversation

knedlsepp
Copy link
Member

@knedlsepp knedlsepp commented Jan 5, 2021

Motivation for this change

This should simplify using nix-shell -A or nix develop to develop
CMake based projects. CMake features a mechanism to use a different
RPATH for all executables in the build directory and only rewrite these
RPATHs on installation. This makes it possible to run executables
already from the build directory without having to set LD_LIBRARY_PATH.
This should simplify the checkPhase for cmake based projects and
hopefully not break anything.

Fixes: #22060

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to go through staging.

@knedlsepp
Copy link
Member Author

This needs to go through staging.

👍 Currently stuck at building bluez with this anyway.

@r-burns
Copy link
Contributor

r-burns commented Jan 25, 2021

Darwin stdenv still builds with this fyi

@knedlsepp
Copy link
Member Author

Darwin stdenv still builds with this fyi

Thanks for the heads up. I'll try to figure out what was my issue with the bluez build then we can move this forward.

@stale
Copy link

stale bot commented Jul 26, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 26, 2021
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 26, 2021
@knedlsepp knedlsepp changed the base branch from master to staging July 26, 2021 16:21
@github-actions github-actions bot removed the 6.topic: GNOME GNOME desktop environment and its underlying platform label Jul 26, 2021
@knedlsepp
Copy link
Member Author

After #123011 was merged I can build bluez and continue with this.

@knedlsepp knedlsepp changed the title [WIP] cmake/setup-hook.sh: Don't skip build-RPATH cmake/setup-hook.sh: Don't skip build-RPATH Jul 26, 2021
@knedlsepp knedlsepp force-pushed the cmake-rpath branch 2 times, most recently from 405e616 to 7affe0c Compare July 26, 2021 16:55
@github-actions github-actions bot added the 6.topic: GNOME GNOME desktop environment and its underlying platform label Jul 26, 2021
@knedlsepp
Copy link
Member Author

For reference. The last attempt was reverted because of #15881

@knedlsepp knedlsepp marked this pull request as ready for review July 26, 2021 19:09
@knedlsepp knedlsepp requested a review from ttuegel as a code owner July 26, 2021 19:09
@knedlsepp
Copy link
Member Author

cc @ttuegel You once tried to drop the SKIP_BUILD_RPATH in 8ec5adc, which had to be reverted in d5d4606 because of a broken Darwin stdenv.
I'd like to try again to get rid of it since it's somewhat of an inconvenience that by default tests in cmake builds don't work unless you set LD_LIBRARY_PATH or disable the SKIP_BUILD_RPATH.

I tried nixpkgs-review on this, but it requires downloading 128GB of stuff, so I just rebuilt the projects on a best effort basis and everything seems to work fine on x86_64-linux.

@ttuegel ttuegel merged commit e1e0d5d into NixOS:staging Jul 11, 2022
Staging automation moved this from Needs review to Done Jul 11, 2022
@trofi
Copy link
Contributor

trofi commented Jul 11, 2022

Bisect says f7414e2 cmake/setup-hook.sh: Don't skip build-RPATH broke metis in staging and staging-next:

$ nix build -f. -L metis
...
metis> CMake Error at programs/cmake_install.cmake:55 (file):
metis>   file RPATH_CHANGE could not write new RPATH:
metis>   to the file:
metis>     /nix/store/9dvvz70jchfzg066yfscbxb3j0czi6h0-metis-5.1.0/bin/gpmetis
metis>   The current RUNPATH is:
metis>     /nix/store/9dvvz70jchfzg066yfscbxb3j0czi6h0-metis-5.1.0/lib64:/nix/store/9dvvz70jchfzg066yfscbxb3j0czi6h0-metis-5.1.0/lib:/nix/store/mhhlymrg2m70r8h94cwhv2d7a0c8l7g6-glibc-2.34-210/lib:/nix/store/xjmm9xg5sq47q63cbf447qalz9fmwp0v-gcc-11.3.0-lib/lib
metis>   which does not contain:
metis>     /home/karypis/local/lib:
metis>   as was expected.
metis> Call Stack (most recent call first):
metis>   cmake_install.cmake:49 (include)
metis>
metis> make: *** [Makefile:100: install] Error 1

@Artturin
Copy link
Member

Metis hasnt been updated in nixpkgs since 2015 so either lets mark it broken or try updating it or set the cmake flag removed here for it

@danshapero also @jtojnar due to him having created an issue in https://github.com/KarypisLab/METIS

@Artturin
Copy link
Member

KarypisLab/METIS@521a2c3 fixed it

@trofi
Copy link
Contributor

trofi commented Jul 12, 2022

Bisect says f7414e2 cmake/setup-hook.sh: Don't skip build-RPATH broke spirv-llvm-translator in staging and staging-next (a popular dependency):

$ nix build -f. spirv-llvm-translator -L
...
SPIRV-LLVM-Translator-unstable> post-installation fixup
SPIRV-LLVM-Translator-unstable> shrinking RPATHs of ELF executables and libraries in /nix/store/8x0j0ivggqri5ncdp2lmw4ngpr12gaqj-SPIRV-LLVM-Translator-unstable-2022-05-04
SPIRV-LLVM-Translator-unstable> shrinking /nix/store/8x0j0ivggqri5ncdp2lmw4ngpr12gaqj-SPIRV-LLVM-Translator-unstable-2022-05-04/lib/libLLVMSPIRVLib.so.11
SPIRV-LLVM-Translator-unstable> shrinking /nix/store/8x0j0ivggqri5ncdp2lmw4ngpr12gaqj-SPIRV-LLVM-Translator-unstable-2022-05-04/bin/llvm-spirv
SPIRV-LLVM-Translator-unstable> strip is /nix/store/6mx62f4mdv5y1awi7y2mp23a76gg0ba7-gcc-wrapper-11.3.0/bin/strip
SPIRV-LLVM-Translator-unstable> stripping (with command strip and flags -S) in /nix/store/8x0j0ivggqri5ncdp2lmw4ngpr12gaqj-SPIRV-LLVM-Translator-unstable-2022-05-04/lib  /nix/store/8x0j0ivggqri5ncdp2lmw4ngpr12gaqj-SPIRV-LLVM-Translator-unstable-2022-05-04/bin
SPIRV-LLVM-Translator-unstable> patching script interpreter paths in /nix/store/8x0j0ivggqri5ncdp2lmw4ngpr12gaqj-SPIRV-LLVM-Translator-unstable-2022-05-04
SPIRV-LLVM-Translator-unstable> checking for references to /build/ in /nix/store/8x0j0ivggqri5ncdp2lmw4ngpr12gaqj-SPIRV-LLVM-Translator-unstable-2022-05-04...
SPIRV-LLVM-Translator-unstable> RPATH of binary /nix/store/8x0j0ivggqri5ncdp2lmw4ngpr12gaqj-SPIRV-LLVM-Translator-unstable-2022-05-04/bin/llvm-spirv contains a forbidden reference to /build/

@MatthewCroughan
Copy link
Contributor

I can confirm that spirv-llvm-translator was broken on staging-next, as I ran into this specific issue when trying to use staging-next to cross compile an sdImage for riscv64.

@trofi
Copy link
Contributor

trofi commented Jul 12, 2022

@Artturin
Copy link
Member

#181243

@Artturin
Copy link
Member

files which still set LD_LIBRARY_PATH to pwd

pkgs/development/compilers/ldc/generic.nix
pkgs/development/libraries/hotpatch/default.nix
pkgs/development/libraries/grpc/default.nix
pkgs/development/libraries/eccodes/default.nix
rg "LD_LIBRARY_PATH.+pwd" --files-with-matches | xargs rg "cmake" --files-with-matches

@trofi
Copy link
Contributor

trofi commented Jul 14, 2022

Skimming through staging-next hydra failures there still seem to be more of these:

A few mor will probably pop up when qt fixes will get grabbed by hydra.

@Artturin
Copy link
Member

#181540

@trofi
Copy link
Contributor

trofi commented Jul 16, 2022

@vcunat
Copy link
Member

vcunat commented Jul 19, 2022

@Artturin
Copy link
Member

#182097

@strager
Copy link
Contributor

strager commented Jul 22, 2022

My program (quick-lint-js) successfully installs with Nix on Linux. However, due to this patch, the installed executable can't find libstdc++.so.6.

I don't know if this bug happens with other programs.

How should I work around this problem?


Notice how the gcc-11.3.0-lib runpath entry is missing from the executable after this patch:

Nixpkgs commit e1e0d5d (after this patch was merged):

$ nix-shell -I ~/Projects/ -p quick-lint-js --run 'quick-lint-js --version'
quick-lint-js: error while loading shared libraries: libstdc++.so.6: cannot open shared object file: No such file or directory

$ readelf -a "$(nix-shell -I ~/Projects/ -p quick-lint-js --run 'which quick-lint-js')" | egrep -i 'r(un)?path'
 0x000000000000001d (RUNPATH)            Library runpath: [/nix/store/mhhlymrg2m70r8h94cwhv2d7a0c8l7g6-glibc-2.34-210/lib]

Nixpkgs commit de1ff46 (before this patch was merged):

$ nix-shell -I ~/Projects/ -p quick-lint-js --run 'quick-lint-js --version'
quick-lint-js version 2.6.0

$ readelf -a "$(nix-shell -I ~/Projects/ -p quick-lint-js --run 'which quick-lint-js')" | egrep -i 'r(un)?path'
 0x000000000000001d (RUNPATH)            Library runpath: [/nix/store/xjmm9xg5sq47q63cbf447qalz9fmwp0v-gcc-11.3.0-lib/lib:/nix/store/mhhlymrg2m70r8h94cwhv2d7a0c8l7g6-glibc-2.34-210/lib]

@r-burns
Copy link
Contributor

r-burns commented Jul 22, 2022

@strager sorry to hear that, I don't see why this change should have broken that. For now, you can manually revert to the old behavior on a per-package basis, which seems to fix the rpath:

--- a/pkgs/development/tools/quick-lint-js/default.nix
+++ b/pkgs/development/tools/quick-lint-js/default.nix
@@ -15,6 +15,10 @@ stdenv.mkDerivation rec {
   nativeBuildInputs = [ cmake ninja ];
   doCheck = true;
 
+  cmakeFlags = [
+    "-DCMAKE_SKIP_BUILD_RPATH=ON"
+  ];

Turning on CMAKE_BUILD_WITH_INSTALL_RPATH also seems to fix it, so it seems there is something wrong with the rpath rewriting for this package.

@Artturin
Copy link
Member

@strager sorry to hear that, I don't see why this change should have broken that. For now, you can manually revert to the old behavior on a per-package basis, which seems to fix the rpath:

--- a/pkgs/development/tools/quick-lint-js/default.nix
+++ b/pkgs/development/tools/quick-lint-js/default.nix
@@ -15,6 +15,10 @@ stdenv.mkDerivation rec {
   nativeBuildInputs = [ cmake ninja ];
   doCheck = true;
 
+  cmakeFlags = [
+    "-DCMAKE_SKIP_BUILD_RPATH=ON"
+  ];

Turning on CMAKE_BUILD_WITH_INSTALL_RPATH also seems to fix it, so it seems there is something wrong with the rpath rewriting for this package.

i tried autoPatchelfHook and first got

quick-lint-js> setting interpreter of /nix/store/1yn1yg8xyc9w1wv897444hasj5j1zlh1-quick-lint-js-2.6.0/bin/quick-lint-js
quick-lint-js> searching for dependencies of /nix/store/1yn1yg8xyc9w1wv897444hasj5j1zlh1-quick-lint-js-2.6.0/bin/quick-lint-js
quick-lint-js>     libstdc++.so.6 -> not found!
quick-lint-js> auto-patchelf: 1 dependencies could not be satisfied
quick-lint-js> error: auto-patchelf could not satisfy dependency libstdc++.so.6 wanted by /nix/store/1yn1yg8xyc9w1wv897444hasj5j1zlh1-quick-lint-js-2.6.0/bin/quick-lint-js
quick-lint-js> auto-patchelf failed to find all the required dependencies.
quick-lint-js> Add the missing dependencies to --libs or use `--ignore-missing="foo.so.1 bar.so etc.so"`.

then added stdenv.cc.cc.lib to buildInputs and then it works

setting interpreter of /nix/store/nipi33sz18117nvzpjz2svg4cm813dym-quick-lint-js-2.6.0/bin/quick-lint-js
searching for dependencies of /nix/store/nipi33sz18117nvzpjz2svg4cm813dym-quick-lint-js-2.6.0/bin/quick-lint-js
    libstdc++.so.6 -> found: /nix/store/xjmm9xg5sq47q63cbf447qalz9fmwp0v-gcc-11.3.0-lib/lib
setting RPATH to: /nix/store/xjmm9xg5sq47q63cbf447qalz9fmwp0v-gcc-11.3.0-lib/lib
auto-patchelf: 0 dependencies could not be satisfied

autoPatchelfHook is usually used for random binaries from other distros so its not the proper fix for this case

Looking in to finding a proper fix

strager added a commit to quick-lint/quick-lint-js that referenced this pull request Jul 24, 2022
Our Nix builds are broken due to upstream changes [1]. Disable the
failing code for now to make CI green again.

[1] NixOS/nixpkgs#108496
strager added a commit to quick-lint/quick-lint-js that referenced this pull request Jul 24, 2022
Our Nix builds are broken due to upstream changes [1]. Disable the
failing code for now to make CI green again.

[1] NixOS/nixpkgs#108496
@strager strager mentioned this pull request Sep 7, 2022
13 tasks
strager added a commit to strager/nixpkgs that referenced this pull request Sep 7, 2022
quick-lint-js fails to run after installation:

    $ quick-lint-js --version
    quick-lint-js: error while loading shared libraries: libstdc++.so.6: cannot open shared object file: No such file or directory

Cause: NixOS#108496 (comment)

Work around this error.
strager added a commit to quick-lint/quick-lint-js that referenced this pull request Sep 22, 2022
quick-lint-js fails to run after installation:

    $ quick-lint-js --version
    quick-lint-js: error while loading shared libraries: libstdc++.so.6: cannot open shared object file: No such file or directory

Cause: NixOS/nixpkgs#108496 (comment)

Work around this error.

This workaround has landed in Nixpkgs upstream:
NixOS/nixpkgs#190091
@chkno chkno mentioned this pull request Sep 25, 2023
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Staging
  
Done
Development

Successfully merging this pull request may close these issues.

No in-build execution for CMake managed builds