`lib.fileset.difference`: init #259065

infinisil · 2023-10-04T21:27:27Z

Description of changes

This is another split off from the WIP file set combinators PR. This PR adds one function:

lib.fileset.difference: Subtracts the second file set from the first.

# Recursively includes all files in ./. except ones under ./a/b
lib.fileset.difference ./. ./a/b

This work is sponsored by Antithesis ✨

Things done

Documentation
Tests

roberth · 2023-10-24T09:35:53Z

lib/fileset/default.nix

+      difference ./. ./tests
+
+      let
+        # A list of Nix files


Suggested change

# A list of Nix files

# A set of Nix-related files

nix/ may contain non-.nix files, which could be considered non-Nix.

roberth · 2023-10-24T09:37:49Z

lib/fileset/internal.nix

+            fileset2._internalBaseComponents
+        );
+
+      # The filesetTree's need to have the same base path in order to be able to compute the difference between them.


Suggested change

# The filesetTree's need to have the same base path in order to be able to compute the difference between them.

# The filesetTree's need to have the same base path in order to be able to compute the difference between them.

# Technically an unrelated root in the second, negative argument could be considered not to match any files in

# the first argument, but users would be clueless as to why their filter doesn't work.

Hmm not sure what you mean here. If the root of the second argument is unrelated (e.g. difference /foo /bar), then no files inside /foo match /bar, meaning nothing gets subtracted, so the result is /foo. And that should be expected.

I was thinking of entirely unrelated trees, such as when one is an absolute path, but the other is a fetchGit lazy tree.

Rejecting difference /foo /bar seems excessive, I agree.

Ah yeah, it will error for different filesystem roots, there's a comment on this in the comments for _difference:

# The filesets must already be coerced and validated to be in the same filesystem root.

The public difference function passes its arguments through _coerceMany, which checks against this, before calling the internal one.

roberth · 2023-10-24T09:42:50Z

lib/fileset/internal.nix

+  # Compute the set difference between two file sets.
+  # The filesets must already be coerced and validated to be in the same filesystem root.
+  # Type: Fileset -> Fileset -> Fileset
+  _difference = fileset1: fileset2:


Suggested change

_difference = fileset1: fileset2:

_difference = positive: negative:

Suggested change

_difference = fileset1: fileset2:

_difference = baseSet: negatingSet:

Suggested change

_difference = fileset1: fileset2:

_difference = fileset: subtrahend:

Nice! I'll also apply this to the user-facing function. I like positive/negative the most

roberth · 2023-10-24T09:46:57Z

lib/fileset/internal.nix

+          # The common prefix is the same as the first path, so the second path is equal or longer.
+          # We need to shorten the second arguments tree to the same base path as the first one


Not clear to me why.
What is tree2 for?

Same as the tree of the negative file set, but with the same base path as the positive file set, such that they are comparable and can be operated on.

E.g. if you have difference /foo /foo/bar, the common path would be /foo, meaning the first branch applies.

It then takes the negative file set, (base: /foo/bar, tree: "directory") in this case, and shortens the base to /foo instead, such that you get (base: /foo, tree: { bar = "directory"; }). tree2 here only contains the tree part though, so { bar = "directory"; }.

Now we have two trees, "directory" from the positive file set, and { bar = "directory"; } from the negative file set, that have the same base /foo, so they can be operated on, and it matches the base path that the result should have.

I'll put something like this in the code docs, and I'll improve the variable name, negativeTreeWithPositiveBasePath or so (maybe shorter :P)

roberth · 2023-10-24T09:49:56Z

lib/fileset/internal.nix

+      # The result is always the left hand side, because nothing gets removed
+      lhs
+    else
+      # Otherwise we always have two attribute sets which we can recurse into


Suggested change

# Otherwise we always have two attribute sets which we can recurse into

# Otherwise we always have two attribute sets to recurse into

roberth · 2023-10-24T09:55:48Z

lib/fileset/tests.sh

+    [c]=1
+)
+checkFileset 'difference ./. ./a/x'
+


How about removing the parent? e.g.

difference ./a ./.

Maybe the test could be more systematic, by framing it as a matrix of possible arguments. Perhaps not an actual matrix in terms of code, but laid out in a way that you can see that all combinations are tested.
That way you would naturally test difference ./a ./..

infinisil · 2023-10-25T20:27:53Z

@roberth Should be good now!

