From 8594f3cd5ad34838b2b7997af4909160e5887d73 Mon Sep 17 00:00:00 2001 From: Bryan Lai Date: Wed, 31 Jan 2024 19:41:17 +0800 Subject: [PATCH 1/6] libutil/url: fix git+file:./ parse error Previously, the "file:./" prefix was not correctly recognized in fixGitURL; instead, it was mistaken as a file path, which resulted in a parsed url of the form "file://file:./". This commit fixes the issue by properly detecting the "file:" prefix. Note, however, that unlike "file://", the "file:./" URI is _not_ standardized, but has been widely used to referred to relative file paths. In particular, the "git+file:./" did work for nix<=2.18, and was broken since nix 2.19.0. Finally, this commit fixes the issue completely for the 2.19 series, but is still inadequate for the 2.20 series due to new behaviors from the switch to libgit2. However, it does improve the correctness of parsing even though it is not yet a complete solution. --- src/libutil/url.cc | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/libutil/url.cc b/src/libutil/url.cc index c6561441d..f4178f87f 100644 --- a/src/libutil/url.cc +++ b/src/libutil/url.cc @@ -171,16 +171,16 @@ std::string fixGitURL(const std::string & url) std::regex scpRegex("([^/]*)@(.*):(.*)"); if (!hasPrefix(url, "/") && std::regex_match(url, scpRegex)) return std::regex_replace(url, scpRegex, "ssh://$1@$2/$3"); - else { - if (url.find("://") == std::string::npos) { - return (ParsedURL { - .scheme = "file", - .authority = "", - .path = url - }).to_string(); - } else - return url; + if (hasPrefix(url, "file:")) + return url; + if (url.find("://") == std::string::npos) { + return (ParsedURL { + .scheme = "file", + .authority = "", + .path = url + }).to_string(); } + return url; } // https://www.rfc-editor.org/rfc/rfc3986#section-3.1 From 358c26fd13a902d9a4032a00e6683571be07a384 Mon Sep 17 00:00:00 2001 From: DavHau Date: Sat, 17 Feb 2024 19:36:32 +0700 Subject: [PATCH 2/6] fetchTree: shallow git fetching by default Motivation: make git fetching more efficient for most repos by default --- .../shallow-git-fetching-by-default.md | 12 +++++ src/libexpr/primops/fetchTree.cc | 27 ++++++++--- .../test-cases/fetchTree-shallow/default.nix | 45 +++++++++++++++++++ 3 files changed, 78 insertions(+), 6 deletions(-) create mode 100644 doc/manual/rl-next/shallow-git-fetching-by-default.md create mode 100644 tests/nixos/fetch-git/test-cases/fetchTree-shallow/default.nix diff --git a/doc/manual/rl-next/shallow-git-fetching-by-default.md b/doc/manual/rl-next/shallow-git-fetching-by-default.md new file mode 100644 index 000000000..4d044f881 --- /dev/null +++ b/doc/manual/rl-next/shallow-git-fetching-by-default.md @@ -0,0 +1,12 @@ +--- +synopsis: "`fetchTree` now fetches git repositories shallowly by default" +prs: 10028 +--- + +`builtins.fetchTree` now clones git repositories shallowly by default, which reduces network traffic and disk usage significantly in many cases. + +Previously, the default behavior was to clone the full history of a specific tag or branch (eg. `ref`) and only afterwards extract the files of one specific revision. + +From now on, the `ref` and `allRefs` arguments will be ignored, except if shallow cloning is disabled by setting `shallow = false`. + +The defaults for `builtins.fetchGit` remain unchanged. Here, shallow cloning has to be enabled manually by passing `shallow = true`. diff --git a/src/libexpr/primops/fetchTree.cc b/src/libexpr/primops/fetchTree.cc index 1997d5513..2926e3161 100644 --- a/src/libexpr/primops/fetchTree.cc +++ b/src/libexpr/primops/fetchTree.cc @@ -138,6 +138,11 @@ static void fetchTree( attrs.emplace("exportIgnore", Explicit{true}); } + // fetchTree should fetch git repos with shallow = true by default + if (type == "git" && !params.isFetchGit && !attrs.contains("shallow")) { + attrs.emplace("shallow", Explicit{true}); + } + if (!params.allowNameArgument) if (auto nameIter = attrs.find("name"); nameIter != attrs.end()) state.error( @@ -321,6 +326,8 @@ static RegisterPrimOp primop_fetchTree({ - `ref` (String, optional) + By default, this has no effect. This becomes relevant only once `shallow` cloning is disabled. + A [Git reference](https://git-scm.com/book/en/v2/Git-Internals-Git-References), such as a branch or tag name. Default: `"HEAD"` @@ -334,8 +341,9 @@ static RegisterPrimOp primop_fetchTree({ - `shallow` (Bool, optional) Make a shallow clone when fetching the Git tree. + When this is enabled, the options `ref` and `allRefs` have no effect anymore. - Default: `false` + Default: `true` - `submodules` (Bool, optional) @@ -345,8 +353,11 @@ static RegisterPrimOp primop_fetchTree({ - `allRefs` (Bool, optional) - If set to `true`, always fetch the entire repository, even if the latest commit is still in the cache. - Otherwise, only the latest commit is fetched if it is not already cached. + By default, this has no effect. This becomes relevant only once `shallow` cloning is disabled. + + Whether to fetch all references (eg. branches and tags) of the repository. + With this argument being true, it's possible to load a `rev` from *any* `ref`. + (Without setting this option, only `rev`s from the specified `ref` are supported). Default: `false` @@ -600,6 +611,8 @@ static RegisterPrimOp primop_fetchGit({ [Git reference]: https://git-scm.com/book/en/v2/Git-Internals-Git-References + This option has no effect once `shallow` cloning is enabled. + By default, the `ref` value is prefixed with `refs/heads/`. As of 2.3.0, Nix will not prefix `refs/heads/` if `ref` starts with `refs/`. @@ -617,13 +630,15 @@ static RegisterPrimOp primop_fetchGit({ - `shallow` (default: `false`) Make a shallow clone when fetching the Git tree. - + When this is enabled, the options `ref` and `allRefs` have no effect anymore. - `allRefs` - Whether to fetch all references of the repository. - With this argument being true, it's possible to load a `rev` from *any* `ref` + Whether to fetch all references (eg. branches and tags) of the repository. + With this argument being true, it's possible to load a `rev` from *any* `ref`. (by default only `rev`s from the specified `ref` are supported). + This option has no effect once `shallow` cloning is enabled. + - `verifyCommit` (default: `true` if `publicKey` or `publicKeys` are provided, otherwise `false`) Whether to check `rev` for a signature matching `publicKey` or `publicKeys`. diff --git a/tests/nixos/fetch-git/test-cases/fetchTree-shallow/default.nix b/tests/nixos/fetch-git/test-cases/fetchTree-shallow/default.nix new file mode 100644 index 000000000..f635df1f8 --- /dev/null +++ b/tests/nixos/fetch-git/test-cases/fetchTree-shallow/default.nix @@ -0,0 +1,45 @@ +{ + description = "fetchTree fetches git repos shallowly by default"; + script = '' + # purge nix git cache to make sure we start with a clean slate + client.succeed("rm -rf ~/.cache/nix") + + # add two commits to the repo: + # - one with a large file (2M) + # - another one making the file small again + client.succeed(f""" + dd if=/dev/urandom of={repo.path}/thailand bs=1M count=2 \ + && {repo.git} add thailand \ + && {repo.git} commit -m 'commit1' \ + && echo 'ThaigerSprint' > {repo.path}/thailand \ + && {repo.git} add thailand \ + && {repo.git} commit -m 'commit2' \ + && {repo.git} push origin main + """) + + # memoize the revision + commit2_rev = client.succeed(f""" + {repo.git} rev-parse HEAD + """).strip() + + # construct the fetcher call + fetchGit_expr = f""" + builtins.fetchTree {{ + type = "git"; + url = "{repo.remote}"; + rev = "{commit2_rev}"; + }} + """ + + # fetch the repo via nix + fetched1 = client.succeed(f""" + nix eval --impure --raw --expr '({fetchGit_expr}).outPath' + """) + + # check that the size of ~/.cache/nix is less than 1M + cache_size = client.succeed(""" + du -s ~/.cache/nix + """).strip().split()[0] + assert int(cache_size) < 1024, f"cache size is {cache_size}K which is larger than 1M" + ''; +} From 0ed356f3c0d6d7e716d113e563c89dcdbd92b11e Mon Sep 17 00:00:00 2001 From: "J. Dekker" Date: Thu, 30 May 2024 15:03:20 +0200 Subject: [PATCH 3/6] scripts/install.in: add riscv64 support to installer The artifacts are already built and hosted, the install script just needs to be taught about riscv64. Signed-off-by: J. Dekker --- scripts/install.in | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/scripts/install.in b/scripts/install.in index 7d2e52b26..b4e808d8e 100755 --- a/scripts/install.in +++ b/scripts/install.in @@ -50,6 +50,11 @@ case "$(uname -s).$(uname -m)" in path=@tarballPath_armv7l-linux@ system=armv7l-linux ;; + Linux.riscv64) + hash=@tarballHash_riscv64-linux@ + path=@tarballPath_riscv64-linux@ + system=riscv64-linux + ;; Darwin.x86_64) hash=@tarballHash_x86_64-darwin@ path=@tarballPath_x86_64-darwin@ From 73f9afd71626120c2ffcd06b7ae66a7ec6787e36 Mon Sep 17 00:00:00 2001 From: "J. Dekker" Date: Thu, 30 May 2024 20:11:28 +0200 Subject: [PATCH 4/6] upload-release.pl: add riscv64 to nix-fallback-paths.nix This uses the x86_64-linux's cross-compiled output as we don't have a native riscv64 builder. Signed-off-by: J. Dekker --- maintainers/upload-release.pl | 1 + 1 file changed, 1 insertion(+) diff --git a/maintainers/upload-release.pl b/maintainers/upload-release.pl index 9e73524a6..4c4e2bd6f 100755 --- a/maintainers/upload-release.pl +++ b/maintainers/upload-release.pl @@ -243,6 +243,7 @@ write_file("$tmpDir/fallback-paths.nix", " x86_64-linux = \"" . getStorePath("build.x86_64-linux") . "\";\n" . " i686-linux = \"" . getStorePath("build.i686-linux") . "\";\n" . " aarch64-linux = \"" . getStorePath("build.aarch64-linux") . "\";\n" . + " riscv64-linux = \"" . getStorePath("buildCross.riscv64-unknown-linux-gnu.x86_64-linux") . "\";\n" . " x86_64-darwin = \"" . getStorePath("build.x86_64-darwin") . "\";\n" . " aarch64-darwin = \"" . getStorePath("build.aarch64-darwin") . "\";\n" . "}\n"); From cfc18a77395b5ef49763f77c3cc67a95f762a0eb Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 16 May 2024 18:46:38 -0400 Subject: [PATCH 5/6] Modernize `Hash` ordering with C++20 `<=>` Progress on #10832 This doesn't switch to auto-deriving the fields, but by defining `<=>` we allow deriving `<=>` in downstream types where `Hash` is used. --- src/libutil/hash.cc | 17 +++++------------ src/libutil/hash.hh | 11 +++-------- 2 files changed, 8 insertions(+), 20 deletions(-) diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index d4c9d6533..2f2ed8138 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -50,21 +50,14 @@ bool Hash::operator == (const Hash & h2) const } -bool Hash::operator != (const Hash & h2) const +std::strong_ordering Hash::operator <=> (const Hash & h) const { - return !(*this == h2); -} - - -bool Hash::operator < (const Hash & h) const -{ - if (hashSize < h.hashSize) return true; - if (hashSize > h.hashSize) return false; + if (auto cmp = algo <=> h.algo; cmp != 0) return cmp; + if (auto cmp = hashSize <=> h.hashSize; cmp != 0) return cmp; for (unsigned int i = 0; i < hashSize; i++) { - if (hash[i] < h.hash[i]) return true; - if (hash[i] > h.hash[i]) return false; + if (auto cmp = hash[i] <=> h.hash[i]; cmp != 0) return cmp; } - return false; + return std::strong_ordering::equivalent; } diff --git a/src/libutil/hash.hh b/src/libutil/hash.hh index e14aae43c..ef96d08c9 100644 --- a/src/libutil/hash.hh +++ b/src/libutil/hash.hh @@ -86,19 +86,14 @@ private: public: /** - * Check whether two hash are equal. + * Check whether two hashes are equal. */ bool operator == (const Hash & h2) const; /** - * Check whether two hash are not equal. + * Compare how two hashes are ordered. */ - bool operator != (const Hash & h2) const; - - /** - * For sorting. - */ - bool operator < (const Hash & h) const; + std::strong_ordering operator <=> (const Hash & h2) const; /** * Returns the length of a base-16 representation of this hash. From 1e99f324d907bc51b99822d3fa3ba7eee21a1d46 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 3 Jun 2024 09:36:05 -0400 Subject: [PATCH 6/6] Fix shellcheck issue 8b86f415c1a8565085e70475933e459a29d67283 was merged from a CI run that predated the new linting. --- tests/functional/flakes/eval-cache.sh | 2 ++ 1 file changed, 2 insertions(+) mode change 100644 => 100755 tests/functional/flakes/eval-cache.sh diff --git a/tests/functional/flakes/eval-cache.sh b/tests/functional/flakes/eval-cache.sh old mode 100644 new mode 100755 index 90c7abd3c..0f8df1b91 --- a/tests/functional/flakes/eval-cache.sh +++ b/tests/functional/flakes/eval-cache.sh @@ -1,3 +1,5 @@ +#!/usr/bin/env bash + source ./common.sh requireGit