From 872d93eb13f22e8705e03903b65c7eba8b26a99b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Wed, 10 Apr 2024 15:17:39 +0200 Subject: [PATCH 1/3] Add a test for depending on a symlink store path Regression test for https://github.com/NixOS/nix/issues/9579 --- tests/functional/linux-sandbox.sh | 3 +++ tests/functional/symlink-derivation.nix | 36 +++++++++++++++++++++++++ 2 files changed, 39 insertions(+) create mode 100644 tests/functional/symlink-derivation.nix diff --git a/tests/functional/linux-sandbox.sh b/tests/functional/linux-sandbox.sh index ff7d257bd..04209277b 100644 --- a/tests/functional/linux-sandbox.sh +++ b/tests/functional/linux-sandbox.sh @@ -73,3 +73,6 @@ testCert missing fixed-output "$nocert" # Cert in sandbox when ssl-cert-file is set to an existing file testCert present fixed-output "$cert" + +# Symlinks should be added in the sandbox directly and not followed +nix-sandbox-build symlink-derivation.nix diff --git a/tests/functional/symlink-derivation.nix b/tests/functional/symlink-derivation.nix new file mode 100644 index 000000000..17ba37424 --- /dev/null +++ b/tests/functional/symlink-derivation.nix @@ -0,0 +1,36 @@ +with import ./config.nix; + +let + foo_in_store = builtins.toFile "foo" "foo"; + foo_symlink = mkDerivation { + name = "foo-symlink"; + buildCommand = '' + ln -s ${foo_in_store} $out + ''; + }; + symlink_to_not_in_store = mkDerivation { + name = "symlink-to-not-in-store"; + buildCommand = '' + ln -s ${builtins.toString ./.} $out + ''; + }; +in +mkDerivation { + name = "depends-on-symlink"; + buildCommand = '' + ( + set -x + + # `foo_symlink` should be a symlink pointing to `foo_in_store` + [[ -L ${foo_symlink} ]] + [[ $(readlink ${foo_symlink}) == ${foo_in_store} ]] + + # `symlink_to_not_in_store` should be a symlink pointing to `./.`, which + # is not available in the sandbox + [[ -L ${symlink_to_not_in_store} ]] + [[ $(readlink ${symlink_to_not_in_store}) == ${builtins.toString ./.} ]] + (! ls ${symlink_to_not_in_store}/) + ) + echo "Success!" > $out + ''; +} From 913db9f7385b8717d9eaf6269e9f319e78e4c564 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Wed, 10 Apr 2024 15:19:18 +0200 Subject: [PATCH 2/3] Fix permission denied when building symlink derivation which points to a symlink out of the store Bind-mounting symlinks is apparently not possible, which is why the thing was failing. Fortunately, symlinks are small, so we can fallback to copy them at no cost. Fix https://github.com/NixOS/nix/issues/9579 Co-authored-by: Artturin --- src/libstore/build/local-derivation-goal.cc | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index ab66195b8..68c387a9d 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -24,6 +24,7 @@ #include #include +#include #include #include #include @@ -396,20 +397,30 @@ void LocalDerivationGoal::cleanupPostOutputsRegisteredModeNonCheck() static void doBind(const Path & source, const Path & target, bool optional = false) { debug("bind mounting '%1%' to '%2%'", source, target); struct stat st; - if (stat(source.c_str(), &st) == -1) { + + auto bindMount = [&]() { + if (mount(source.c_str(), target.c_str(), "", MS_BIND | MS_REC, 0) == -1) + throw SysError("bind mount from '%1%' to '%2%' failed", source, target); + }; + + if (lstat(source.c_str(), &st) == -1) { if (optional && errno == ENOENT) return; else throw SysError("getting attributes of path '%1%'", source); } - if (S_ISDIR(st.st_mode)) + if (S_ISDIR(st.st_mode)) { createDirs(target); - else { + bindMount(); + } else if (S_ISLNK(st.st_mode)) { + // Symlinks can (apparently) not be bind-mounted, so just copy it + createDirs(dirOf(target)); + copyFile(source, target, /* andDelete */ false); + } else { createDirs(dirOf(target)); writeFile(target, ""); + bindMount(); } - if (mount(source.c_str(), target.c_str(), "", MS_BIND | MS_REC, 0) == -1) - throw SysError("bind mount from '%1%' to '%2%' failed", source, target); }; #endif From ae4737294e91ab93526612b17950e1bc4f0b47f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Wed, 10 Apr 2024 15:17:56 +0200 Subject: [PATCH 3/3] doBind: Use our own lstat wrapper Doesn't change much, but brings a bit more consistency to the code --- src/libstore/build/local-derivation-goal.cc | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 68c387a9d..db12af810 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -24,7 +24,6 @@ #include #include -#include #include #include #include @@ -396,19 +395,21 @@ void LocalDerivationGoal::cleanupPostOutputsRegisteredModeNonCheck() #if __linux__ static void doBind(const Path & source, const Path & target, bool optional = false) { debug("bind mounting '%1%' to '%2%'", source, target); - struct stat st; auto bindMount = [&]() { if (mount(source.c_str(), target.c_str(), "", MS_BIND | MS_REC, 0) == -1) throw SysError("bind mount from '%1%' to '%2%' failed", source, target); }; - if (lstat(source.c_str(), &st) == -1) { - if (optional && errno == ENOENT) + auto maybeSt = maybeLstat(source); + if (!maybeSt) { + if (optional) return; else throw SysError("getting attributes of path '%1%'", source); } + auto st = *maybeSt; + if (S_ISDIR(st.st_mode)) { createDirs(target); bindMount();