`lib.fileset.gitTracked`: init #264891

infinisil · 2023-11-01T21:34:40Z

Description of changes

This adds two new functions to lib.fileset:

lib.fileset.gitTracked :: Path -> FileSet: Create a file set containing all Git-tracked files in the working tree of a local Git repository.
```
lib.fileset.gitTracked ./.
```
lib.fileset.gitTrackedWith :: { recurseSubmodules :: Bool ? false } -> Path -> FileSet: Same, but optionally also recurse into submodules.
```
lib.fileset.gitTrackedWith { recurseSubmodules = true; } ./.
```

These functions are useful since it's very common to only want to include files tracked by Git. So much so that Flakes does it by default.

Combined with the fact that Flakes currently always copies all files to the Nix store, these functions are somewhat useless with Flakes:

If you use this for a git+file: input, Flakes will have already implicitly filtered out all files not tracked by Git, including .git, so this function can't even be called on the local directory then.
If you use a path:. input instead of implicitly letting Flakes treat it as a Git directory, it will import all files into the Nix store, which is exactly what this library is trying to let you control.
Even disregarding the above, there's a problem preventing this function from being usable here as is: builtins.fetchGit dirty mode almost unusable in pure evaluation mode nix#9292

But for stable Nix, these functions are a requirement, since it's not otherwise possible to create a file set from just Git-tracked files (builtins.fetchGit returns a store path, which doesn't work with file sets).

This work is sponsored by Antithesis ✨

Things done

Interface decided
Code comments
User documentation
Tests

roberth · 2023-11-06T11:10:36Z

gitTracked should be a consumer of the library rather than be part of it.
Its domain is git, not sets.

Please rename it to, say, lib.git.trackedFileSet.

lib/fileset/internal.nix

fricklerhandwerk · 2023-11-08T00:54:44Z

While this adds means to strengthen build purity, which user demand is driving this change?

infinisil · 2023-11-08T01:19:08Z

@fricklerhandwerk Pretty much every Git-based project using stable Nix can benefit from this function, since it provides a good base set of files to further customise. Even manually it's not practically possible to do this without this function, because you'd have to list each file tracked in Git individually in a unions, there's no nice way to get the list of git-tracked files.

Comparatively, this is already possible with lib.sources, because you can just do lib.cleanSourceWith { src = builtins.fetchGit ./.; filter = ...; } or so.

There is a possibility of having a lib.git.lsTrackedFiles :: Path -> [ Path ], then one could do

# Equivalent to the proposed lib.fileset.gitTracked ./.
lib.fileset.unions (lib.git.lsTrackedFiles ./.)

But:

It's less lazy and less efficient, since everything file has to go through the length-strict list
It would be less discoverable and be more annoying to type

I'd prefer just having lib.fileset.gitTracked for convenience (whose importance we shouldn't underestimate).

fricklerhandwerk · 2023-11-08T07:16:41Z

The fact that it can be done with existing library functions, especially as trivially as you're showing, and especially given you're already working on fromSource, indicates to me we shouldn't spend more energy here.

infinisil · 2023-11-08T20:52:37Z

@fricklerhandwerk No it cannot be done with existing functions (unless you pretty much copy what I did in this PR). lib.git.lsTrackedFiles doesn't exist, and even if it did, it would be a bunch less efficient than the function in this PR.

I really think something like this PR is required for an enjoyable experience with the file set library. Nobody (me included) wants to have to manually filter out files not tracked by Git.

roberth

It's not pretty, the way it is, or where it is, but it's useful. One request; otherwise lgtm.

lib/fileset/default.nix

roberth · 2023-11-10T19:04:46Z

Perhaps also relevant is a helper that only applies this when there's a .git at the expected location. That way users can support local development and tarballs simultaneously, e.g. if the code will be used in another project via niv or flakes.

roberth · 2023-11-10T19:07:45Z

Nobody (me included) wants to have to manually filter out files not tracked by Git.

Am I nobody to you? ;)

EDIT: actually it's not possible, because tracked and non-ignored are different sets, where tracked requires reading the staging area, which I don't think is feasible.

infinisil · 2023-11-10T19:36:50Z

Perhaps also relevant is a helper that only applies this when there's a .git at the expected location. That way users can support local development and tarballs simultaneously, e.g. if the code will be used in another project via niv or flakes.

This could be done with

if builtins.pathExists ./.git then
  gitTracked ./.
else
  ./.

But yeah having a convenience function for this might be nice. Better in a separate PR though :)

EDIT: actually it's not possible, because tracked and non-ignored are different sets, where tracked requires reading the staging area, which I don't think is feasible.

Hehe yeah, I wonder if we should have a combination of the two though, something like "all files that would be tracked if you ran git add -A". Probably that would be

let
  tracked = gitTracked ./.;
  ignored = gitIgnored ./.;
