Merge pull request #263478 from tweag/fileset/unknown-type-error

`lib.fileset.toSource`: Improve error for unknown file types

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.'