File set library tracking issue and feature requests

[infinisil]

Context

With sponsoring from Antithesis ✨, following the lib.path library, I've been able to work on lib.fileset in recent months, a new library in Nixpkgs to select local files to use for sources. Originally I opened a WIP PR as a proof-of-concept and to get feedback on the general design. Later I also created a Discourse post to gather more feedback. After not getting a lot of feedback though (but also nobody complaining), we (me, @roberth and @fricklerhandwerk) were content enough with its general design and therefore agreed to accept my PRs to incrementally build up the library in Nixpkgs.

So over the last months I made a bunch of PRs, and had them reviewed and merged by @roberth and @fricklerhandwerk. Today I'm happy to say that the library is very usable, check out its reference docs! It's not perfect, but the basic feature set is there and is tested thoroughly.

Originally I used the WIP PR to summarise the on-going work, but it would be better to have a dedicated tracking issue for this instead, so I'm creating this issue for that. Furthermore I'm going to use this issue to collect feature requests for the file set library.

Finished, ongoing and proposed interface changes

• File set combinators base: lib.fileset.toSource #245623
• lib.fileset.union, lib.fileset.unions: init #255025
• lib.fileset.trace, lib.fileset.traceVal: init #256417
• lib.fileset.intersection: init #257356
• lib.fileset.difference: init #259065
• lib.fileset.fileFilter: init #260265
• lib.fileset.fromSource: init #261732
• lib.fileset: Lazier path existence check #267091
• fileset.fileFilter: Restrict second argument to paths #267384
• lib.fileset.gitTracked: init #264891
• lib.fileset.maybeMissing: init #265964
• lib.fileset.fileFilter: Predicate attribute for file extension #266362
• lib.fileset.gitTracked should support out-of-tree builds #269283
• Turn lib.fileset into a list of file paths #288592
• lib.fileset: Implicitly coerce lists to unions #267387
  Status: Unsure
• Add lib.fileset.optional :: Bool -> FileSet -> FileSet #266106
  Status: Unsure
• lib.fileset.empty feature request #266366
  Status: Unsure
• lib.fileset.trace should show whether directories include files by default #268878
  Status: Unsure
• lib.fileset.fileFilter should allow filtering by file subpath/components #269504
  Status: Unsure
• lib.fileset needs something like lib.cleanSource #269517
  Status: Unsure
• lib.fileset should have a way to filter directories #271307
  Status: Unsure

Indirectly related:

• Function for transforming store path contents #264541
  Definitely out of scope for file set library
  Status: Unsure
• Add a way to control inclusion of empty directories in builtins.path results nix#8820
  This is a proposed addition to the Nix builtins to improve file set library performance.
  Status: Unsure
• builtins.fetchGit dirty mode almost unusable in pure evaluation mode nix#9292

If you're trying to use the library but run into problems related to any of the above issues, please leave a comment there or 👍 it, especially if its status is unsure. And if you have a request for a new feature, please comment it here, ideally with an issue/PR!

[nixos-discourse]

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/easy-source-filtering-with-file-sets/29117/12

[accelbread]

It seems the library has some issue on Nix 2.12 and earlier.

For example, take the following flake:

{
  outputs = { nixpkgs, ... }:
    let pkgs = nixpkgs.legacyPackages.x86_64-linux; in {
      packages.x86_64-linux.default = pkgs.stdenv.mkDerivation {
        name = "test";
        src = nixpkgs.lib.fileset.toSource {
          root = ./.;
          fileset = nixpkgs.lib.fileset.maybeMissing ./value;
        };
        buildPhase="touch $out";
      };
    };
}

Building the package on Nix 2.13 and after works with no issue.

With older Nix, I get:

error: access to absolute path '/nix/store' is forbidden in pure eval mode (use '--impure' to override)

This only occurs when root is the root dir of the flake; if it is a subdir there is no error.

I don't really need support for older Nix myself, but this did take a while to debug since it was failing with an old script that had pinned Nix.

I don't know if it is a real issue, but bringing it up in case it is or if it should be documented.

[roberth]

The issue is real, but as a Nix 2.12 issue, and that version has been beyond the Nix team's support range since July.

Perhaps Nix should warn when it is called after its planned EOL date?

[infinisil]