in
union tracked
  (intersection
    (difference ./. tracked)
    (difference ./. ignored)
  )

(and probably some nice set identity could be used to make this smaller).

But that's also for another PR :)

roberth · 2023-11-10T19:38:26Z

lib/fileset/default.nix

+  */
+  gitTracked =
+    /*
+      The [path](https://nixos.org/manual/nix/stable/language/values#type-path) to a local working directory of the Git repository whose tracked files to include.


Suggested change

The [path](https://nixos.org/manual/nix/stable/language/values#type-path) to a local working directory of the Git repository whose tracked files to include.

The [path](https://nixos.org/manual/nix/stable/language/values#type-path) to a local Git repository.

Remove "working directory" because that's less unambiguously the root of the repo.

Don't state too much of the obvious.

Not sure where to document the return value, but I'll make another suggestion.

I do think for correctness we should use working directory, because there's also bare repositories without a working directory, see https://git-scm.com/docs/gitrepository-layout#_description.

I do agree to remove the "whose tracked files to include" though.

Suggested change

The [path](https://nixos.org/manual/nix/stable/language/values#type-path) to a local working directory of the Git repository whose tracked files to include.

The [path](https://nixos.org/manual/nix/stable/language/values#type-path) to a local working directory of the Git repository.

This directory must contain a `.git` file or subdirectory.

Suggested change

The [path](https://nixos.org/manual/nix/stable/language/values#type-path) to a local working directory of the Git repository whose tracked files to include.

The [path](https://nixos.org/manual/nix/stable/language/values#type-path) to the working directory of a local Git repository.

This directory must contain a `.git` file or subdirectory.

Switch "the" and "a" around, so we don't suggest there's any choice of directories within a repo. (Technically it may have worktrees but that's not even of secondary importance, and can be fully explained by a small vagueness on our end when it comes to the definition of repository.)

If that's not ok, consider

Suggested change

The [path](https://nixos.org/manual/nix/stable/language/values#type-path) to a local working directory of the Git repository whose tracked files to include.

The [path](https://nixos.org/manual/nix/stable/language/values#type-path) to a local non-bare Git repository.

This directory must contain a `.git` file or subdirectory.

or if you don't mind an association with the worktree feature, we can be really brief:

Suggested change

The [path](https://nixos.org/manual/nix/stable/language/values#type-path) to a local working directory of the Git repository whose tracked files to include.

The [path](https://nixos.org/manual/nix/stable/language/values#type-path) to a local Git worktree with a valid `.git` file or directory.

I like the first suggestion, will apply.

roberth · 2023-11-10T19:40:57Z

lib/fileset/default.nix

@@ -510,4 +516,107 @@ If a directory does not recursively contain any file, it is omitted from the sto
      # We could also return the original fileset argument here,
      # but that would then duplicate work for consumers of the fileset, because then they have to coerce it again
      actualFileset;
+
+  /*
+    Like [`gitTrackedWith`](#function-library-lib.fileset.gitTrackedWith) with `{}` as the first argument, therefore using the defaults.


Suggested change

Like [`gitTrackedWith`](#function-library-lib.fileset.gitTrackedWith) with `{}` as the first argument, therefore using the defaults.

A fileset with all files from a repository; typically passed to [`intersect`](#function-library-lib.fileset.intersect).

This function behaves like [`gitTrackedWith { }`](#function-library-lib.fileset.gitTrackedWith) - using the defaults.

Suggested change

Like [`gitTrackedWith`](#function-library-lib.fileset.gitTrackedWith) with `{}` as the first argument, therefore using the defaults.

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.

This function behaves like [`gitTrackedWith { }`](#function-library-lib.fileset.gitTrackedWith) - using the defaults.

A configuration parameter for gitTrackedWith will be introduced in the next commit

infinisil · 2023-11-16T00:15:05Z

Force push to apply the suggestions: (diff)

Now rebasing on top of master, I think this is ready.

roberth

LGTM after

roberth · 2023-11-16T10:57:33Z

lib/fileset/default.nix

+      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."


Suggested change

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

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

The test needs it too :/

Haha right, it's annoying but I'd be very much screwed without these tests

Pushed

github-actions bot added the 6.topic: lib The Nixpkgs function library label Nov 1, 2023

infinisil mentioned this pull request Nov 1, 2023

[WIP] File set combinators #222981

Closed

7 tasks

infinisil changed the title ~~lib.fileset.gitTracked: init~~ lib.fileset.gitTracked: init Nov 1, 2023

ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Nov 1, 2023

infinisil mentioned this pull request Nov 1, 2023

lib.fileset.fromSource: init #261732

Merged

3 tasks

infinisil requested review from fricklerhandwerk and roberth November 1, 2023 22:50

infinisil force-pushed the fileset.gitTracked branch from 61d564a to 12d8a38 Compare November 3, 2023 00:37

infinisil marked this pull request as ready for review November 3, 2023 00:39

infinisil requested a review from edolstra as a code owner November 3, 2023 00:39

infinisil commented Nov 6, 2023

View reviewed changes

lib/fileset/internal.nix Show resolved Hide resolved

infinisil mentioned this pull request Nov 9, 2023

File set library tracking issue and feature requests #266356

Open

roberth reviewed Nov 10, 2023

View reviewed changes

lib/fileset/default.nix Show resolved Hide resolved

roberth reviewed Nov 10, 2023

View reviewed changes

ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Nov 10, 2023

infinisil force-pushed the fileset.gitTracked branch from f034955 to a87f43e Compare November 16, 2023 00:09

infinisil added 2 commits November 16, 2023 01:12

lib.fileset.gitTracked/gitTrackedWith: init

2dfb1d3

A configuration parameter for gitTrackedWith will be introduced in the next commit

lib.fileset.gitTrackedWith: Introduce recurseSubmodules parameter

d33f1a6

infinisil force-pushed the fileset.gitTracked branch from a87f43e to bd13e36 Compare November 16, 2023 00:15

ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Nov 16, 2023

roberth reviewed Nov 16, 2023

View reviewed changes

infinisil force-pushed the fileset.gitTracked branch from bd13e36 to 948534a Compare November 16, 2023 12:10

lib.fileset.gitTracked: Better error in pure eval

ada680b

infinisil force-pushed the fileset.gitTracked branch from 948534a to ada680b Compare November 16, 2023 13:42

roberth approved these changes Nov 16, 2023

View reviewed changes

infinisil merged commit b399b68 into NixOS:master Nov 16, 2023
19 checks passed

infinisil deleted the fileset.gitTracked branch November 16, 2023 14:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`lib.fileset.gitTracked`: init #264891

`lib.fileset.gitTracked`: init #264891

infinisil commented Nov 1, 2023 •

edited

Loading

roberth commented Nov 6, 2023

fricklerhandwerk commented Nov 8, 2023

infinisil commented Nov 8, 2023 •

edited

Loading

fricklerhandwerk commented Nov 8, 2023

infinisil commented Nov 8, 2023

roberth left a comment

roberth commented Nov 10, 2023

roberth commented Nov 10, 2023 •

edited

Loading

infinisil commented Nov 10, 2023

roberth Nov 10, 2023

infinisil Nov 10, 2023

infinisil Nov 10, 2023

roberth Nov 10, 2023

infinisil Nov 16, 2023

roberth Nov 10, 2023

infinisil Nov 10, 2023

infinisil commented Nov 16, 2023 •

edited

Loading

roberth left a comment

roberth Nov 16, 2023

infinisil Nov 16, 2023

roberth Nov 16, 2023

infinisil Nov 16, 2023

	The [path](https://nixos.org/manual/nix/stable/language/values#type-path) to a local working directory of the Git repository whose tracked files to include.
	The [path](https://nixos.org/manual/nix/stable/language/values#type-path) to a local Git repository.

	The [path](https://nixos.org/manual/nix/stable/language/values#type-path) to a local working directory of the Git repository whose tracked files to include.
	The [path](https://nixos.org/manual/nix/stable/language/values#type-path) to a local working directory of the Git repository.
	This directory must contain a `.git` file or subdirectory.

-    Like [`gitTrackedWith`](#function-library-lib.fileset.gitTrackedWith) with `{}` as the first argument, therefore using the defaults.
+    A fileset with all files from a repository; typically passed to [`intersect`](#function-library-lib.fileset.intersect).
+    This function behaves like [`gitTrackedWith { }`](#function-library-lib.fileset.gitTrackedWith) - using the defaults.

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

lib.fileset.gitTracked: init #264891

lib.fileset.gitTracked: init #264891

Conversation

infinisil commented Nov 1, 2023 • edited Loading

Description of changes

Things done

roberth commented Nov 6, 2023

fricklerhandwerk commented Nov 8, 2023

infinisil commented Nov 8, 2023 • edited Loading

fricklerhandwerk commented Nov 8, 2023

infinisil commented Nov 8, 2023

roberth left a comment

Choose a reason for hiding this comment

roberth commented Nov 10, 2023

roberth commented Nov 10, 2023 • edited Loading

infinisil commented Nov 10, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

infinisil commented Nov 16, 2023 • edited Loading

roberth left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

`lib.fileset.gitTracked`: init #264891

`lib.fileset.gitTracked`: init #264891

infinisil commented Nov 1, 2023 •

edited

Loading

infinisil commented Nov 8, 2023 •

edited

Loading

roberth commented Nov 10, 2023 •

edited

Loading

infinisil commented Nov 16, 2023 •

edited

Loading