Improve support for subflakes #10089

edolstra · 2024-02-26T18:53:34Z

Motivation

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. In fact, if subflakes worked at all, it was by accident.

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.

Depends on #10088.

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.

dpulls · 2024-03-04T13:40:43Z

🎉 All dependencies have been resolved !

thufschmitt · 2024-03-05T07:40:34Z

Discussed during the Nix maintainers meeting on 2024-03-04.

Idea approved
Document that directly accessing .. in subflakes is likely to break in the future
Use path literals as the url for subflakes

Great overall
Right now, the PR allows import ../. in a subflake. Do we want that?
- Directly accessing .. is convenient, but breaks the encapsulation property of flakes
  - Relates to nix flakes: flakes in subdirectories can access files in parent directories (breaking the implied seal of flake.nix) #4414
- We can however access other parts of the tree with relative inputs (that can go up until the root)
  - inputs.something_else = ../foo
- From an implementation perspective, hard to forbid without lazy trees
- Decision:
  - Don't formally forbid direct access to .. since it's hard without lazy trees
  - Once lazy trees lands, make that conditional on a flag
  - Potentially make the default restrictive. Would be a breaking change, but
    - can be mitigated by only making that change effective after a flake update
    - small breakage as getting the old behavior back would only be a one-line change
Syntax for relative flakes?
- Currently reuses url = "path:./foo" that already kinda works on master
- A more natural syntax would be url = ./foo
  - This is actually what most people seem to try first when they want subflakes

nixos-discourse · 2024-03-05T07:41:42Z

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

This wasn't caught by CI because #10149 and #10152 pass individually... It doesn't happen on lazy-trees either because we never try to fetch relative path flakes (#10089).

…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.

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.

NixOS#10089 (comment)

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

nixos-discourse · 2024-11-27T21:43:39Z

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

nixos-discourse · 2024-12-18T21:24:34Z

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

MagicRB · 2024-12-19T00:30:04Z

Does this also mean that subflakes will auto-update? In other words, nix flake update subflake ought to be unnecessary with this right?

roberth

Reviewed together in meeting

Release note: Generalizing the flake format can break tools that consume flake.nix files. For instance toJSON (import ./flake.nix).inputs will not work as expected anymore for flakes that use path values.

InputPath -> InputChain?

roberth · 2024-11-27T20:47:18Z

src/libfetchers/fetchers.hh

+     * Only for relative path flakes, i.e. 'path:./foo', returns the
+     * relative path, i.e. './foo'.
+     */
+    std::optional<std::string> isRelative() const;


This should be in the expr / flake layer.
Open issue?

roberth · 2024-12-23T14:25:23Z

src/nix/flake.md

+    the directory containing that `flake.nix`. However, the resolved
+    path must be in the same tree. For instance, a `flake.nix` in the
+    root of a tree can use `path:./foo` to access the flake in
+    subdirectory `foo`, but `path:../bar` is illegal.


Worth mentioning that foo can refer to its ../bar

roberth · 2024-12-23T14:41:05Z

src/libflake/flake/flakeref.hh

@@ -84,7 +84,8 @@ FlakeRef parseFlakeRef(
    const std::string & url,
    const std::optional<Path> & baseDir = {},
    bool allowMissing = false,
-    bool isFlake = true);
+    bool isFlake = true,
+    bool allowRelative = false);


Suggested change

bool allowRelative = false);

bool preserveRelativePaths = false);

roberth · 2024-12-23T14:45:47Z

src/libflake/flake/flake.cc

@@ -386,13 +408,29 @@ LockedFlake lockFlake(

        debug("old lock file: %s", oldLockFile);

-        std::map<InputPath, FlakeInput> overrides;
+        struct Override


Suggested change

struct Override

struct OverrideValue

roberth · 2024-12-23T14:48:52Z

src/libflake/flake/flake.cc

+                    /* Respect the "flakeness" of the input even if we
+                       override it. */
+                    if (hasOverride)
+                        input.isFlake = input2.isFlake;


Off-topic: perhaps flake-ness should be "type-checked"

(existing code, moved)

roberth · 2024-12-23T15:09:18Z

src/libflake/flake/flakeref.cc

    }

    fetchers::Attrs attrs;
    attrs.insert_or_assign("type", "path");
    attrs.insert_or_assign("path", path);

