Skip to content

Commit 1c31764

Browse files
pks-tgitster
pks-t
authored and
gitster
committed
fetch: print left-hand side when fetching HEAD:foo
`store_updated_refs()` parses the remote reference for two purposes: - It gets used as a note when writing FETCH_HEAD. - It is passed through to `display_ref_update()` to display updated references in the following format: ``` * branch master -> master ``` In most cases, the parsed remote reference is the prettified reference name and can thus be used for both cases. But if the remote reference is HEAD, the parsed remote reference becomes empty. This is intended when we write the FETCH_HEAD, where we skip writing the note in that case. But when displaying the updated references this leads to inconsistent output where the left-hand side of reference updates is missing in some cases: ``` $ git fetch origin HEAD HEAD:explicit-head :implicit-head main From https://github.com/git/git * branch HEAD -> FETCH_HEAD * [new ref] -> explicit-head * [new ref] -> implicit-head * branch main -> FETCH_HEAD ``` This behaviour has existed ever since the table-based output has been introduced for git-fetch(1) via 165f390 (git-fetch: more terse fetch output, 2007-11-03) and was never explicitly documented either in the commit message or in any of our tests. So while it may not be a bug per se, it feels like a weird inconsistency and not like it was a concious design decision. The logic of how we compute the remote reference name that we ultimately pass to `display_ref_update()` is not easy to follow. There are three different cases here: - When the remote reference name is "HEAD" we set the remote reference name to the empty string. This is the case that causes the left-hand side to go missing, where we would indeed want to print "HEAD" instead of the empty string. This is what `prettify_refname()` would return. - When the remote reference name has a well-known prefix then we strip this prefix. This matches what `prettify_refname()` does. - Otherwise, we keep the fully qualified reference name. This also matches what `prettify_refname()` does. As the return value of `prettify_refname()` would do the correct thing for us in all three cases, we can thus fix the inconsistency by passing through the full remote reference name to `display_ref_update()`, which learns to call `prettify_refname()`. At the same time, this also simplifies the code a bit. Note that this patch also changes formatting of the block that computes the "kind" (which is the category like "branch" or "tag") and "what" (which is the prettified reference name like "master" or "v1.0") variables. This is done on purpose so that it is part of the diff, hopefully making the change easier to comprehend. Signed-off-by: Patrick Steinhardt <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 3daf655 commit 1c31764

File tree

2 files changed

+66
-18
lines changed

2 files changed

+66
-18
lines changed

‎builtin/fetch.c

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -920,12 +920,14 @@ static void display_ref_update(struct display_state *display_state, char code,
920920
}
921921

922922
width = (summary_width + strlen(summary) - gettext_width(summary));
923+
remote = prettify_refname(remote);
924+
local = prettify_refname(local);
923925

924926
strbuf_addf(&display_state->buf, " %c %-*s ", code, width, summary);
925927
if (!display_state->compact_format)
926-
print_remote_to_local(display_state, remote, prettify_refname(local));
928+
print_remote_to_local(display_state, remote, local);
927929
else
928-
print_compact(display_state, remote, prettify_refname(local));
930+
print_compact(display_state, remote, local);
929931
if (error)
930932
strbuf_addf(&display_state->buf, " (%s)", error);
931933
strbuf_addch(&display_state->buf, '\n');
@@ -936,7 +938,7 @@ static void display_ref_update(struct display_state *display_state, char code,
936938
static int update_local_ref(struct ref *ref,
937939
struct ref_transaction *transaction,
938940
struct display_state *display_state,
939-
const char *remote, const struct ref *remote_ref,
941+
const struct ref *remote_ref,
940942
int summary_width)
941943
{
942944
struct commit *current = NULL, *updated;
@@ -948,7 +950,7 @@ static int update_local_ref(struct ref *ref,
948950
if (oideq(&ref->old_oid, &ref->new_oid)) {
949951
if (verbosity > 0)
950952
display_ref_update(display_state, '=', _("[up to date]"), NULL,
951-
remote, ref->name, summary_width);
953+
remote_ref->name, ref->name, summary_width);
952954
return 0;
953955
}
954956

@@ -961,7 +963,7 @@ static int update_local_ref(struct ref *ref,
961963
*/
962964
display_ref_update(display_state, '!', _("[rejected]"),
963965
_("can't fetch into checked-out branch"),
964-
remote, ref->name, summary_width);
966+
remote_ref->name, ref->name, summary_width);
965967
return 1;
966968
}
967969

@@ -972,12 +974,12 @@ static int update_local_ref(struct ref *ref,
972974
r = s_update_ref("updating tag", ref, transaction, 0);
973975
display_ref_update(display_state, r ? '!' : 't', _("[tag update]"),
974976
r ? _("unable to update local ref") : NULL,
975-
remote, ref->name, summary_width);
977+
remote_ref->name, ref->name, summary_width);
976978
return r;
977979
} else {
978980
display_ref_update(display_state, '!', _("[rejected]"),
979981
_("would clobber existing tag"),
980-
remote, ref->name, summary_width);
982+
remote_ref->name, ref->name, summary_width);
981983
return 1;
982984
}
983985
}
@@ -1010,7 +1012,7 @@ static int update_local_ref(struct ref *ref,
10101012
r = s_update_ref(msg, ref, transaction, 0);
10111013
display_ref_update(display_state, r ? '!' : '*', what,
10121014
r ? _("unable to update local ref") : NULL,
1013-
remote, ref->name, summary_width);
1015+
remote_ref->name, ref->name, summary_width);
10141016
return r;
10151017
}
10161018

