-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
path-info: implement --filter-substitutable #7587
base: master
Are you sure you want to change the base?
Conversation
b706d6c
to
dca6010
Compare
12e0743
to
59c7db4
Compare
@roberth, it seems this implementation is fast, but it suffers from only every querying the official cache (cache.nixos.org). Trying to nail down why exactly that is has been sort of a PITA. Do you happen to have any insight into that? Update |
src/nix/path-info.cc
Outdated
substitutablePaths.find(store->followLinksToStorePath(storePathS)) == | ||
substitutablePaths.end() | ||
? v["substitutable"] = false | ||
: v["substitutable"] = true; |
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.
substitutablePaths.find(store->followLinksToStorePath(storePathS)) == | |
substitutablePaths.end() | |
? v["substitutable"] = false | |
: v["substitutable"] = true; | |
v["substitutable"] = (bool) substitutablePaths.count(store->followLinksToStorePath(storePathS)); |
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.
Maybe instead of a substitutable
bool, it should return an array of substituters?
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.
Good idea
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.
So now that I've figured out why Store::querySubstitutablePaths
wasn't working: #7587 (comment), I'm not sure how to easily do this. If I stick with defining the logic myself it should be easy though.
Attempts at using querySubstitutablePathInfos
result in much slower queries which is why I moved away from that implementation. One possibility would be to add a flag to queryMissing
to ignore the local store, but that would break the API.
ab201d1
to
a895e82
Compare
69f6d20
to
a895e82
Compare
I'm starting to think that I am just gonna have to make a new sub-command for this functionality, as the Also have some async refactor work here that I'd like to include, but that might be better off as a separate PR. edit Running all queries asynchronously is faster than waiting to query subsequent caches based on results from previous caches in my benchmarks so far. In addition, returning a json of uncached output dependencies seems like the right move so that they can be easily passed to a build command. I've also opted to filter out derivations with |
That's awesome work @nrdxp |
src/nix/path-info.cc
Outdated
@@ -45,6 +46,14 @@ struct CmdPathInfo : StorePathsCommand, MixJSON | |||
.description = "Show signatures.", | |||
.handler = {&showSigs, true}, | |||
}); | |||
|
|||
addFlag({ | |||
.longName = "query-substitutes", |
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.
Maybe this should be named --filter-substitutable
or something like that? Since it doesn't really query what the substitutes are.
Fixes NixOS#3946 Filters out any paths from the output which are already available in the configured substituters. Useful in CI environments, to avoid builds that are already cached.
b68f15f
to
fb30259
Compare
@nrdxp please add a release note. |
Oops… misclick. |
Fixes #3946
Alternative to #7526 which implements the same logic in the new cli instead of the legacy
nix-store
Checklist for maintainers
Maintainers: tick if completed or explain if not relevant
tests/**.sh
src/*/tests
tests/nixos/*