-    return std::make_pair(FlakeRef(fetchers::Input::fromAttrs(fetchSettings, std::move(attrs)), ""), fragment);
+    return fromParsedURL(fetchSettings, {
+        .url = path,


This is a bit wrong.
We could either

remove this and always reconstruct URLs for display, or

the field name should be something like displayText.

roberth · 2024-12-23T15:11:42Z

src/libflake/flake/lockfile.hh

@@ -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;


Suggested change

std::optional<InputPath> parentPath;

std::optional<InputPath> parentInputChain;

as mentioned in the review header comment

roberth · 2024-12-23T15:24:21Z

src/nix/flake.md

+    path must be in the same tree. For instance, a `flake.nix` in the
+    root of a tree can use `path:./foo` to access the flake in
+    subdirectory `foo`, but `path:../bar` is illegal.
+


Suggested change

Path inputs can be specified with path values in `flake.nix`. Path values are a syntax for `path` inputs, and they are converted by

1. resolving them into relative paths, relative to the base directory of `flake.nix`

2. escaping URL characters (refer to IETF RFC?)

3. prepending `path:`

Note that the allowed syntax for path values in flake `inputs` may be more restrictive than general Nix, so you may need to use `path:` if your path contains certain special characters. See [Path literals](@docroot@/language/syntax#path-literal)

roberth · 2024-12-23T15:41:20Z

tests/functional/flakes/relative-paths.sh

+# Test circular relative path flakes. FIXME: doesn't work at the moment.
+if false; then


Is it an ok-ish error or a crash?

nixos-discourse · 2024-12-23T16:08:20Z

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

edolstra marked this pull request as draft February 26, 2024 18:53

github-actions bot added documentation new-cli Relating to the "nix" command with-tests Issues related to testing. PRs with tests have some priority fetching Networking with the outside (non-Nix) world, input locking labels Feb 26, 2024

thufschmitt added the idea approved The given proposal has been discussed and approved by the Nix team. An implementation is welcome. label Mar 5, 2024

edolstra mentioned this pull request Mar 7, 2024

PathInputScheme::getFingerprint(): Don't barf on relative paths #10176

Merged

edolstra mentioned this pull request Apr 15, 2024

fetchToStore(): Avoid duplicate copying if the input is already a store path #10511

Open

edolstra force-pushed the relative-flakes branch from 47ccb1d to 690281f Compare April 23, 2024 14:01

edolstra force-pushed the relative-flakes branch from 690281f to b2be6fe Compare May 15, 2024 18:30

edolstra added 2 commits May 17, 2024 16:38

call-flake.nix: Fix relative path resolution

49f592d

`parentNode.sourceInfo.outPath` does not include the subdir of the parent flake, while `parentNode.outPath` does. So we need to use the latter.

Allow the 'url' flake input attribute to be a path literal

3180671

NixOS#10089 (comment)

edolstra mentioned this pull request Sep 11, 2024

Input::fetchToStore(): Don't try to substitute #10612

Merged

edolstra added 8 commits September 16, 2024 13:45

Merge remote-tracking branch 'origin/master' into relative-flakes

21fc07c

shellcheck

09d7197

parentPath -> parentInputPath

71d4bb8

Merge remote-tracking branch 'origin/master' into relative-flakes

91e7d49

tests/functional/flakes/relative-paths.sh: Fix build failure in hydra…

f206325

…Jobs.tests.functional_user

Merge remote-tracking branch 'origin/master' into relative-flakes

0b00bf7

Remove FIXME

00b99b8

Remove FIXME

985b2f9

edolstra marked this pull request as ready for review November 27, 2024 14:25

edolstra requested a review from fricklerhandwerk as a code owner November 27, 2024 14:25

This was referenced Nov 27, 2024

add dual use edolstra/flake-compat#55

Closed

Add support for "path:./relative/path" edolstra/flake-compat#18

Open

roberth requested review from roberth and removed request for fricklerhandwerk November 27, 2024 20:38

roberth assigned tomberek and roberth Nov 27, 2024

Enzime mentioned this pull request Dec 4, 2024

Disallow writing path input to lock file #12004

Open

2 tasks

roberth mentioned this pull request Dec 11, 2024

path:${builtins.storeDir}/... input is not fetchable when eval store is relocated #12040

Closed

2 tasks

Merge remote-tracking branch 'origin/master' into relative-flakes

8534c42

Remove dead code

9223d64

roberth reviewed Dec 23, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve support for subflakes #10089

Improve support for subflakes #10089

edolstra commented Feb 26, 2024 •

edited

Loading

dpulls bot commented Mar 4, 2024

thufschmitt commented Mar 5, 2024

nixos-discourse commented Mar 5, 2024

nixos-discourse commented Nov 27, 2024

nixos-discourse commented Dec 18, 2024

MagicRB commented Dec 19, 2024

roberth left a comment

roberth Nov 27, 2024

roberth Dec 23, 2024

roberth Dec 23, 2024

roberth Dec 23, 2024

roberth Dec 23, 2024

roberth Dec 23, 2024

roberth Dec 23, 2024

roberth Dec 23, 2024

roberth Dec 23, 2024

nixos-discourse commented Dec 23, 2024

	bool allowRelative = false);
	bool preserveRelativePaths = false);

	std::optional<InputPath> parentPath;
	std::optional<InputPath> parentInputChain;

+Path inputs can be specified with path values in `flake.nix`. Path values are a syntax for `path` inputs, and they are converted by
+. resolving them into relative paths, relative to the base directory of `flake.nix`
+. escaping URL characters (refer to IETF RFC?)
+. prepending `path:`
+Note that the allowed syntax for path values in flake `inputs` may be more restrictive than general Nix, so you may need to use `path:` if your path contains certain special characters. See [Path literals](@docroot@/language/syntax#path-literal)

		# Test circular relative path flakes. FIXME: doesn't work at the moment.
		if false; then

Improve support for subflakes #10089

Are you sure you want to change the base?

Improve support for subflakes #10089

Conversation

edolstra commented Feb 26, 2024 • edited Loading

Motivation

Context

Priorities and Process

dpulls bot commented Mar 4, 2024

thufschmitt commented Mar 5, 2024

nixos-discourse commented Mar 5, 2024

nixos-discourse commented Nov 27, 2024

nixos-discourse commented Dec 18, 2024

MagicRB commented Dec 19, 2024

roberth left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nixos-discourse commented Dec 23, 2024

edolstra commented Feb 26, 2024 •

edited

Loading