@matheusbsbs / matheus-bagliano / commits / 583bc41923

fetch: make `--atomic` flag cover pruning of refs

When fetching with the `--prune` flag we will delete any local references matching the fetch refspec which have disappeared on the remote. This step is not currently covered by the `--atomic` flag: we delete branches even though updating of local references has failed, which means that the fetch is not an all-or-nothing operation. Fix this bug by passing in the global transaction into `prune_refs()`: if one is given, then we'll only queue up deletions and not commit them right away. This change also improves performance when pruning many branches in a repository with a big packed-refs file: every references is pruned in its own transaction, which means that we potentially have to rewrite the packed-refs files for every single reference we're about to prune. The following benchmark demonstrates this: it performs a pruning fetch from a repository with a single reference into a repository with 100k references, which causes us to prune all but one reference. This is of course a very artificial setup, but serves to demonstrate the impact of only having to write the packed-refs file once: Benchmark 1: git fetch --prune --atomic +refs/*:refs/* (HEAD~) Time (mean ± σ): 2.366 s ± 0.021 s [User: 0.858 s, System: 1.508 s] Range (min … max): 2.328 s … 2.407 s 10 runs Benchmark 2: git fetch --prune --atomic +refs/*:refs/* (HEAD) Time (mean ± σ): 1.369 s ± 0.017 s [User: 0.715 s, System: 0.641 s] Range (min … max): 1.346 s … 1.400 s 10 runs Summary 'git fetch --prune --atomic +refs/*:refs/* (HEAD)' ran 1.73 ± 0.03 times faster than 'git fetch --prune --atomic +refs/*:refs/* (HEAD~)' Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Feb 17, 2022 at 14:04 UTC 583bc419235cedc6a2ba12593f058a9f812b9594

2 files changed +24 -14

builtin/fetch.c

+22 -8

@@ -1338,11 +1338,14 @@ out:

1338 return ret;

1339 }

1340

1341 -static int prune_refs(struct refspec *rs, struct ref *ref_map,

1341 +static int prune_refs(struct refspec *rs,

1342 + struct ref_transaction *transaction,

1343 + struct ref *ref_map,

1344 const char *raw_url)

1345 {

1346 int url_len, i, result = 0;

1347 struct ref *ref, *stale_refs = get_stale_heads(rs, ref_map);

1348 + struct strbuf err = STRBUF_INIT;

1349 char *url;

1350 int summary_width = transport_summary_width(stale_refs);

1351 const char *dangling_msg = dry_run

@@ -1363,13 +1366,22 @@ static int prune_refs(struct refspec *rs, struct ref *ref_map,

1366 url_len = i - 3;

1367

1368 if (!dry_run) {

1366 - struct string_list refnames = STRING_LIST_INIT_NODUP;

1369 + if (transaction) {

1370 + for (ref = stale_refs; ref; ref = ref->next) {

1371 + result = ref_transaction_delete(transaction, ref->name, NULL, 0,

1372 + "fetch: prune", &err);

1373 + if (result)

1374 + goto cleanup;

1375 + }

1376 + } else {

1377 + struct string_list refnames = STRING_LIST_INIT_NODUP;

1378

1368 - for (ref = stale_refs; ref; ref = ref->next)

1369 - string_list_append(&refnames, ref->name);

1379 + for (ref = stale_refs; ref; ref = ref->next)

1380 + string_list_append(&refnames, ref->name);

1381

1371 - result = delete_refs("fetch: prune", &refnames, 0);

1372 - string_list_clear(&refnames, 0);

1382 + result = delete_refs("fetch: prune", &refnames, 0);

1383 + string_list_clear(&refnames, 0);

1384 + }

1385 }

1386

1387 if (verbosity >= 0) {

@@ -1388,6 +1400,8 @@ static int prune_refs(struct refspec *rs, struct ref *ref_map,

1400 }

1401 }

1402

1403 +cleanup:

1404 + strbuf_release(&err);

1405 free(url);

1406 free_refs(stale_refs);

1407 return result;

@@ -1629,10 +1643,10 @@ static int do_fetch(struct transport *transport,

1643 * don't care whether --tags was specified.

1644 */

1645 if (rs->nr) {

1632 - retcode = prune_refs(rs, ref_map, transport->url);

1646 + retcode = prune_refs(rs, transaction, ref_map, transport->url);

1647 } else {

1648 retcode = prune_refs(&transport->remote->fetch,

1635 - ref_map,

1649 + transaction, ref_map,

1650 transport->url);

1651 }

1652 if (retcode != 0)

t/t5510-fetch.sh

+2 -6

@@ -354,17 +354,13 @@ test_expect_success 'fetch --atomic --prune executes a single reference transact

354 head_oid=$(git rev-parse HEAD) &&

355

356 # Fetching with the `--atomic` flag should update all references in a

357 - # single transaction. It is currently missing coverage of pruned

358 - # references though, and as a result those may be committed to disk

359 - # even if updating references fails later.

357 + # single transaction.

358 cat >expected <<-EOF &&

359 prepared

360 $ZERO_OID $ZERO_OID refs/remotes/origin/scheduled-for-deletion

363 - committed

364 - $ZERO_OID $ZERO_OID refs/remotes/origin/scheduled-for-deletion

365 - prepared

361 $ZERO_OID $head_oid refs/remotes/origin/new-branch

362 committed

363 + $ZERO_OID $ZERO_OID refs/remotes/origin/scheduled-for-deletion

364 $ZERO_OID $head_oid refs/remotes/origin/new-branch

365 EOF

366