diff --git a/doc/manual/rl-next/filterSource-filter-chroot-stores.md b/doc/manual/rl-next/filterSource-filter-chroot-stores.md new file mode 100644 index 000000000..206e5d9c3 --- /dev/null +++ b/doc/manual/rl-next/filterSource-filter-chroot-stores.md @@ -0,0 +1,19 @@ +--- +synopsis: fix usage of `builtins.filterSource` and `builtins.path` with the filter argument when using chroot stores +issues: [11503] +credits: [lily, alois31, horrors] +category: Fixes +--- + +The semantics of `builtins.filterSource` (and the `filter` argument for +`builtins.path`) have been adjusted regarding how paths inside the Nix store +are handled. + +Previously, when evaluating whether a path should be included, the filtering +function received the **physical path** if the source was inside the chroot store. + +Now, it receives the **logical path** instead. + +This ensures consistency in path handling and avoids potential +misinterpretations of paths within the evaluator, which led to various fallouts +mentioned in . diff --git a/lix/libexpr/primops.cc b/lix/libexpr/primops.cc index 7e6bd459e..13f2a1b6b 100644 --- a/lix/libexpr/primops.cc +++ b/lix/libexpr/primops.cc @@ -1508,31 +1508,38 @@ static void addPath( // FIXME: handle CA derivation outputs (where path needs to // be rewritten to the actual output). auto rewrites = state.aio.blockOn(state.ctx.paths.realiseContext(context)); - path = state.ctx.paths.toRealPath(rewriteStrings(path, rewrites), context); + path = rewriteStrings(path, rewrites); + + Path realPath = path; StorePathSet refs; + // If the path is in the store, it can mean either a physical path or a logical path in a + // chroot store. Query the chroot store for its presence to find out which is the case. if (state.ctx.store->isInStore(path)) { try { auto [storePath, subPath] = state.ctx.store->toStorePath(path); // FIXME: we should scanForReferences on the path before adding it refs = state.ctx.store->queryPathInfo(storePath)->references; - path = state.ctx.store->toRealPath(storePath) + subPath; + realPath = state.ctx.store->toRealPath(path); } catch (Error &) { // FIXME: should be InvalidPathError } } - path = evalSettings.pureEval && expectedHash - ? path - : state.ctx.paths.checkSourcePath(CanonPath(path)).canonical().abs(); + realPath = evalSettings.pureEval && expectedHash + ? realPath + : state.ctx.paths.checkSourcePath(CanonPath(realPath)).canonical().abs(); - PathFilter filter = filterFun ? ([&](const Path & path) { - auto st = lstat(path); + PathFilter filter = filterFun ? ([&](const Path & p) { + auto st = lstat(p); /* Call the filter function. The first argument is the path, the second is a string indicating the type of the file. */ Value arg1; - arg1.mkString(path); + if (isInDir(p, realPath)) + arg1.mkString(path + "/" + std::string(p, realPath.size() + 1)); + else + arg1.mkString(p); Value arg2; arg2.mkString( @@ -1559,7 +1566,7 @@ static void addPath( if (!expectedHash || !state.ctx.store->isValidPath(*expectedStorePath)) { auto dstPath = state.aio.blockOn(fetchToStore( *state.ctx.store, - state.ctx.paths.checkSourcePath(CanonPath(path)), + state.ctx.paths.checkSourcePath(CanonPath(realPath)), name, method, &filter, diff --git a/tests/functional/filter-source.nix b/tests/functional/filter-source.nix index 907163639..7d9f712b4 100644 --- a/tests/functional/filter-source.nix +++ b/tests/functional/filter-source.nix @@ -1,3 +1,5 @@ +{ filterin }: + with import ./config.nix; mkDerivation { @@ -6,7 +8,8 @@ mkDerivation { input = let filter = path: type: type != "symlink" + && (builtins.substring 0 (builtins.stringLength filterin) (builtins.toString path) == filterin) && baseNameOf path != "foo" && !((import ./lang/lib.nix).hasSuffix ".bak" (baseNameOf path)); - in builtins.filterSource filter ((builtins.getEnv "TEST_ROOT") + "/filterin"); + in builtins.filterSource filter filterin; } diff --git a/tests/functional/filter-source.sh b/tests/functional/filter-source.sh index ba34d2eac..864a65395 100644 --- a/tests/functional/filter-source.sh +++ b/tests/functional/filter-source.sh @@ -18,8 +18,18 @@ checkFilter() { test ! -L $1/link } -nix-build ./filter-source.nix -o $TEST_ROOT/filterout1 +nix-build ./filter-source.nix --argstr filterin $TEST_ROOT/filterin -o $TEST_ROOT/filterout1 checkFilter $TEST_ROOT/filterout1 -nix-build ./path.nix -o $TEST_ROOT/filterout2 +nix-build ./path.nix --argstr filterin $TEST_ROOT/filterin -o $TEST_ROOT/filterout2 checkFilter $TEST_ROOT/filterout2 + +if canUseSandbox; then + filterinStorePath=$(nix-store --add-fixed --recursive sha256 $TEST_ROOT/filterin --store $TEST_HOME/.local/share/nix/root) + + nix-build ./filter-source.nix --argstr filterin $filterinStorePath -o $TEST_ROOT/filterout3 --store $TEST_HOME/.local/share/nix/root --builders 'auto - - 1 1' + checkFilter $TEST_ROOT/filterout3 + + nix-build ./path.nix --argstr filterin $filterinStorePath -o $TEST_ROOT/filterout4 --store $TEST_HOME/.local/share/nix/root --builders 'auto - - 1 1' + checkFilter $TEST_ROOT/filterout4 +fi diff --git a/tests/functional/meson.build b/tests/functional/meson.build index ff6116329..1c15e079c 100644 --- a/tests/functional/meson.build +++ b/tests/functional/meson.build @@ -200,7 +200,8 @@ functional_tests_scripts = [ 'substitute-truncated-nar.sh', 'regression-484.sh', 'regression-reference-checks.sh', - 'external-commands.sh' + 'external-commands.sh', + 'redirected-filter-source.sh', ] # Plugin tests require shared libraries support. diff --git a/tests/functional/path.nix b/tests/functional/path.nix index 883c3c41b..2d87c4745 100644 --- a/tests/functional/path.nix +++ b/tests/functional/path.nix @@ -1,3 +1,5 @@ +{ filterin }: + with import ./config.nix; mkDerivation { @@ -5,9 +7,10 @@ mkDerivation { builder = builtins.toFile "builder" "ln -s $input $out"; input = builtins.path { - path = ((builtins.getEnv "TEST_ROOT") + "/filterin"); + path = filterin; filter = path: type: - type != "symlink" + type != "symlink" + && (builtins.substring 0 (builtins.stringLength filterin) (builtins.toString path) == filterin) && baseNameOf path != "foo" && !((import ./lang/lib.nix).hasSuffix ".bak" (baseNameOf path)); }; diff --git a/tests/functional/redirected-filter-source.nix b/tests/functional/redirected-filter-source.nix new file mode 100644 index 000000000..88d206ac4 --- /dev/null +++ b/tests/functional/redirected-filter-source.nix @@ -0,0 +1,17 @@ +{ path }: +let + pathStr = builtins.toString path; +in +builtins.path { + path = builtins.toString path; + name = "src"; + filter = + fullPath: type: + let + obtained = builtins.substring 0 (builtins.stringLength pathStr) fullPath; + in + if obtained == pathStr then + true + else + builtins.throw "bug detected, expected: ${pathStr}, got: ${obtained}"; +} diff --git a/tests/functional/redirected-filter-source.sh b/tests/functional/redirected-filter-source.sh new file mode 100644 index 000000000..cd9824b25 --- /dev/null +++ b/tests/functional/redirected-filter-source.sh @@ -0,0 +1,21 @@ +# This exercises some of the behaviors we expect under redirected stores w.r.t. to `filterSource`. +# They serve as anti regression testing towards the `lib.fileset` breakage in nixpkgs. +# See: https://github.com/NixOS/nixpkgs/pull/369694. + +source common.sh + +TEST_STORE="$TEST_ROOT/teststore" +TEST_DIR="$TEST_ROOT/testsrc" + +mkdir -p "$TEST_STORE" +mkdir -p "$TEST_DIR" +chmod -R u+w $TEST_STORE +rm -fr $TEST_STORE + +cp redirected-filter-source.nix "$TEST_DIR/default.nix" + +nix-instantiate --eval --store "$TEST_STORE" "$TEST_DIR" --arg path "\"$TEST_DIR\"" + +path_in_store="$(nix-store --add "$TEST_DIR")" +nix-store --store "$TEST_STORE" --add "$TEST_DIR" > /dev/null +nix-instantiate --eval --store "$TEST_STORE" "$TEST_DIR" --arg path "builtins.storePath \"$path_in_store\""