Merge "libexpr: rectify filtering logic for filter builtins under chroot stores" into main
This commit is contained in:
Raito Bezarius
@@ -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 <https://github.com/NixOS/nixpkgs/pull/369694>.
|
||||
+16
-9
@@ -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,
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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));
|
||||
};
|
||||
|
||||
@@ -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}";
|
||||
}
|
||||
@@ -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\""
|
||||
Reference in New Issue
Block a user