Skip to content
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 #12107

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

bryango
Copy link
Member

@bryango bryango commented Dec 26, 2024

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:

@github-actions github-actions bot added the fetching Networking with the outside (non-Nix) world, input locking label Dec 26, 2024
@bryango bryango changed the title git: make relative path absolute for local repo fetchers/git: make relative path absolute for local repo Dec 26, 2024
@github-actions github-actions bot added the with-tests Issues related to testing. PRs with tests have some priority label Dec 27, 2024
@bryango bryango marked this pull request as ready for review December 27, 2024 13:29
@bryango bryango requested a review from edolstra as a code owner December 27, 2024 13:29
Copy link

@aij aij left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I gave it a quick try and it does work for me:

ivan@tobati:~/nixops/aij$ /nix/store/bn0djaymav6qa4zg5vaskzcaija5nq2p-nix-2.26.0pre20241227_9111645/bin/nix flake update 
warning: updating lock file '/home/ivan/nixops/aij/flake.lock':
• Updated input 'unstable':
    'git+file:unstable?rev=3f0036916daca9836ba2c3f236b0ae051949a0a8' (2024-12-21)
  → 'git+file:unstable?ref=refs/heads/ipmiutil&rev=174ae1b581ddd6d5a6c55684da392e0baf500b1e' (2024-12-27)

Copy link
Member

@roberth roberth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partial fix

@edolstra
Copy link
Member

For the example in #9709, namely

inputs.nixpkgs.url = git+file:./nixpkgs;

doesn't this result in a lock file that has the absolute path of the original Git repo in it? I.e. if I run nix flake lock on such a flake in /home/bob/repo, doesn't it result in a lock file that references /home/bob/repo/nixpkgs?

@bryango
Copy link
Member Author

bryango commented Jan 1, 2025

inputs.nixpkgs.url = git+file:./nixpkgs;

doesn't this result in a lock file that has the absolute path of the original Git repo in it? I.e. if I run nix flake lock on such a flake in /home/bob/repo, doesn't it result in a lock file that references /home/bob/repo/nixpkgs?

@edolstra Actually, no! It still gets locked to the relative path and shows up in flake.lock as file:./nixpkgs, which is great! I have pushed another test to cement this behavior.

Comment on lines 70 to 96
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 flake.nix"
(
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" ]]
)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

submodule does get locked to the relative path file:./submodule. I don't understand why but hey, it works!

@bryango
Copy link
Member Author

bryango commented Jan 4, 2025

Draft-ing and un-draft-ing to try to unlock the new CI...
Update: doesn't seem to work, no idea what I should do now haha

@bryango bryango marked this pull request as draft January 4, 2025 12:27
@bryango bryango marked this pull request as ready for review January 4, 2025 12:28
@roberth
Copy link
Member

roberth commented Jan 4, 2025

Rebased for GHA config update

@Mic92 Mic92 requested a review from roberth January 7, 2025 19:15
@bryango bryango marked this pull request as draft January 9, 2025 09:50
@bryango bryango marked this pull request as ready for review January 9, 2025 11:02
@bryango
Copy link
Member Author

bryango commented Jan 9, 2025

Rebased on current master, conflicts resolved, CI happy! Friendly ping @roberth @edolstra for re-review!

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.
@bryango
Copy link
Member Author

bryango commented Jan 10, 2025

Rebased again, conflicts resolved, CI happy.

tests/functional/flakes/flakes.sh Show resolved Hide resolved
@roberth
Copy link
Member

roberth commented Jan 10, 2025

Thanks!

@mergify queue

@roberth roberth added backport 2.24-maintenance Automatically creates a PR against the branch backport 2.25-maintenance Automatically creates a PR against the branch labels Jan 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.24-maintenance Automatically creates a PR against the branch backport 2.25-maintenance Automatically creates a PR against the branch fetching Networking with the outside (non-Nix) world, input locking with-tests Issues related to testing. PRs with tests have some priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flake input git+file:./${submodule} no longer works
4 participants