lib.fileset.intersection: init#257356
Conversation
ofborg
bot
added
10.rebuild-darwin: 0
infinisil
force-pushed
the
fileset.intersect
branch
2 times, most recently
from
cc723f2 to
98346b0
Compare
infinisil
changed the title
lib.fileset,intersect, lib.fileset.intersects: initlib.fileset,intersection: init
infinisil
changed the title
lib.fileset,intersection: initlib.fileset.intersection: init
lib/fileset/README.md
Outdated
There was a problem hiding this comment.
@roberth @fricklerhandwerk This PR isn't done yet, but I wonder what you think of this design decision.
It's an interesting duality:
- Union is mostly needed between many file sets (e.g. combining together the exact files you need)
- Intersection is mostly needed between few file sets (e.g. limiting the selected files to some directory)
We might really only need unions (taking a list) and intersection (binary operation).
union (binary operation) doesn't seem that important, but I can imagine some situations where it could be useful and we have it already.
Notably Haskell's Data.Set also provides union, unions, intersection but no intersections (there is one, but it's in Data.Set.Internal).
There was a problem hiding this comment.
We could have an interface that requires at least one set: intersect :: a -> [a] -> a. But probably a binary one is good enough. One can still fold over a list, then defining the base set will be required for sane results. That again we can add to documentation as an example.
There was a problem hiding this comment.
In math it's easy to invoke a somewhat implicitly defined "universe" set. Us finite mortals would have to make that explicit, which is another way to land on a -> List a -> a whereas @fricklerhandwerk seems to have arrived through NonEmptyList a -> a.
infinisil
force-pushed
the
fileset.intersect
branch
3 times, most recently
from
10a4700 to
81bb565
Compare
|
This is ready to review now. There's tests and documentation now. |
infinisil
force-pushed
the
fileset.intersect
branch
from
81bb565 to
86b6d46
Compare
delroth
added
the
12.approvals: 1
|
@roberth This PR is ready to be merged imo, so I'm planning to merge it myself in a couple days unless you have some concerns until then. And if it looks good to you I wouldn't mind it being merged earlier of course :) |
lib/fileset/README.md
Outdated
There was a problem hiding this comment.
Wasn't it already defined to be the most specific, longest path that contains them all? Or is it not feasible to make that truly an invariant?
There was a problem hiding this comment.
It's not defined like that yet. It's a bit tricky because file sets can be empty but still have a base path, but I think it could be done with some careful wording
roberth
left a comment
roberth
left a comment
There was a problem hiding this comment.
A suggestion and some thoughts. LGTM.
lib/fileset/default.nix
Outdated
There was a problem hiding this comment.
For a more realistic example that readily applies in many projects:
let buildFiles = fileFilter (f: f.ext != "nix") ../.;
in intersection buildFiles (unions [ ../build-support ./src ./Makefile ])Not sure if I got the filter right. Had to change LICENSE in order to realistically encounter .nix files in the union.
There was a problem hiding this comment.
Initially I wrote the fileFilter without a path (../.). This would have been feasible if in a design where we allow infinite, non-enumerable sets such as intersections [ ].
There was a problem hiding this comment.
buildFiles is particularly useful if you have multiple derivations that each build some part of that set.
There was a problem hiding this comment.
fileFilter doesn't exist yet, but I just opened the PR for it yesterday: #260265
I can add this example in the PR once this one is merged
delroth
added
12.approvals: 2
Co-authored-by: Robert Hensing <[email protected]>
infinisil
force-pushed
the
fileset.intersect
branch
from
86b6d46 to
389be8d
Compare
delroth
removed
the
12.approvals: 2
While this change is backwards-incompatible, I think it's okay because: - The `fileFilter` function is not yet in a stable NixOS release, it was only merged about [a month ago](NixOS#257356). - All public uses of the function on GitHub only pass a path - Any `fileFilter pred fileset` can also be expressed as `intersection fileset (fileFilter pred path)` without loss of functionality. - This is furthermore pointed out in the new error message when a file set is passed
While this change is backwards-incompatible, I think it's okay because: - The `fileFilter` function is not yet in a stable NixOS release, it was only merged about [a month ago](NixOS/nixpkgs#257356). - All public uses of the function on GitHub only pass a path - Any `fileFilter pred fileset` can also be expressed as `intersection fileset (fileFilter pred path)` without loss of functionality. - This is furthermore pointed out in the new error message when a file set is passed
While this change is backwards-incompatible, I think it's okay because: - The `fileFilter` function is not yet in a stable NixOS release, it was only merged about [a month ago](NixOS/nixpkgs#257356). - All public uses of the function on GitHub only pass a path - Any `fileFilter pred fileset` can also be expressed as `intersection fileset (fileFilter pred path)` without loss of functionality. - This is furthermore pointed out in the new error message when a file set is passed
4 participants
Description of changes
This is another split off from the WIP fileset combinators PR.
This adds one function:
lib.fileset.intersection: Computes the intersection between two file setsThis work is sponsored by Antithesis ✨
Things done