roberth · 2023-10-26T09:55:31Z

lib/fileset/default.nix

+        {
+          context = "first argument";
+          value = positive;
+        }
+        {
+          context = "second argument";
+          value = negative;
+        }


Suggested change

{

context = "first argument";

value = positive;

}

{

context = "second argument";

value = negative;

}

{

context = "first argument (positive set)";

value = positive;

}

{

context = "second argument (negative set)";

value = negative;

}

Not 100% sure, but might be nice.

roberth · 2023-10-26T09:56:29Z

lib/fileset/internal.nix

+      #   because under the base path of `/foo/bar`, everything from the negative file set is included
+      # For `difference /foo /bar`, `negativeTreePositiveBase = null`
+      #   because under the base path of `/foo`, nothing from the negative file set is included
+      negativeTreePositiveBase =


Suggested change

negativeTreePositiveBase =

negativeTreeWithPositiveBase =

infinisil · 2023-10-30T11:52:56Z

Done now, unless you do it before me, I'll merge once CI passes :)

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

infinisil mentioned this pull request Oct 4, 2023

[WIP] File set combinators #222981

Closed

7 tasks

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

infinisil force-pushed the fileset.difference branch from bee5e1d to b93c133 Compare October 11, 2023 15:35

infinisil marked this pull request as ready for review October 11, 2023 15:36

infinisil requested review from roberth and fricklerhandwerk October 11, 2023 15:36

infinisil marked this pull request as draft October 11, 2023 15:36

infinisil force-pushed the fileset.difference branch from b93c133 to 4f1dd7b Compare October 11, 2023 16:07

infinisil marked this pull request as ready for review October 11, 2023 16:11

fricklerhandwerk approved these changes Oct 23, 2023

View reviewed changes

roberth reviewed Oct 24, 2023

View reviewed changes

infinisil force-pushed the fileset.difference branch 2 times, most recently from 2d4b2e4 to cb9a53d Compare October 25, 2023 20:22

roberth approved these changes Oct 26, 2023

View reviewed changes

lib.fileset.difference: init

50df7f9

infinisil force-pushed the fileset.difference branch from cb9a53d to 50df7f9 Compare October 30, 2023 11:52

infinisil merged commit fc28c5e into NixOS:master Nov 1, 2023
6 checks passed

infinisil deleted the fileset.difference branch November 1, 2023 18:40

infinisil mentioned this pull request Nov 9, 2023

File set library tracking issue and feature requests #266356

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`lib.fileset.difference`: init #259065

`lib.fileset.difference`: init #259065

infinisil commented Oct 4, 2023 •

edited

Loading

roberth Oct 24, 2023

roberth Oct 24, 2023

infinisil Oct 25, 2023 •

edited

Loading

roberth Oct 25, 2023

infinisil Oct 25, 2023

roberth Oct 24, 2023

infinisil Oct 25, 2023

roberth Oct 24, 2023

infinisil Oct 25, 2023 •

edited

Loading

roberth Oct 24, 2023

roberth Oct 24, 2023

roberth Oct 24, 2023

infinisil Oct 25, 2023

infinisil commented Oct 25, 2023

roberth Oct 26, 2023

roberth Oct 26, 2023

infinisil commented Oct 30, 2023

	_difference = fileset1: fileset2:
	_difference = positive: negative:

	_difference = fileset1: fileset2:
	_difference = baseSet: negatingSet:

	_difference = fileset1: fileset2:
	_difference = fileset: subtrahend:

		# The common prefix is the same as the first path, so the second path is equal or longer.
		# We need to shorten the second arguments tree to the same base path as the first one

	# Otherwise we always have two attribute sets which we can recurse into
	# Otherwise we always have two attribute sets to recurse into

lib.fileset.difference: init #259065

lib.fileset.difference: init #259065

Conversation

infinisil commented Oct 4, 2023 • edited Loading

Description of changes

Things done

Choose a reason for hiding this comment

Choose a reason for hiding this comment

infinisil Oct 25, 2023 • edited Loading

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 Oct 25, 2023 • edited Loading

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 Oct 25, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

infinisil commented Oct 30, 2023

`lib.fileset.difference`: init #259065

`lib.fileset.difference`: init #259065

infinisil commented Oct 4, 2023 •

edited

Loading

infinisil Oct 25, 2023 •

edited

Loading

infinisil Oct 25, 2023 •

edited

Loading