kernel
/
git
mirror of https://git.kernel.org/pub/scm/git/git.git
1
0
Fork 0
Code Releases Activity

Merge branch 'ps/fetch-atomic' into ps/fetch-mirror-optim 

* ps/fetch-atomic:
  fetch: make `--atomic` flag cover pruning of refs
  fetch: make `--atomic` flag cover backfilling of tags
  refs: add interface to iterate over queued transactional updates
  fetch: report errors when backfilling tags fails
  fetch: control lifecycle of FETCH_HEAD in a single place
  fetch: backfill tags before setting upstream
  fetch: increase test coverage of fetches
maint
Junio C Hamano 2022-03-01 10:11:00 -08:00
parent e6ebfd0e8c 583bc41923
commit 34363403a2
5 changed files with 263 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

180
builtin/fetch.c
View File

@ -349,7 +349,19 @@ static void clear_item(struct refname_hash_entry *item)
item->ignore = 1; item->ignore = 1;
} }



static void add_already_queued_tags(const char *refname,
const struct object_id *old_oid,
const struct object_id *new_oid,
void *cb_data)
{
struct hashmap *queued_tags = cb_data;
if (starts_with(refname, "refs/tags/") && new_oid)
(void) refname_hash_add(queued_tags, refname, new_oid);
}

