fetch: delay user information post committing of transaction
In Git 2.50 and earlier, we would display failure codes and error message as part of the status display: $ git fetch . v1.0.0:refs/heads/foo error: cannot update ref 'refs/heads/foo': trying to write non-commit object f665776185ad074b236c00751d666da7d1977dbe to branch 'refs/heads/foo' From . ! [new tag] v1.0.0 -> foo (unable to update local ref) With the addition of batched updates, this information is no longer shown to the user: $ git fetch . v1.0.0:refs/heads/foo From . * [new tag] v1.0.0 -> foo error: cannot update ref 'refs/heads/foo': trying to write non-commit object f665776185ad074b236c00751d666da7d1977dbe to branch 'refs/heads/foo' Since reference updates are batched and processed together at the end, information around the outcome is not available during individual reference parsing. To overcome this, collate and delay the output to the end. Introduce `ref_update_display_info` which will hold individual update's information and also whether the update failed or succeeded. This finally allows us to iterate over all such updates and print them to the user. Using an dynamic array and strmap does add some overhead to 'git-fetch(1)', but from benchmarking this seems to be not too bad: Benchmark 1: fetch: many refs (refformat = files, refcount = 1000, revision = master) Time (mean ± σ): 42.6 ms ± 1.2 ms [User: 13.1 ms, System: 29.8 ms] Range (min … max): 40.1 ms … 45.8 ms 47 runs Benchmark 2: fetch: many refs (refformat = files, refcount = 1000, revision = HEAD) Time (mean ± σ): 43.1 ms ± 1.2 ms [User: 12.7 ms, System: 30.7 ms] Range (min … max): 40.5 ms … 45.8 ms 48 runs Summary fetch: many refs (refformat = files, refcount = 1000, revision = master) ran 1.01 ± 0.04 times faster than fetch: many refs (refformat = files, refcount = 1000, revision = HEAD) Another approach would be to move the status printing logic to be handled post the transaction being committed. That however would require adding an iterator to the ref transaction that tracks both the outcome (success/failure) and the original refspec information for each update, which is more involved infrastructure work compared to the strmap approach here. Helped-by: Phillip Wood <phillip.wood123@gmail.com> Reported-by: Jeff King <peff@peff.net> Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Karthik Nayak committed
Jan 25, 2026 at 23:52 UTC
eff9299eacb9d88ded6efdc2a78024dc5fc20eea
2 files changed
+193
-54
builtin/fetch.c
+192
-54
@@ -861,12 +861,87 @@ static void display_ref_update(struct display_state *display_state, char code,
861
fputs(display_state->buf.buf, f);
862
}
863
864
+struct ref_update_display_info {
865
+ bool failed;
866
+ char success_code;
867
+ char fail_code;
868
+ char *summary;
869
+ char *fail_detail;
870
+ char *success_detail;
871
+ char *ref;
872
+ char *remote;
873
+ struct object_id old_oid;
874
+ struct object_id new_oid;
875
+};
876
+
877
+struct ref_update_display_info_array {
878
+ struct ref_update_display_info *info;
879
+ size_t alloc, nr;
880
+};
881
+
882
+static struct ref_update_display_info *ref_update_display_info_append(
883
+ struct ref_update_display_info_array *array,
884
+ char success_code,
885
+ char fail_code,
886
+ const char *summary,
887
+ const char *success_detail,
888
+ const char *fail_detail,
889
+ const char *ref,
890
+ const char *remote,
891
+ const struct object_id *old_oid,
892
+ const struct object_id *new_oid)
893
+{
894
+ struct ref_update_display_info *info;
895
+
896
+ ALLOC_GROW(array->info, array->nr + 1, array->alloc);
897
+ info = &array->info[array->nr++];
898
+
899
+ info->failed = false;
900
+ info->success_code = success_code;
901
+ info->fail_code = fail_code;
902
+ info->summary = xstrdup(summary);
903
+ info->success_detail = xstrdup_or_null(success_detail);
904
+ info->fail_detail = xstrdup_or_null(fail_detail);
905
+ info->remote = xstrdup(remote);
906
+ info->ref = xstrdup(ref);
907
+
908
+ oidcpy(&info->old_oid, old_oid);
909
+ oidcpy(&info->new_oid, new_oid);
910
+
911
+ return info;
912
+}
913
+
914
+static void ref_update_display_info_set_failed(struct ref_update_display_info *info)
915
+{
916
+ info->failed = true;
917
+}
918
+
919
+static void ref_update_display_info_free(struct ref_update_display_info *info)
920
+{
921
+ free(info->summary);
922
+ free(info->success_detail);
923
+ free(info->fail_detail);
924
+ free(info->remote);
925
+ free(info->ref);
926
+}
927
+
928
+static void ref_update_display_info_display(struct ref_update_display_info *info,
929
+ struct display_state *display_state,
930
+ int summary_width)
931
+{
932
+ display_ref_update(display_state,
933
+ info->failed ? info->fail_code : info->success_code,
934
+ info->summary,
935
+ info->failed ? info->fail_detail : info->success_detail,
936
+ info->remote, info->ref, &info->old_oid,
937
+ &info->new_oid, summary_width);
938
+}
939
+
940
static int update_local_ref(struct ref *ref,
941
struct ref_transaction *transaction,
866
- struct display_state *display_state,
942
const struct ref *remote_ref,
868
- int summary_width,
869
- const struct fetch_config *config)
943
+ const struct fetch_config *config,
944
+ struct ref_update_display_info_array *display_array)
945
{
946
struct commit *current = NULL, *updated;
947
int fast_forward = 0;
@@ -877,41 +952,56 @@ static int update_local_ref(struct ref *ref,
952
953
if (oideq(&ref->old_oid, &ref->new_oid)) {
954
if (verbosity > 0)
880
- display_ref_update(display_state, '=', _("[up to date]"), NULL,
881
- remote_ref->name, ref->name,
882
- &ref->old_oid, &ref->new_oid, summary_width);
955
+ ref_update_display_info_append(display_array, '=', '=',
956
+ _("[up to date]"), NULL,
957
+ NULL, ref->name,
958
+ remote_ref->name, &ref->old_oid,
959
+ &ref->new_oid);
960
return 0;
961
}
962
963
if (!update_head_ok &&
964
!is_null_oid(&ref->old_oid) &&
965
branch_checked_out(ref->name)) {
966
+ struct ref_update_display_info *info;
967
/*
968
* If this is the head, and it's not okay to update
969
* the head, and the old value of the head isn't empty...
970
*/
893
- display_ref_update(display_state, '!', _("[rejected]"),
894
- _("can't fetch into checked-out branch"),
895
- remote_ref->name, ref->name,
896
- &ref->old_oid, &ref->new_oid, summary_width);
971
+ info = ref_update_display_info_append(display_array, '!', '!',
972
+ _("[rejected]"), NULL,
973
+ _("can't fetch into checked-out branch"),
974
+ ref->name, remote_ref->name,
975
+ &ref->old_oid, &ref->new_oid);
976
+ ref_update_display_info_set_failed(info);
977
return 1;
978
}
979
980
if (!is_null_oid(&ref->old_oid) &&
981
starts_with(ref->name, "refs/tags/")) {
982
+ struct ref_update_display_info *info;
983
+
984
if (force || ref->force) {
985
int r;
986
+
987
r = s_update_ref("updating tag", ref, transaction, 0);
905
- display_ref_update(display_state, r ? '!' : 't', _("[tag update]"),
906
- r ? _("unable to update local ref") : NULL,
907
- remote_ref->name, ref->name,
908
- &ref->old_oid, &ref->new_oid, summary_width);
988
+
989
+ info = ref_update_display_info_append(display_array, 't', '!',
990
+ _("[tag update]"), NULL,
991
+ _("unable to update local ref"),
992
+ ref->name, remote_ref->name,
993
+ &ref->old_oid, &ref->new_oid);
994
+ if (r)
995
+ ref_update_display_info_set_failed(info);
996
+
997
return r;
998
} else {
911
- display_ref_update(display_state, '!', _("[rejected]"),
912
- _("would clobber existing tag"),
913
- remote_ref->name, ref->name,
914
- &ref->old_oid, &ref->new_oid, summary_width);
999
+ info = ref_update_display_info_append(display_array, '!', '!',
1000
+ _("[rejected]"), NULL,
1001
+ _("would clobber existing tag"),
1002
+ ref->name, remote_ref->name,
1003
+ &ref->old_oid, &ref->new_oid);
1004
+ ref_update_display_info_set_failed(info);
1005
return 1;
1006
}
1007
}
@@ -921,6 +1011,7 @@ static int update_local_ref(struct ref *ref,
1011
updated = lookup_commit_reference_gently(the_repository,
1012
&ref->new_oid, 1);
1013
if (!current || !updated) {
1014
+ struct ref_update_display_info *info;
1015
const char *msg;
1016
const char *what;
1017
int r;
@@ -941,10 +1032,15 @@ static int update_local_ref(struct ref *ref,
1032
}
1033
1034
r = s_update_ref(msg, ref, transaction, 0);
944
- display_ref_update(display_state, r ? '!' : '*', what,
945
- r ? _("unable to update local ref") : NULL,
946
- remote_ref->name, ref->name,
947
- &ref->old_oid, &ref->new_oid, summary_width);
1035
+
1036
+ info = ref_update_display_info_append(display_array, '*', '!',
1037
+ what, NULL,
1038
+ _("unable to update local ref"),
1039
+ ref->name, remote_ref->name,
1040
+ &ref->old_oid, &ref->new_oid);
1041
+ if (r)
1042
+ ref_update_display_info_set_failed(info);
1043
+
1044
return r;
1045
}
1046
@@ -960,6 +1056,7 @@ static int update_local_ref(struct ref *ref,
1056
}
1057
1058
if (fast_forward) {
1059
+ struct ref_update_display_info *info;
1060
struct strbuf quickref = STRBUF_INIT;
1061
int r;
1062
@@ -967,29 +1064,46 @@ static int update_local_ref(struct ref *ref,
1064
strbuf_addstr(&quickref, "..");
1065
strbuf_add_unique_abbrev(&quickref, &ref->new_oid, DEFAULT_ABBREV);
1066
r = s_update_ref("fast-forward", ref, transaction, 1);
970
- display_ref_update(display_state, r ? '!' : ' ', quickref.buf,
971
- r ? _("unable to update local ref") : NULL,
972
- remote_ref->name, ref->name,
973
- &ref->old_oid, &ref->new_oid, summary_width);
1067
+
1068
+ info = ref_update_display_info_append(display_array, ' ', '!',
1069
+ quickref.buf, NULL,
1070
+ _("unable to update local ref"),
1071
+ ref->name, remote_ref->name,
1072
+ &ref->old_oid, &ref->new_oid);
1073
+ if (r)
1074
+ ref_update_display_info_set_failed(info);
1075
+
1076
strbuf_release(&quickref);
1077
return r;
1078
} else if (force || ref->force) {
1079
+ struct ref_update_display_info *info;
1080
struct strbuf quickref = STRBUF_INIT;
1081
int r;
1082
+
1083
strbuf_add_unique_abbrev(&quickref, ¤t->object.oid, DEFAULT_ABBREV);
1084
strbuf_addstr(&quickref, "...");
1085
strbuf_add_unique_abbrev(&quickref, &ref->new_oid, DEFAULT_ABBREV);
1086
r = s_update_ref("forced-update", ref, transaction, 1);
983
- display_ref_update(display_state, r ? '!' : '+', quickref.buf,
984
- r ? _("unable to update local ref") : _("forced update"),
985
- remote_ref->name, ref->name,
986
- &ref->old_oid, &ref->new_oid, summary_width);
1087
+
1088
+ info = ref_update_display_info_append(display_array, '+', '!',
1089
+ quickref.buf, _("forced update"),
1090
+ _("unable to update local ref"),
1091
+ ref->name, remote_ref->name,
1092
+ &ref->old_oid, &ref->new_oid);
1093
+
1094
+ if (r)
1095
+ ref_update_display_info_set_failed(info);
1096
+
1097
strbuf_release(&quickref);
1098
return r;
1099
} else {
990
- display_ref_update(display_state, '!', _("[rejected]"), _("non-fast-forward"),
991
- remote_ref->name, ref->name,
992
- &ref->old_oid, &ref->new_oid, summary_width);
1100
+ struct ref_update_display_info *info;
1101
+ info = ref_update_display_info_append(display_array, '!', '!',
1102
+ _("[rejected]"), NULL,
1103
+ _("non-fast-forward"),
1104
+ ref->name, remote_ref->name,
1105
+ &ref->old_oid, &ref->new_oid);
1106
+ ref_update_display_info_set_failed(info);
1107
return 1;
1108
}
1109
}
@@ -1103,17 +1217,14 @@ static int store_updated_refs(struct display_state *display_state,
1217
int connectivity_checked,
1218
struct ref_transaction *transaction, struct ref *ref_map,
1219
struct fetch_head *fetch_head,
1106
- const struct fetch_config *config)
1220
+ const struct fetch_config *config,
1221
+ struct ref_update_display_info_array *display_array)
1222
{
1223
int rc = 0;
1224
struct strbuf note = STRBUF_INIT;
1225
const char *what, *kind;
1226
struct ref *rm;
1227
int want_status;
1113
- int summary_width = 0;
1114
-
1115
- if (verbosity >= 0)
1116
- summary_width = transport_summary_width(ref_map);
1228
1229
if (!connectivity_checked) {
1230
struct check_connected_options opt = CHECK_CONNECTED_INIT;
@@ -1218,8 +1329,8 @@ static int store_updated_refs(struct display_state *display_state,
1329
display_state->url_len);
1330
1331
if (ref) {
1221
- rc |= update_local_ref(ref, transaction, display_state,
1222
- rm, summary_width, config);
1332
+ rc |= update_local_ref(ref, transaction, rm,
1333
+ config, display_array);
1334
free(ref);
1335
} else if (write_fetch_head || dry_run) {
1336
/*
@@ -1227,12 +1338,12 @@ static int store_updated_refs(struct display_state *display_state,
1338
* would be written to FETCH_HEAD, if --dry-run
1339
* is set).
1340
*/
1230
- display_ref_update(display_state, '*',
1231
- *kind ? kind : "branch", NULL,
1232
- rm->name,
1233
- "FETCH_HEAD",
1234
- &rm->new_oid, &rm->old_oid,
1235
- summary_width);
1341
+
1342
+ ref_update_display_info_append(display_array, '*', '*',
1343
+ *kind ? kind : "branch",
1344
+ NULL, NULL, "FETCH_HEAD",
1345
+ rm->name, &rm->new_oid,
1346
+ &rm->old_oid);
1347
}
1348
}
1349
}
@@ -1300,7 +1411,8 @@ static int fetch_and_consume_refs(struct display_state *display_state,
1411
struct ref_transaction *transaction,
1412
struct ref *ref_map,
1413
struct fetch_head *fetch_head,
1303
- const struct fetch_config *config)
1414
+ const struct fetch_config *config,
1415
+ struct ref_update_display_info_array *display_array)
1416
{
1417
int connectivity_checked = 1;
1418
int ret;
@@ -1322,7 +1434,8 @@ static int fetch_and_consume_refs(struct display_state *display_state,
1434
1435
trace2_region_enter("fetch", "consume_refs", the_repository);
1436
ret = store_updated_refs(display_state, connectivity_checked,
1325
- transaction, ref_map, fetch_head, config);
1437
+ transaction, ref_map, fetch_head, config,
1438
+ display_array);
1439
trace2_region_leave("fetch", "consume_refs", the_repository);
1440
1441
out:
@@ -1493,7 +1606,8 @@ static int backfill_tags(struct display_state *display_state,
1606
struct ref_transaction *transaction,
1607
struct ref *ref_map,
1608
struct fetch_head *fetch_head,
1496
- const struct fetch_config *config)
1609
+ const struct fetch_config *config,
1610
+ struct ref_update_display_info_array *display_array)
1611
{
1612
int retcode, cannot_reuse;
1613
@@ -1515,7 +1629,7 @@ static int backfill_tags(struct display_state *display_state,
1629
transport_set_option(transport, TRANS_OPT_DEPTH, "0");
1630
transport_set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, NULL);
1631
retcode = fetch_and_consume_refs(display_state, transport, transaction, ref_map,
1518
- fetch_head, config);
1632
+ fetch_head, config, display_array);
1633
1634
if (gsecondary) {
1635
transport_disconnect(gsecondary);
@@ -1641,6 +1755,7 @@ struct ref_rejection_data {
1755
bool conflict_msg_shown;
1756
bool case_sensitive_msg_shown;
1757
const char *remote_name;
1758
+ struct strmap *rejected_refs;
1759
};
1760
1761
static void ref_transaction_rejection_handler(const char *refname,
@@ -1681,6 +1796,7 @@ static void ref_transaction_rejection_handler(const char *refname,
1796
refname, ref_transaction_error_msg(err));
1797
}
1798
1799
+ strmap_put(data->rejected_refs, refname, NULL);
1800
*data->retcode = 1;
1801
}
1802
@@ -1690,6 +1806,7 @@ static void ref_transaction_rejection_handler(const char *refname,
1806
*/
1807
static int commit_ref_transaction(struct ref_transaction **transaction,
1808
bool is_atomic, const char *remote_name,
1809
+ struct strmap *rejected_refs,
1810
struct strbuf *err)
1811
{
1812
int retcode = ref_transaction_commit(*transaction, err);
@@ -1701,6 +1818,7 @@ static int commit_ref_transaction(struct ref_transaction **transaction,
1818
.conflict_msg_shown = 0,
1819
.remote_name = remote_name,
1820
.retcode = &retcode,
1821
+ .rejected_refs = rejected_refs,
1822
};
1823
1824
ref_transaction_for_each_rejected_update(*transaction,
@@ -1729,6 +1847,9 @@ static int do_fetch(struct transport *transport,
1847
struct fetch_head fetch_head = { 0 };
1848
struct strbuf err = STRBUF_INIT;
1849
int do_set_head = 0;
1850
+ struct ref_update_display_info_array display_array = { 0 };
1851
+ struct strmap rejected_refs = STRMAP_INIT;
1852
+ int summary_width = 0;
1853
1854
if (tags == TAGS_DEFAULT) {
1855
if (transport->remote->fetch_tags == 2)
@@ -1853,7 +1974,7 @@ static int do_fetch(struct transport *transport,
1974
}
1975
1976
if (fetch_and_consume_refs(&display_state, transport, transaction, ref_map,
1856
- &fetch_head, config)) {
1977
+ &fetch_head, config, &display_array)) {
1978
retcode = 1;
1979
goto cleanup;
1980
}
@@ -1876,7 +1997,7 @@ static int do_fetch(struct transport *transport,
1997
* the transaction and don't commit anything.
1998
*/
1999
if (backfill_tags(&display_state, transport, transaction, tags_ref_map,
1879
- &fetch_head, config))
2000
+ &fetch_head, config, &display_array))
2001
retcode = 1;
2002
}
2003
@@ -1886,8 +2007,12 @@ static int do_fetch(struct transport *transport,
2007
if (retcode)
2008
goto cleanup;
2009
2010
+ if (verbosity >= 0)
2011
+ summary_width = transport_summary_width(ref_map);
2012
+
2013
retcode = commit_ref_transaction(&transaction, atomic_fetch,
1890
- transport->remote->name, &err);
2014
+ transport->remote->name,
2015
+ &rejected_refs, &err);
2016
/*
2017
* With '--atomic', bail out if the transaction fails. Without '--atomic',
2018
* continue to fetch head and perform other post-fetch operations.
@@ -1965,7 +2090,17 @@ cleanup:
2090
*/
2091
if (retcode && !atomic_fetch && transaction)
2092
commit_ref_transaction(&transaction, false,
1968
- transport->remote->name, &err);
2093
+ transport->remote->name,
2094
+ &rejected_refs, &err);
2095
+
2096
+ for (size_t i = 0; i < display_array.nr; i++) {
2097
+ struct ref_update_display_info *info = &display_array.info[i];
2098
+
2099
+ if (!info->failed && strmap_contains(&rejected_refs, info->ref))
2100
+ ref_update_display_info_set_failed(info);
2101
+ ref_update_display_info_display(info, &display_state, summary_width);
2102
+ ref_update_display_info_free(info);
2103
+ }
2104
2105
if (retcode) {
2106
if (err.len) {
@@ -1980,6 +2115,9 @@ cleanup:
2115
2116
if (transaction)
2117
ref_transaction_free(transaction);
2118
+
2119
+ free(display_array.info);
2120
+ strmap_clear(&rejected_refs, 0);
2121
display_state_release(&display_state);
2122
close_fetch_head(&fetch_head);
2123
strbuf_release(&err);
t/t5516-fetch-push.sh
+1
@@ -1893,6 +1893,7 @@ test_expect_success 'pushing non-commit objects should report error' '
1893
1894
tagsha=$(git rev-parse test^{tag}) &&
1895
test_must_fail git push ../dest "$tagsha:refs/heads/branch" 2>err &&
1896
+ test_grep "! \[remote rejected\] $tagsha -> branch (invalid new value provided)" err &&
1897
test_grep "trying to write non-commit object $tagsha to branch ${SQ}refs/heads/branch${SQ}" err
1898
)
1899
'