Skip to content

Commit db40094

Browse files
committed
Merge branch 'jk/fetch-always-update-tracking'
"git fetch origin master" unlike "git fetch origin" or "git fetch" did not update "refs/remotes/origin/master"; this was an early design decision to keep the update of remote tracking branches predictable, but in practice it turns out that people find it more convenient to opportunisticly update them whenever we have a chance, and we have been updating them when we run "git push" which already breaks the original "predictability" anyway. Now such a fetch does update refs/remotes/origin/master. * jk/fetch-always-update-tracking: fetch: don't try to update unfetched tracking refs fetch: opportunistically update tracking refs refactor "ref->merge" flag fetch/pull doc: untangle meaning of bare <ref> t5510: start tracking-ref tests from a known state
2 parents 67b57a9 + 823c6d5 commit db40094

File tree

4 files changed

+102
-30
lines changed

4 files changed

+102
-30
lines changed

Documentation/pull-fetch-param.txt

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,11 @@ Some short-cut notations are also supported.
6868
+
6969
* `tag <tag>` means the same as `refs/tags/<tag>:refs/tags/<tag>`;
7070
it requests fetching everything up to the given tag.
71-
* A parameter <ref> without a colon is equivalent to
72-
<ref>: when pulling/fetching, so it merges <ref> into the current
73-
branch without storing the remote branch anywhere locally
71+
ifndef::git-pull[]
72+
* A parameter <ref> without a colon fetches that ref into FETCH_HEAD,
73+
endif::git-pull[]
74+
ifdef::git-pull[]
75+
* A parameter <ref> without a colon merges <ref> into the current
76+
branch,
77+
endif::git-pull[]
78+
and updates the remote-tracking branches (if any).

builtin/fetch.c

Lines changed: 51 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ static void add_merge_config(struct ref **head,
119119

120120
for (rm = *head; rm; rm = rm->next) {
121121
if (branch_merge_matches(branch, i, rm->name)) {
122-
rm->merge = 1;
122+
rm->fetch_head_status = FETCH_HEAD_MERGE;
123123
break;
124124
}
125125
}
@@ -140,7 +140,7 @@ static void add_merge_config(struct ref **head,
140140
refspec.src = branch->merge[i]->src;
141141
get_fetch_map(remote_refs, &refspec, tail, 1);
142142
for (rm = *old_tail; rm; rm = rm->next)
143-
rm->merge = 1;
143+
rm->fetch_head_status = FETCH_HEAD_MERGE;
144144
}
145145
}
146146

