diff --git a/.version b/.version index 7780cec2961..544fe5d4382 100644 --- a/.version +++ b/.version @@ -1 +1 @@ -2.32.1 +2.32.2 diff --git a/src/libfetchers-tests/git-utils.cc b/src/libfetchers-tests/git-utils.cc index a7eb55fc206..8c306655d6f 100644 --- a/src/libfetchers-tests/git-utils.cc +++ b/src/libfetchers-tests/git-utils.cc @@ -176,6 +176,12 @@ TEST_F(GitUtilsTest, peel_reference) TEST(GitUtils, isLegalRefName) { + ASSERT_TRUE(isLegalRefName("A/b")); + ASSERT_TRUE(isLegalRefName("AaA/b")); + ASSERT_TRUE(isLegalRefName("FOO/BAR/BAZ")); + ASSERT_TRUE(isLegalRefName("HEAD")); + ASSERT_TRUE(isLegalRefName("refs/tags/1.2.3")); + ASSERT_TRUE(isLegalRefName("refs/heads/master")); ASSERT_TRUE(isLegalRefName("foox")); ASSERT_TRUE(isLegalRefName("1337")); ASSERT_TRUE(isLegalRefName("foo.baz")); diff --git a/src/libfetchers/git-utils.cc b/src/libfetchers/git-utils.cc index 603b8958741..ae7cc2b6585 100644 --- a/src/libfetchers/git-utils.cc +++ b/src/libfetchers/git-utils.cc @@ -15,6 +15,7 @@ #include #include +#include #include #include #include @@ -31,6 +32,7 @@ #include #include #include +#include #include #include @@ -1388,63 +1390,33 @@ GitRepo::WorkdirInfo GitRepo::getCachedWorkdirInfo(const std::filesystem::path & return workdirInfo; } -/** - * Checks that the git reference is valid and normalizes slash '/' sequences. - * - * Accepts shorthand references (one-level refnames are allowed). - */ -bool isValidRefNameAllowNormalizations(const std::string & refName) -{ - /* Unfortunately libgit2 doesn't expose the limit in headers, but its internal - limit is also 1024. */ - std::array normalizedRefBuffer; - - /* It would be nice to have a better API like git_reference_name_is_valid, but - * with GIT_REFERENCE_FORMAT_REFSPEC_SHORTHAND flag. libgit2 uses it internally - * but doesn't expose it in public headers [1]. - * [1]: - * https://github.com/libgit2/libgit2/blob/9d5f1bacc23594c2ba324c8f0d41b88bf0e9ef04/src/libgit2/refs.c#L1362-L1365 - */ - - auto res = git_reference_normalize_name( - normalizedRefBuffer.data(), - normalizedRefBuffer.size(), - refName.c_str(), - GIT_REFERENCE_FORMAT_ALLOW_ONELEVEL | GIT_REFERENCE_FORMAT_REFSPEC_SHORTHAND); - - return res == 0; -} - bool isLegalRefName(const std::string & refName) { initLibGit2(); - /* Since `git_reference_normalize_name` is the best API libgit2 has for verifying - * reference names with shorthands (see comment in normalizeRefName), we need to - * ensure that exceptions to the validity checks imposed by normalization [1] are checked - * explicitly. - * [1]: https://git-scm.com/docs/git-check-ref-format#Documentation/git-check-ref-format.txt---normalize - */ - /* Check for cases that don't get rejected by libgit2. * FIXME: libgit2 should reject this. */ if (refName == "@") return false; - /* Leading slashes and consecutive slashes are stripped during normalizatiton. */ - if (refName.starts_with('/') || refName.find("//") != refName.npos) - return false; - - /* Refer to libgit2. */ - if (!isValidRefNameAllowNormalizations(refName)) - return false; - /* libgit2 doesn't barf on DEL symbol. * FIXME: libgit2 should reject this. */ if (refName.find('\177') != refName.npos) return false; - return true; + for (auto * func : { + git_reference_name_is_valid, + git_branch_name_is_valid, + git_tag_name_is_valid, + }) { + int valid = 0; + if (func(&valid, refName.c_str())) + throw Error("checking git reference '%s': %s", refName, git_error_last()->message); + if (valid) + return true; + } + + return false; } } // namespace nix diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index 727792db0a2..b06ee9c8368 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -496,6 +496,36 @@ struct GitInputScheme : InputScheme Git interprets them as part of the file name. So get rid of them. */ url.query.clear(); + /* Backward compatibility hack: In old versions of Nix, if you had + a flake input like + + inputs.foo.url = "git+https://foo/bar?dir=subdir"; + + it would result in a lock file entry like + + "original": { + "dir": "subdir", + "type": "git", + "url": "https://foo/bar?dir=subdir" + } + + New versions of Nix remove `?dir=subdir` from the `url` field, + since the subdirectory is intended for `FlakeRef`, not the + fetcher (and specifically the remote server), that is, the + flakeref is parsed into + + "original": { + "dir": "subdir", + "type": "git", + "url": "https://foo/bar" + } + + However, new versions of nix parsing old flake.lock files would pass the dir= + query parameter in the "url" attribute to git, which will then complain. + + For this reason, we are filtering the `dir` query parameter from the URL + before passing it to git. */ + url.query.erase("dir"); repoInfo.location = url; } diff --git a/src/libfetchers/include/nix/fetchers/git-utils.hh b/src/libfetchers/include/nix/fetchers/git-utils.hh index 6656793f26d..19b5f0f6b0f 100644 --- a/src/libfetchers/include/nix/fetchers/git-utils.hh +++ b/src/libfetchers/include/nix/fetchers/git-utils.hh @@ -156,9 +156,12 @@ struct Setter }; /** - * Checks that the git reference is valid and normalized. + * Checks that the string can be a valid git reference, branch or tag name. + * Accepts shorthand references (one-level refnames are allowed), pseudorefs + * like `HEAD`. * - * Accepts shorthand references (one-level refnames are allowed). + * @note This is a coarse test to make sure that the refname is at least something + * that Git can make sense of. */ bool isLegalRefName(const std::string & refName); diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc index 5de4d778d7e..065c90c725e 100644 --- a/src/libmain/shared.cc +++ b/src/libmain/shared.cc @@ -325,34 +325,29 @@ int handleExceptions(const std::string & programName, std::function fun) std::string error = ANSI_RED "error:" ANSI_NORMAL " "; try { try { - try { - fun(); - } catch (...) { - /* Subtle: we have to make sure that any `interrupted' - condition is discharged before we reach printMsg() - below, since otherwise it will throw an (uncaught) - exception. */ - setInterruptThrown(); - throw; - } - } catch (Exit & e) { - return e.status; - } catch (UsageError & e) { - logError(e.info()); - printError("\nTry '%1% --help' for more information.", programName); - return 1; - } catch (BaseError & e) { - logError(e.info()); - return e.info().status; - } catch (std::bad_alloc & e) { - printError(error + "out of memory"); - return 1; - } catch (std::exception & e) { - printError(error + e.what()); - return 1; + fun(); + } catch (...) { + /* Subtle: we have to make sure that any `interrupted' + condition is discharged before we reach printMsg() + below, since otherwise it will throw an (uncaught) + exception. */ + setInterruptThrown(); + throw; } - } catch (...) { - /* In case logger also throws just give up. */ + } catch (Exit & e) { + return e.status; + } catch (UsageError & e) { + logError(e.info()); + printError("\nTry '%1% --help' for more information.", programName); + return 1; + } catch (BaseError & e) { + logError(e.info()); + return e.info().status; + } catch (std::bad_alloc & e) { + printError(error + "out of memory"); + return 1; + } catch (std::exception & e) { + printError(error + e.what()); return 1; } diff --git a/src/libstore/include/nix/store/serve-protocol-connection.hh b/src/libstore/include/nix/store/serve-protocol-connection.hh index 873277db902..fa50132c88b 100644 --- a/src/libstore/include/nix/store/serve-protocol-connection.hh +++ b/src/libstore/include/nix/store/serve-protocol-connection.hh @@ -82,6 +82,8 @@ struct ServeProto::BasicClientConnection BuildResult getBuildDerivationResponse(const StoreDirConfig & store); void narFromPath(const StoreDirConfig & store, const StorePath & path, std::function fun); + + void importPaths(const StoreDirConfig & store, std::function fun); }; struct ServeProto::BasicServerConnection diff --git a/src/libstore/include/nix/store/serve-protocol.hh b/src/libstore/include/nix/store/serve-protocol.hh index 5cb8a89a8d3..974bf42d58d 100644 --- a/src/libstore/include/nix/store/serve-protocol.hh +++ b/src/libstore/include/nix/store/serve-protocol.hh @@ -108,8 +108,13 @@ enum struct ServeProto::Command : uint64_t { QueryValidPaths = 1, QueryPathInfos = 2, DumpStorePath = 3, - // ImportPaths = 4, // removed - // ExportPaths = 5, // removed + /** + * @note This is no longer used by Nix (as a client), but it is used + * by Hydra. We should therefore not remove it until Hydra no longer + * uses it either. + */ + ImportPaths = 4, + // ExportPaths = 5, BuildPaths = 6, QueryClosure = 7, BuildDerivation = 8, diff --git a/src/libstore/serve-protocol-connection.cc b/src/libstore/serve-protocol-connection.cc index a90b104a68e..baa3bf0ce71 100644 --- a/src/libstore/serve-protocol-connection.cc +++ b/src/libstore/serve-protocol-connection.cc @@ -93,4 +93,14 @@ void ServeProto::BasicClientConnection::narFromPath( fun(from); } +void ServeProto::BasicClientConnection::importPaths(const StoreDirConfig & store, std::function fun) +{ + to << ServeProto::Command::ImportPaths; + fun(to); + to.flush(); + + if (readInt(from) != 1) + throw Error("remote machine failed to import closure"); +} + } // namespace nix diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index 75766437a91..cca142cd6b6 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -1722,7 +1722,6 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() if (buildMode == bmRepair) { /* Path already exists, need to replace it */ replaceValidPath(store.toRealPath(finalDestPath), actualPath); - actualPath = store.toRealPath(finalDestPath); } else if (buildMode == bmCheck) { /* Path already exists, and we want to compare, so we leave out new path in place. */ @@ -1736,7 +1735,6 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() auto destPath = store.toRealPath(finalDestPath); deletePath(destPath); movePath(actualPath, destPath); - actualPath = destPath; } } @@ -1789,7 +1787,9 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() debug("unreferenced input: '%1%'", store.printStorePath(i)); } - store.optimisePath(actualPath, NoRepair); // FIXME: combine with scanForReferences() + if (!store.isValidPath(newInfo.path)) + store.optimisePath( + store.toRealPath(finalDestPath), NoRepair); // FIXME: combine with scanForReferences() newInfo.deriver = drvPath; newInfo.ultimate = true; diff --git a/src/nix/nix-store/nix-store.cc b/src/nix/nix-store/nix-store.cc index 35563404c16..2f632510c23 100644 --- a/src/nix/nix-store/nix-store.cc +++ b/src/nix/nix-store/nix-store.cc @@ -986,6 +986,16 @@ static void opServe(Strings opFlags, Strings opArgs) store->narFromPath(store->parseStorePath(readString(in)), out); break; + case ServeProto::Command::ImportPaths: { + if (!writeAllowed) + throw Error("importing paths is not allowed"); + // FIXME: should we skip sig checking? + importPaths(*store, in, NoCheckSigs); + // indicate success + out << 1; + break; + } + case ServeProto::Command::BuildPaths: { if (!writeAllowed) diff --git a/tests/functional/fetchGitRefs.sh b/tests/functional/fetchGitRefs.sh index 288b26591ad..a7d1a2a2931 100755 --- a/tests/functional/fetchGitRefs.sh +++ b/tests/functional/fetchGitRefs.sh @@ -59,6 +59,9 @@ invalid_ref() { } +valid_ref 'A/b' +valid_ref 'AaA/b' +valid_ref 'FOO/BAR/BAZ' valid_ref 'foox' valid_ref '1337' valid_ref 'foo.baz'