@@ -1033,7 +1035,7 @@ static int update_local_ref(struct ref *ref,
10331035
r = s_update_ref("fast-forward", ref, transaction, 1);
10341036
display_ref_update(display_state, r ? '!' : ' ', quickref.buf,
10351037
r ? _("unable to update local ref") : NULL,
1036-
remote, ref->name, summary_width);
1038+
remote_ref->name, ref->name, summary_width);
10371039
strbuf_release(&quickref);
10381040
return r;
10391041
} else if (force || ref->force) {
@@ -1045,12 +1047,12 @@ static int update_local_ref(struct ref *ref,
10451047
r = s_update_ref("forced-update", ref, transaction, 1);
10461048
display_ref_update(display_state, r ? '!' : '+', quickref.buf,
10471049
r ? _("unable to update local ref") : _("forced update"),
1048-
remote, ref->name, summary_width);
1050+
remote_ref->name, ref->name, summary_width);
10491051
strbuf_release(&quickref);
10501052
return r;
10511053
} else {
10521054
display_ref_update(display_state, '!', _("[rejected]"), _("non-fast-forward"),
1053-
remote, ref->name, summary_width);
1055+
remote_ref->name, ref->name, summary_width);
10541056
return 1;
10551057
}
10561058
}
@@ -1255,14 +1257,13 @@ static int store_updated_refs(struct display_state *display_state,
12551257
if (!strcmp(rm->name, "HEAD")) {
12561258
kind = "";
12571259
what = "";
1258-
}
1259-
else if (skip_prefix(rm->name, "refs/heads/", &what))
1260+
} else if (skip_prefix(rm->name, "refs/heads/", &what)) {
12601261
kind = "branch";
1261-
else if (skip_prefix(rm->name, "refs/tags/", &what))
1262+
} else if (skip_prefix(rm->name, "refs/tags/", &what)) {
12621263
kind = "tag";
1263-
else if (skip_prefix(rm->name, "refs/remotes/", &what))
1264+
} else if (skip_prefix(rm->name, "refs/remotes/", &what)) {
12641265
kind = "remote-tracking branch";
1265-
else {
1266+
} else {
12661267
kind = "";
12671268
what = rm->name;
12681269
}
@@ -1280,7 +1281,7 @@ static int store_updated_refs(struct display_state *display_state,
12801281
display_state->url_len);
12811282

12821283
if (ref) {
1283-
rc |= update_local_ref(ref, transaction, display_state, what,
1284+
rc |= update_local_ref(ref, transaction, display_state,
12841285
rm, summary_width);
12851286
free(ref);
12861287
} else if (write_fetch_head || dry_run) {
@@ -1291,7 +1292,7 @@ static int store_updated_refs(struct display_state *display_state,
12911292
*/
12921293
display_ref_update(display_state, '*',
12931294
*kind ? kind : "branch", NULL,
1294-
*what ? what : "HEAD",
1295+
rm->name,
12951296
"FETCH_HEAD", summary_width);
12961297
}
12971298
}

‎t/t5574-fetch-output.sh

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,53 @@ test_expect_success 'fetch compact output' '
6161
test_cmp expect actual
6262
'
6363

64+
test_expect_success 'fetch output with HEAD' '
65+
test_when_finished "rm -rf head" &&
66+
git clone . head &&
67+
68+
git -C head fetch --dry-run origin HEAD >actual.out 2>actual.err &&
69+
cat >expect <<-EOF &&
70+
From $(test-tool path-utils real_path .)/.
71+
* branch HEAD -> FETCH_HEAD
72+
EOF
73+
test_must_be_empty actual.out &&
74+
test_cmp expect actual.err &&
75+
76+
git -C head fetch origin HEAD >actual.out 2>actual.err &&
77+
test_must_be_empty actual.out &&
78+
test_cmp expect actual.err &&
79+
80+
git -C head fetch --dry-run origin HEAD:foo >actual.out 2>actual.err &&
81+
cat >expect <<-EOF &&
82+
From $(test-tool path-utils real_path .)/.
83+
* [new ref] HEAD -> foo
84+
EOF
85+
test_must_be_empty actual.out &&
86+
test_cmp expect actual.err &&
87+
88+
git -C head fetch origin HEAD:foo >actual.out 2>actual.err &&
89+
test_must_be_empty actual.out &&
90+
test_cmp expect actual.err
91+
'
92+
93+
test_expect_success 'fetch output with object ID' '
94+
test_when_finished "rm -rf object-id" &&
95+
git clone . object-id &&
96+
commit=$(git rev-parse HEAD) &&
97+
98+
git -C object-id fetch --dry-run origin $commit:object-id >actual.out 2>actual.err &&
99+
cat >expect <<-EOF &&
100+
From $(test-tool path-utils real_path .)/.
101+
* [new ref] $commit -> object-id
102+
EOF
103+
test_must_be_empty actual.out &&
104+
test_cmp expect actual.err &&
105+
106+
git -C object-id fetch origin $commit:object-id >actual.out 2>actual.err &&
107+
test_must_be_empty actual.out &&
108+
test_cmp expect actual.err
109+
'
110+
64111
test_expect_success '--no-show-forced-updates' '
65112
mkdir forced-updates &&
66113
(

0 commit comments

Comments
 (0)