From cbc0b8728f344269a37962d499e1ab0f7aae8d83 Mon Sep 17 00:00:00 2001 From: Kristofer Karlsson Date: Fri, 30 Oct 2026 12:07:35 +0100 Subject: [PATCH] odb: do not list our own refs as alternate refs in the quarantine Since 722ff7f876 (receive-pack: quarantine objects until pre-receive accepts, 2016-10-03), receive-pack writes incoming objects to a temporary object directory. Child processes see it as their primary object directory, and tmp-objdir passes the repository's own object directory to them via GIT_ALTERNATE_OBJECT_DIRECTORIES. The connectivity check passes --alternate-refs to rev-list since 39b44ba771 (check_everything_connected: assume alternate ref tips are valid, 2019-07-01). Inside the quarantine, the repository's own object directory is one of the alternates, so --alternate-refs runs for-each-ref on the repository itself and lists all of its refs. That includes hidden refs. bcec6780b2 (receive-pack: only use visible refs for connectivity check, 2022-11-17) added --exclude-hidden=receive to the check so that repositories with many hidden refs do not have to load all of them on each push. The hidden refs are excluded from --all, but they all come back through --alternate-refs. Skip our own object directory when listing alternate refs. The quarantine is created directly inside it, as documented in "QUARANTINE ENVIRONMENT" in git-receive-pack(1), so it is the parent of GIT_QUARANTINE_PATH. This is safe: alternate refs only serve as negative tips that are assumed to be complete, so dropping some can only make the traversal walk further, never make it accept a missing object. Add a test that our own refs are no longer listed, both by the connectivity check and by rev-list run from a pre-receive hook, a test that the refs of a real alternate still are, and a perf test pushing a single commit into a repository with a visible branch and about 200,000 refs hidden by receive.hideRefs, and into a copy of it that hides nothing: Test HEAD~1 HEAD ----------------------------------------------------------------------- 5552.2: push, hidden refs 0.29(0.31+0.06) 0.10(0.07+0.03) -65.5% 5552.3: push, no hidden refs 1.18(1.00+0.67) 0.98(0.78+0.62) -16.9% With hidden refs, most of the time went to the for-each-ref child that listed the alternate refs. Without hidden refs, every ref was listed twice, once for --all and once for --alternate-refs. Signed-off-by: Kristofer Karlsson --- odb.c | 19 ++++++++++++++++ t/perf/p5552-push-hidden-refs.sh | 37 +++++++++++++++++++++++++++++++ t/t5547-push-quarantine.sh | 38 ++++++++++++++++++++++++++++++++ 3 files changed, 94 insertions(+) create mode 100755 t/perf/p5552-push-hidden-refs.sh diff --git a/odb.c b/odb.c index 2c93a9490c6b18..4313bb795e6442 100644 --- a/odb.c +++ b/odb.c @@ -2,6 +2,7 @@ #include "abspath.h" #include "commit-graph.h" #include "config.h" +#include "dir.h" #include "environment.h" #include "gettext.h" #include "hashmap.h" @@ -183,11 +184,28 @@ static int refs_from_alternate_cb(struct odb_files_dir *alternate, void *payload) { struct strbuf path = STRBUF_INIT; + struct strbuf self = STRBUF_INIT; + const char *quarantine; size_t base_len; struct alternate_refs_data *cb = payload; if (!strbuf_realpath(&path, alternate->abspath, 0)) goto out; + + /* + * Inside an object quarantine, our own object directory is one of + * our alternates, but its refs are our own refs, not alternate refs. + * The quarantine is created directly inside it, so it is the parent + * of GIT_QUARANTINE_PATH. + */ + quarantine = getenv(GIT_QUARANTINE_ENVIRONMENT); + if (quarantine && strbuf_realpath(&self, quarantine, 0)) { + strbuf_strip_file_from_path(&self); + strbuf_trim_trailing_dir_sep(&self); + if (!fspathcmp(path.buf, self.buf)) + goto out; + } + if (!strbuf_strip_suffix(&path, "/objects")) goto out; base_len = path.len; @@ -202,6 +220,7 @@ static int refs_from_alternate_cb(struct odb_files_dir *alternate, out: strbuf_release(&path); + strbuf_release(&self); return 0; } diff --git a/t/perf/p5552-push-hidden-refs.sh b/t/perf/p5552-push-hidden-refs.sh new file mode 100755 index 00000000000000..e9aa20629875c7 --- /dev/null +++ b/t/perf/p5552-push-hidden-refs.sh @@ -0,0 +1,37 @@ +#!/bin/sh + +test_description='speed of push into a repository with many hidden refs' +. ./perf-lib.sh + +test_perf_fresh_repo + +test_expect_success 'set up repository with many hidden refs' ' + git init --bare dst.git && + test_commit_bulk --ref=refs/hidden/base --id=hidden 1000 && + git rev-list refs/hidden/base >commits && + for i in $(test_seq 200) + do + sed "s|^\(.*\)$|create refs/hidden/$i/\1 \1|" commits || return 1 + done >input && + git push dst.git refs/hidden/base && + git -C dst.git update-ref --stdin >"$(pwd)/alternate-refs.log" + EOF + git -C hidden.git config core.alternateRefsCommand ./alternate-refs && + test_commit hidden-push && + git push hidden.git HEAD:refs/heads/new && + test_path_is_missing alternate-refs.log +' + +test_expect_success 'quarantine lists refs of other alternates' ' + test_when_finished "rm -f alternate-refs.log" && + git init --bare other.git && + git push other.git HEAD:refs/heads/other && + echo "$(pwd)/other.git/objects" >hidden.git/objects/info/alternates && + test_commit hidden-push-2 && + git push hidden.git HEAD:refs/heads/new2 && + (cd other.git && pwd -P) >expect && + sort -u alternate-refs.log >actual && + test_cmp expect actual +' + +test_expect_success 'rev-list --alternate-refs in a hook skips own refs' ' + test_when_finished "rm -rf altref.git" && + git init --bare altref.git && + git push altref.git HEAD:refs/heads/main && + test_hook -C altref.git pre-receive <<-\EOF && + git rev-list --no-walk --alternate-refs --stdin ../hook-alternate-refs + EOF + test_commit altref-push && + git push altref.git HEAD:refs/heads/new && + test_must_be_empty hook-alternate-refs +' + test_done