From 6e44bc2a89a4eb66b07d42750fb1047a9e4b4a62 Mon Sep 17 00:00:00 2001 From: Bryan Lai Date: Thu, 26 Dec 2024 20:50:36 +0800 Subject: [PATCH 1/7] fetchers/git: make path absolute for local repo (cherry picked from commit 96bd9bad2f434fe3489b039ac665679878460b0c) --- src/libfetchers/git.cc | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index 6c5bda470..ae30de8fb 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -425,7 +425,16 @@ struct GitInputScheme : InputScheme auto url = parseURL(getStrAttr(input.attrs, "url")); bool isBareRepository = url.scheme == "file" && !pathExists(url.path + "/.git"); repoInfo.isLocal = url.scheme == "file" && !forceHttp && !isBareRepository; - 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(); // If this is a local directory and no ref or revision is // given, then allow the use of an unclean working tree. From 84e3f4ad795499c6f24c9071a9100dda6c2666ba Mon Sep 17 00:00:00 2001 From: Bryan Lai Date: Fri, 27 Dec 2024 17:35:44 +0800 Subject: [PATCH 2/7] tests/flakes: check git+file:./${submodule} protocol Relative, local git repo used to work (for submodules), but it fails after 3e0129ce3b9eb094d4a3cc8023884f372f1d7ff6. This commit adds a test to prevent such failure in the future. (cherry picked from commit 9d088fa50242c5fe2ad94bffb61a4742f232ff0b) --- tests/functional/flakes/flakes.sh | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/functional/flakes/flakes.sh b/tests/functional/flakes/flakes.sh index 26b91eda7..43e740adb 100755 --- a/tests/functional/flakes/flakes.sh +++ b/tests/functional/flakes/flakes.sh @@ -228,6 +228,14 @@ nix build -o "$TEST_ROOT/result" "git+file://$flake1Dir#default" nix build -o "$TEST_ROOT/result" "$flake1Dir?ref=HEAD#default" nix build -o "$TEST_ROOT/result" "git+file://$flake1Dir?ref=HEAD#default" +# Check that relative paths are allowed for git flakes. +# This may change in the future once git submodule support is refined. +# See: https://discourse.nixos.org/t/57783 and #9708. +( + cd "$flake1Dir/.." + nix build -o "$TEST_ROOT/result" "git+file:./$(basename "$flake1Dir")" +) + # Check that store symlinks inside a flake are not interpreted as flakes. nix build -o "$flake1Dir/result" "git+file://$flake1Dir" nix path-info "$flake1Dir/result" From d4f0e8f4e3628b1d6a88a6449b2d16927c63709c Mon Sep 17 00:00:00 2001 From: Bryan Lai Date: Wed, 1 Jan 2025 14:53:04 +0800 Subject: [PATCH 3/7] tests/flake-in-submodule: git+file:./* input (cherry picked from commit 37ac18d1d9faba24ce09653723342465bcac3e0b) --- tests/functional/flakes/flake-in-submodule.sh | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/functional/flakes/flake-in-submodule.sh b/tests/functional/flakes/flake-in-submodule.sh index 817f77783..74b2e76c3 100755 --- a/tests/functional/flakes/flake-in-submodule.sh +++ b/tests/functional/flakes/flake-in-submodule.sh @@ -63,3 +63,21 @@ flakeref=git+file://$rootRepo\?submodules=1\&dir=submodule echo '"foo"' > "$rootRepo"/submodule/sub.nix [[ $(nix eval --json "$flakeref#sub" ) = '"foo"' ]] [[ $(nix flake metadata --json "$flakeref" | jq -r .locked.rev) = null ]] + +# 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 < Date: Fri, 10 Jan 2025 09:57:54 +0100 Subject: [PATCH 4/7] Clarify cd call in tests/functional/flakes/flakes.sh (cherry picked from commit d9a50c0af2c73d0fb0b890975d9bcb0ff8a31b76) --- tests/functional/flakes/flakes.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/functional/flakes/flakes.sh b/tests/functional/flakes/flakes.sh index 43e740adb..423f0abfe 100755 --- a/tests/functional/flakes/flakes.sh +++ b/tests/functional/flakes/flakes.sh @@ -232,6 +232,7 @@ nix build -o "$TEST_ROOT/result" "git+file://$flake1Dir?ref=HEAD#default" # This may change in the future once git submodule support is refined. # See: https://discourse.nixos.org/t/57783 and #9708. ( + # This `cd` should not be required and is indicative of aforementioned bug. cd "$flake1Dir/.." nix build -o "$TEST_ROOT/result" "git+file:./$(basename "$flake1Dir")" ) From d944cb7f582ab16172764c60bcf8c2e3adb18796 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 14 Jan 2025 17:42:26 +0100 Subject: [PATCH 5/7] Use isAbsolute() (cherry picked from commit ff9d886f3cb28db13bfb88d1dc4d6fe3dc83270f) --- src/libexpr/eval.cc | 2 +- src/libfetchers/path.cc | 2 +- src/libfetchers/registry.cc | 2 +- src/libflake/flake/flakeref.cc | 2 +- src/libutil/file-system.cc | 7 +------ src/libutil/file-system.hh | 5 +++++ src/libutil/source-accessor.cc | 2 +- 7 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 4de464678..15a289360 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -438,7 +438,7 @@ void EvalState::checkURI(const std::string & uri) /* If the URI is a path, then check it against allowedPaths as well. */ - if (hasPrefix(uri, "/")) { + if (isAbsolute(uri)) { if (auto rootFS2 = rootFS.dynamic_pointer_cast()) rootFS2->checkAccess(CanonPath(uri)); return; diff --git a/src/libfetchers/path.cc b/src/libfetchers/path.cc index fca0df84b..e255dd228 100644 --- a/src/libfetchers/path.cc +++ b/src/libfetchers/path.cc @@ -96,7 +96,7 @@ struct PathInputScheme : InputScheme std::optional isRelative(const Input & input) const { auto path = getStrAttr(input.attrs, "path"); - if (hasPrefix(path, "/")) + if (isAbsolute(path)) return std::nullopt; else return path; diff --git a/src/libfetchers/registry.cc b/src/libfetchers/registry.cc index 3c893c8ea..7f2552143 100644 --- a/src/libfetchers/registry.cc +++ b/src/libfetchers/registry.cc @@ -156,7 +156,7 @@ static std::shared_ptr getGlobalRegistry(const Settings & settings, re return std::make_shared(settings, Registry::Global); // empty registry } - if (!hasPrefix(path, "/")) { + if (!isAbsolute(path)) { auto storePath = downloadFile(store, path, "flake-registry.json").storePath; if (auto store2 = store.dynamic_pointer_cast()) store2->addPermRoot(storePath, getCacheDir() + "/nix/flake-registry.json"); diff --git a/src/libflake/flake/flakeref.cc b/src/libflake/flake/flakeref.cc index a57fce9f3..8fd15a52d 100644 --- a/src/libflake/flake/flakeref.cc +++ b/src/libflake/flake/flakeref.cc @@ -175,7 +175,7 @@ std::pair parsePathFlakeRefWithFragment( } } else { - if (!hasPrefix(path, "/")) + if (!isAbsolute(path)) throw BadURL("flake reference '%s' is not an absolute path", url); path = canonPath(path + "/" + getOr(query, "dir", "")); } diff --git a/src/libutil/file-system.cc b/src/libutil/file-system.cc index 732e0a327..558593ca3 100644 --- a/src/libutil/file-system.cc +++ b/src/libutil/file-system.cc @@ -29,12 +29,7 @@ namespace fs = std::filesystem; namespace nix { -/** - * Treat the string as possibly an absolute path, by inspecting the - * start of it. Return whether it was probably intended to be - * absolute. - */ -static bool isAbsolute(PathView path) +bool isAbsolute(PathView path) { return fs::path { path }.is_absolute(); } diff --git a/src/libutil/file-system.hh b/src/libutil/file-system.hh index 92168cbb9..7d2f6a916 100644 --- a/src/libutil/file-system.hh +++ b/src/libutil/file-system.hh @@ -42,6 +42,11 @@ namespace nix { struct Sink; struct Source; +/** + * Return whether the path denotes an absolute path. + */ +bool isAbsolute(PathView path); + /** * @return An absolutized path, resolving paths relative to the * specified directory, or the current directory otherwise. The path diff --git a/src/libutil/source-accessor.cc b/src/libutil/source-accessor.cc index e797951c7..1876db92c 100644 --- a/src/libutil/source-accessor.cc +++ b/src/libutil/source-accessor.cc @@ -94,7 +94,7 @@ CanonPath SourceAccessor::resolveSymlinks( throw Error("infinite symlink recursion in path '%s'", showPath(path)); auto target = readLink(res); res.pop(); - if (hasPrefix(target, "/")) + if (isAbsolute(target)) res = CanonPath::root; todo.splice(todo.begin(), tokenizeString>(target, "/")); } From f27e263f8966bd723bae0dad38045cdfb5268da7 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 16 Jan 2025 17:22:54 +0100 Subject: [PATCH 6/7] Warn against the use of relative 'git+file:' flake inputs (cherry picked from commit 12e14956e22a42c26715ca084bb6b17d35628bb9) --- src/libfetchers/git.cc | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index ae30de8fb..c2d3b42ed 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -434,7 +434,16 @@ struct GitInputScheme : InputScheme // // See: https://discourse.nixos.org/t/57783 and #9708 // - repoInfo.url = repoInfo.isLocal ? std::filesystem::absolute(url.path).string() : url.to_string(); + if (repoInfo.isLocal) { + if (!isAbsolute(url.path)) { + warn( + "Fetching Git repository '%s', which uses a path relative to the current directory. " + "This is not supported and will stop working in a future release.", + url.to_string()); + } + repoInfo.url = std::filesystem::absolute(url.path).string(); + } else + repoInfo.url = url.to_string(); // If this is a local directory and no ref or revision is // given, then allow the use of an unclean working tree. From 876d724061dd7274a1a4b7b2943fddf610034220 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 17 Jan 2025 12:33:23 +0100 Subject: [PATCH 7/7] Add link to tracking issue (cherry picked from commit 3197c19a31f5ce42b099d257b6ba22fb25e707c4) --- src/libfetchers/git.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index c2d3b42ed..eca678bde 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -438,7 +438,8 @@ struct GitInputScheme : InputScheme if (!isAbsolute(url.path)) { warn( "Fetching Git repository '%s', which uses a path relative to the current directory. " - "This is not supported and will stop working in a future release.", + "This is not supported and will stop working in a future release. " + "See https://github.com/NixOS/nix/issues/12281 for details.", url.to_string()); } repoInfo.url = std::filesystem::absolute(url.path).string();