From 0f6cc8018c7b7f9244f06ad8072ab017808ad0c2 Mon Sep 17 00:00:00 2001
From: Silvan Mosberger <silvan.mosberger@tweag.io>
Date: Thu, 26 Oct 2023 01:56:42 +0200
Subject: lib.fileset.toSource: Improve error for unknown file types
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This does decrease performance unfortunately

Benchmarking expression toSource { root = ./.; fileset = ./.; }
Mean CPU time 0.103747 (σ = 0.012415) for 10 runs is 97.32181384964636% (σ = 16.34179537413021%) of the old value 0.106602 (σ = 0.0125571)
Statistic .envs.elements (205920) is 105.5842% (+10891) of the old value 195029
Statistic .gc.totalBytes (20247696) is 101.7495% (+348160) of the old value 19899536
Statistic .nrThunks (134824) is 108.7878% (+10891) of the old value 123933
Statistic .symbols.number (996) is 100.1005% (+1) of the old value 995
Statistic .values.number (275238) is 104.1199% (+10891) of the old value 264347
---
 lib/fileset/internal.nix | 49 +++++++++++++++++++++++++++++-------------------
 lib/fileset/tests.sh     |  4 +++-
 2 files changed, 33 insertions(+), 20 deletions(-)

(limited to 'lib/fileset')

diff --git a/lib/fileset/internal.nix b/lib/fileset/internal.nix
index 2d52a8cb410..afaa8363373 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.
@@ -432,26 +432,37 @@ rec {
           # but builtins.path doesn't call the filter function on the `path` argument itself,
           # meaning this function can never receive "/" as an argument
           pathSlash = path + "/";
+
+          include =
+            # 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));
         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
+        # 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
+        if include && type == "unknown" then
+          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.''
         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));
+          include;
     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 86ef1989f60..5b756b8fc59 100755
--- a/lib/fileset/tests.sh
+++ b/lib/fileset/tests.sh
@@ -356,7 +356,9 @@ rm -rf -- *
 
 # non-regular and non-symlink files cannot be added to the Nix store
 mkfifo a
-expectFailure 'toSource { root = ./.; fileset = ./a; }' 'file '\'"$work"'/a'\'' has an unsupported type'
+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
-- 
cgit 1.4.1