Skip to content

receive-pack: connectivity check does not exclude hidden refs in the quarantine #2285

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
spkrka wants to merge 1 commit into
base: master
Choose a base branch
from
Open

receive-pack: connectivity check does not exclude hidden refs in the quarantine #2285

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions odb.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,13 +183,28 @@ static int refs_from_alternate_cb(struct odb_files_dir *alternate,
void *payload)
{
struct strbuf path = STRBUF_INIT;
struct strbuf self = STRBUF_INIT;
size_t base_len;
struct alternate_refs_data *cb = payload;

if (!strbuf_realpath(&path, alternate->abspath, 0))
goto out;
if (!strbuf_strip_suffix(&path, "/objects"))
goto out;

/*
* NEEDSWORK: this is a hack, mainly here to measure the cost of
* the bug. Inside an object quarantine, tmp-objdir passes our own
* object directory to child processes as an alternate, so we would
* list our own refs as alternate refs, including the hidden refs
* that the connectivity check of receive-pack deliberately skips.
* Comparing paths papers over that; the quarantine should rather
* tell its children which directory is ours.
*/
if (cb->repo->commondir &&
strbuf_realpath(&self, cb->repo->commondir, 0) &&
!strcmp(path.buf, self.buf))
goto out;
base_len = path.len;

/* Is this a git repository with refs? */
Expand All @@ -202,6 +217,7 @@ static int refs_from_alternate_cb(struct odb_files_dir *alternate,

out:
strbuf_release(&path);
strbuf_release(&self);
return 0;
}

Expand Down
37 changes: 37 additions & 0 deletions t/perf/p5552-push-hidden-refs.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 <input &&
git -C dst.git pack-refs --all &&
cp -R dst.git dst-visible.git &&
git -C dst.git config receive.hideRefs refs/hidden &&
git symbolic-ref HEAD refs/heads/main &&
test_commit_bulk --id=visible 1 &&
git push dst.git main &&
git push dst-visible.git main
'

test_perf 'push, hidden refs' '
git commit --allow-empty -m new &&
git push -f dst.git HEAD:refs/heads/new
'

test_perf 'push, no hidden refs' '
git commit --allow-empty -m new &&
git push -f dst-visible.git HEAD:refs/heads/new
'

test_done
25 changes: 25 additions & 0 deletions t/t5547-push-quarantine.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,4 +101,29 @@ test_expect_success '.keep file is removed after push' '
test_path_is_missing "$keep"
'

test_expect_success 'quarantine does not list own refs as alternate refs' '
git init --bare hidden.git &&
git push hidden.git HEAD:refs/heads/main HEAD:refs/hidden/one &&
git -C hidden.git config receive.hideRefs refs/hidden &&
write_script hidden.git/alternate-refs <<-EOF &&
echo "\$1" >>"$(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_done
Loading