From: Junio C Hamano Date: Mon, 17 Sep 2018 20:54:00 +0000 (-0700) Subject: Merge branch 'ab/fetch-tags-noclobber' X-Git-Tag: v2.20.0-rc0~229 X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/d39cab3989f9e660cae124f78143369b13ad2901?ds=inline;hp=-c Merge branch 'ab/fetch-tags-noclobber' The rules used by "git push" and "git fetch" to determine if a ref can or cannot be updated were inconsistent; specifically, fetching to update existing tags were allowed even though tags are supposed to be unmoving anchoring points. "git fetch" was taught to forbid updates to existing tags without the "--force" option. * ab/fetch-tags-noclobber: fetch: stop clobbering existing tags without --force fetch: document local ref updates with/without --force push doc: correct lies about how push refspecs work push doc: move mention of "tag " later in the prose push doc: remove confusing mention of remote merger fetch tests: add a test for clobbering tag behavior push tests: use spaces in interpolated string push tests: make use of unused $1 in test description fetch: change "branch" to "reference" in --force -h output --- d39cab3989f9e660cae124f78143369b13ad2901 diff --combined Documentation/fetch-options.txt index 8bc36af4b1,5b624caf58..fa0a3151b3 --- a/Documentation/fetch-options.txt +++ b/Documentation/fetch-options.txt @@@ -42,25 -42,6 +42,25 @@@ the current repository has the same his .git/shallow. This option updates .git/shallow and accept such refs. +--negotiation-tip=:: + By default, Git will report, to the server, commits reachable + from all local refs to find common commits in an attempt to + reduce the size of the to-be-received packfile. If specified, + Git will only report commits reachable from the given tips. + This is useful to speed up fetches when the user knows which + local ref is likely to have commits in common with the + upstream ref being fetched. ++ +This option may be specified more than once; if so, Git will report +commits reachable from any of the given commits. ++ +The argument to this option may be a glob on ref names, a ref, or the (possibly +abbreviated) SHA-1 of a commit. Specifying a glob is equivalent to specifying +this option multiple times, one for each matching ref name. ++ +See also the `fetch.negotiationAlgorithm` configuration variable +documented in linkgit:git-config[1]. + ifndef::git-pull[] --dry-run:: Show what would be done, without making any changes. @@@ -68,11 -49,16 +68,16 @@@ endif::git-pull[ -f:: --force:: - When 'git fetch' is used with `:` - refspec, it refuses to update the local branch - `` unless the remote branch `` it - fetches is a descendant of ``. This option - overrides that check. + When 'git fetch' is used with `:` refspec it may + refuse to update the local branch as discussed + ifdef::git-pull[] + in the `` part of the linkgit:git-fetch[1] + documentation. + endif::git-pull[] + ifndef::git-pull[] + in the `` part below. + endif::git-pull[] + This option overrides that check. -k:: --keep:: diff --combined builtin/fetch.c index dc0931fb46,683f70d71e..0696abfc2a --- a/builtin/fetch.c +++ b/builtin/fetch.c @@@ -22,7 -22,6 +22,7 @@@ #include "utf8.h" #include "packfile.h" #include "list-objects-filter-options.h" +#include "commit-reach.h" static const char * const builtin_fetch_usage[] = { N_("git fetch [] [ [...]]"), @@@ -65,7 -64,6 +65,7 @@@ static int shown_url = 0 static struct refspec refmap = REFSPEC_INIT_FETCH; static struct list_objects_filter_options filter_options; static struct string_list server_options = STRING_LIST_INIT_DUP; +static struct string_list negotiation_tip = STRING_LIST_INIT_NODUP; static int git_fetch_config(const char *k, const char *v, void *cb) { @@@ -115,7 -113,7 +115,7 @@@ static struct option builtin_fetch_opti N_("append to .git/FETCH_HEAD instead of overwriting")), OPT_STRING(0, "upload-pack", &upload_pack, N_("path"), N_("path to upload pack on remote end")), - OPT__FORCE(&force, N_("force overwrite of local branch"), 0), + OPT__FORCE(&force, N_("force overwrite of local reference"), 0), OPT_BOOL('m', "multiple", &multiple, N_("fetch from multiple remotes")), OPT_SET_INT('t', "tags", &tags, @@@ -164,8 -162,6 +164,8 @@@ TRANSPORT_FAMILY_IPV4), OPT_SET_INT('6', "ipv6", &family, N_("use IPv6 addresses only"), TRANSPORT_FAMILY_IPV6), + OPT_STRING_LIST(0, "negotiation-tip", &negotiation_tip, N_("revision"), + N_("report that we have only objects reachable from this object")), OPT_PARSE_LIST_OBJECTS_FILTER(&filter_options), OPT_END() }; @@@ -239,7 -235,7 +239,7 @@@ static int will_fetch(struct ref **head { struct ref *rm = *head; while (rm) { - if (!hashcmp(rm->old_oid.hash, sha1)) + if (hasheq(rm->old_oid.hash, sha1)) return 1; rm = rm->next; } @@@ -508,7 -504,7 +508,7 @@@ static void adjust_refcol_width(const s int max, rlen, llen, len; /* uptodate lines are only shown on high verbosity level */ - if (!verbosity && !oidcmp(&ref->peer_ref->old_oid, &ref->old_oid)) + if (!verbosity && oideq(&ref->peer_ref->old_oid, &ref->old_oid)) return; max = term_columns(); @@@ -645,7 -641,7 +645,7 @@@ static int update_local_ref(struct ref if (type < 0) die(_("object %s not found"), oid_to_hex(&ref->new_oid)); - if (!oidcmp(&ref->old_oid, &ref->new_oid)) { + if (oideq(&ref->old_oid, &ref->new_oid)) { if (verbosity > 0) format_display(display, '=', _("[up to date]"), NULL, remote, pretty_ref, summary_width); @@@ -668,18 -664,22 +668,24 @@@ if (!is_null_oid(&ref->old_oid) && starts_with(ref->name, "refs/tags/")) { - int r; - r = s_update_ref("updating tag", ref, 0); - format_display(display, r ? '!' : 't', _("[tag update]"), - r ? _("unable to update local ref") : NULL, - remote, pretty_ref, summary_width); - return r; + if (force || ref->force) { + int r; + r = s_update_ref("updating tag", ref, 0); + format_display(display, r ? '!' : 't', _("[tag update]"), + r ? _("unable to update local ref") : NULL, + remote, pretty_ref, summary_width); + return r; + } else { + format_display(display, '!', _("[rejected]"), _("would clobber existing tag"), + remote, pretty_ref, summary_width); + return 1; + } } - current = lookup_commit_reference_gently(&ref->old_oid, 1); - updated = lookup_commit_reference_gently(&ref->new_oid, 1); + current = lookup_commit_reference_gently(the_repository, + &ref->old_oid, 1); + updated = lookup_commit_reference_gently(the_repository, + &ref->new_oid, 1); if (!current || !updated) { const char *msg; const char *what; @@@ -814,8 -814,7 +820,8 @@@ static int store_updated_refs(const cha continue; } - commit = lookup_commit_reference_gently(&rm->old_oid, + commit = lookup_commit_reference_gently(the_repository, + &rm->old_oid, 1); if (!commit) rm->fetch_head_status = FETCH_HEAD_NOT_FOR_MERGE; @@@ -943,11 -942,13 +949,11 @@@ static int quickfetch(struct ref *ref_m return check_connected(iterate_ref_map, &rm, &opt); } -static int fetch_refs(struct transport *transport, struct ref *ref_map, - struct ref **updated_remote_refs) +static int fetch_refs(struct transport *transport, struct ref *ref_map) { int ret = quickfetch(ref_map); if (ret) - ret = transport_fetch_refs(transport, ref_map, - updated_remote_refs); + ret = transport_fetch_refs(transport, ref_map); if (!ret) /* * Keep the new pack's ".keep" file around to allow the caller @@@ -1062,40 -1063,6 +1068,40 @@@ static void set_option(struct transpor name, transport->url); } + +static int add_oid(const char *refname, const struct object_id *oid, int flags, + void *cb_data) +{ + struct oid_array *oids = cb_data; + + oid_array_append(oids, oid); + return 0; +} + +static void add_negotiation_tips(struct git_transport_options *smart_options) +{ + struct oid_array *oids = xcalloc(1, sizeof(*oids)); + int i; + + for (i = 0; i < negotiation_tip.nr; i++) { + const char *s = negotiation_tip.items[i].string; + int old_nr; + if (!has_glob_specials(s)) { + struct object_id oid; + if (get_oid(s, &oid)) + die("%s is not a valid object", s); + oid_array_append(oids, &oid); + continue; + } + old_nr = oids->nr; + for_each_glob_ref(add_oid, s, oids); + if (old_nr == oids->nr) + warning("Ignoring --negotiation-tip=%s because it does not match any refs", + s); + } + smart_options->negotiation_tips = oids; +} + static struct transport *prepare_transport(struct remote *remote, int deepen) { struct transport *transport; @@@ -1122,12 -1089,6 +1128,12 @@@ filter_options.filter_spec); set_option(transport, TRANS_OPT_FROM_PROMISOR, "1"); } + if (negotiation_tip.nr) { + if (transport->smart_options) + add_negotiation_tips(transport->smart_options); + else + warning("Ignoring --negotiation-tip because the protocol does not support it."); + } return transport; } @@@ -1152,7 -1113,7 +1158,7 @@@ static void backfill_tags(struct transp transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL); transport_set_option(transport, TRANS_OPT_DEPTH, "0"); transport_set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, NULL); - if (!fetch_refs(transport, ref_map, NULL)) + if (!fetch_refs(transport, ref_map)) consume_refs(transport, ref_map); if (gsecondary) { @@@ -1168,6 -1129,7 +1174,6 @@@ static int do_fetch(struct transport *t int autotags = (transport->remote->fetch_tags == 1); int retcode = 0; const struct ref *remote_refs; - struct ref *updated_remote_refs = NULL; struct argv_array ref_prefixes = ARGV_ARRAY_INIT; if (tags == TAGS_DEFAULT) { @@@ -1190,7 -1152,7 +1196,7 @@@ refspec_ref_prefixes(&transport->remote->fetch, &ref_prefixes); if (ref_prefixes.argc && - (tags == TAGS_SET || (tags == TAGS_DEFAULT && !rs->nr))) { + (tags == TAGS_SET || (tags == TAGS_DEFAULT))) { argv_array_push(&ref_prefixes, "refs/tags/"); } @@@ -1218,7 -1180,24 +1224,7 @@@ transport->url); } } - - if (fetch_refs(transport, ref_map, &updated_remote_refs)) { - free_refs(ref_map); - retcode = 1; - goto cleanup; - } - if (updated_remote_refs) { - /* - * Regenerate ref_map using the updated remote refs. This is - * to account for additional information which may be provided - * by the transport (e.g. shallow info). - */ - free_refs(ref_map); - ref_map = get_ref_map(transport->remote, updated_remote_refs, rs, - tags, &autotags); - free_refs(updated_remote_refs); - } - if (consume_refs(transport, ref_map)) { + if (fetch_refs(transport, ref_map) || consume_refs(transport, ref_map)) { free_refs(ref_map); retcode = 1; goto cleanup; diff --combined t/t5516-fetch-push.sh index 539c25aada,c545b16d13..7a8f56db53 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@@ -923,7 -923,7 +923,7 @@@ test_expect_success 'push into aliased ( cd child1 && git branch foo && - git symbolic-ref refs/heads/bar refs/heads/foo + git symbolic-ref refs/heads/bar refs/heads/foo && git config receive.denyCurrentBranch false ) && ( @@@ -945,7 -945,7 +945,7 @@@ test_expect_success 'push into aliased ( cd child1 && git branch foo && - git symbolic-ref refs/heads/bar refs/heads/foo + git symbolic-ref refs/heads/bar refs/heads/foo && git config receive.denyCurrentBranch false ) && ( @@@ -969,7 -969,7 +969,7 @@@ test_force_push_tag () tag_type_description=$1 tag_args=$2 - test_expect_success 'force pushing required to update lightweight tag' " + test_expect_success "force pushing required to update $tag_type_description" " mk_test testrepo heads/master && mk_child testrepo child1 && mk_child testrepo child2 && @@@ -1009,7 -1009,32 +1009,32 @@@ } test_force_push_tag "lightweight tag" "-f" - test_force_push_tag "annotated tag" "-f -a -m'msg'" + test_force_push_tag "annotated tag" "-f -a -m'tag message'" + + test_force_fetch_tag () { + tag_type_description=$1 + tag_args=$2 + + test_expect_success "fetch will not clobber an existing $tag_type_description without --force" " + mk_test testrepo heads/master && + mk_child testrepo child1 && + mk_child testrepo child2 && + ( + cd testrepo && + git tag testTag && + git -C ../child1 fetch origin tag testTag && + >file1 && + git add file1 && + git commit -m 'file1' && + git tag $tag_args testTag && + test_must_fail git -C ../child1 fetch origin tag testTag && + git -C ../child1 fetch origin '+refs/tags/*:refs/tags/*' + ) + " + } + + test_force_fetch_tag "lightweight tag" "-f" + test_force_fetch_tag "annotated tag" "-f -a -m'tag message'" test_expect_success 'push --porcelain' ' mk_empty testrepo && @@@ -1036,7 -1061,7 +1061,7 @@@ test_expect_success 'push --porcelain r mk_empty testrepo && git push testrepo refs/heads/master:refs/remotes/origin/master && (cd testrepo && - git reset --hard origin/master^ + git reset --hard origin/master^ && git config receive.denyCurrentBranch true) && echo >.git/foo "To testrepo" && @@@ -1050,7 -1075,7 +1075,7 @@@ test_expect_success 'push --porcelain - mk_empty testrepo && git push testrepo refs/heads/master:refs/remotes/origin/master && (cd testrepo && - git reset --hard origin/master + git reset --hard origin/master && git config receive.denyCurrentBranch true) && echo >.git/foo "To testrepo" && @@@ -1358,7 -1383,7 +1383,7 @@@ test_expect_success 'push --follow-tag git commit --allow-empty -m "future commit" && git tag -m "future" future && git checkout master && - git for-each-ref refs/heads/master refs/tags/tag >../expect + git for-each-ref refs/heads/master refs/tags/tag >../expect && git push --follow-tag ../dst master ) && ( diff --combined t/t5612-clone-refspec.sh index 5582b3d5fd,6ea8f50dae..e36ac01661 --- a/t/t5612-clone-refspec.sh +++ b/t/t5612-clone-refspec.sh @@@ -97,13 -97,14 +97,13 @@@ test_expect_success 'clone with --no-ta git fetch && git for-each-ref refs/tags >../actual ) && - >expect && - test_cmp expect actual + test_must_be_empty actual ' test_expect_success '--single-branch while HEAD pointing at master' ' ( cd dir_master && - git fetch && + git fetch --force && git for-each-ref refs/remotes/origin | sed -e "/HEAD$/d" \ -e "s|/remotes/origin/|/heads/|" >../actual @@@ -114,7 -115,7 +114,7 @@@ test_cmp expect actual && ( cd dir_master && - git fetch --tags && + git fetch --tags --force && git for-each-ref refs/tags >../actual ) && git for-each-ref refs/tags >expect && @@@ -139,7 -140,8 +139,7 @@@ test_expect_success '--single-branch wh git fetch && git for-each-ref refs/tags >../actual ) && - >expect && - test_cmp expect actual && + test_must_be_empty actual && test_line_count = 0 actual && # get tags with --tags overrides tagOpt ( @@@ -228,7 -230,8 +228,7 @@@ test_expect_success '--single-branch wi -e "s|/remotes/origin/|/heads/|" >../actual ) && # nothing - >expect && - test_cmp expect actual + test_must_be_empty actual ' test_done