-
-
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
fetchers/git: make relative path absolute for local repo (backport #12107) #12265
base: 2.24-maintenance
Are you sure you want to change the base?
Conversation
Cherry-pick of 96bd9ba has failed:
Cherry-pick of 37ac18d has failed:
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 |
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.
@roberth proposed resolutions of backport conflicts!
<<<<<<< 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) |
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.
<<<<<<< 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(); |
<<<<<<< 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) |
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.
<<<<<<< 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" ]] | |
) |
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).) |
Agree, let's see how:
... are finalized and then decide what to do. |
Motivation
Closes #9708 (again).
This patch:
git+file:./${submodule}
to continue working.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:git+file:./${submodule}
no longer works #9708A better fix is outlined by @roberth in https://discourse.nixos.org/t/57783, namely:
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.
Friendly pings:
This is an automatic backport of pull request #12107 done by [Mergify](https://mergify.com).