New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
types: Fix path.check for cleanSourceWith output #64691
base: master
Are you sure you want to change the base?
Conversation
Advantage of `cleanSourceWith` is that it composes without copying intermediate output to the store. Currently using `path.check` breaks this. Consider: ```nix let cleanedSource1 = cleanSourceWith { src = ./.; filter = filter1; }; in assert (path.check cleanedSource1); cleanSourceWith { src = cleanedSource1; filter = filter2; } ``` Currently this will copy all the files that pass filter1 to the store in order to check that the resulting path starts with "/". Luckily `cleanSourceWith` exposes a `origSrc` property that we can check.
Alternatively one may call |
|
Thank you for your contributions.
|
I marked this as stale due to inactivity. → More info |
Is this still needed? |
@@ -243,7 +243,7 @@ rec { | |||
path = mkOptionType { | |||
name = "path"; | |||
# Hacky: there is no ‘isPath’ primop. | |||
check = x: builtins.substring 0 1 (toString x) == "/"; | |||
check = x: builtins.substring 0 1 (toString (x.origSrc or x)) == "/"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps something like this, for a more general and decoupled solution. Needs a new attr in cleanSource and friends.
check = x: builtins.substring 0 1 (toString (x.origSrc or x)) == "/"; | |
check = x: x._isPath or builtins.substring 0 1 (toString x) == "/"; |
Advantage of
cleanSourceWith
is that it composes without copying intermediate output to the store. Currently usingpath.check
breaks this. Consider:Currently this will copy all the files that pass filter1 to the store in order to check that the resulting path starts with "/".
Luckily
cleanSourceWith
exposes aorigSrc
property that we can check.Motivation for this change
Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)