static void find_non_local_tags(const struct ref *refs, static void find_non_local_tags(const struct ref *refs,
struct ref_transaction *transaction,
struct ref **head, struct ref **head,
struct ref ***tail) struct ref ***tail)
{ {
@ -367,6 +379,16 @@ static void find_non_local_tags(const struct ref *refs,
create_fetch_oidset(head, &fetch_oids); create_fetch_oidset(head, &fetch_oids);


for_each_ref(add_one_refname, &existing_refs); for_each_ref(add_one_refname, &existing_refs);

/*
* If we already have a transaction, then we need to filter out all
* tags which have already been queued up.
*/
if (transaction)
ref_transaction_for_each_queued_update(transaction,
add_already_queued_tags,
&existing_refs);

for (ref = refs; ref; ref = ref->next) { for (ref = refs; ref; ref = ref->next) {
if (!starts_with(ref->name, "refs/tags/")) if (!starts_with(ref->name, "refs/tags/"))
continue; continue;
@ -600,7 +622,7 @@ static struct ref *get_ref_map(struct remote *remote,
/* also fetch all tags */ /* also fetch all tags */
get_fetch_map(remote_refs, tag_refspec, &tail, 0); get_fetch_map(remote_refs, tag_refspec, &tail, 0);
else if (tags == TAGS_DEFAULT && *autotags) else if (tags == TAGS_DEFAULT && *autotags)
find_non_local_tags(remote_refs, &ref_map, &tail); find_non_local_tags(remote_refs, NULL, &ref_map, &tail);


/* Now append any refs to be updated opportunistically: */ /* Now append any refs to be updated opportunistically: */
*tail = orefs; *tail = orefs;
@ -1083,23 +1105,18 @@ N_("it took %.2f seconds to check forced updates; you can use\n"
"to avoid this check\n"); "to avoid this check\n");


static int store_updated_refs(const char *raw_url, const char *remote_name, static int store_updated_refs(const char *raw_url, const char *remote_name,
int connectivity_checked, struct ref *ref_map, int connectivity_checked,
struct worktree **worktrees) struct ref_transaction *transaction, struct ref *ref_map,
struct fetch_head *fetch_head, struct worktree **worktrees)
{ {
struct fetch_head fetch_head;
int url_len, i, rc = 0; int url_len, i, rc = 0;
struct strbuf note = STRBUF_INIT, err = STRBUF_INIT; struct strbuf note = STRBUF_INIT, err = STRBUF_INIT;
struct ref_transaction *transaction = NULL;
const char *what, *kind; const char *what, *kind;
struct ref *rm; struct ref *rm;
char *url; char *url;
int want_status; int want_status;
int summary_width = transport_summary_width(ref_map); int summary_width = transport_summary_width(ref_map);


rc = open_fetch_head(&fetch_head);
if (rc)
return -1;

if (raw_url) if (raw_url)
url = transport_anonymize_url(raw_url); url = transport_anonymize_url(raw_url);
else else
@ -1115,14 +1132,6 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
} }
} }


if (atomic_fetch) {
transaction = ref_transaction_begin(&err);
if (!transaction) {
error("%s", err.buf);
goto abort;
}
}

prepare_format_display(ref_map); prepare_format_display(ref_map);


/* /*
@ -1206,7 +1215,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
strbuf_addf(&note, "'%s' of ", what); strbuf_addf(&note, "'%s' of ", what);
} }


append_fetch_head(&fetch_head, &rm->old_oid, append_fetch_head(fetch_head, &rm->old_oid,
rm->fetch_head_status, rm->fetch_head_status,
note.buf, url, url_len); note.buf, url, url_len);


@ -1238,17 +1247,6 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
} }
} }


if (!rc && transaction) {
rc = ref_transaction_commit(transaction, &err);
if (rc) {
error("%s", err.buf);
goto abort;
}
}

if (!rc)
commit_fetch_head(&fetch_head);

if (rc & STORE_REF_ERROR_DF_CONFLICT) if (rc & STORE_REF_ERROR_DF_CONFLICT)
error(_("some local refs could not be updated; try running\n" error(_("some local refs could not be updated; try running\n"
" 'git remote prune %s' to remove any old, conflicting " " 'git remote prune %s' to remove any old, conflicting "
@ -1266,9 +1264,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
abort: abort:
strbuf_release(&note); strbuf_release(&note);
strbuf_release(&err); strbuf_release(&err);
ref_transaction_free(transaction);
free(url); free(url);
close_fetch_head(&fetch_head);
return rc; return rc;
} }


@ -1308,7 +1304,9 @@ static int check_exist_and_connected(struct ref *ref_map)
} }


static int fetch_and_consume_refs(struct transport *transport, static int fetch_and_consume_refs(struct transport *transport,
struct ref_transaction *transaction,
struct ref *ref_map, struct ref *ref_map,
struct fetch_head *fetch_head,
struct worktree **worktrees) struct worktree **worktrees)
{ {
int connectivity_checked = 1; int connectivity_checked = 1;
@ -1331,7 +1329,8 @@ static int fetch_and_consume_refs(struct transport *transport,


trace2_region_enter("fetch", "consume_refs", the_repository); trace2_region_enter("fetch", "consume_refs", the_repository);
ret = store_updated_refs(transport->url, transport->remote->name, ret = store_updated_refs(transport->url, transport->remote->name,
connectivity_checked, ref_map, worktrees); connectivity_checked, transaction, ref_map,
fetch_head, worktrees);
trace2_region_leave("fetch", "consume_refs", the_repository); trace2_region_leave("fetch", "consume_refs", the_repository);


out: out:
@ -1339,11 +1338,14 @@ out:
return ret; return ret;
} }


static int prune_refs(struct refspec *rs, struct ref *ref_map, static int prune_refs(struct refspec *rs,
struct ref_transaction *transaction,
struct ref *ref_map,
const char *raw_url) const char *raw_url)
{ {
int url_len, i, result = 0; int url_len, i, result = 0;
struct ref *ref, *stale_refs = get_stale_heads(rs, ref_map); struct ref *ref, *stale_refs = get_stale_heads(rs, ref_map);
struct strbuf err = STRBUF_INIT;
char *url; char *url;
int summary_width = transport_summary_width(stale_refs); int summary_width = transport_summary_width(stale_refs);
const char *dangling_msg = dry_run const char *dangling_msg = dry_run
@ -1364,6 +1366,14 @@ static int prune_refs(struct refspec *rs, struct ref *ref_map,
url_len = i - 3; url_len = i - 3;


if (!dry_run) { if (!dry_run) {
if (transaction) {
for (ref = stale_refs; ref; ref = ref->next) {
result = ref_transaction_delete(transaction, ref->name, NULL, 0,
"fetch: prune", &err);
if (result)
goto cleanup;
}
} else {
struct string_list refnames = STRING_LIST_INIT_NODUP; struct string_list refnames = STRING_LIST_INIT_NODUP;


for (ref = stale_refs; ref; ref = ref->next) for (ref = stale_refs; ref; ref = ref->next)
@ -1372,6 +1382,7 @@ static int prune_refs(struct refspec *rs, struct ref *ref_map,
result = delete_refs("fetch: prune", &refnames, 0); result = delete_refs("fetch: prune", &refnames, 0);
string_list_clear(&refnames, 0); string_list_clear(&refnames, 0);
} }
}


if (verbosity >= 0) { if (verbosity >= 0) {
for (ref = stale_refs; ref; ref = ref->next) { for (ref = stale_refs; ref; ref = ref->next) {
@ -1389,6 +1400,8 @@ static int prune_refs(struct refspec *rs, struct ref *ref_map,
} }
} }


cleanup:
strbuf_release(&err);
free(url); free(url);
free_refs(stale_refs); free_refs(stale_refs);
return result; return result;
@ -1503,10 +1516,13 @@ static struct transport *prepare_transport(struct remote *remote, int deepen)
return transport; return transport;
} }


static void backfill_tags(struct transport *transport, struct ref *ref_map, static int backfill_tags(struct transport *transport,
struct ref_transaction *transaction,
struct ref *ref_map,
struct fetch_head *fetch_head,
struct worktree **worktrees) struct worktree **worktrees)
{ {
int cannot_reuse; int retcode, cannot_reuse;


/* /*
* Once we have set TRANS_OPT_DEEPEN_SINCE, we can't unset it * Once we have set TRANS_OPT_DEEPEN_SINCE, we can't unset it
@ -1525,18 +1541,21 @@ static void backfill_tags(struct transport *transport, struct ref *ref_map,
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL); transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL);
transport_set_option(transport, TRANS_OPT_DEPTH, "0"); transport_set_option(transport, TRANS_OPT_DEPTH, "0");
transport_set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, NULL); transport_set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, NULL);
fetch_and_consume_refs(transport, ref_map, worktrees); retcode = fetch_and_consume_refs(transport, transaction, ref_map, fetch_head, worktrees);


if (gsecondary) { if (gsecondary) {
transport_disconnect(gsecondary); transport_disconnect(gsecondary);
gsecondary = NULL; gsecondary = NULL;
} }

return retcode;
} }


static int do_fetch(struct transport *transport, static int do_fetch(struct transport *transport,
struct refspec *rs) struct refspec *rs)
{ {
struct ref *ref_map; struct ref_transaction *transaction = NULL;
struct ref *ref_map = NULL;
int autotags = (transport->remote->fetch_tags == 1); int autotags = (transport->remote->fetch_tags == 1);
int retcode = 0; int retcode = 0;
const struct ref *remote_refs; const struct ref *remote_refs;
@ -1544,6 +1563,8 @@ static int do_fetch(struct transport *transport,
TRANSPORT_LS_REFS_OPTIONS_INIT; TRANSPORT_LS_REFS_OPTIONS_INIT;
int must_list_refs = 1; int must_list_refs = 1;
struct worktree **worktrees = get_worktrees(); struct worktree **worktrees = get_worktrees();
struct fetch_head fetch_head = { 0 };
struct strbuf err = STRBUF_INIT;


if (tags == TAGS_DEFAULT) { if (tags == TAGS_DEFAULT) {
if (transport->remote->fetch_tags == 2) if (transport->remote->fetch_tags == 2)
@ -1601,6 +1622,18 @@ static int do_fetch(struct transport *transport,
if (!update_head_ok) if (!update_head_ok)
check_not_current_branch(ref_map, worktrees); check_not_current_branch(ref_map, worktrees);


retcode = open_fetch_head(&fetch_head);
if (retcode)
goto cleanup;

if (atomic_fetch) {
transaction = ref_transaction_begin(&err);
if (!transaction) {
retcode = error("%s", err.buf);
goto cleanup;
}
}

if (tags == TAGS_DEFAULT && autotags) if (tags == TAGS_DEFAULT && autotags)
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, "1"); transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, "1");
if (prune) { if (prune) {
@ -1610,21 +1643,61 @@ static int do_fetch(struct transport *transport,
* don't care whether --tags was specified. * don't care whether --tags was specified.
*/ */
if (rs->nr) { if (rs->nr) {
retcode = prune_refs(rs, ref_map, transport->url); retcode = prune_refs(rs, transaction, ref_map, transport->url);
} else { } else {
retcode = prune_refs(&transport->remote->fetch, retcode = prune_refs(&transport->remote->fetch,
ref_map, transaction, ref_map,
transport->url); transport->url);
} }
if (retcode != 0) if (retcode != 0)
retcode = 1; retcode = 1;
} }
if (fetch_and_consume_refs(transport, ref_map, worktrees)) {
free_refs(ref_map); if (fetch_and_consume_refs(transport, transaction, ref_map, &fetch_head, worktrees)) {
retcode = 1; retcode = 1;
goto cleanup; goto cleanup;
} }


/*
* If neither --no-tags nor --tags was specified, do automated tag
* following.
*/
if (tags == TAGS_DEFAULT && autotags) {
struct ref *tags_ref_map = NULL, **tail = &tags_ref_map;

find_non_local_tags(remote_refs, transaction, &tags_ref_map, &tail);
if (tags_ref_map) {
/*
* If backfilling of tags fails then we want to tell
* the user so, but we have to continue regardless to
* populate upstream information of the references we
* have already fetched above. The exception though is
* when `--atomic` is passed: in that case we'll abort
* the transaction and don't commit anything.
*/
if (backfill_tags(transport, transaction, tags_ref_map,
&fetch_head, worktrees))
retcode = 1;
}

free_refs(tags_ref_map);
}

if (transaction) {
if (retcode)
goto cleanup;

retcode = ref_transaction_commit(transaction, &err);
if (retcode) {
error("%s", err.buf);
ref_transaction_free(transaction);
transaction = NULL;
goto cleanup;
}
}

commit_fetch_head(&fetch_head);

if (set_upstream) { if (set_upstream) {
struct branch *branch = branch_get("HEAD"); struct branch *branch = branch_get("HEAD");
struct ref *rm; struct ref *rm;
@ -1644,7 +1717,7 @@ static int do_fetch(struct transport *transport,
if (!rm->peer_ref) { if (!rm->peer_ref) {
if (source_ref) { if (source_ref) {
warning(_("multiple branches detected, incompatible with --set-upstream")); warning(_("multiple branches detected, incompatible with --set-upstream"));
goto skip; goto cleanup;
} else { } else {
source_ref = rm; source_ref = rm;
} }
@ -1658,7 +1731,7 @@ static int do_fetch(struct transport *transport,
warning(_("could not set upstream of HEAD to '%s' from '%s' when " warning(_("could not set upstream of HEAD to '%s' from '%s' when "
"it does not point to any branch."), "it does not point to any branch."),
shortname, transport->remote->name); shortname, transport->remote->name);
goto skip; goto cleanup;
} }


if (!strcmp(source_ref->name, "HEAD") || if (!strcmp(source_ref->name, "HEAD") ||
@ -1678,21 +1751,16 @@ static int do_fetch(struct transport *transport,
"you need to specify exactly one branch with the --set-upstream option")); "you need to specify exactly one branch with the --set-upstream option"));
} }
} }
skip:
free_refs(ref_map);