The problem is that builtins.readFileType only exists in 2.13 and up (and 2.13 only because it was backported: NixOS/nix#8763). Before that, the only way to get the file type was to use builtins.readDir on the parent directory (see lib.pathType), which in case of store paths, is /nix/store, which is why that error gets thrown.

I did actually write a workaround for this in #222981 (see patch in details below), but it's a bit hacky, it assumes that any /nix/store/* path in pure evaluation is always a directory, which isn't necessarily the case, though it would be good enough for file sets, since it can only filter directories anyways.

We could consider applying this patch, probably locally for just file sets. And alternatively, throw a better error message if such a case is detected.

Nixpkgs patch

diff --git a/lib/filesystem.nix b/lib/filesystem.nix
index 5569b8ac80fd..0d0019771a13 100644
--- a/lib/filesystem.nix
+++ b/lib/filesystem.nix
@@ -9,6 +9,24 @@ let
   inherit (builtins)
     readDir
     pathExists
+    storeDir
+    ;
+
+  inherit (lib.trivial)
+    inPureEvalMode
+    ;
+
+  inherit (lib.path)
+    deconstruct
+    ;
+
+  inherit (lib.path.components)
+    fromSubpath
+    ;
+
+  inherit (lib.lists)
+    take
+    length
     ;
 
   inherit (lib.filesystem)
@@ -33,20 +51,41 @@ in
       => "regular"
   */
   pathType =
-    builtins.readFileType or
+    let
+      storeDirComponents = fromSubpath "./${storeDir}";
+
+      legacy = path:
+        if ! pathExists path
+        # Fail irrecoverably to mimic the historic behavior of this function and
+        # the new builtins.readFileType
+        then abort "lib.filesystem.pathType: Path ${toString path} does not exist."
+        # The filesystem root is the only path where `dirOf / == /` and
+        # `baseNameOf /` is not valid. We can detect this and directly return
+        # "directory", since we know the filesystem root can't be anything else.
+        else if dirOf path == path
+        then "directory"
+        else (readDir (dirOf path)).${baseNameOf path};
+
+      legacyPureEval = path:
+        let
+          # This is a workaround for the fact that `lib.filesystem.pathType` doesn't work correctly on /nix/store/...-name paths in pure evaluation mode. For file set combinators it's safe to assume that
+          pathParts = deconstruct path;
+          assumeDirectory =
+            storeDirComponents == take (length pathParts.components - 1) pathParts.components;
+        in
+        if assumeDirectory then
+          "directory"
+        else
+          legacy path;
+
+    in
+    if builtins ? readFileType then
+      builtins.readFileType
     # Nix <2.14 compatibility shim
-    (path:
-      if ! pathExists path
-      # Fail irrecoverably to mimic the historic behavior of this function and
-      # the new builtins.readFileType
-      then abort "lib.filesystem.pathType: Path ${toString path} does not exist."
-      # The filesystem root is the only path where `dirOf / == /` and
-      # `baseNameOf /` is not valid. We can detect this and directly return
-      # "directory", since we know the filesystem root can't be anything else.
-      else if dirOf path == path
-      then "directory"
-      else (readDir (dirOf path)).${baseNameOf path}
-    );
+    else if inPureEvalMode then
+      legacyPureEval
+    else
+      legacy;
 
   /*
     Whether a path exists and is a directory.

[roberth]

The problem is that readFileType supports in all supported versions?
Hardly a problem.

If you're stuck on an old version, please upgrade. If you can't upgrade, please let us know what is the problem with that.

I consider such hacks a waste of human potential. Let's move forward instead of in circles.

[accelbread]

Perhaps it would make sense to not fall back to builtins.readDir, since all supported Nix verisons have builtins.readFileType? If the error was something like "builtins does not have attribute readFileType", it would be easy to tell that one is using an old Nix.

[nixos-discourse]

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/umport-automatic-import-of-modules/39455/4

[mightyiam]

You know how sometimes apps reveal the commit hash from which they were built in the UI? I don't actually need this feature, but it's existing, and I'm trying to refactor to build using Nix. Anyway, I'm not sure how to obtain it within the sandbox in pure evaluation. I couldn't find such a utility specifically for that, nor did lib.fileset.unions [./foo ./.git/HEAD ./.git/refs/heads] succeed. Is there no way around the exclusion of .git using lib.fileset?

[roberth]

With Flakes, the exclusion of .git is almost unavoidable. You'd have to use file:// installables on the command line to avoid the normal git-based behavior, but this is not feasible for flakes that are imported using Flake inputs.

Instead, Flakes gives you a .rev attribute that contains the commit hash.

This solves a reproducibility problem: if you include for example .git/refs/heads, your derivation hash will depend on the set of all refs, which changes all the time as the project develops. When building a past version, branches that were created or deleted since then will cause changes in the hash, causing different inputs hashes, rebuilds and cache misses, unnecessary redeployments, and frustrated collaborators.

If you're in a monorepo, or really any large enough repo where the majority of files isn't consumed by a single derivation, I would recommend against even using .rev because it causes unnecessary rebuilds just to update the version everywhere, even when a change only happened in some corner of the repo.
Or at least use it in fewer places. Using it in a wrapper derivation or config file that's added after the build may be ok, if the main concern is build time. It doesn't solve the redeployments, and not 100% of the cache misses, but those may not be an real problem in some cases.