`lib.fileset.difference`: init by infinisil · Pull Request #259065 · NixOS/nixpkgs

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

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

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

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

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

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

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

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

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 This PR causes between 1 and 10 packages to rebuild on Linux. 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 fricklerhandwerk and roberth 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

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

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

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

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

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

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

-        {
-          context = "first argument";
-          value = positive;
-        }
-        {
-          context = "second argument";
-          value = negative;
-        }
+        {
+          context = "first argument (positive set)";
+          value = positive;
+        }
+        {
+          context = "second argument (negative set)";
+          value = negative;
+        }

Uh oh!

Comments

Conversation

infinisil commented Oct 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of changes

Things done

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

infinisil Oct 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

infinisil Oct 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

infinisil commented Oct 25, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

infinisil commented Oct 30, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

infinisil commented Oct 4, 2023 •

edited

Loading

infinisil Oct 25, 2023 •

edited

Loading

infinisil Oct 25, 2023 •

edited

Loading