diff options
author | Robert Hensing <roberth@users.noreply.github.com> | 2023-10-30 13:14:22 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-10-30 13:14:22 +0100 |
commit | 1c92b1f03bd4c8c98f355554592e21a9d3e8a6da (patch) | |
tree | d0f8f91f7ff2f4689718df82275b63c2067d68c6 | |
parent | 3e93e2445ab48d167d27cf0d45f684cb0a389810 (diff) | |
parent | 47c81d3286bd027d5b55fa581f9502eff4ea8822 (diff) |
Merge pull request #263478 from tweag/fileset/unknown-type-error
`lib.fileset.toSource`: Improve error for unknown file types
-rw-r--r-- | lib/fileset/internal.nix | 52 | ||||
-rwxr-xr-x | lib/fileset/tests.sh | 15 |
2 files changed, 43 insertions, 24 deletions
diff --git a/lib/fileset/internal.nix b/lib/fileset/internal.nix index 2d52a8cb410b1..76b95c6ae471b 100644 --- a/lib/fileset/internal.nix +++ b/lib/fileset/internal.nix @@ -424,7 +424,7 @@ rec { # Filter suited when there's some files # This can't be used for when there's no files, because the base directory is always included nonEmpty = - path: _: + path: type: let # Add a slash to the path string, turning "/foo" to "/foo/", # making sure to not have any false prefix matches below. @@ -433,25 +433,37 @@ rec { # meaning this function can never receive "/" as an argument pathSlash = path + "/"; in - # Same as `hasPrefix pathSlash baseString`, but more efficient. - # With base /foo/bar we need to include /foo: - # hasPrefix "/foo/" "/foo/bar/" - if substring 0 (stringLength pathSlash) baseString == pathSlash then - true - # Same as `! hasPrefix baseString pathSlash`, but more efficient. - # With base /foo/bar we need to exclude /baz - # ! hasPrefix "/baz/" "/foo/bar/" - else if substring 0 baseLength pathSlash != baseString then - false - else - # Same as `removePrefix baseString path`, but more efficient. - # From the above code we know that hasPrefix baseString pathSlash holds, so this is safe. - # We don't use pathSlash here because we only needed the trailing slash for the prefix matching. - # With base /foo and path /foo/bar/baz this gives - # inTree (split "/" (removePrefix "/foo/" "/foo/bar/baz")) - # == inTree (split "/" "bar/baz") - # == inTree [ "bar" "baz" ] - inTree (split "/" (substring baseLength (-1) path)); + ( + # Same as `hasPrefix pathSlash baseString`, but more efficient. + # With base /foo/bar we need to include /foo: + # hasPrefix "/foo/" "/foo/bar/" + if substring 0 (stringLength pathSlash) baseString == pathSlash then + true + # Same as `! hasPrefix baseString pathSlash`, but more efficient. + # With base /foo/bar we need to exclude /baz + # ! hasPrefix "/baz/" "/foo/bar/" + else if substring 0 baseLength pathSlash != baseString then + false + else + # Same as `removePrefix baseString path`, but more efficient. + # From the above code we know that hasPrefix baseString pathSlash holds, so this is safe. + # We don't use pathSlash here because we only needed the trailing slash for the prefix matching. + # With base /foo and path /foo/bar/baz this gives + # inTree (split "/" (removePrefix "/foo/" "/foo/bar/baz")) + # == inTree (split "/" "bar/baz") + # == inTree [ "bar" "baz" ] + inTree (split "/" (substring baseLength (-1) path)) + ) + # This is a way have an additional check in case the above is true without any significant performance cost + && ( + # This relies on the fact that Nix only distinguishes path types "directory", "regular", "symlink" and "unknown", + # so everything except "unknown" is allowed, seems reasonable to rely on that + type != "unknown" + || throw '' + lib.fileset.toSource: `fileset` contains a file that cannot be added to the store: ${path} + This file is neither a regular file nor a symlink, the only file types supported by the Nix store. + Therefore the file set cannot be added to the Nix store as is. Make sure to not include that file to avoid this error.'' + ); in # Special case because the code below assumes that the _internalBase is always included in the result # which shouldn't be done when we have no files at all in the base diff --git a/lib/fileset/tests.sh b/lib/fileset/tests.sh index d8d8dd4131894..5b756b8fc5931 100755 --- a/lib/fileset/tests.sh +++ b/lib/fileset/tests.sh @@ -332,7 +332,7 @@ expectFailure 'with ((import <nixpkgs/lib>).extend (import <nixpkgs/lib/fileset/ \s*`root`: root "'"$work"'/foo/mock-root" \s*`fileset`: root "'"$work"'/bar/mock-root" \s*Different roots are not supported.' -rm -rf * +rm -rf -- * # `root` needs to exist expectFailure 'toSource { root = ./a; fileset = ./.; }' 'lib.fileset.toSource: `root` \('"$work"'/a\) does not exist.' @@ -342,7 +342,7 @@ 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: \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.' -rm -rf * +rm -rf -- * # The fileset argument should be evaluated, even if the directory is empty expectFailure 'toSource { root = ./.; fileset = abort "This should be evaluated"; }' 'evaluation aborted with the following error message: '\''This should be evaluated'\' @@ -352,7 +352,14 @@ 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: \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.' -rm -rf * +rm -rf -- * + +# non-regular and non-symlink files cannot be added to the Nix store +mkfifo a +expectFailure 'toSource { root = ./.; fileset = ./a; }' 'lib.fileset.toSource: `fileset` contains a file that cannot be added to the store: '"$work"'/a +\s*This file is neither a regular file nor a symlink, the only file types supported by the Nix store. +\s*Therefore the file set cannot be added to the Nix store as is. Make sure to not include that file to avoid this error.' +rm -rf -- * # Path coercion only works for paths expectFailure 'toSource { root = ./.; fileset = 10; }' 'lib.fileset.toSource: `fileset` is of type int, but it should be a file set or a path instead.' @@ -493,7 +500,7 @@ expectFailure 'with ((import <nixpkgs/lib>).extend (import <nixpkgs/lib/fileset/ \s*element 0: root "'"$work"'/foo/mock-root" \s*element 1: root "'"$work"'/bar/mock-root" \s*Different roots are not supported.' -rm -rf * +rm -rf -- * # Coercion errors show the correct context expectFailure 'toSource { root = ./.; fileset = union ./a ./.; }' 'lib.fileset.union: first argument \('"$work"'/a\) does not exist.' |