EvalState::realiseContext(): Allow access to the entire closure#12045
EvalState::realiseContext(): Allow access to the entire closure#12045roberth merged 1 commit intoNixOS:masterfrom
Conversation
edolstra
force-pushed
the
allow-closure
branch
from
48d9696 to
9aa5039
Compare
github-actions
bot
added
the
with-tests
|
Hmm, makes me wonder if we ever want |
What about |
| StorePathSet closure; | ||
| store->computeFSClosure(storePath, closure); |
There was a problem hiding this comment.
Computing the closure from scratch could cause a performance problem.
n calls to allowClosure would be O(n * closure size), which may be significantly larger than O(total allowed closure size).
The latter can be achieved by computing the closure of paths that aren't in an allowed closure yet, i.e. checking the references recursively and skipping ones that have been of the closure of any preceding allowClosure call.
There was a problem hiding this comment.
The cost of the call to computeFSClosure() is likely to be trivial compared to the actual build of the derivation we're importing from.
There was a problem hiding this comment.
Not entirely trivial to implement this optimization given the current code. I'm ok to keep this as is.
edolstra
force-pushed
the
allow-closure
branch
from
9aa5039 to
a6dc5d4
Compare
This would allow any store path to be included not just into derivations but into IFD by doing withStorePath = storePathStr: f:
let path = builtins.appendContext ... storePathStr;
in builtins.seq
(builtins.toFile "dummy" "${path}")
(f path) |
edolstra
force-pushed
the
allow-closure
branch
from
a6dc5d4 to
08361f0
Compare
🟠 Waiting for conditions to matchDetails
|
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
…2045 EvalState::realiseContext(): Allow access to the entire closure (backport #12045)
roberth
added
automatic backport
5 participants
Motivation
When doing IFD, we should allow access to the entire closure of the store paths built by the derivation, not just the top-level paths.
Fixes #11030.
Context
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.