Input: Replace 'locked' bool by isLocked() method #10024
Conversation
github-actions
bot
added
new-cli
It's better to just check whether the input has all the attributes needed to consider itself locked (e.g. whether a Git input has an 'rev' attribute). Also, the 'locked' field was actually incorrect for Git inputs: it would be set to true even for dirty worktrees. As a result, we got away with using fetchTree() internally even though fetchTree() requires a locked input in pure mode. In particular, this allowed '--override-input' to work by accident. The fix is to pass a set of "overrides" to call-flake.nix for all the unlocked inputs (i.e. the top-level flake and any --override-inputs).
edolstra
force-pushed
the
remove-locked-flag
branch
from
69f6cea
to
071dd2b
Compare
|
I reviewed the |
|
@Ericson2314 Good point. We don't need a NAR hash for Git/Mercurial for an input to be considered locked, but we do for GitHub, since we can't verify the tarball from the rev alone. So I've changed the GitHub fetcher to require a NAR hash. |
|
OK I see. Ideally there would be something a bit more "constructive" e.g. |
|
Yeah, right now the callers only care whether an input is locked, not how it is locked. |
|
(N.B. I didn't review the rest, but I guess it is closer to that sort of evidence-passing thing I envisioned.) |
| bool isLocked(const Input & input) const override | ||
| { | ||
| /* Since we can't verify the integrity of the tarball from the | ||
| Git revision alone, we also require a NAR hash for | ||
| locking. FIXME: in the future, we may want to require a Git | ||
| tree hash instead of a NAR hash. */ | ||
| return input.getRev().has_value() && input.getNarHash().has_value(); | ||
| } | ||
|
|
There was a problem hiding this comment.
This is quite a breaking change (and it also breaks the githubFlakes test, although that was hidden behind other breakages)
There was a problem hiding this comment.
(not to say that we necessarily shouldn't have it, but it should at the very least get a release note)
Motivation
It's better to just check whether the input has all the attributes needed to consider itself locked (e.g. whether a Git input has an
revattribute).Also, the
lockedfield was actually incorrect for Git inputs: it would be set to true even for dirty worktrees. As a result, we got away with usingfetchTree()internally even thoughfetchTree()requires a locked input in pure mode. In particular, this allowed--override-inputto work by accident.The fix is to pass a set of "overrides" to
call-flake.nixfor all the unlocked inputs (i.e. the top-level flake and any--override-inputs). This is more efficient anyway since it avoids refetching inputs thatlockFlake()already fetched.Backported from the lazy-trees branch.
Context
Priorities and Process
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.