@@ -160,16 +160,32 @@ static struct ref *get_ref_map(struct transport *transport,
160160
const struct ref *remote_refs = transport_get_remote_refs(transport);
161161

162162
if (ref_count || tags == TAGS_SET) {
163+
struct ref **old_tail;
164+
163165
for (i = 0; i < ref_count; i++) {
164166
get_fetch_map(remote_refs, &refs[i], &tail, 0);
165167
if (refs[i].dst && refs[i].dst[0])
166168
*autotags = 1;
167169
}
168170
/* Merge everything on the command line, but not --tags */
169171
for (rm = ref_map; rm; rm = rm->next)
170-
rm->merge = 1;
172+
rm->fetch_head_status = FETCH_HEAD_MERGE;
171173
if (tags == TAGS_SET)
172174
get_fetch_map(remote_refs, tag_refspec, &tail, 0);
175+
176+
/*
177+
* For any refs that we happen to be fetching via command-line
178+
* arguments, take the opportunity to update their configured
179+
* counterparts. However, we do not want to mention these
180+
* entries in FETCH_HEAD at all, as they would simply be
181+
* duplicates of existing entries.
182+
*/
183+
old_tail = tail;
184+
for (i = 0; i < transport->remote->fetch_refspec_nr; i++)
185+
get_fetch_map(ref_map, &transport->remote->fetch[i],
186+
&tail, 1);
187+
for (rm = *old_tail; rm; rm = rm->next)
188+
rm->fetch_head_status = FETCH_HEAD_IGNORE;
173189
} else {
174190
/* Use the defaults */
175191
struct remote *remote = transport->remote;
@@ -186,7 +202,7 @@ static struct ref *get_ref_map(struct transport *transport,
186202
*autotags = 1;
187203
if (!i && !has_merge && ref_map &&
188204
!remote->fetch[0].pattern)
189-
ref_map->merge = 1;
205+
ref_map->fetch_head_status = FETCH_HEAD_MERGE;
190206
}
191207
/*
192208
* if the remote we're fetching from is the same
@@ -202,7 +218,7 @@ static struct ref *get_ref_map(struct transport *transport,
202218
ref_map = get_remote_ref(remote_refs, "HEAD");
203219
if (!ref_map)
204220
die(_("Couldn't find remote ref HEAD"));
205-
ref_map->merge = 1;
221+
ref_map->fetch_head_status = FETCH_HEAD_MERGE;
206222
tail = &ref_map->next;
207223
}
208224
}
@@ -389,7 +405,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
389405
const char *what, *kind;
390406
struct ref *rm;
391407
char *url, *filename = dry_run ? "/dev/null" : git_path("FETCH_HEAD");
392-
int want_merge;
408+
int want_status;
393409

394410
fp = fopen(filename, "a");
395411
if (!fp)
@@ -407,19 +423,22 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
407423
}
408424

409425
/*
410-
* The first pass writes objects to be merged and then the
411-
* second pass writes the rest, in order to allow using
412-
* FETCH_HEAD as a refname to refer to the ref to be merged.
426+
* We do a pass for each fetch_head_status type in their enum order, so
427+
* merged entries are written before not-for-merge. That lets readers
428+
* use FETCH_HEAD as a refname to refer to the ref to be merged.
413429
*/
414-
for (want_merge = 1; 0 <= want_merge; want_merge--) {
430+
for (want_status = FETCH_HEAD_MERGE;
431+
want_status <= FETCH_HEAD_IGNORE;
432+
want_status++) {
415433
for (rm = ref_map; rm; rm = rm->next) {
416434
struct ref *ref = NULL;
435+
const char *merge_status_marker = "";
417436

418437
commit = lookup_commit_reference_gently(rm->old_sha1, 1);
419438
if (!commit)
420-
rm->merge = 0;
439+
rm->fetch_head_status = FETCH_HEAD_NOT_FOR_MERGE;
421440

422-
if (rm->merge != want_merge)
441+
if (rm->fetch_head_status != want_status)
423442
continue;
424443

425444
if (rm->peer_ref) {
@@ -465,16 +484,26 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
465484
strbuf_addf(&note, "%s ", kind);
466485
strbuf_addf(&note, "'%s' of ", what);
467486
}
468-
fprintf(fp, "%s\t%s\t%s",
469-
sha1_to_hex(rm->old_sha1),
470-
rm->merge ? "" : "not-for-merge",
471-
note.buf);
472-
for (i = 0; i < url_len; ++i)
473-
if ('\n' == url[i])
474-
fputs("\\n", fp);
475-
else
476-
fputc(url[i], fp);
477-
fputc('\n', fp);
487+
switch (rm->fetch_head_status) {
488+
case FETCH_HEAD_NOT_FOR_MERGE:
489+
merge_status_marker = "not-for-merge";
490+
/* fall-through */
491+
case FETCH_HEAD_MERGE:
492+
fprintf(fp, "%s\t%s\t%s",
493+
sha1_to_hex(rm->old_sha1),
494+
merge_status_marker,
495+
note.buf);
496+
for (i = 0; i < url_len; ++i)
497+
if ('\n' == url[i])
498+
fputs("\\n", fp);
499+
else
500+
fputc(url[i], fp);
501+
fputc('\n', fp);
502+
break;
503+
default:
504+
/* do not write anything to FETCH_HEAD */
505+
break;
506+
}
478507

479508
strbuf_reset(&note);
480509
if (ref) {

cache.h

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1025,9 +1025,21 @@ struct ref {
10251025
unsigned int
10261026
force:1,
10271027
forced_update:1,
1028-
merge:1,
10291028
deletion:1,
10301029
matched:1;
1030+
1031+
/*
1032+
* Order is important here, as we write to FETCH_HEAD
1033+
* in numeric order. And the default NOT_FOR_MERGE
1034+
* should be 0, so that xcalloc'd structures get it
1035+
* by default.
1036+
*/
1037+
enum {
1038+
FETCH_HEAD_MERGE = -1,
1039+
FETCH_HEAD_NOT_FOR_MERGE = 0,
1040+
FETCH_HEAD_IGNORE = 1
1041+
} fetch_head_status;
1042+
10311043
enum {
10321044
REF_STATUS_NONE = 0,
10331045
REF_STATUS_OK,

t/t5510-fetch.sh

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -370,30 +370,39 @@ test_expect_success 'bundle should record HEAD correctly' '
370370
371371
'
372372

373-
test_expect_success 'explicit fetch should not update tracking' '
373+
test_expect_success 'mark initial state of origin/master' '
374+
(
375+
cd three &&
376+
git tag base-origin-master refs/remotes/origin/master
377+
)
378+
'
379+
380+
test_expect_success 'explicit fetch should update tracking' '
374381
375382
cd "$D" &&
376383
git branch -f side &&
377384
(
378385
cd three &&
386+
git update-ref refs/remotes/origin/master base-origin-master &&
379387
o=$(git rev-parse --verify refs/remotes/origin/master) &&
380388
git fetch origin master &&
381389
n=$(git rev-parse --verify refs/remotes/origin/master) &&
382-
test "$o" = "$n" &&
390+
test "$o" != "$n" &&
383391
test_must_fail git rev-parse --verify refs/remotes/origin/side
384392
)
385393
'
386394

387-
test_expect_success 'explicit pull should not update tracking' '
395+
test_expect_success 'explicit pull should update tracking' '
388396
389397
cd "$D" &&
390398
git branch -f side &&
391399
(
392400
cd three &&
401+
git update-ref refs/remotes/origin/master base-origin-master &&
393402
o=$(git rev-parse --verify refs/remotes/origin/master) &&
394403
git pull origin master &&
395404
n=$(git rev-parse --verify refs/remotes/origin/master) &&
396-
test "$o" = "$n" &&
405+
test "$o" != "$n" &&
397406
test_must_fail git rev-parse --verify refs/remotes/origin/side
398407
)
399408
'
@@ -404,6 +413,7 @@ test_expect_success 'configured fetch updates tracking' '
404413
git branch -f side &&
405414
(
406415
cd three &&
416+
git update-ref refs/remotes/origin/master base-origin-master &&
407417
o=$(git rev-parse --verify refs/remotes/origin/master) &&
408418
git fetch origin &&
409419
n=$(git rev-parse --verify refs/remotes/origin/master) &&
@@ -412,6 +422,22 @@ test_expect_success 'configured fetch updates tracking' '
412422
)
413423
'
414424

425+
test_expect_success 'non-matching refspecs do not confuse tracking update' '
426+
cd "$D" &&
427+
git update-ref refs/odd/location HEAD &&
428+
(
429+
cd three &&
430+
git update-ref refs/remotes/origin/master base-origin-master &&
431+
git config --add remote.origin.fetch \
432+
refs/odd/location:refs/remotes/origin/odd &&
433+
o=$(git rev-parse --verify refs/remotes/origin/master) &&
434+
git fetch origin master &&
435+
n=$(git rev-parse --verify refs/remotes/origin/master) &&
436+
test "$o" != "$n" &&
437+
test_must_fail git rev-parse --verify refs/remotes/origin/odd
438+
)
439+
'
440+
415441
test_expect_success 'pushing nonexistent branch by mistake should not segv' '
416442
417443
cd "$D" &&

0 commit comments

Comments
 (0)