Try to substitute tree for all uses of fetchTree instead of just in flakes #3520
Conversation
| if (narHash) { | ||
| try { | ||
| auto tree = substituteTree(store); | ||
| return {std::move(tree), shared_from_this()}; |
There was a problem hiding this comment.
Unfortunately, this is not correct because this will return a different Tree and Input than fetchTreeInternal(). For instance, for a Git input, fetchTreeInternal() will return an Input that has rev set, and a Tree that has revCount and lastModified set.
So we can only substitute the store path if we can get those Input / Tree attributes in some other way. It works for flake inputs because flake.lock provides the missing attributes.
There was a problem hiding this comment.
Ok makes sense. So basically a locked ref always needs to be returned? Not that you couldn't substitute an unlocked ref with a narHash, but that wouldn't allow things like lockfile creation? If so was wondering why LockedRef isn't a subclass, which fetchTree could then return.
Trying to fix the FIXME in fetchTree.cc:86
Wasn't quite sure how having a nar hash interacts with resolved refs. Since narHash is optional, a resolved ref could be immutable, but then possibly not locked? But then it doesn't make sense to assume that fetchOrSubstituteTree returns a lockedRef:
nix/src/libexpr/flake/flake.cc
Line 210 in 8c75621
Should that be treated as an immutable ref but not locked? Or should substitute only occur for locked refs?