lib.fileset: Minor changes from feedback

Co-authored-by: Robert Hensing <robert@roberthensing.nl>
Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
This commit is contained in:
Silvan Mosberger 2023-09-15 00:08:04 +02:00
parent fe6c1539cc
commit 94e103ee3f
4 changed files with 52 additions and 58 deletions

View File

@ -50,11 +50,11 @@ An attribute set with these values:
- `_internalBaseRoot` (path): - `_internalBaseRoot` (path):
The filesystem root of `_internalBase`, same as `(lib.path.splitRoot _internalBase).root`. The filesystem root of `_internalBase`, same as `(lib.path.splitRoot _internalBase).root`.
This is here because this needs to be computed anyways, and this computation shouldn't be duplicated. This is here because this needs to be computed anyway, and this computation shouldn't be duplicated.
- `_internalBaseComponents` (list of strings): - `_internalBaseComponents` (list of strings):
The path components of `_internalBase`, same as `lib.path.subpath.components (lib.path.splitRoot _internalBase).subpath`. The path components of `_internalBase`, same as `lib.path.subpath.components (lib.path.splitRoot _internalBase).subpath`.
This is here because this needs to be computed anyways, and this computation shouldn't be duplicated. This is here because this needs to be computed anyway, and this computation shouldn't be duplicated.
- `_internalTree` ([filesetTree](#filesettree)): - `_internalTree` ([filesetTree](#filesettree)):
A tree representation of all included files under `_internalBase`. A tree representation of all included files under `_internalBase`.

View File

@ -36,6 +36,10 @@ let
cleanSourceWith cleanSourceWith
; ;
inherit (lib.trivial)
pipe
;
in { in {
/* /*
@ -111,7 +115,7 @@ in {
Paths in [strings](https://nixos.org/manual/nix/stable/language/values.html#type-string), including Nix store paths, cannot be passed as `root`. Paths in [strings](https://nixos.org/manual/nix/stable/language/values.html#type-string), including Nix store paths, cannot be passed as `root`.
`root` has to be a directory. `root` has to be a directory.
<!-- Ignore the indentation here, this is a nixdoc rendering bug that needs to be fixed --> <!-- Ignore the indentation here, this is a nixdoc rendering bug that needs to be fixed: https://github.com/nix-community/nixdoc/issues/75 -->
:::{.note} :::{.note}
Changing `root` only affects the directory structure of the resulting store path, it does not change which files are added to the store. Changing `root` only affects the directory structure of the resulting store path, it does not change which files are added to the store.
The only way to change which files get added to the store is by changing the `fileset` attribute. The only way to change which files get added to the store is by changing the `fileset` attribute.
@ -124,7 +128,7 @@ The only way to change which files get added to the store is by changing the `fi
This argument can also be a path, This argument can also be a path,
which gets [implicitly coerced to a file set](#sec-fileset-path-coercion). which gets [implicitly coerced to a file set](#sec-fileset-path-coercion).
<!-- Ignore the indentation here, this is a nixdoc rendering bug that needs to be fixed --> <!-- Ignore the indentation here, this is a nixdoc rendering bug that needs to be fixed: https://github.com/nix-community/nixdoc/issues/75 -->
:::{.note} :::{.note}
If a directory does not recursively contain any file, it is omitted from the store path contents. If a directory does not recursively contain any file, it is omitted from the store path contents.
::: :::
@ -134,18 +138,18 @@ If a directory does not recursively contain any file, it is omitted from the sto
}: }:
let let
# We cannot rename matched attribute arguments, so let's work around it with an extra `let in` statement # We cannot rename matched attribute arguments, so let's work around it with an extra `let in` statement
maybeFileset = fileset; filesetArg = fileset;
in in
let let
fileset = _coerce "lib.fileset.toSource: `fileset`" maybeFileset; fileset = _coerce "lib.fileset.toSource: `fileset`" filesetArg;
rootFilesystemRoot = (splitRoot root).root; rootFilesystemRoot = (splitRoot root).root;
filesetFilesystemRoot = (splitRoot fileset._internalBase).root; filesetFilesystemRoot = (splitRoot fileset._internalBase).root;
filter = _toSourceFilter fileset; sourceFilter = _toSourceFilter fileset;
in in
if ! isPath root then if ! isPath root then
if isStringLike root then if isStringLike root then
throw '' throw ''
lib.fileset.toSource: `root` "${toString root}" is a string-like value, but it should be a path instead. lib.fileset.toSource: `root` ("${toString root}") is a string-like value, but it should be a path instead.
Paths in strings are not supported by `lib.fileset`, use `lib.sources` or derivations instead.'' Paths in strings are not supported by `lib.fileset`, use `lib.sources` or derivations instead.''
else else
throw '' throw ''
@ -154,29 +158,29 @@ If a directory does not recursively contain any file, it is omitted from the sto
# See also ../path/README.md # See also ../path/README.md
else if rootFilesystemRoot != filesetFilesystemRoot then else if rootFilesystemRoot != filesetFilesystemRoot then
throw '' throw ''
lib.fileset.toSource: Filesystem roots are not the same for `fileset` and `root` "${toString root}": lib.fileset.toSource: Filesystem roots are not the same for `fileset` and `root` ("${toString root}"):
`root`: root "${toString rootFilesystemRoot}" `root`: root "${toString rootFilesystemRoot}"
`fileset`: root "${toString filesetFilesystemRoot}" `fileset`: root "${toString filesetFilesystemRoot}"
Different roots are not supported.'' Different roots are not supported.''
else if ! pathExists root then else if ! pathExists root then
throw '' throw ''
lib.fileset.toSource: `root` ${toString root} does not exist.'' lib.fileset.toSource: `root` (${toString root}) does not exist.''
else if pathType root != "directory" then else if pathType root != "directory" then
throw '' throw ''
lib.fileset.toSource: `root` ${toString root} is a file, but it should be a directory instead. Potential solutions: lib.fileset.toSource: `root` (${toString root}) is a file, but it should be a directory instead. Potential solutions:
- If you want to import the file into the store _without_ a containing directory, use string interpolation or `builtins.path` instead of this function. - If you want to import the file into the store _without_ a containing directory, use string interpolation or `builtins.path` instead of this function.
- If you want to import the file into the store _with_ a containing directory, set `root` to the containing directory, such as ${toString (dirOf root)}, and set `fileset` to the file path.'' - If you want to import the file into the store _with_ a containing directory, set `root` to the containing directory, such as ${toString (dirOf root)}, and set `fileset` to the file path.''
else if ! hasPrefix root fileset._internalBase then else if ! hasPrefix root fileset._internalBase then
throw '' throw ''
lib.fileset.toSource: `fileset` could contain files in ${toString fileset._internalBase}, which is not under the `root` ${toString root}. Potential solutions: lib.fileset.toSource: `fileset` could contain files in ${toString fileset._internalBase}, which is not under the `root` (${toString root}). Potential solutions:
- Set `root` to ${toString fileset._internalBase} or any directory higher up. This changes the layout of the resulting store path. - Set `root` to ${toString fileset._internalBase} or any directory higher up. This changes the layout of the resulting store path.
- Set `fileset` to a file set that cannot contain files outside the `root` ${toString root}. This could change the files included in the result.'' - Set `fileset` to a file set that cannot contain files outside the `root` (${toString root}). This could change the files included in the result.''
else else
builtins.seq filter builtins.seq sourceFilter
cleanSourceWith { cleanSourceWith {
name = "source"; name = "source";
src = root; src = root;
inherit filter; filter = sourceFilter;
}; };
/* /*
@ -209,8 +213,8 @@ If a directory does not recursively contain any file, it is omitted from the sto
# This argument can also be a path, # This argument can also be a path,
# which gets [implicitly coerced to a file set](#sec-fileset-path-coercion). # which gets [implicitly coerced to a file set](#sec-fileset-path-coercion).
fileset2: fileset2:
let _unionMany
filesets = _coerceMany "lib.fileset.union" [ (_coerceMany "lib.fileset.union" [
{ {
context = "first argument"; context = "first argument";
value = fileset1; value = fileset1;
@ -219,9 +223,7 @@ If a directory does not recursively contain any file, it is omitted from the sto
context = "second argument"; context = "second argument";
value = fileset2; value = fileset2;
} }
]; ]);
in
_unionMany filesets;
/* /*
The file set containing all files that are in any of the given file sets. The file set containing all files that are in any of the given file sets.
@ -260,25 +262,20 @@ If a directory does not recursively contain any file, it is omitted from the sto
# The elements can also be paths, # The elements can also be paths,
# which get [implicitly coerced to file sets](#sec-fileset-path-coercion). # which get [implicitly coerced to file sets](#sec-fileset-path-coercion).
filesets: filesets:
let if ! isList filesets then
# We cannot rename matched attribute arguments, so let's work around it with an extra `let in` statement throw "lib.fileset.unions: Expected argument to be a list, but got a ${typeOf filesets}."
maybeFilesets = filesets; else if filesets == [ ] then
in # TODO: This could be supported, but requires an extra internal representation for the empty file set, which would be special for not having a base path.
let
# Annotate the elements with context, used by _coerceMany for better errors
annotated = imap0 (i: el: {
context = "element ${toString i} of the argument";
value = el;
}) maybeFilesets;
filesets = _coerceMany "lib.fileset.unions" annotated;
in
if ! isList maybeFilesets then
throw "lib.fileset.unions: Expected argument to be a list, but got a ${typeOf maybeFilesets}."
else if maybeFilesets == [ ] then
# TODO: This could be supported, but requires an extra internal representation for the empty file set
throw "lib.fileset.unions: Expected argument to be a list with at least one element, but it contains no elements." throw "lib.fileset.unions: Expected argument to be a list with at least one element, but it contains no elements."
else else
_unionMany filesets; pipe filesets [
# Annotate the elements with context, used by _coerceMany for better errors
(imap0 (i: el: {
context = "element ${toString i}";
value = el;
}))
(_coerceMany "lib.fileset.unions")
_unionMany
];
} }

View File

@ -135,14 +135,14 @@ rec {
else if ! isPath value then else if ! isPath value then
if isStringLike value then if isStringLike value then
throw '' throw ''
${context} "${toString value}" is a string-like value, but it should be a path instead. ${context} ("${toString value}") is a string-like value, but it should be a path instead.
Paths represented as strings are not supported by `lib.fileset`, use `lib.sources` or derivations instead.'' Paths represented as strings are not supported by `lib.fileset`, use `lib.sources` or derivations instead.''
else else
throw '' throw ''
${context} is of type ${typeOf value}, but it should be a path instead.'' ${context} is of type ${typeOf value}, but it should be a path instead.''
else if ! pathExists value then else if ! pathExists value then
throw '' throw ''
${context} ${toString value} does not exist.'' ${context} (${toString value}) does not exist.''
else else
_singleton value; _singleton value;
@ -341,9 +341,6 @@ rec {
# The common base path assembled from a filesystem root and the common components # The common base path assembled from a filesystem root and the common components
commonBase = append first._internalBaseRoot (join commonBaseComponents); commonBase = append first._internalBaseRoot (join commonBaseComponents);
# The number of path components common to all base paths
commonBaseComponentsCount = length commonBaseComponents;
# A list of filesetTree's that all have the same base path # A list of filesetTree's that all have the same base path
# This is achieved by nesting the trees into the components they have over the common base path # This is achieved by nesting the trees into the components they have over the common base path
# E.g. `union /foo/bar /foo/baz` has the base path /foo # E.g. `union /foo/bar /foo/baz` has the base path /foo
@ -352,7 +349,7 @@ rec {
# Therefore allowing combined operations over them. # Therefore allowing combined operations over them.
trees = map (fileset: trees = map (fileset:
setAttrByPath setAttrByPath
(drop commonBaseComponentsCount fileset._internalBaseComponents) (drop (length commonBaseComponents) fileset._internalBaseComponents)
fileset._internalTree fileset._internalTree
) filesets; ) filesets;

View File

@ -236,7 +236,7 @@ checkFileset() (
#### Error messages ##### #### Error messages #####
# Absolute paths in strings cannot be passed as `root` # Absolute paths in strings cannot be passed as `root`
expectFailure 'toSource { root = "/nix/store/foobar"; fileset = ./.; }' 'lib.fileset.toSource: `root` "/nix/store/foobar" is a string-like value, but it should be a path instead. expectFailure 'toSource { root = "/nix/store/foobar"; fileset = ./.; }' 'lib.fileset.toSource: `root` \("/nix/store/foobar"\) is a string-like value, but it should be a path instead.
\s*Paths in strings are not supported by `lib.fileset`, use `lib.sources` or derivations instead.' \s*Paths in strings are not supported by `lib.fileset`, use `lib.sources` or derivations instead.'
# Only paths are accepted as `root` # Only paths are accepted as `root`
@ -246,18 +246,18 @@ expectFailure 'toSource { root = 10; fileset = ./.; }' 'lib.fileset.toSource: `r
mkdir -p {foo,bar}/mock-root mkdir -p {foo,bar}/mock-root
expectFailure 'with ((import <nixpkgs/lib>).extend (import <nixpkgs/lib/fileset/mock-splitRoot.nix>)).fileset; expectFailure 'with ((import <nixpkgs/lib>).extend (import <nixpkgs/lib/fileset/mock-splitRoot.nix>)).fileset;
toSource { root = ./foo/mock-root; fileset = ./bar/mock-root; } toSource { root = ./foo/mock-root; fileset = ./bar/mock-root; }
' 'lib.fileset.toSource: Filesystem roots are not the same for `fileset` and `root` "'"$work"'/foo/mock-root": ' 'lib.fileset.toSource: Filesystem roots are not the same for `fileset` and `root` \("'"$work"'/foo/mock-root"\):
\s*`root`: root "'"$work"'/foo/mock-root" \s*`root`: root "'"$work"'/foo/mock-root"
\s*`fileset`: root "'"$work"'/bar/mock-root" \s*`fileset`: root "'"$work"'/bar/mock-root"
\s*Different roots are not supported.' \s*Different roots are not supported.'
rm -rf * rm -rf *
# `root` needs to exist # `root` needs to exist
expectFailure 'toSource { root = ./a; fileset = ./.; }' 'lib.fileset.toSource: `root` '"$work"'/a does not exist.' expectFailure 'toSource { root = ./a; fileset = ./.; }' 'lib.fileset.toSource: `root` \('"$work"'/a\) does not exist.'
# `root` needs to be a file # `root` needs to be a file
touch a touch a
expectFailure 'toSource { root = ./a; fileset = ./a; }' 'lib.fileset.toSource: `root` '"$work"'/a is a file, but it should be a directory instead. Potential solutions: expectFailure 'toSource { root = ./a; fileset = ./a; }' 'lib.fileset.toSource: `root` \('"$work"'/a\) is a file, but it should be a directory instead. Potential solutions:
\s*- If you want to import the file into the store _without_ a containing directory, use string interpolation or `builtins.path` instead of this function. \s*- If you want to import the file into the store _without_ a containing directory, use string interpolation or `builtins.path` instead of this function.
\s*- If you want to import the file into the store _with_ a containing directory, set `root` to the containing directory, such as '"$work"', and set `fileset` to the file path.' \s*- If you want to import the file into the store _with_ a containing directory, set `root` to the containing directory, such as '"$work"', and set `fileset` to the file path.'
rm -rf * rm -rf *
@ -267,18 +267,18 @@ expectFailure 'toSource { root = ./.; fileset = abort "This should be evaluated"
# Only paths under `root` should be able to influence the result # Only paths under `root` should be able to influence the result
mkdir a mkdir a
expectFailure 'toSource { root = ./a; fileset = ./.; }' 'lib.fileset.toSource: `fileset` could contain files in '"$work"', which is not under the `root` '"$work"'/a. Potential solutions: expectFailure 'toSource { root = ./a; fileset = ./.; }' 'lib.fileset.toSource: `fileset` could contain files in '"$work"', which is not under the `root` \('"$work"'/a\). Potential solutions:
\s*- Set `root` to '"$work"' or any directory higher up. This changes the layout of the resulting store path. \s*- Set `root` to '"$work"' or any directory higher up. This changes the layout of the resulting store path.
\s*- Set `fileset` to a file set that cannot contain files outside the `root` '"$work"'/a. This could change the files included in the result.' \s*- Set `fileset` to a file set that cannot contain files outside the `root` \('"$work"'/a\). This could change the files included in the result.'
rm -rf * rm -rf *
# Path coercion only works for paths # Path coercion only works for paths
expectFailure 'toSource { root = ./.; fileset = 10; }' 'lib.fileset.toSource: `fileset` is of type int, but it should be a path instead.' expectFailure 'toSource { root = ./.; fileset = 10; }' 'lib.fileset.toSource: `fileset` is of type int, but it should be a path instead.'
expectFailure 'toSource { root = ./.; fileset = "/some/path"; }' 'lib.fileset.toSource: `fileset` "/some/path" is a string-like value, but it should be a path instead. expectFailure 'toSource { root = ./.; fileset = "/some/path"; }' 'lib.fileset.toSource: `fileset` \("/some/path"\) is a string-like value, but it should be a path instead.
\s*Paths represented as strings are not supported by `lib.fileset`, use `lib.sources` or derivations instead.' \s*Paths represented as strings are not supported by `lib.fileset`, use `lib.sources` or derivations instead.'
# Path coercion errors for non-existent paths # Path coercion errors for non-existent paths
expectFailure 'toSource { root = ./.; fileset = ./a; }' 'lib.fileset.toSource: `fileset` '"$work"'/a does not exist.' expectFailure 'toSource { root = ./.; fileset = ./a; }' 'lib.fileset.toSource: `fileset` \('"$work"'/a\) does not exist.'
# File sets cannot be evaluated directly # File sets cannot be evaluated directly
expectFailure 'union ./. ./.' 'lib.fileset: Directly evaluating a file set is not supported. Use `lib.fileset.toSource` to turn it into a usable source instead.' expectFailure 'union ./. ./.' 'lib.fileset: Directly evaluating a file set is not supported. Use `lib.fileset.toSource` to turn it into a usable source instead.'
@ -395,16 +395,16 @@ expectFailure 'with ((import <nixpkgs/lib>).extend (import <nixpkgs/lib/fileset/
expectFailure 'with ((import <nixpkgs/lib>).extend (import <nixpkgs/lib/fileset/mock-splitRoot.nix>)).fileset; expectFailure 'with ((import <nixpkgs/lib>).extend (import <nixpkgs/lib/fileset/mock-splitRoot.nix>)).fileset;
toSource { root = ./.; fileset = unions [ ./foo/mock-root ./bar/mock-root ]; } toSource { root = ./.; fileset = unions [ ./foo/mock-root ./bar/mock-root ]; }
' 'lib.fileset.unions: Filesystem roots are not the same: ' 'lib.fileset.unions: Filesystem roots are not the same:
\s*element 0 of the argument: root "'"$work"'/foo/mock-root" \s*element 0: root "'"$work"'/foo/mock-root"
\s*element 1 of the argument: root "'"$work"'/bar/mock-root" \s*element 1: root "'"$work"'/bar/mock-root"
\s*Different roots are not supported.' \s*Different roots are not supported.'
rm -rf * rm -rf *
# Coercion errors show the correct context # Coercion errors show the correct context
expectFailure 'toSource { root = ./.; fileset = union ./a ./.; }' 'lib.fileset.union: first argument '"$work"'/a does not exist.' expectFailure 'toSource { root = ./.; fileset = union ./a ./.; }' 'lib.fileset.union: first argument \('"$work"'/a\) does not exist.'
expectFailure 'toSource { root = ./.; fileset = union ./. ./b; }' 'lib.fileset.union: second argument '"$work"'/b does not exist.' expectFailure 'toSource { root = ./.; fileset = union ./. ./b; }' 'lib.fileset.union: second argument \('"$work"'/b\) does not exist.'
expectFailure 'toSource { root = ./.; fileset = unions [ ./a ./. ]; }' 'lib.fileset.unions: element 0 of the argument '"$work"'/a does not exist.' expectFailure 'toSource { root = ./.; fileset = unions [ ./a ./. ]; }' 'lib.fileset.unions: element 0 \('"$work"'/a\) does not exist.'
expectFailure 'toSource { root = ./.; fileset = unions [ ./. ./b ]; }' 'lib.fileset.unions: element 1 of the argument '"$work"'/b does not exist.' expectFailure 'toSource { root = ./.; fileset = unions [ ./. ./b ]; }' 'lib.fileset.unions: element 1 \('"$work"'/b\) does not exist.'
# unions needs a list with at least 1 element # unions needs a list with at least 1 element
expectFailure 'toSource { root = ./.; fileset = unions null; }' 'lib.fileset.unions: Expected argument to be a list, but got a null.' expectFailure 'toSource { root = ./.; fileset = unions null; }' 'lib.fileset.unions: Expected argument to be a list, but got a null.'