Skip to content

Commit 851d2f0

Browse files
gitstergitster
committed
Merge branch 'ps/fetch-atomic'
"git fetch" can make two separate fetches, but ref updates coming from them were in two separate ref transactions under "--atomic", which has been corrected. * 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
2 parents 1a48745 + 583bc41 commit 851d2f0

File tree

5 files changed

+262
-61
lines changed

5 files changed

+262
-61
lines changed

‎builtin/fetch.c‎

Lines changed: 129 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,19 @@ static void clear_item(struct refname_hash_entry *item)
349349
item->ignore = 1;
350350
}
351351

352+
353+
static void add_already_queued_tags(const char *refname,
354+
const struct object_id *old_oid,
355+
const struct object_id *new_oid,
356+
void *cb_data)
357+
{
358+
struct hashmap *queued_tags = cb_data;
359+
if (starts_with(refname, "refs/tags/") && new_oid)
360+
(void) refname_hash_add(queued_tags, refname, new_oid);
361+
}
362+
352363
static void find_non_local_tags(const struct ref *refs,
364+
struct ref_transaction *transaction,
353365
struct ref **head,
354366
struct ref ***tail)
355367
{
@@ -367,6 +379,16 @@ static void find_non_local_tags(const struct ref *refs,
367379
create_fetch_oidset(head, &fetch_oids);
368380

369381
for_each_ref(add_one_refname, &existing_refs);
382+
383+
/*
384+
* If we already have a transaction, then we need to filter out all
385+
* tags which have already been queued up.
386+
*/
387+
if (transaction)
388+
ref_transaction_for_each_queued_update(transaction,
389+
add_already_queued_tags,
390+
&existing_refs);
391+
370392
for (ref = refs; ref; ref = ref->next) {
371393
if (!starts_with(ref->name, "refs/tags/"))
372394
continue;
@@ -600,7 +622,7 @@ static struct ref *get_ref_map(struct remote *remote,
600622
/* also fetch all tags */
601623
get_fetch_map(remote_refs, tag_refspec, &tail, 0);
602624
else if (tags == TAGS_DEFAULT && *autotags)
603-
find_non_local_tags(remote_refs, &ref_map, &tail);
625+
find_non_local_tags(remote_refs, NULL, &ref_map, &tail);
604626

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

10851107
static int store_updated_refs(const char *raw_url, const char *remote_name,
1086-
int connectivity_checked, struct ref *ref_map,
1087-
struct worktree **worktrees)
1108+
int connectivity_checked,
1109+
struct ref_transaction *transaction, struct ref *ref_map,
1110+
struct fetch_head *fetch_head, struct worktree **worktrees)
10881111
{
1089-
struct fetch_head fetch_head;
10901112
int url_len, i, rc = 0;
10911113
struct strbuf note = STRBUF_INIT, err = STRBUF_INIT;
1092-
struct ref_transaction *transaction = NULL;
10931114
const char *what, *kind;
10941115
struct ref *rm;
10951116
char *url;
10961117
int want_status;
10971118
int summary_width = 0;
10981119

1099-
rc = open_fetch_head(&fetch_head);
1100-
if (rc)
1101-
return -1;
1102-
11031120
if (verbosity >= 0)
11041121
summary_width = transport_summary_width(ref_map);
11051122

@@ -1118,14 +1135,6 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
11181135
}
11191136
}
11201137

1121-
if (atomic_fetch) {
1122-
transaction = ref_transaction_begin(&err);
1123-
if (!transaction) {
1124-
error("%s", err.buf);
1125-
goto abort;
1126-
}
1127-
}
1128-
11291138
prepare_format_display(ref_map);
11301139

11311140
/*
@@ -1209,7 +1218,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
12091218
strbuf_addf(&note, "'%s' of ", what);
12101219
}
12111220

1212-
append_fetch_head(&fetch_head, &rm->old_oid,
1221+
append_fetch_head(fetch_head, &rm->old_oid,
12131222
rm->fetch_head_status,
12141223
note.buf, url, url_len);
12151224

@@ -1241,17 +1250,6 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
12411250
}
12421251
}
12431252

1244-
if (!rc && transaction) {
1245-
rc = ref_transaction_commit(transaction, &err);
1246-
if (rc) {
1247-
error("%s", err.buf);
1248-
goto abort;
1249-
}
1250-
}
1251-
1252-
if (!rc)
1253-
commit_fetch_head(&fetch_head);
1254-
12551253
if (rc & STORE_REF_ERROR_DF_CONFLICT)
12561254
error(_("some local refs could not be updated; try running\n"
12571255
" 'git remote prune %s' to remove any old, conflicting "
@@ -1269,9 +1267,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
12691267
abort:
12701268
strbuf_release(&note);
12711269
strbuf_release(&err);
1272-
ref_transaction_free(transaction);
12731270
free(url);
1274-
close_fetch_head(&fetch_head);
12751271
return rc;
12761272
}
12771273

@@ -1311,7 +1307,9 @@ static int check_exist_and_connected(struct ref *ref_map)
13111307
}
13121308

13131309
static int fetch_and_consume_refs(struct transport *transport,
1310+
struct ref_transaction *transaction,
13141311
struct ref *ref_map,
1312+
struct fetch_head *fetch_head,
13151313
struct worktree **worktrees)
13161314
{
13171315
int connectivity_checked = 1;
@@ -1334,19 +1332,23 @@ static int fetch_and_consume_refs(struct transport *transport,
13341332

13351333
trace2_region_enter("fetch", "consume_refs", the_repository);
13361334
ret = store_updated_refs(transport->url, transport->remote->name,
1337-
connectivity_checked, ref_map, worktrees);
1335+
connectivity_checked, transaction, ref_map,
1336+
fetch_head, worktrees);
13381337
trace2_region_leave("fetch", "consume_refs", the_repository);
13391338

13401339
out:
13411340
transport_unlock_pack(transport, 0);
13421341
return ret;
13431342
}
13441343

1345-
static int prune_refs(struct refspec *rs, struct ref *ref_map,
1344+
static int prune_refs(struct refspec *rs,
1345+
struct ref_transaction *transaction,
1346+
struct ref *ref_map,
13461347
const char *raw_url)
13471348
{
13481349
int url_len, i, result = 0;
13491350
struct ref *ref, *stale_refs = get_stale_heads(rs, ref_map);
1351+
struct strbuf err = STRBUF_INIT;
13501352
char *url;
13511353
const char *dangling_msg = dry_run
13521354
? _(" (%s will become dangling)")
@@ -1366,13 +1368,22 @@ static int prune_refs(struct refspec *rs, struct ref *ref_map,
13661368
url_len = i - 3;
13671369

13681370
if (!dry_run) {
1369-
struct string_list refnames = STRING_LIST_INIT_NODUP;
1371+
if (transaction) {
1372+
for (ref = stale_refs; ref; ref = ref->next) {
1373+
result = ref_transaction_delete(transaction, ref->name, NULL, 0,
1374+
"fetch: prune", &err);
1375+
if (result)
1376+
goto cleanup;
1377+
}
1378+
} else {
1379+
struct string_list refnames = STRING_LIST_INIT_NODUP;
13701380

1371-
for (ref = stale_refs; ref; ref = ref->next)
1372-
string_list_append(&refnames, ref->name);
1381+
for (ref = stale_refs; ref; ref = ref->next)
1382+
string_list_append(&refnames, ref->name);
13731383

1374-
result = delete_refs("fetch: prune", &refnames, 0);
1375-
string_list_clear(&refnames, 0);
1384+
result = delete_refs("fetch: prune", &refnames, 0);
1385+
string_list_clear(&refnames, 0);
1386+
}
13761387
}
13771388

13781389
if (verbosity >= 0) {
@@ -1393,6 +1404,8 @@ static int prune_refs(struct refspec *rs, struct ref *ref_map,
13931404
}
13941405
}
13951406

1407+
cleanup:
1408+
strbuf_release(&err);
13961409
free(url);
13971410
free_refs(stale_refs);
13981411
return result;
@@ -1507,10 +1520,13 @@ static struct transport *prepare_transport(struct remote *remote, int deepen)
15071520
return transport;
15081521
}
15091522

1510-
static void backfill_tags(struct transport *transport, struct ref *ref_map,
1511-
struct worktree **worktrees)
1523+
static int backfill_tags(struct transport *transport,
1524+
struct ref_transaction *transaction,
1525+
struct ref *ref_map,
1526+
struct fetch_head *fetch_head,
1527+
struct worktree **worktrees)
15121528
{
1513-
int cannot_reuse;
1529+
int retcode, cannot_reuse;
15141530

15151531
/*
15161532
* Once we have set TRANS_OPT_DEEPEN_SINCE, we can't unset it
@@ -1529,25 +1545,30 @@ static void backfill_tags(struct transport *transport, struct ref *ref_map,
15291545
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL);
15301546
transport_set_option(transport, TRANS_OPT_DEPTH, "0");
15311547
transport_set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, NULL);
1532-
fetch_and_consume_refs(transport, ref_map, worktrees);
1548+
retcode = fetch_and_consume_refs(transport, transaction, ref_map, fetch_head, worktrees);
15331549

15341550
if (gsecondary) {
15351551
transport_disconnect(gsecondary);
15361552
gsecondary = NULL;
15371553
}
1554+
1555+
return retcode;
15381556
}
15391557

15401558
static int do_fetch(struct transport *transport,
15411559
struct refspec *rs)
15421560
{
1543-
struct ref *ref_map;
1561+
struct ref_transaction *transaction = NULL;
1562+
struct ref *ref_map = NULL;
15441563
int autotags = (transport->remote->fetch_tags == 1);
15451564
int retcode = 0;
15461565
const struct ref *remote_refs;
15471566
struct transport_ls_refs_options transport_ls_refs_options =
15481567
TRANSPORT_LS_REFS_OPTIONS_INIT;
15491568
int must_list_refs = 1;
15501569
struct worktree **worktrees = get_worktrees();
1570+
struct fetch_head fetch_head = { 0 };
1571+
struct strbuf err = STRBUF_INIT;
15511572

15521573
if (tags == TAGS_DEFAULT) {
15531574
if (transport->remote->fetch_tags == 2)
@@ -1605,6 +1626,18 @@ static int do_fetch(struct transport *transport,
16051626
if (!update_head_ok)
16061627
check_not_current_branch(ref_map, worktrees);
16071628

1629+
retcode = open_fetch_head(&fetch_head);
1630+
if (retcode)
1631+
goto cleanup;
1632+
1633+
if (atomic_fetch) {
1634+
transaction = ref_transaction_begin(&err);
1635+
if (!transaction) {
1636+
retcode = error("%s", err.buf);
1637+
goto cleanup;
1638+
}
1639+
}
1640+
16081641
if (tags == TAGS_DEFAULT && autotags)
16091642
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, "1");
16101643
if (prune) {
@@ -1614,21 +1647,61 @@ static int do_fetch(struct transport *transport,
16141647
* don't care whether --tags was specified.
16151648
*/
16161649
if (rs->nr) {
1617-
retcode = prune_refs(rs, ref_map, transport->url);
1650+
retcode = prune_refs(rs, transaction, ref_map, transport->url);
16181651
} else {
16191652
retcode = prune_refs(&transport->remote->fetch,
1620-
ref_map,
1653+
transaction, ref_map,
16211654
transport->url);
16221655
}
16231656
if (retcode != 0)
16241657
retcode = 1;
16251658
}
1626-
if (fetch_and_consume_refs(transport, ref_map, worktrees)) {
1627-
free_refs(ref_map);
1659+
1660+
if (fetch_and_consume_refs(transport, transaction, ref_map, &fetch_head, worktrees)) {
16281661
retcode = 1;
16291662
goto cleanup;
16301663
}
16311664

1665+
/*
1666+
* If neither --no-tags nor --tags was specified, do automated tag
1667+
* following.
1668+
*/
1669+
if (tags == TAGS_DEFAULT && autotags) {
1670+
struct ref *tags_ref_map = NULL, **tail = &tags_ref_map;
1671+
1672+
find_non_local_tags(remote_refs, transaction, &tags_ref_map, &tail);
1673+
if (tags_ref_map) {
1674+
/*
1675+
* If backfilling of tags fails then we want to tell
1676+
* the user so, but we have to continue regardless to
1677+
* populate upstream information of the references we
1678+
* have already fetched above. The exception though is
1679+
* when `--atomic` is passed: in that case we'll abort
1680+
* the transaction and don't commit anything.
1681+
*/
1682+
if (backfill_tags(transport, transaction, tags_ref_map,
1683+
&fetch_head, worktrees))
1684+
retcode = 1;
1685+
}
1686+
1687+
free_refs(tags_ref_map);
1688+
}
1689+
1690+
if (transaction) {
1691+
if (retcode)
1692+
goto cleanup;
1693+
1694+
retcode = ref_transaction_commit(transaction, &err);
1695+
if (retcode) {
1696+
error("%s", err.buf);
1697+
ref_transaction_free(transaction);
1698+
transaction = NULL;
1699+
goto cleanup;
1700+
}
1701+
}
1702+
1703+
commit_fetch_head(&fetch_head);
1704+
16321705
if (set_upstream) {
16331706
struct branch *branch = branch_get("HEAD");
16341707
struct ref *rm;
@@ -1648,7 +1721,7 @@ static int do_fetch(struct transport *transport,
16481721
if (!rm->peer_ref) {
16491722
if (source_ref) {
16501723
warning(_("multiple branches detected, incompatible with --set-upstream"));
1651-
goto skip;
1724+
goto cleanup;
16521725
} else {
16531726
source_ref = rm;
16541727
}
@@ -1662,7 +1735,7 @@ static int do_fetch(struct transport *transport,
16621735
warning(_("could not set upstream of HEAD to '%s' from '%s' when "
16631736
"it does not point to any branch."),
16641737
shortname, transport->remote->name);
1665-
goto skip;
1738+
goto cleanup;
16661739
}
16671740

16681741
if (!strcmp(source_ref->name, "HEAD") ||
@@ -1682,21 +1755,16 @@ static int do_fetch(struct transport *transport,
16821755
"you need to specify exactly one branch with the --set-upstream option"));
16831756
}
16841757
}
1685-
skip:
1686-
free_refs(ref_map);
16871758

1688-
/* if neither --no-tags nor --tags was specified, do automated tag
1689-
* following ... */
1690-
if (tags == TAGS_DEFAULT && autotags) {
1691-
struct ref **tail = &ref_map;
1692-
ref_map = NULL;
1693-
find_non_local_tags(remote_refs, &ref_map, &tail);
1694-
if (ref_map)
1695-
backfill_tags(transport, ref_map, worktrees);
1696-
free_refs(ref_map);
1759+
cleanup:
1760+
if (retcode && transaction) {
1761+
ref_transaction_abort(transaction, &err);
1762+
error("%s", err.buf);
16971763
}
16981764

1699-
cleanup:
1765+
close_fetch_head(&fetch_head);
1766+
strbuf_release(&err);
1767+
free_refs(ref_map);
17001768
free_worktrees(worktrees);
17011769
return retcode;
17021770
}

0 commit comments

Comments
 (0)