/* if neither --no-tags nor --tags was specified, do automated tag
* following ... */
if (tags == TAGS_DEFAULT && autotags) {
struct ref **tail = &ref_map;
ref_map = NULL;
find_non_local_tags(remote_refs, &ref_map, &tail);
if (ref_map)
backfill_tags(transport, ref_map, worktrees);
free_refs(ref_map);
}


cleanup: cleanup:
if (retcode && transaction) {
ref_transaction_abort(transaction, &err);
error("%s", err.buf);
}

close_fetch_head(&fetch_head);
strbuf_release(&err);
free_refs(ref_map);
free_worktrees(worktrees); free_worktrees(worktrees);
return retcode; return retcode;
} }

16
refs.c
View File

@ -2417,6 +2417,22 @@ int initial_ref_transaction_commit(struct ref_transaction *transaction,
return refs->be->initial_transaction_commit(refs, transaction, err); return refs->be->initial_transaction_commit(refs, transaction, err);
} }


void ref_transaction_for_each_queued_update(struct ref_transaction *transaction,
ref_transaction_for_each_queued_update_fn cb,
void *cb_data)
{
int i;

for (i = 0; i < transaction->nr; i++) {
struct ref_update *update = transaction->updates[i];

cb(update->refname,
(update->flags & REF_HAVE_OLD) ? &update->old_oid : NULL,
(update->flags & REF_HAVE_NEW) ? &update->new_oid : NULL,
cb_data);
}
}

int refs_delete_refs(struct ref_store *refs, const char *logmsg, int refs_delete_refs(struct ref_store *refs, const char *logmsg,
struct string_list *refnames, unsigned int flags) struct string_list *refnames, unsigned int flags)
{ {

14
refs.h
View File

@ -776,6 +776,20 @@ int ref_transaction_abort(struct ref_transaction *transaction,
int initial_ref_transaction_commit(struct ref_transaction *transaction, int initial_ref_transaction_commit(struct ref_transaction *transaction,
struct strbuf *err); struct strbuf *err);


/*
* Execute the given callback function for each of the reference updates which
* have been queued in the given transaction. `old_oid` and `new_oid` may be
* `NULL` pointers depending on whether the update has these object IDs set or
* not.
*/
typedef void ref_transaction_for_each_queued_update_fn(const char *refname,
const struct object_id *old_oid,
const struct object_id *new_oid,
void *cb_data);
void ref_transaction_for_each_queued_update(struct ref_transaction *transaction,
ref_transaction_for_each_queued_update_fn cb,
void *cb_data);

/* /*
* Free `*transaction` and all associated data. * Free `*transaction` and all associated data.
*/ */

74
t/t5503-tagfollow.sh
View File

@ -160,4 +160,78 @@ test_expect_success 'new clone fetch main and tags' '
test_cmp expect actual test_cmp expect actual
' '


test_expect_success 'atomic fetch with failing backfill' '
git init clone3 &&

# We want to test whether a failure when backfilling tags correctly
# aborts the complete transaction when `--atomic` is passed: we should
# neither create the branch nor should we create the tag when either
# one of both fails to update correctly.
#
# To trigger failure we simply abort when backfilling a tag.
write_script clone3/.git/hooks/reference-transaction <<-\EOF &&
while read oldrev newrev reference
do
if test "$reference" = refs/tags/tag1
then
exit 1
fi
done
EOF

test_must_fail git -C clone3 fetch --atomic .. $B:refs/heads/something &&
test_must_fail git -C clone3 rev-parse --verify refs/heads/something &&
test_must_fail git -C clone3 rev-parse --verify refs/tags/tag2
'

test_expect_success 'atomic fetch with backfill should use single transaction' '
git init clone4 &&

# Fetching with the `--atomic` flag should update all references in a
# single transaction, including backfilled tags. We thus expect to see
# a single reference transaction for the created branch and tags.
cat >expected <<-EOF &&
prepared
$ZERO_OID $B refs/heads/something
$ZERO_OID $S refs/tags/tag2
$ZERO_OID $T refs/tags/tag1
committed
$ZERO_OID $B refs/heads/something
$ZERO_OID $S refs/tags/tag2
$ZERO_OID $T refs/tags/tag1
EOF

write_script clone4/.git/hooks/reference-transaction <<-\EOF &&
( echo "$*" && cat ) >>actual
EOF

git -C clone4 fetch --atomic .. $B:refs/heads/something &&
test_cmp expected clone4/actual
'

test_expect_success 'backfill failure causes command to fail' '
git init clone5 &&

write_script clone5/.git/hooks/reference-transaction <<-EOF &&
while read oldrev newrev reference
do
if test "\$reference" = refs/tags/tag1
then
# Create a nested tag below the actual tag we
# wanted to write, which causes a D/F conflict
# later when we want to commit refs/tags/tag1.
# We cannot just `exit 1` here given that this
# would cause us to die immediately.
git update-ref refs/tags/tag1/nested $B
exit \$!
fi
done
EOF

test_must_fail git -C clone5 fetch .. $B:refs/heads/something &&
test $B = $(git -C clone5 rev-parse --verify refs/heads/something) &&
test $S = $(git -C clone5 rev-parse --verify tag2) &&
test_must_fail git -C clone5 rev-parse --verify tag1
'

test_done test_done

29
t/t5510-fetch.sh
View File

@ -343,6 +343,35 @@ test_expect_success 'fetch --atomic --append appends to FETCH_HEAD' '
test_cmp expected atomic/.git/FETCH_HEAD test_cmp expected atomic/.git/FETCH_HEAD
' '


test_expect_success 'fetch --atomic --prune executes a single reference transaction only' '
test_when_finished "rm -rf \"$D\"/atomic" &&

cd "$D" &&
git branch scheduled-for-deletion &&
git clone . atomic &&
git branch -D scheduled-for-deletion &&
git branch new-branch &&
head_oid=$(git rev-parse HEAD) &&

# Fetching with the `--atomic` flag should update all references in a
# single transaction.
cat >expected <<-EOF &&
prepared
$ZERO_OID $ZERO_OID refs/remotes/origin/scheduled-for-deletion
$ZERO_OID $head_oid refs/remotes/origin/new-branch
committed
$ZERO_OID $ZERO_OID refs/remotes/origin/scheduled-for-deletion
$ZERO_OID $head_oid refs/remotes/origin/new-branch
EOF

write_script atomic/.git/hooks/reference-transaction <<-\EOF &&
( echo "$*" && cat ) >>actual
EOF

git -C atomic fetch --atomic --prune origin &&
test_cmp expected atomic/actual
'

test_expect_success '--refmap="" ignores configured refspec' ' test_expect_success '--refmap="" ignores configured refspec' '
cd "$TRASH_DIRECTORY" && cd "$TRASH_DIRECTORY" &&
git clone "$D" remote-refs && git clone "$D" remote-refs &&