From eec765f9e6a862c82990df8d3ceedf51f4600330 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Mon, 11 Dec 2023 22:56:11 +0100 Subject: [PATCH 1/2] lib.fileset: Refactor gitTracked and gitTrackedWith Introduce an internal function for them to share more behavior. This makes future changes easier. --- lib/fileset/default.nix | 58 ++++++++++++---------------------------- lib/fileset/internal.nix | 31 +++++++++++++++++++++ 2 files changed, 48 insertions(+), 41 deletions(-) diff --git a/lib/fileset/default.nix b/lib/fileset/default.nix index 75e609a072e7..18acb9a980ca 100644 --- 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; + }); } diff --git a/lib/fileset/internal.nix b/lib/fileset/internal.nix index 35d556e78391..ade21fd87610 100644 --- 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,31 @@ 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." + # 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; + } From 6e3be6ddb0c9789d8b891318c526a9401dab481e Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Mon, 11 Dec 2023 23:05:51 +0100 Subject: [PATCH 2/2] lib.fileset.gitTracked: Improve error when passing files --- lib/fileset/internal.nix | 2 ++ lib/fileset/tests.sh | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/lib/fileset/internal.nix b/lib/fileset/internal.nix index ade21fd87610..9de5590d3eff 100644 --- a/lib/fileset/internal.nix +++ b/lib/fileset/internal.nix @@ -874,6 +874,8 @@ rec { 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`), diff --git a/lib/fileset/tests.sh b/lib/fileset/tests.sh index 077aefe371c3..d55c4fbfdbeb 100755 --- 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.'