fetchers/git: make relative path absolute for local repo (backport #12107) by mergify[bot] · Pull Request #12265 · NixOS/nix

mergify · 2025-01-15T19:55:35Z

Motivation

Closes #9708 (again).

This patch:

(naively) Resolves relative paths to local git repos, returning absolute paths, thus allowing flake URIs such as git+file:./${submodule} to continue working.
Adds tests to prevent future breakages.

Context

Flake URIs such as git+file:./${submodule} used to work before 2.18, but broke in 2.19, was fixed later, and broke again recently since 3e0129c, resulting in a core-dumped assertion error in #9708 (comment).

This PR suppresses the error and allows git+file:./${submodule}to continue working. However, this fix is not ideal and should potentially be superseded by a better submodule implementation in the future. See:

A better fix is outlined by @roberth in https://discourse.nixos.org/t/57783, namely:

This bug seems to have allowed relative git flake inputs to be resolved against the current working directory (as in POSIX), and this tends to work out ok in the context of flakes, but is the wrong behavior, as it should resolve against the flake.nix base directory instead.
Supporting relative paths in fetchTree would add significant value.
This behavior was used as a workaround for lacking relative flake input or “subflake” support.

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

Friendly pings:

@roberth and @tomberek for reviews
@DamienCassou and @aij for testing

This is an automatic backport of pull request #12107 done by [Mergify](https://mergify.com).

(cherry picked from commit 96bd9ba) # Conflicts: # src/libfetchers/git.cc

Relative, local git repo used to work (for submodules), but it fails after 3e0129c. This commit adds a test to prevent such failure in the future. (cherry picked from commit 9d088fa)

(cherry picked from commit 37ac18d) # Conflicts: # tests/functional/flakes/flake-in-submodule.sh

(cherry picked from commit d9a50c0)

mergify · 2025-01-15T19:55:36Z

Cherry-pick of 96bd9ba has failed:

On branch mergify/bp/2.24-maintenance/pr-12107
Your branch is up to date with 'origin/2.24-maintenance'.

You are currently cherry-picking commit 96bd9bad2.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   src/libfetchers/git.cc

no changes added to commit (use "git add" and/or "git commit -a")

Cherry-pick of 37ac18d has failed:

On branch mergify/bp/2.24-maintenance/pr-12107
Your branch is ahead of 'origin/2.24-maintenance' by 2 commits.
  (use "git push" to publish your local commits)

You are currently cherry-picking commit 37ac18d1d.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   tests/functional/flakes/flake-in-submodule.sh

no changes added to commit (use "git add" and/or "git commit -a")

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

bryango

@roberth proposed resolutions of backport conflicts!

bryango · 2025-01-16T13:29:44Z

src/libfetchers/git.cc

+<<<<<<< HEAD
        repoInfo.url = repoInfo.isLocal ? url.path : url.base;
+=======
+        //
+        // FIXME: here we turn a possibly relative path into an absolute path.
+        // This allows relative git flake inputs to be resolved against the
+        // **current working directory** (as in POSIX), which tends to work out
+        // ok in the context of flakes, but is the wrong behavior,
+        // as it should resolve against the flake.nix base directory instead.
+        //
+        // See: https://discourse.nixos.org/t/57783 and #9708
+        //
+        repoInfo.url = repoInfo.isLocal ? std::filesystem::absolute(url.path).string() : url.to_string();
+>>>>>>> 96bd9bad2 (fetchers/git: make path absolute for local repo)


Suggested change

<<<<<<< HEAD

repoInfo.url = repoInfo.isLocal ? url.path : url.base;

=======

//

// FIXME: here we turn a possibly relative path into an absolute path.

// This allows relative git flake inputs to be resolved against the

// **current working directory** (as in POSIX), which tends to work out

// ok in the context of flakes, but is the wrong behavior,

// as it should resolve against the flake.nix base directory instead.

//

// See: https://discourse.nixos.org/t/57783 and #9708

//

repoInfo.url = repoInfo.isLocal ? std::filesystem::absolute(url.path).string() : url.to_string();

>>>>>>> 96bd9bad2 (fetchers/git: make path absolute for local repo)

//

// FIXME: here we turn a possibly relative path into an absolute path.

// This allows relative git flake inputs to be resolved against the

// **current working directory** (as in POSIX), which tends to work out

// ok in the context of flakes, but is the wrong behavior,

// as it should resolve against the flake.nix base directory instead.

//

// See: https://discourse.nixos.org/t/57783 and #9708

//

repoInfo.url = repoInfo.isLocal ? std::filesystem::absolute(url.path).string() : url.to_string();

bryango · 2025-01-16T13:31:29Z

tests/functional/flakes/flake-in-submodule.sh

+<<<<<<< HEAD
+=======
+
+# Test that `nix flake metadata` parses `submodule` correctly.
+cat > "$rootRepo"/flake.nix <<EOF
+{
+    outputs = { self }: {
+    };
+}
+EOF
+git -C "$rootRepo" add flake.nix
+git -C "$rootRepo" commit -m "Add flake.nix"
+
+storePath=$(nix flake metadata --json "$rootRepo?submodules=1" | jq -r .path)
+[[ -e "$storePath/submodule" ]]
+
+# The root repo may use the submodule repo as an input
+# through the relative path. This may change in the future;
+# see: https://discourse.nixos.org/t/57783 and #9708.
+cat > "$rootRepo"/flake.nix <<EOF
+{
+    inputs.subRepo.url = "git+file:./submodule";
+    outputs = { ... }: { };
+}
+EOF
+git -C "$rootRepo" add flake.nix
+git -C "$rootRepo" commit -m "Add subRepo input"
+(
+  cd "$rootRepo"
+  # The submodule must be locked to the relative path,
+  # _not_ the absolute path:
+  [[ $(nix flake metadata --json | jq -r .locks.nodes.subRepo.locked.url) = "file:./submodule" ]]
+)
+>>>>>>> 37ac18d1d (tests/flake-in-submodule: git+file:./* input)


Suggested change

<<<<<<< HEAD

=======

# Test that `nix flake metadata` parses `submodule` correctly.

cat > "$rootRepo"/flake.nix <<EOF

{

outputs = { self }: {

};

}

EOF

git -C "$rootRepo" add flake.nix

git -C "$rootRepo" commit -m "Add flake.nix"

storePath=$(nix flake metadata --json "$rootRepo?submodules=1" | jq -r .path)

[[ -e "$storePath/submodule" ]]

# The root repo may use the submodule repo as an input

# through the relative path. This may change in the future;

# see: https://discourse.nixos.org/t/57783 and #9708.

cat > "$rootRepo"/flake.nix <<EOF

{

inputs.subRepo.url = "git+file:./submodule";

outputs = { ... }: { };

}

EOF

git -C "$rootRepo" add flake.nix

git -C "$rootRepo" commit -m "Add subRepo input"

(

cd "$rootRepo"

# The submodule must be locked to the relative path,

# _not_ the absolute path:

[[ $(nix flake metadata --json | jq -r .locks.nodes.subRepo.locked.url) = "file:./submodule" ]]

)

>>>>>>> 37ac18d1d (tests/flake-in-submodule: git+file:./* input)

# Test that `nix flake metadata` parses `submodule` correctly.

cat > "$rootRepo"/flake.nix <<EOF

{

outputs = { self }: {

};

}

EOF

git -C "$rootRepo" add flake.nix

git -C "$rootRepo" commit -m "Add flake.nix"

storePath=$(nix flake metadata --json "$rootRepo?submodules=1" | jq -r .path)

[[ -e "$storePath/submodule" ]]

# The root repo may use the submodule repo as an input

# through the relative path. This may change in the future;

# see: https://discourse.nixos.org/t/57783 and #9708.

cat > "$rootRepo"/flake.nix <<EOF

{

inputs.subRepo.url = "git+file:./submodule";

outputs = { ... }: { };

}

EOF

git -C "$rootRepo" add flake.nix

git -C "$rootRepo" commit -m "Add subRepo input"

(

cd "$rootRepo"

# The submodule must be locked to the relative path,

# _not_ the absolute path:

[[ $(nix flake metadata --json | jq -r .locks.nodes.subRepo.locked.url) = "file:./submodule" ]]

)

edolstra · 2025-01-16T15:53:36Z

To be honest, I'd prefer to hold off on backporting this PR, since the behaviour of making the fetcher depend on the cwd seems extremely questionable to me. (See #12107 (comment).)

bryango · 2025-01-17T03:40:07Z

Agree, let's see how:

... are finalized and then decide what to do.

roberth · 2025-01-21T22:18:50Z

Done in Backport git+file:./ fixes to 2.24 (#12107 + #12277) #12287

bryango and others added 4 commits January 15, 2025 19:55

fetchers/git: make path absolute for local repo

a31d865

(cherry picked from commit 96bd9ba) # Conflicts: # src/libfetchers/git.cc

tests/flakes: check git+file:./${submodule} protocol

da989e7

Relative, local git repo used to work (for submodules), but it fails after 3e0129c. This commit adds a test to prevent such failure in the future. (cherry picked from commit 9d088fa)

tests/flake-in-submodule: git+file:./* input

4a6d797

(cherry picked from commit 37ac18d) # Conflicts: # tests/functional/flakes/flake-in-submodule.sh

Clarify cd call in tests/functional/flakes/flakes.sh

809b771

(cherry picked from commit d9a50c0)

mergify bot requested a review from edolstra as a code owner January 15, 2025 19:55

mergify bot added conflicts merge-queue labels Jan 15, 2025

github-actions bot added with-tests Issues related to testing. PRs with tests have some priority fetching Networking with the outside (non-Nix) world, input locking labels Jan 15, 2025

bryango suggested changes Jan 16, 2025

View reviewed changes

bryango mentioned this pull request Jan 17, 2025

Backport git+file:./ fixes to 2.24 (#12107 + #12277) #12287

Merged

roberth closed this Jan 21, 2025

xokdvium deleted the mergify/bp/2.24-maintenance/pr-12107 branch October 7, 2025 21:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fetchers/git: make relative path absolute for local repo (backport #12107)#12265

fetchers/git: make relative path absolute for local repo (backport #12107)#12265
mergify[bot] wants to merge 4 commits into2.24-maintenancefrom
mergify/bp/2.24-maintenance/pr-12107

mergify bot commented Jan 15, 2025

Uh oh!

mergify bot commented Jan 15, 2025

Uh oh!

bryango left a comment

Uh oh!

bryango Jan 16, 2025

Uh oh!

bryango Jan 16, 2025

Uh oh!

edolstra commented Jan 16, 2025

Uh oh!

bryango commented Jan 17, 2025

Uh oh!

roberth commented Jan 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

mergify bot commented Jan 15, 2025

Motivation

Context

Uh oh!

mergify bot commented Jan 15, 2025

Uh oh!

bryango left a comment

Choose a reason for hiding this comment

Uh oh!

bryango Jan 16, 2025

Choose a reason for hiding this comment

Uh oh!

bryango Jan 16, 2025

Choose a reason for hiding this comment

Uh oh!

edolstra commented Jan 16, 2025

Uh oh!

bryango commented Jan 17, 2025

Uh oh!

roberth commented Jan 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants