From 13f1764c6cb4ab17ee887d7e92205a5b2a111f47 Mon Sep 17 00:00:00 2001 From: Sonke Hahn Date: Mon, 10 Aug 2026 10:19:11 -0400 Subject: [PATCH 1/2] libfetchers: only consult git attributes for plausible git-lfs pointers `GitSourceAccessor::readBlob` asked libgit2 for the `filter` attribute of every single blob it read. That lookup is O(number of `.gitattributes` rules): `git_attr_get_ext` is passed no attribute session, so it redoes its attribute setup and rematches every rule of every parent directory's `.gitattributes` on each call. On a World-shaped tree (a root `.gitattributes` with ~1400 anchored LFS rules) that is ~25us per file. Sampling `nix eval` on a synthetic clone of that shape (10k files) showed `lfs::Fetch::shouldFetch` taking 513 of 2273 main-thread samples -- 23% of the whole ingestion. A blob can only need smudging if it is a git-lfs pointer, and the spec requires a pointer to start with the `version` key. Check that first and only ask git about the handful of blobs that pass. After the change the same profile shows 2 samples, and total CPU for the benchmark drops from 3.82s to 3.20s (this is an -O0 build, so the win is larger in a release build where libgit2 is a bigger share). Behaviour is unchanged for the emitted content. The only visible difference is that an lfs-enrolled file whose blob does not even start with `version ` (i.e. content committed without the filter running) is now passed through silently instead of warning "should have been a git-lfs pointer" -- detecting that case is exactly what costs O(rules) per file. Files that do start with `version ` still warn as before. Note that `GitExportIgnoreSourceAccessor::isExportIgnored` pays the same per-path cost (496 of 2273 samples in the same profile) and is not addressed here. --- src/libfetchers-tests/git-utils.cc | 93 +++++++++++++++++++ src/libfetchers/git-lfs-fetch.cc | 20 +++- src/libfetchers/git-utils.cc | 33 +++---- .../include/nix/fetchers/git-lfs-fetch.hh | 26 +++++- 4 files changed, 152 insertions(+), 20 deletions(-) diff --git a/src/libfetchers-tests/git-utils.cc b/src/libfetchers-tests/git-utils.cc index 4ab9eeb703..56d1124f87 100644 --- a/src/libfetchers-tests/git-utils.cc +++ b/src/libfetchers-tests/git-utils.cc @@ -407,4 +407,97 @@ TEST(GitUtils, isLegalRefName) ASSERT_FALSE(isLegalRefName("")); } +// ============================================================================ +// git-lfs smudging +// ============================================================================ + +static const char * lfsPointer = + "version https://git-lfs.github.com/spec/v1\n" + "oid sha256:f5e02aa71e67f41d79023a128ca35bad86cf7b6656967bfe0884b3a3c4325eaf\n" + "size 10000000\n"; + +class GitLfsTest : public GitUtilsTest +{ +protected: + git_repository * rawRepo = nullptr; + git_oid commitOid; + + /** + * Commit `files` (path -> contents) as the initial commit and record + * its oid in `commitOid`. + */ + void commitFiles(const std::map & files) + { + ASSERT_EQ(git_repository_open(&rawRepo, tmpDir.string().c_str()), 0); + + git_treebuilder * builder = nullptr; + ASSERT_EQ(git_treebuilder_new(&builder, rawRepo, nullptr), 0); + + for (auto & [name, contents] : files) { + git_oid blobOid; + ASSERT_EQ(git_blob_create_from_buffer(&blobOid, rawRepo, contents.data(), contents.size()), 0); + ASSERT_EQ(git_treebuilder_insert(nullptr, builder, name.c_str(), &blobOid, GIT_FILEMODE_BLOB), 0); + } + + git_oid treeOid; + ASSERT_EQ(git_treebuilder_write(&treeOid, builder), 0); + git_treebuilder_free(builder); + + git_tree * tree = nullptr; + ASSERT_EQ(git_tree_lookup(&tree, rawRepo, &treeOid), 0); + + git_signature * sig = nullptr; + ASSERT_EQ(git_signature_now(&sig, "nix", "nix@example.com"), 0); + ASSERT_EQ(git_commit_create_v(&commitOid, rawRepo, "HEAD", sig, sig, nullptr, "initial commit", tree, 0), 0); + git_signature_free(sig); + git_tree_free(tree); + } + + void TearDown() override + { + if (rawRepo) + git_repository_free(rawRepo); + GitUtilsTest::TearDown(); + } +}; + +TEST_F(GitLfsTest, pointer_candidate_only_accepts_pointer_shaped_content) +{ + ASSERT_TRUE(lfs::Fetch::isPointerCandidate(lfsPointer)); + // Malformed pointers are still candidates: only the attribute lookup can + // tell us whether to warn about them. + ASSERT_TRUE(lfs::Fetch::isPointerCandidate("version https://git-lfs.github.com/spec/v9\n")); + + ASSERT_FALSE(lfs::Fetch::isPointerCandidate("")); + ASSERT_FALSE(lfs::Fetch::isPointerCandidate("hello world")); + ASSERT_FALSE(lfs::Fetch::isPointerCandidate("versionless")); + ASSERT_FALSE(lfs::Fetch::isPointerCandidate(std::string_view("\0\1\2binary", 9))); +} + +/** + * Looking up git attributes is O(number of `.gitattributes` rules) per path, + * so it must not happen for blobs that cannot be a pointer in the first place. + */ +TEST_F(GitLfsTest, should_fetch_skips_the_attribute_lookup_for_non_pointer_content) +{ + commitFiles({ + {".gitattributes", "enrolled filter=lfs -text\n"}, + {"enrolled", "this is not a pointer"}, + {"plain", lfsPointer}, + }); + + lfs::Fetch fetch(rawRepo, commitOid); + + // The attribute really is set, ... + ASSERT_TRUE(fetch.hasLfsFilterAttribute(CanonPath("enrolled"))); + ASSERT_FALSE(fetch.hasLfsFilterAttribute(CanonPath("plain"))); + + // ... but content that cannot be a pointer is rejected without consulting it. + ASSERT_FALSE(fetch.shouldFetch(CanonPath("enrolled"), "this is not a pointer")); + ASSERT_TRUE(fetch.shouldFetch(CanonPath("enrolled"), lfsPointer)); + + // Pointer-shaped content that is not enrolled is still not smudged. + ASSERT_FALSE(fetch.shouldFetch(CanonPath("plain"), lfsPointer)); +} + } // namespace nix diff --git a/src/libfetchers/git-lfs-fetch.cc b/src/libfetchers/git-lfs-fetch.cc index b85e811cf8..23594933e9 100644 --- a/src/libfetchers/git-lfs-fetch.cc +++ b/src/libfetchers/git-lfs-fetch.cc @@ -270,7 +270,25 @@ Fetch::Fetch(git_repository * repo, git_oid rev, std::string attrPathPrefix) this->url = nix::fixGitURL(remoteUrl).canonicalise(); } -bool Fetch::shouldFetch(const CanonPath & path) const +bool Fetch::isPointerCandidate(std::string_view content) +{ + // The spec requires `version` to be the first line of a pointer file, so + // any blob that does not start with it cannot be one. Note that we do not + // also reject blobs that are too large to be a pointer (see `fetch()`): + // those are exactly the ones worth warning about, and there are few enough + // of them that the attribute lookup does not matter. + return content.starts_with("version "); +} + +bool Fetch::shouldFetch(const CanonPath & path, std::string_view content) const +{ + /* Consulting git attributes is expensive (see `hasLfsFilterAttribute()`) + and this runs for every blob of a source tree, so reject the blobs that + cannot be a pointer without asking git about them. */ + return isPointerCandidate(content) && hasLfsFilterAttribute(path); +} + +bool Fetch::hasLfsFilterAttribute(const CanonPath & path) const { const char * attr = nullptr; git_attr_options opts = GIT_ATTR_OPTIONS_INIT; diff --git a/src/libfetchers/git-utils.cc b/src/libfetchers/git-utils.cc index 3d02f01208..585936ead8 100644 --- a/src/libfetchers/git-utils.cc +++ b/src/libfetchers/git-utils.cc @@ -905,27 +905,24 @@ struct GitSourceAccessor : SourceAccessor const auto blob = getBlob(*state, path, symlink); - if (state->lfsFetch) { - if (state->lfsFetch->shouldFetch(path)) { - StringSink s; - try { - // FIXME: do we need to hold the state lock while - // doing this? - auto contents = - std::string((const char *) git_blob_rawcontent(blob.get()), git_blob_rawsize(blob.get())); - state->lfsFetch->fetch(contents, path, s, [&s](uint64_t size) { s.s.reserve(size); }); - } catch (Error & e) { - e.addTrace({}, "while smudging git-lfs file '%s'", path); - throw; - } - sizeCallback(s.s.size()); - StringSource source{s.s}; - source.drainInto(sink); - return; + auto view = std::string_view((const char *) git_blob_rawcontent(blob.get()), git_blob_rawsize(blob.get())); + + if (state->lfsFetch && state->lfsFetch->shouldFetch(path, view)) { + StringSink s; + try { + // FIXME: do we need to hold the state lock while + // doing this? + state->lfsFetch->fetch(std::string(view), path, s, [&s](uint64_t size) { s.s.reserve(size); }); + } catch (Error & e) { + e.addTrace({}, "while smudging git-lfs file '%s'", path); + throw; } + sizeCallback(s.s.size()); + StringSource source{s.s}; + source.drainInto(sink); + return; } - auto view = std::string_view((const char *) git_blob_rawcontent(blob.get()), git_blob_rawsize(blob.get())); sizeCallback(view.size()); StringSource source{view}; source.drainInto(sink); diff --git a/src/libfetchers/include/nix/fetchers/git-lfs-fetch.hh b/src/libfetchers/include/nix/fetchers/git-lfs-fetch.hh index 8fea1c1798..27fdf93e5e 100644 --- a/src/libfetchers/include/nix/fetchers/git-lfs-fetch.hh +++ b/src/libfetchers/include/nix/fetchers/git-lfs-fetch.hh @@ -30,7 +30,31 @@ struct Fetch std::string attrPathPrefix; Fetch(git_repository * repo, git_oid rev, std::string attrPathPrefix = ""); - bool shouldFetch(const CanonPath & path) const; + + /** + * Whether `content` is shaped like a git-lfs pointer file. + * + * This is a cheap *necessary* condition for smudging, used to avoid the + * expensive attribute lookup in `shouldFetch()` for the overwhelming + * majority of blobs. It deliberately accepts malformed pointers, so that + * `fetch()` can still warn about them. + */ + static bool isPointerCandidate(std::string_view content); + + /** + * Whether the `filter` attribute of `path` is `lfs`. + * + * Expensive: libgit2 redoes its attribute setup and rematches every + * `.gitattributes` rule of every parent directory on each call, so this + * costs O(number of rules) per path. Prefer `shouldFetch()`, which only + * gets here for blobs that could actually be a pointer. + */ + bool hasLfsFilterAttribute(const CanonPath & path) const; + + /** + * Whether the blob at `path` holding `content` must be smudged. + */ + bool shouldFetch(const CanonPath & path, std::string_view content) const; void fetch( const std::string & content, const CanonPath & pointerFilePath, From 1afd29e448d4513651f188f433cba0c9fb6ced03 Mon Sep 17 00:00:00 2001 From: Sonke Hahn Date: Mon, 10 Aug 2026 11:02:43 -0400 Subject: [PATCH 2/2] libfetchers: decide export-ignore per directory instead of per path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `GitExportIgnoreSourceAccessor::isAllowedUncached` asked libgit2 for the `export-ignore` attribute of every path it filtered. `git_attr_get_ext` is called without an attribute session, so each call re-runs `attr_setup` and then rematches *every* rule of *every* applicable `.gitattributes` with `wildmatch` — there is no literal fast path in `git_attr_fnmatch__match`. Ingesting a directory of a monorepo whose root `.gitattributes` carries ~1400 (mostly git-lfs) rules therefore costs O(rules x files). Nothing about the *content* of a file can rule out `export-ignore`, so the trick used for git-lfs pointers does not apply. What can be ruled out cheaply is a whole directory: the attribute can only be assigned by a source that spells its name out, and the sources that apply to a directory's entries are its ancestors' `.gitattributes` files plus `$GIT_DIR/info/attributes` and the global `core.attributesfile` (`GIT_ATTR_CHECK_NO_SYSTEM` excludes only the system-wide file). So `GitRepo::mayExportIgnore(commit, path)` substring-searches those sources once, and the accessor memoises the answer per directory, chaining each directory onto its parent's. Where it comes out false — the common case — the libgit2 lookup is skipped entirely, turning O(files) attribute resolutions into O(directories) tree lookups. The gate is conservative in both directions: an unreadable external attribute file, a mere mention in a comment, or a working-directory accessor (where libgit2 resolves attributes against the index, which we cannot read through `next`) all fall back to asking libgit2. Measured on a synthetic monorepo-shaped repository (10000 files, the World root `.gitattributes`), `fetchGit { exportIgnore = true; }`: `isExportIgnored` drops from 551/2266 profile samples (24%) to 11/2594 (0.4%), and CPU time from 3.00s to 2.14s in an -O0 build. Also generalises the unit-test fixture to build nested trees, and adds tests for a rule in a subdirectory and for a rule above `attrPathPrefix` — neither was covered before (the functional test's `exportIgnore = true` case is commented out). --- src/libfetchers-tests/git-utils.cc | 209 ++++++++++++++---- src/libfetchers/git-utils.cc | 180 ++++++++++++++- .../include/nix/fetchers/git-utils.hh | 17 ++ 3 files changed, 356 insertions(+), 50 deletions(-) diff --git a/src/libfetchers-tests/git-utils.cc b/src/libfetchers-tests/git-utils.cc index 56d1124f87..d86cc1a1a5 100644 --- a/src/libfetchers-tests/git-utils.cc +++ b/src/libfetchers-tests/git-utils.cc @@ -26,6 +26,78 @@ class GitUtilsTest : public ::testing::Test protected: std::filesystem::path tmpDir; + /** Set by `commitFiles`. */ + git_repository * rawRepo = nullptr; + git_oid commitOid; + + /** + * Write `files` (repo-relative path -> contents; paths may name + * subdirectories) as the initial commit and record its oid in + * `commitOid`. + */ + void commitFiles(const std::map & files) + { + ASSERT_EQ(git_repository_open(&rawRepo, tmpDir.string().c_str()), 0); + + auto treeOid = buildTree(files); + + git_tree * tree = nullptr; + ASSERT_EQ(git_tree_lookup(&tree, rawRepo, &treeOid), 0); + + git_signature * sig = nullptr; + ASSERT_EQ(git_signature_now(&sig, "nix", "nix@example.com"), 0); + ASSERT_EQ(git_commit_create_v(&commitOid, rawRepo, "HEAD", sig, sig, nullptr, "initial commit", tree, 0), 0); + git_signature_free(sig); + git_tree_free(tree); + } + + /** The hash of the commit created by `commitFiles`. */ + Hash commitHash() const + { + return toHash(commitOid); + } + + static Hash toHash(const git_oid & oid) + { + char sha[GIT_OID_SHA1_HEXSIZE + 1]; + git_oid_tostr(sha, sizeof(sha), &oid); + return Hash::parseNonSRIUnprefixed(sha, HashAlgorithm::SHA1); + } + +private: + git_oid buildTree(const std::map & files) + { + std::map blobs; + std::map> subdirs; + + for (auto & [path, contents] : files) { + auto slash = path.find('/'); + if (slash == std::string::npos) + blobs.emplace(path, contents); + else + subdirs[path.substr(0, slash)].emplace(path.substr(slash + 1), contents); + } + + git_treebuilder * builder = nullptr; + EXPECT_EQ(git_treebuilder_new(&builder, rawRepo, nullptr), 0); + + for (auto & [name, contents] : blobs) { + git_oid blobOid; + EXPECT_EQ(git_blob_create_from_buffer(&blobOid, rawRepo, contents.data(), contents.size()), 0); + EXPECT_EQ(git_treebuilder_insert(nullptr, builder, name.c_str(), &blobOid, GIT_FILEMODE_BLOB), 0); + } + + for (auto & [name, entries] : subdirs) { + auto subtreeOid = buildTree(entries); + EXPECT_EQ(git_treebuilder_insert(nullptr, builder, name.c_str(), &subtreeOid, GIT_FILEMODE_TREE), 0); + } + + git_oid treeOid; + EXPECT_EQ(git_treebuilder_write(&treeOid, builder), 0); + git_treebuilder_free(builder); + return treeOid; + } + public: void SetUp() override { @@ -42,6 +114,9 @@ class GitUtilsTest : public ::testing::Test void TearDown() override { + if (rawRepo) + git_repository_free(rawRepo); + // Destroy the AutoDelete, triggering removal // not AutoDelete::reset(), which would cancel the deletion. delTmpDir.reset(); @@ -417,49 +492,7 @@ static const char * lfsPointer = "size 10000000\n"; class GitLfsTest : public GitUtilsTest -{ -protected: - git_repository * rawRepo = nullptr; - git_oid commitOid; - - /** - * Commit `files` (path -> contents) as the initial commit and record - * its oid in `commitOid`. - */ - void commitFiles(const std::map & files) - { - ASSERT_EQ(git_repository_open(&rawRepo, tmpDir.string().c_str()), 0); - - git_treebuilder * builder = nullptr; - ASSERT_EQ(git_treebuilder_new(&builder, rawRepo, nullptr), 0); - - for (auto & [name, contents] : files) { - git_oid blobOid; - ASSERT_EQ(git_blob_create_from_buffer(&blobOid, rawRepo, contents.data(), contents.size()), 0); - ASSERT_EQ(git_treebuilder_insert(nullptr, builder, name.c_str(), &blobOid, GIT_FILEMODE_BLOB), 0); - } - - git_oid treeOid; - ASSERT_EQ(git_treebuilder_write(&treeOid, builder), 0); - git_treebuilder_free(builder); - - git_tree * tree = nullptr; - ASSERT_EQ(git_tree_lookup(&tree, rawRepo, &treeOid), 0); - - git_signature * sig = nullptr; - ASSERT_EQ(git_signature_now(&sig, "nix", "nix@example.com"), 0); - ASSERT_EQ(git_commit_create_v(&commitOid, rawRepo, "HEAD", sig, sig, nullptr, "initial commit", tree, 0), 0); - git_signature_free(sig); - git_tree_free(tree); - } - - void TearDown() override - { - if (rawRepo) - git_repository_free(rawRepo); - GitUtilsTest::TearDown(); - } -}; +{}; TEST_F(GitLfsTest, pointer_candidate_only_accepts_pointer_shaped_content) { @@ -500,4 +533,94 @@ TEST_F(GitLfsTest, should_fetch_skips_the_attribute_lookup_for_non_pointer_conte ASSERT_FALSE(fetch.shouldFetch(CanonPath("plain"), lfsPointer)); } +// ============================================================================ +// export-ignore +// ============================================================================ + +class GitExportIgnoreTest : public GitUtilsTest +{}; + +/** + * Looking up a git attribute costs O(rules in all applicable `.gitattributes`) + * per path, so there has to be a cheap way to rule out `export-ignore` for a + * whole directory at once. + */ +TEST_F(GitExportIgnoreTest, may_export_ignore_reports_where_the_attribute_can_apply) +{ + commitFiles({ + {".gitattributes", "*.png filter=lfs diff=lfs merge=lfs -text\n"}, + {"a/.gitattributes", "# nothing to see here\n"}, + {"a/file", "content"}, + {"a/b/.gitattributes", "generated/ export-ignore\n"}, + {"a/b/file", "content"}, + {"c/.gitattributes", "[attr]internal export-ignore\n"}, + {"c/file", "content"}, + }); + + auto repo = openRepo(); + auto commit = commitHash(); + + ASSERT_FALSE(repo->mayExportIgnore(commit, "")); + ASSERT_FALSE(repo->mayExportIgnore(commit, "a")); + // A rule in a subdirectory only applies below that subdirectory. + ASSERT_TRUE(repo->mayExportIgnore(commit, "a/b")); + // A macro definition counts too: it can assign `export-ignore` indirectly. + ASSERT_TRUE(repo->mayExportIgnore(commit, "c")); + ASSERT_FALSE(repo->mayExportIgnore(commit, "does/not/exist")); +} + +TEST_F(GitExportIgnoreTest, may_export_ignore_sees_rules_in_the_root) +{ + commitFiles({ + {".gitattributes", "dropped export-ignore\n"}, + {"a/file", "content"}, + }); + + auto repo = openRepo(); + + ASSERT_TRUE(repo->mayExportIgnore(commitHash(), "")); + ASSERT_TRUE(repo->mayExportIgnore(commitHash(), "a")); +} + +TEST_F(GitExportIgnoreTest, a_rule_in_a_subdirectory_is_honoured) +{ + commitFiles({ + {".gitattributes", "*.png filter=lfs -text\n"}, + {"kept", "content"}, + {"sub/.gitattributes", "dropped export-ignore\n"}, + {"sub/dropped", "content"}, + {"sub/kept", "content"}, + }); + + auto repo = openRepo(); + auto accessor = repo->getAccessor(commitHash(), {.exportIgnore = true}, ""); + + ASSERT_TRUE(accessor->pathExists(CanonPath("kept"))); + ASSERT_TRUE(accessor->pathExists(CanonPath("sub/kept"))); + ASSERT_FALSE(accessor->pathExists(CanonPath("sub/dropped"))); +} + +/** + * Zone accessors are rooted at a subtree, so the `.gitattributes` files that + * apply to them live above the accessor's root. + */ +TEST_F(GitExportIgnoreTest, a_rule_above_the_attr_path_prefix_is_honoured) +{ + commitFiles({ + {".gitattributes", "dropped export-ignore\n"}, + {"zone/dropped", "content"}, + {"zone/kept", "content"}, + }); + + auto repo = openRepo(); + auto commit = commitHash(); + auto zoneTree = repo->getSubtreeSha(repo->getCommitTree(commit), "zone"); + + auto accessor = + repo->getAccessor(zoneTree, {.exportIgnore = true, .attrCommitRev = commit, .attrPathPrefix = "zone"}, ""); + + ASSERT_TRUE(accessor->pathExists(CanonPath("kept"))); + ASSERT_FALSE(accessor->pathExists(CanonPath("dropped"))); +} + } // namespace nix diff --git a/src/libfetchers/git-utils.cc b/src/libfetchers/git-utils.cc index 585936ead8..73bfbcbdb7 100644 --- a/src/libfetchers/git-utils.cc +++ b/src/libfetchers/git-utils.cc @@ -34,6 +34,7 @@ #include #include #include +#include #include #include #include @@ -264,6 +265,59 @@ static void initRepoAtomically(std::filesystem::path & path, GitRepo::Options op delTmpDir.cancel(); } +/** + * Whether the contents of a `.gitattributes` file can assign `export-ignore`. + * Every rule and every macro definition that sets, unsets or unspecifies the + * attribute has to spell its name out, so a substring search is a sound (if + * conservative: it also fires on comments) test. + */ +static bool mentionsExportIgnore(std::string_view contents) +{ + return contents.find("export-ignore") != std::string_view::npos; +} + +/** + * The attribute files outside the repository's tree that libgit2 consults: + * `$GIT_DIR/info/attributes` and the global `core.attributesfile` (which + * defaults to the XDG location). `GIT_ATTR_CHECK_NO_SYSTEM` only excludes the + * system-wide file, so these two can still assign attributes. + */ +static std::vector externalAttrFiles(git_repository * repo) +{ + std::vector result; + + auto toPath = [](const git_buf & buf) { return std::filesystem::path(std::string(buf.ptr, buf.size)); }; + + { + git_buf info = GIT_BUF_INIT; + if (!git_repository_item_path(&info, repo, GIT_REPOSITORY_ITEM_INFO) && info.size) + result.push_back(toPath(info) / "attributes"); + git_buf_dispose(&info); + } + + GitConfig config; + if (!git_repository_config_snapshot(Setter(config), repo)) { + git_buf configured = GIT_BUF_INIT; + if (!git_config_get_path(&configured, config.get(), "core.attributesfile")) { + if (configured.size) + result.push_back(toPath(configured)); + } else { + /* Unset: libgit2 falls back to `/attributes`. */ + git_buf searchPath = GIT_BUF_INIT; + if (!git_libgit2_opts(GIT_OPT_GET_SEARCH_PATH, GIT_CONFIG_LEVEL_XDG, &searchPath) && searchPath.size) + for (auto & dir : tokenizeString>( + std::string(searchPath.ptr, searchPath.size), std::string(1, GIT_PATH_LIST_SEPARATOR))) + result.push_back(std::filesystem::path(dir) / "attributes"); + git_buf_dispose(&searchPath); + } + git_buf_dispose(&configured); + } + + git_error_clear(); + + return result; +} + struct GitRepoImpl : GitRepo, std::enable_shared_from_this { /** Location of the repository on disk. */ @@ -653,23 +707,27 @@ struct GitRepoImpl : GitRepo, std::enable_shared_from_this return toHash(*git_object_id(tree.get())); } - std::vector getGitAttributesAlongPath(const Hash & commitSha, const std::string & path) override + /** + * Call `fn` with the `.gitattributes` entry of the root tree and of each + * directory along `path`, outermost first, stopping at the first missing + * directory component. `fn` returns false to stop the walk early. + */ + void forEachGitAttributesAlongPath( + const Hash & commitSha, const std::string & path, std::function fn) { - std::vector result; auto oid = hashToOID(getCommitTree(commitSha)); - typedef std::unique_ptr> Tree; Tree tree; if (git_tree_lookup(Setter(tree), *this, &oid)) throw Error("looking up tree: %s", git_error_last()->message); auto checkAttrs = [&] { auto e = git_tree_entry_byname(tree.get(), ".gitattributes"); - if (e && git_tree_entry_type(e) == GIT_OBJECT_BLOB) - result.push_back(toHash(*git_tree_entry_id(e))); + return e && git_tree_entry_type(e) == GIT_OBJECT_BLOB ? fn(e) : true; }; - checkAttrs(); + if (!checkAttrs()) + return; for (auto & component : tokenizeString>(path, "/")) { auto e = git_tree_entry_byname(tree.get(), component.c_str()); @@ -679,9 +737,62 @@ struct GitRepoImpl : GitRepo, std::enable_shared_from_this if (git_tree_lookup(Setter(next), *this, git_tree_entry_id(e))) break; tree = std::move(next); - checkAttrs(); + if (!checkAttrs()) + return; } + } + + std::vector getGitAttributesAlongPath(const Hash & commitSha, const std::string & path) override + { + std::vector result; + forEachGitAttributesAlongPath(commitSha, path, [&](const git_tree_entry * e) { + result.push_back(toHash(*git_tree_entry_id(e))); + return true; + }); + return result; + } + /** + * Memoised result of scanning the attribute files outside the repository's + * tree; see `externalAttrFiles`. + */ + std::once_flag externalAttrFilesScanned; + bool externalAttrFilesMayExportIgnore_ = false; + + bool externalAttrFilesMayExportIgnore() + { + std::call_once(externalAttrFilesScanned, [&]() { + for (auto & file : externalAttrFiles(*this)) { + try { + if (nix::pathExists(file) && mentionsExportIgnore(nix::readFile(file))) { + externalAttrFilesMayExportIgnore_ = true; + return; + } + } catch (SysError &) { + // Unreadable: assume the worst rather than silently + // dropping rules libgit2 might still see. + externalAttrFilesMayExportIgnore_ = true; + return; + } + } + }); + return externalAttrFilesMayExportIgnore_; + } + + bool mayExportIgnore(const Hash & commitSha, const std::string & path) override + { + if (externalAttrFilesMayExportIgnore()) + return true; + + bool result = false; + forEachGitAttributesAlongPath(commitSha, path, [&](const git_tree_entry * e) { + Blob blob; + if (git_tree_entry_to_object((git_object **) (git_blob **) Setter(blob), *this, e)) + throw GitError("reading a '.gitattributes' file"); + result = mentionsExportIgnore( + std::string_view((const char *) git_blob_rawcontent(blob.get()), git_blob_rawsize(blob.get()))); + return !result; + }); return result; } @@ -1171,6 +1282,12 @@ struct GitExportIgnoreSourceAccessor : CachingFilteringSourceAccessor ref repo; std::optional rev; GitAccessorOptions options; + + /** + * Per directory, whether any `.gitattributes` applying to its entries + * mentions `export-ignore`. See `mayExportIgnore`. + */ + boost::unordered_flat_map mayExportIgnoreCache; }; Sync state_; @@ -1212,8 +1329,57 @@ struct GitExportIgnoreSourceAccessor : CachingFilteringSourceAccessor } } + /** + * Whether `dir` can contain export-ignored entries at all. + * + * Resolving an attribute costs O(number of rules in all applicable + * `.gitattributes`) per path, which dominates ingestion of a large + * repository even when nothing uses `export-ignore`. Deciding it once per + * directory instead turns that into O(directories). + * + * The answer for the accessor's root covers everything above it (the + * `.gitattributes` files along `attrPathPrefix` and the attribute files + * outside the tree); every directory below adds its own `.gitattributes` + * to its parent's answer. + * + * Only available when reading from a commit: with a working directory + * libgit2 resolves attributes against the index, which we cannot see + * through `next`. + */ + bool mayExportIgnore(State & state, const CanonPath & dir) + { + if (!state.rev) + return true; + + if (auto i = state.mayExportIgnoreCache.find(dir); i != state.mayExportIgnoreCache.end()) + return i->second; + + bool result; + if (auto parent = dir.parent()) + result = mayExportIgnore(state, *parent) || attrFileMentionsExportIgnore(dir); + else + result = state.repo->mayExportIgnore(*state.rev, state.options.attrPathPrefix); + + state.mayExportIgnoreCache.emplace(dir, result); + return result; + } + + /** Whether `dir`'s own `.gitattributes`, if any, mentions `export-ignore`. */ + bool attrFileMentionsExportIgnore(const CanonPath & dir) + { + auto attrFile = dir / ".gitattributes"; + auto st = next->maybeLstat(attrFile); + return st && st->type == tRegular && mentionsExportIgnore(next->readFile(attrFile)); + } + bool isExportIgnored(const CanonPath & path) { + { + auto state(state_.lock()); + if (!mayExportIgnore(*state, path.parent().value_or(CanonPath::root))) + return false; + } + const char * exportIgnoreEntry = nullptr; // GIT_ATTR_CHECK_INDEX_ONLY: diff --git a/src/libfetchers/include/nix/fetchers/git-utils.hh b/src/libfetchers/include/nix/fetchers/git-utils.hh index e9f63a5b9d..741d533c63 100644 --- a/src/libfetchers/include/nix/fetchers/git-utils.hh +++ b/src/libfetchers/include/nix/fetchers/git-utils.hh @@ -132,6 +132,23 @@ struct GitRepo /** Blob OIDs of .gitattributes at each directory from root to `path`. */ virtual std::vector getGitAttributesAlongPath(const Hash & commitSha, const std::string & path) = 0; + /** + * Whether any Git attribute source that applies to the entries of the + * repo-relative directory `path` mentions `export-ignore`: the + * `.gitattributes` files from the root tree down to `path` as of + * `commitSha`, plus `$GIT_DIR/info/attributes` and the global + * `core.attributesfile` (`GIT_ATTR_CHECK_NO_SYSTEM` only excludes the + * system-wide file). + * + * The test is deliberately conservative — it just looks for the literal + * attribute name, which every rule and macro that assigns it has to spell + * out — but a `false` result guarantees that no entry of `path` can be + * export-ignored. Since resolving an attribute costs O(number of rules in + * all applicable `.gitattributes`) per path, this lets callers skip the + * lookup for whole directories at a time. + */ + virtual bool mayExportIgnore(const Hash & commitSha, const std::string & path) = 0; + virtual ref getAccessor(const Hash & rev, const GitAccessorOptions & options, std::string displayPrefix) = 0;