Merge pull request #273664 from tweag/fileset.gitTracked-improvements

`lib.fileset.gitTracked,gitTrackedWith`: Minor improvements
2023-12-13 05:14:20 +01:00 · 2023-12-13 05:14:20 +01:00 · c4e2e52a21
commit c4e2e52a21
parent f83bb17bb6 6e3be6ddb0
3 changed files with 56 additions and 41 deletions
--- a/lib/fileset/default.nix
+++ b/lib/fileset/default.nix
@ -107,7 +107,7 @@ let
    _printFileset
    _intersection
    _difference
-    _mirrorStorePath
+    _fromFetchGit
    _fetchGitSubmodulesMinver
    _emptyWithoutBase
    ;
@ -148,7 +148,6 @@ let
  inherit (lib.trivial)
    isFunction
    pipe
-    inPureEvalMode
    ;

 in {
@ -754,18 +753,11 @@ in {
      This directory must contain a `.git` file or subdirectory.
    */
    path:
-    # See the gitTrackedWith implementation for more explanatory comments
-    let
-      fetchResult = builtins.fetchGit path;
-    in
-    if inPureEvalMode then
-      throw "lib.fileset.gitTracked: This function is currently not supported in pure evaluation mode, since it currently relies on `builtins.fetchGit`. See https://github.com/NixOS/nix/issues/9292."
-    else if ! isPath path then
-      throw "lib.fileset.gitTracked: Expected the argument to be a path, but it's a ${typeOf path} instead."
-    else if ! pathExists (path + "/.git") then
-      throw "lib.fileset.gitTracked: Expected the argument (${toString path}) to point to a local working tree of a Git repository, but it's not."
-    else
-      _mirrorStorePath path fetchResult.outPath;
+    _fromFetchGit
+      "gitTracked"
+      "argument"
+      path
+      {};

  /*
    Create a file set containing all [Git-tracked files](https://git-scm.com/book/en/v2/Git-Basics-Recording-Changes-to-the-Repository) in a repository.
@ -807,35 +799,19 @@ in {
      This directory must contain a `.git` file or subdirectory.
    */
    path:
-    let
-      # This imports the files unnecessarily, which currently can't be avoided
-      # because `builtins.fetchGit` is the only function exposing which files are tracked by Git.
-      # With the [lazy trees PR](https://github.com/NixOS/nix/pull/6530),
-      # the unnecessarily import could be avoided.
-      # However a simpler alternative still would be [a builtins.gitLsFiles](https://github.com/NixOS/nix/issues/2944).
-      fetchResult = builtins.fetchGit {
-        url = path;
-
-        # This is the only `fetchGit` parameter that makes sense in this context.
-        # We can't just pass `submodules = recurseSubmodules` here because
-        # this would fail for Nix versions that don't support `submodules`.
-        ${if recurseSubmodules then "submodules" else null} = true;
-      };
-    in
-    if inPureEvalMode then
-      throw "lib.fileset.gitTrackedWith: This function is currently not supported in pure evaluation mode, since it currently relies on `builtins.fetchGit`. See https://github.com/NixOS/nix/issues/9292."
-    else if ! isBool recurseSubmodules then
+    if ! isBool recurseSubmodules then
      throw "lib.fileset.gitTrackedWith: Expected the attribute `recurseSubmodules` of the first argument to be a boolean, but it's a ${typeOf recurseSubmodules} instead."
    else if recurseSubmodules && versionOlder nixVersion _fetchGitSubmodulesMinver then
      throw "lib.fileset.gitTrackedWith: Setting the attribute `recurseSubmodules` to `true` is only supported for Nix version ${_fetchGitSubmodulesMinver} and after, but Nix version ${nixVersion} is used."
-    else if ! isPath path then
-      throw "lib.fileset.gitTrackedWith: Expected the second argument to be a path, but it's a ${typeOf path} instead."
-    # We can identify local working directories by checking for .git,
-    # see https://git-scm.com/docs/gitrepository-layout#_description.
-    # Note that `builtins.fetchGit` _does_ work for bare repositories (where there's no `.git`),
-    # even though `git ls-files` wouldn't return any files in that case.
-    else if ! pathExists (path + "/.git") then
-      throw "lib.fileset.gitTrackedWith: Expected the second argument (${toString path}) to point to a local working tree of a Git repository, but it's not."
    else
-      _mirrorStorePath path fetchResult.outPath;
+      _fromFetchGit
+        "gitTrackedWith"
+        "second argument"
+        path
+        # This is the only `fetchGit` parameter that makes sense in this context.
+        # We can't just pass `submodules = recurseSubmodules` here because
+        # this would fail for Nix versions that don't support `submodules`.
+        (lib.optionalAttrs recurseSubmodules {
+          submodules = true;
+        });
 }
--- a/lib/fileset/internal.nix
+++ b/lib/fileset/internal.nix
@ -10,6 +10,7 @@ let
    split
    trace
    typeOf
+    fetchGit
    ;

  inherit (lib.attrsets)
@ -55,6 +56,9 @@ let
    hasSuffix
    ;

+  inherit (lib.trivial)
+    inPureEvalMode
+    ;
 in
 # Rare case of justified usage of rec:
 # - This file is internal, so the return value doesn't matter, no need to make things overridable
@ -852,4 +856,33 @@ rec {
    in
    _create localPath
      (recurse storePath);
+
+  # Create a file set from the files included in the result of a fetchGit call
+  # Type: String -> String -> Path -> Attrs -> FileSet
+  _fromFetchGit = function: argument: path: extraFetchGitAttrs:
+    let
+      # This imports the files unnecessarily, which currently can't be avoided
+      # because `builtins.fetchGit` is the only function exposing which files are tracked by Git.
+      # With the [lazy trees PR](https://github.com/NixOS/nix/pull/6530),
+      # the unnecessarily import could be avoided.
+      # However a simpler alternative still would be [a builtins.gitLsFiles](https://github.com/NixOS/nix/issues/2944).
+      fetchResult = fetchGit ({
+        url = path;
+      } // extraFetchGitAttrs);
+    in
+    if inPureEvalMode then
+      throw "lib.fileset.${function}: This function is currently not supported in pure evaluation mode, since it currently relies on `builtins.fetchGit`. See https://github.com/NixOS/nix/issues/9292."
+    else if ! isPath path then
+      throw "lib.fileset.${function}: Expected the ${argument} to be a path, but it's a ${typeOf path} instead."
+    else if pathType path != "directory" then
+      throw "lib.fileset.${function}: Expected the ${argument} (${toString path}) to be a directory, but it's a file instead."
+    # We can identify local working directories by checking for .git,
+    # see https://git-scm.com/docs/gitrepository-layout#_description.
+    # Note that `builtins.fetchGit` _does_ work for bare repositories (where there's no `.git`),
+    # even though `git ls-files` wouldn't return any files in that case.
+    else if ! pathExists (path + "/.git") then
+      throw "lib.fileset.${function}: Expected the ${argument} (${toString path}) to point to a local working tree of a Git repository, but it's not."
+    else
+      _mirrorStorePath path fetchResult.outPath;
+
 }
--- a/lib/fileset/tests.sh
+++ b/lib/fileset/tests.sh
@ -1317,6 +1317,12 @@ rm -rf -- *
 expectFailure 'gitTracked null' 'lib.fileset.gitTracked: Expected the argument to be a path, but it'\''s a null instead.'
 expectFailure 'gitTrackedWith {} null' 'lib.fileset.gitTrackedWith: Expected the second argument to be a path, but it'\''s a null instead.'

+# The path must be a directory
+touch a
+expectFailure 'gitTracked ./a' 'lib.fileset.gitTracked: Expected the argument \('"$work"'/a\) to be a directory, but it'\''s a file instead'
+expectFailure 'gitTrackedWith {} ./a' 'lib.fileset.gitTrackedWith: Expected the second argument \('"$work"'/a\) to be a directory, but it'\''s a file instead'
+rm -rf -- *
+
 # The path has to contain a .git directory
 expectFailure 'gitTracked ./.' 'lib.fileset.gitTracked: Expected the argument \('"$work"'\) to point to a local working tree of a Git repository, but it'\''s not.'
 expectFailure 'gitTrackedWith {} ./.' 'lib.fileset.gitTrackedWith: Expected the second argument \('"$work"'\) to point to a local working tree of a Git repository, but it'\''s not.'