-
-
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
Improve support for relative path inputs #10089
base: master
Are you sure you want to change the base?
Conversation
🎉 All dependencies have been resolved ! |
Discussed during the Nix maintainers meeting on 2024-03-04.
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/2024-03-04-nix-team-meeting-minute-130/40830/1 |
…re path This is needed for the path:// input scheme (until it returns a FSInputAccessor to the original path, but that's blocked by NixOS#10089) and the Mercurial input scheme.
…re path This is needed for the path:// input scheme (until it returns a FSInputAccessor to the original path, but that's blocked by NixOS#10089) and the Mercurial input scheme.
47ccb1d
to
690281f
Compare
Subflakes are flakes in the same tree, accessed in flake inputs via relative paths (e.g. `inputs.foo.url = "path:./subdir"`). Previously these didn't work very well because they would be separately copied to the store, which is inefficient and makes references to parent directories tricky or impossible. Furthermore, they had their own NAR hash in the lock file, which is superfluous since the parent is already locked. Now subflakes are accessed via the accessor of the calling flake. This avoids the unnecessary copy and makes it possible for subflakes to depend on flakes in a parent directory (so long as they're in the same tree). Lock file nodes for relative flake inputs now have a new `parent` field: { "locked": { "path": "./subdir", "type": "path" }, "original": { "path": "./subdir", "type": "path" }, "parent": [ "foo", "bar" ] } which denotes that `./subdir` is to be interpreted relative to the directory of the `bar` input of the `foo` input of the root flake. Extracted from the lazy-trees branch.
`parentNode.sourceInfo.outPath` does not include the subdir of the parent flake, while `parentNode.outPath` does. So we need to use the latter.
Relative path flakes ("subflakes") are basically fundamentally broken, since they produce lock file entries like "locked": { "lastModified": 1, "narHash": "sha256-/2tW9SKjQbRLzfcJs5SHijli6l3+iPr1235zylGynK8=", "path": "./flakeC", "type": "path" }, that don't specify what "./flakeC" is relative to. They *sometimes* worked by accident because the `narHash` field allowed `fetchToStore()` to get the store path of the subflake *if* it happened to exist in the local store or in a substituter. Subflakes are properly fixed in NixOS#10089 (which adds a "parent" field to the lock file). Rather than come up with some crazy hack to make them work in the interim, let's just disable the only test that depends on the broken behaviour for now.
…Jobs.tests.functional_user
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/2024-11-27-nix-team-meeting-minutes-198/56691/1 |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/2024-12-18-nix-team-meeting-minutes-204/57602/1 |
Does this also mean that subflakes will auto-update? In other words, |
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.
Reviewed together in meeting
- Release note: Generalizing the flake format can break tools that consume
flake.nix
files. For instancetoJSON (import ./flake.nix).inputs
will not work as expected anymore for flakes that use path values.
InputPath -> InputChain?
* Only for relative path flakes, i.e. 'path:./foo', returns the | ||
* relative path, i.e. './foo'. | ||
*/ | ||
std::optional<std::string> isRelative() const; |
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.
This should be in the expr / flake layer.
Open issue?
/* Respect the "flakeness" of the input even if we | ||
override it. */ | ||
if (hasOverride) | ||
input.isFlake = input2.isFlake; |
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.
Off-topic: perhaps flake-ness should be "type-checked"
(existing code, moved)
@@ -38,11 +38,19 @@ struct LockedNode : Node | |||
FlakeRef lockedRef, originalRef; | |||
bool isFlake = true; | |||
|
|||
/* The node relative to which relative source paths | |||
(e.g. 'path:../foo') are interpreted. */ | |||
std::optional<InputPath> parentPath; |
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.
std::optional<InputPath> parentPath; | |
std::optional<InputPath> parentInputChain; |
as mentioned in the review header comment
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.
For consistency, let's postpone this to a future PR that resolves #12098.
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/2024-12-23-nix-team-meeting-minutes-205/57783/1 |
(using: nix-2.26.0pre20241223_9223d64) An update to the input of a subflake does not get automatically picked up by the root when running a build.
This will run the root with the "old" child and the mismatch/update is not detected. At this point the
Not sure if this is intended,,, but was a bit surprising. I'd guess that the root flake would detect the mismatch between its lock and that of the child - similar to how a significant change to an input is detected and locks updated when running things like |
Those are really good observations. "Subflake" suggests some sort of directional relationship other than just an input. We should avoid that term in the release notes, and/or explain what's not implemented.
The alternative term, relative path input would be ok to use for this feature.
Maybe subflakes should be the name for the pattern or feature where all input declarations are moved to the root of the flake? Or is it merely a subdirectory relationship?
Multi flake repositories might also be a useful name for the use case this pr enables.
|
I still need to more thoroughly test it, but I'm afraid the unsync'd lockfiles is a problem. When evaluating the root flake with an outdated/updated/modified lockfile, it will still use the Not sure if this is intended. Idea brainstorming:
note: I do love the new capability to avoid the "infinite updating lockfile"! Excited to keep playing with it. |
I think I'm reading pseudo-syntax that's a little ambiguous, but I would think that the locking of the local sources is fixed by this PR. The locking of transitive dependencies between flakes in the same repo I think should be solved with #7730; otherwise we'll need a lot of new logic to deal with its symptoms. |
That's right, and probably not ideal, but it's consistent with how lock files currently work in general: lock files are independent, and the only time that we use the lock file of an input is if the referring flake doesn't have a lock for that input yet. So a change to the lock file of an input doesn't propagate to the referring flake. This should indeed be fixed by having sparse lock files (#7730).
Yeah we don't use the term "subflake" anywhere (except previously in the PR description, fixed). |
Motivation
We want it to be possible to refer to flake inputs that reside in the same repo via relative paths (e.g.
inputs.foo.url = "path:./subdir"
). Previously this didn't work very well because those inputs would be separately copied to the store, which is inefficient and makes references to parent directories tricky or impossible. Furthermore, they had their own NAR hash in the lock file, which is superfluous since the parent is already locked. In fact, if relative path inputs worked at all, it was by accident.Now relative path inputs are accessed via the accessor of the calling flake. This avoids the unnecessary copy and makes it possible for relative path inputs to depend on flakes in a parent directory (so long as they're in the same tree).
Lock file nodes for relative path inputs now have a new
parent
field:which denotes that
./subdir
is to be interpreted relative to the directory of thebar
input of thefoo
input of the root flake.Extracted 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.