From: Junio C Hamano Date: Mon, 29 Jul 2019 19:38:23 +0000 (-0700) Subject: Merge branch 'fc/fetch-with-import-fix' into maint X-Git-Tag: v2.23.0-rc0~1^2~2 X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/7011ce12b895d172562dcaa7d8b240f3594dc981?ds=inline;hp=-c Merge branch 'fc/fetch-with-import-fix' into maint Code restructuring during 2.20 period broke fetching tags via "import" based transports. * fc/fetch-with-import-fix: fetch: fix regression with transport helpers fetch: make the code more understandable fetch: trivial cleanup t5801 (remote-helpers): add test to fetch tags t5801 (remote-helpers): cleanup refspec stuff --- 7011ce12b895d172562dcaa7d8b240f3594dc981 diff --combined builtin/fetch.c index fc6c879bcf,e485d429c9..c9b92b1e52 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@@ -98,8 -98,6 +98,8 @@@ static int git_fetch_config(const char static int parse_refmap_arg(const struct option *opt, const char *arg, int unset) { + BUG_ON_OPT_NEG(unset); + /* * "git fetch --refmap='' origin foo" * can be used to tell the command not to store anywhere @@@ -239,6 -237,7 +239,7 @@@ static int will_fetch(struct ref **head struct refname_hash_entry { struct hashmap_entry ent; /* must be the first member */ struct object_id oid; + int ignore; char refname[FLEX_ARRAY]; }; @@@ -287,6 -286,11 +288,11 @@@ static int refname_hash_exists(struct h return !!hashmap_get_from_hash(map, strhash(refname), refname); } + static void clear_item(struct refname_hash_entry *item) + { + item->ignore = 1; + } + static void find_non_local_tags(const struct ref *refs, struct ref **head, struct ref ***tail) @@@ -317,9 -321,10 +323,9 @@@ !has_object_file_with_flags(&ref->old_oid, OBJECT_INFO_QUICK) && !will_fetch(head, ref->old_oid.hash) && - !has_sha1_file_with_flags(item->oid.hash, - OBJECT_INFO_QUICK) && + !has_object_file_with_flags(&item->oid, OBJECT_INFO_QUICK) && !will_fetch(head, item->oid.hash)) - oidclr(&item->oid); + clear_item(item); item = NULL; continue; } @@@ -331,9 -336,9 +337,9 @@@ * fetch. */ if (item && - !has_sha1_file_with_flags(item->oid.hash, OBJECT_INFO_QUICK) && + !has_object_file_with_flags(&item->oid, OBJECT_INFO_QUICK) && !will_fetch(head, item->oid.hash)) - oidclr(&item->oid); + clear_item(item); item = NULL; @@@ -352,9 -357,9 +358,9 @@@ * checked to see if it needs fetching. */ if (item && - !has_sha1_file_with_flags(item->oid.hash, OBJECT_INFO_QUICK) && + !has_object_file_with_flags(&item->oid, OBJECT_INFO_QUICK) && !will_fetch(head, item->oid.hash)) - oidclr(&item->oid); + clear_item(item); /* * For all the tags in the remote_refs_list, @@@ -362,19 -367,21 +368,21 @@@ */ for_each_string_list_item(remote_ref_item, &remote_refs_list) { const char *refname = remote_ref_item->string; + struct ref *rm; item = hashmap_get_from_hash(&remote_refs, strhash(refname), refname); if (!item) BUG("unseen remote ref?"); /* Unless we have already decided to ignore this item... */ - if (!is_null_oid(&item->oid)) { - struct ref *rm = alloc_ref(item->refname); - rm->peer_ref = alloc_ref(item->refname); - oidcpy(&rm->old_oid, &item->oid); - **tail = rm; - *tail = &rm->next; - } + if (item->ignore) + continue; + + rm = alloc_ref(item->refname); + rm->peer_ref = alloc_ref(item->refname); + oidcpy(&rm->old_oid, &item->oid); + **tail = rm; + *tail = &rm->next; } hashmap_free(&remote_refs, 1); string_list_clear(&remote_refs_list, 0); @@@ -628,14 -635,9 +636,14 @@@ static int find_and_replace(struct strb const char *needle, const char *placeholder) { - const char *p = strstr(haystack->buf, needle); + const char *p = NULL; int plen, nlen; + nlen = strlen(needle); + if (ends_with(haystack->buf, needle)) + p = haystack->buf + haystack->len - nlen; + else + p = strstr(haystack->buf, needle); if (!p) return 0; @@@ -643,6 -645,7 +651,6 @@@ return 0; plen = strlen(p); - nlen = strlen(needle); if (plen > nlen && p[nlen] != '/') return 0; @@@ -766,6 -769,9 +774,6 @@@ static int update_local_ref(struct ref what = _("[new ref]"); } - if ((recurse_submodules != RECURSE_SUBMODULES_OFF) && - (recurse_submodules != RECURSE_SUBMODULES_ON)) - check_for_new_submodule_commits(&ref->new_oid); r = s_update_ref(msg, ref, 0); format_display(display, r ? '!' : '*', what, r ? _("unable to update local ref") : NULL, @@@ -779,6 -785,9 +787,6 @@@ strbuf_add_unique_abbrev(&quickref, ¤t->object.oid, DEFAULT_ABBREV); strbuf_addstr(&quickref, ".."); strbuf_add_unique_abbrev(&quickref, &ref->new_oid, DEFAULT_ABBREV); - if ((recurse_submodules != RECURSE_SUBMODULES_OFF) && - (recurse_submodules != RECURSE_SUBMODULES_ON)) - check_for_new_submodule_commits(&ref->new_oid); r = s_update_ref("fast-forward", ref, 1); format_display(display, r ? '!' : ' ', quickref.buf, r ? _("unable to update local ref") : NULL, @@@ -791,6 -800,9 +799,6 @@@ strbuf_add_unique_abbrev(&quickref, ¤t->object.oid, DEFAULT_ABBREV); strbuf_addstr(&quickref, "..."); strbuf_add_unique_abbrev(&quickref, &ref->new_oid, DEFAULT_ABBREV); - if ((recurse_submodules != RECURSE_SUBMODULES_OFF) && - (recurse_submodules != RECURSE_SUBMODULES_ON)) - check_for_new_submodule_commits(&ref->new_oid); r = s_update_ref("forced-update", ref, 1); format_display(display, r ? '!' : '+', quickref.buf, r ? _("unable to update local ref") : _("forced update"), @@@ -886,8 -898,6 +894,8 @@@ static int store_updated_refs(const cha ref->force = rm->peer_ref->force; } + if (recurse_submodules != RECURSE_SUBMODULES_OFF) + check_for_new_submodule_commits(&rm->old_oid); if (!strcmp(rm->name, "HEAD")) { kind = ""; @@@ -983,11 -993,10 +991,11 @@@ * everything we are going to fetch already exists and is connected * locally. */ -static int quickfetch(struct ref *ref_map) +static int check_exist_and_connected(struct ref *ref_map) { struct ref *rm = ref_map; struct check_connected_options opt = CHECK_CONNECTED_INIT; + struct ref *r; /* * If we are deepening a shallow clone we already have these @@@ -998,23 -1007,13 +1006,23 @@@ */ if (deepen) return -1; + + /* + * check_connected() allows objects to merely be promised, but + * we need all direct targets to exist. + */ + for (r = rm; r; r = r->next) { + if (!has_object_file(&r->old_oid)) + return -1; + } + opt.quiet = 1; return check_connected(iterate_ref_map, &rm, &opt); } static int fetch_refs(struct transport *transport, struct ref *ref_map) { - int ret = quickfetch(ref_map); + int ret = check_exist_and_connected(ref_map); if (ret) ret = transport_fetch_refs(transport, ref_map); if (!ret) @@@ -1168,7 -1167,6 +1176,7 @@@ static void add_negotiation_tips(struc static struct transport *prepare_transport(struct remote *remote, int deepen) { struct transport *transport; + transport = transport_get(remote, NULL); transport_set_verbosity(transport, verbosity, progress); transport->family = family; @@@ -1188,13 -1186,9 +1196,13 @@@ if (update_shallow) set_option(transport, TRANS_OPT_UPDATE_SHALLOW, "yes"); if (filter_options.choice) { + struct strbuf expanded_filter_spec = STRBUF_INIT; + expand_list_objects_filter_spec(&filter_options, + &expanded_filter_spec); set_option(transport, TRANS_OPT_LIST_OBJECTS_FILTER, - filter_options.filter_spec); + expanded_filter_spec.buf); set_option(transport, TRANS_OPT_FROM_PROMISOR, "1"); + strbuf_release(&expanded_filter_spec); } if (negotiation_tip.nr) { if (transport->smart_options) @@@ -1243,7 -1237,6 +1251,7 @@@ static int do_fetch(struct transport *t int retcode = 0; const struct ref *remote_refs; struct argv_array ref_prefixes = ARGV_ARRAY_INIT; + int must_list_refs = 1; if (tags == TAGS_DEFAULT) { if (transport->remote->fetch_tags == 2) @@@ -1259,36 -1252,17 +1267,36 @@@ goto cleanup; } - if (rs->nr) + if (rs->nr) { + int i; + refspec_ref_prefixes(rs, &ref_prefixes); - else if (transport->remote && transport->remote->fetch.nr) + + /* + * We can avoid listing refs if all of them are exact + * OIDs + */ + must_list_refs = 0; + for (i = 0; i < rs->nr; i++) { + if (!rs->items[i].exact_sha1) { + must_list_refs = 1; + break; + } + } + } else if (transport->remote && transport->remote->fetch.nr) refspec_ref_prefixes(&transport->remote->fetch, &ref_prefixes); - if (ref_prefixes.argc && - (tags == TAGS_SET || (tags == TAGS_DEFAULT))) { - argv_array_push(&ref_prefixes, "refs/tags/"); + if (tags == TAGS_SET || tags == TAGS_DEFAULT) { + must_list_refs = 1; + if (ref_prefixes.argc) + argv_array_push(&ref_prefixes, "refs/tags/"); } - remote_refs = transport_get_remote_refs(transport, &ref_prefixes); + if (must_list_refs) + remote_refs = transport_get_remote_refs(transport, &ref_prefixes); + else + remote_refs = NULL; + argv_array_clear(&ref_prefixes); ref_map = get_ref_map(transport->remote, remote_refs, rs, @@@ -1479,8 -1453,7 +1487,8 @@@ static inline void fetch_one_setup_part */ if (strcmp(remote->name, repository_format_partial_clone)) { if (filter_options.choice) - die(_("--filter can only be used with the remote configured in core.partialClone")); + die(_("--filter can only be used with the remote " + "configured in extensions.partialClone")); return; } @@@ -1556,9 -1529,7 +1564,9 @@@ static int fetch_one(struct remote *rem sigchain_push_common(unlock_pack_on_signal); atexit(unlock_pack); + sigchain_push(SIGPIPE, SIG_IGN); exit_code = do_fetch(gtransport, &rs); + sigchain_pop(SIGPIPE); refspec_clear(&rs); transport_disconnect(gtransport); gtransport = NULL; @@@ -1650,8 -1621,7 +1658,8 @@@ int cmd_fetch(int argc, const char **ar result = fetch_one(remote, argc, argv, prune_tags_ok); } else { if (filter_options.choice) - die(_("--filter can only be used with the remote configured in core.partialClone")); + die(_("--filter can only be used with the remote " + "configured in extensions.partialclone")); /* TODO should this also die if we have a previous partial-clone? */ result = fetch_multiple(&list); } @@@ -1672,7 -1642,7 +1680,7 @@@ string_list_clear(&list, 0); - close_all_packs(the_repository->objects); + close_object_store(the_repository->objects); argv_array_pushl(&argv_gc_auto, "gc", "--auto", NULL); if (verbosity < 0) diff --combined t/t5801-remote-helpers.sh index d04f8007e0,1829ef29c7..2d6c4a281e --- a/t/t5801-remote-helpers.sh +++ b/t/t5801-remote-helpers.sh @@@ -8,8 -8,6 +8,8 @@@ test_description='Test remote-helper im . ./test-lib.sh . "$TEST_DIRECTORY"/lib-gpg.sh +PATH="$TEST_DIRECTORY/t5801:$PATH" + compare_refs() { git --git-dir="$1/.git" rev-parse --verify $2 >expect && git --git-dir="$3/.git" rev-parse --verify $4 >actual && @@@ -126,7 -124,7 +126,7 @@@ test_expect_success 'forced push' ' test_expect_success 'cloning without refspec' ' - GIT_REMOTE_TESTGIT_REFSPEC="" \ + GIT_REMOTE_TESTGIT_NOREFSPEC=1 \ git clone "testgit::${PWD}/server" local2 2>error && test_i18ngrep "this remote helper should implement refspec capability" error && compare_refs local2 HEAD server HEAD @@@ -135,7 -133,7 +135,7 @@@ test_expect_success 'pulling without refspecs' ' (cd local2 && git reset --hard && - GIT_REMOTE_TESTGIT_REFSPEC="" git pull 2>../error) && + GIT_REMOTE_TESTGIT_NOREFSPEC=1 git pull 2>../error) && test_i18ngrep "this remote helper should implement refspec capability" error && compare_refs local2 HEAD server HEAD ' @@@ -145,8 -143,8 +145,8 @@@ test_expect_success 'pushing without re (cd local2 && echo content >>file && git commit -a -m ten && - GIT_REMOTE_TESTGIT_REFSPEC="" && - export GIT_REMOTE_TESTGIT_REFSPEC && + GIT_REMOTE_TESTGIT_NOREFSPEC=1 && + export GIT_REMOTE_TESTGIT_NOREFSPEC && test_must_fail git push 2>../error) && test_i18ngrep "remote-helper doesn.t support push; refspec needed" error ' @@@ -303,4 -301,14 +303,14 @@@ test_expect_success 'fetch url' compare_refs server HEAD local FETCH_HEAD ' + test_expect_success 'fetch tag' ' + (cd server && + git tag v1.0 + ) && + (cd local && + git fetch + ) && + compare_refs local v1.0 server v1.0 + ' + test_done diff --combined t/t5801/git-remote-testgit index 752c763eb6,0000000000..6b9f0b5dc7 mode 100755,000000..100755 --- a/t/t5801/git-remote-testgit +++ b/t/t5801/git-remote-testgit @@@ -1,147 -1,0 +1,151 @@@ +#!/bin/sh +# Copyright (c) 2012 Felipe Contreras + +# The first argument can be a url when the fetch/push command was a url +# instead of a configured remote. In this case, use a generic alias. +if test "$1" = "testgit::$2"; then + alias=_ +else + alias=$1 +fi +url=$2 + +dir="$GIT_DIR/testgit/$alias" - prefix="refs/testgit/$alias" + - default_refspec="refs/heads/*:${prefix}/heads/*" ++h_refspec="refs/heads/*:refs/testgit/$alias/heads/*" ++t_refspec="refs/tags/*:refs/testgit/$alias/tags/*" + - refspec="${GIT_REMOTE_TESTGIT_REFSPEC-$default_refspec}" - - test -z "$refspec" && prefix="refs" ++if test -n "$GIT_REMOTE_TESTGIT_NOREFSPEC" ++then ++ h_refspec="" ++ t_refspec="" ++fi + +GIT_DIR="$url/.git" +export GIT_DIR + +force= + +mkdir -p "$dir" + +if test -z "$GIT_REMOTE_TESTGIT_NO_MARKS" +then + gitmarks="$dir/git.marks" + testgitmarks="$dir/testgit.marks" + test -e "$gitmarks" || >"$gitmarks" + test -e "$testgitmarks" || >"$testgitmarks" +fi + +while read line +do + case $line in + capabilities) + echo 'import' + echo 'export' - test -n "$refspec" && echo "refspec $refspec" ++ test -n "$h_refspec" && echo "refspec $h_refspec" ++ test -n "$t_refspec" && echo "refspec $t_refspec" + if test -n "$gitmarks" + then + echo "*import-marks $gitmarks" + echo "*export-marks $gitmarks" + fi + test -n "$GIT_REMOTE_TESTGIT_SIGNED_TAGS" && echo "signed-tags" + test -n "$GIT_REMOTE_TESTGIT_NO_PRIVATE_UPDATE" && echo "no-private-update" + echo 'option' + echo + ;; + list) - git for-each-ref --format='? %(refname)' 'refs/heads/' ++ git for-each-ref --format='? %(refname)' 'refs/heads/' 'refs/tags/' + head=$(git symbolic-ref HEAD) + echo "@$head HEAD" + echo + ;; + import*) + # read all import lines + while true + do + ref="${line#* }" + refs="$refs $ref" + read line + test "${line%% *}" != "import" && break + done + + if test -n "$gitmarks" + then + echo "feature import-marks=$gitmarks" + echo "feature export-marks=$gitmarks" + fi + + if test -n "$GIT_REMOTE_TESTGIT_FAILURE" + then + echo "feature done" + exit 1 + fi + + echo "feature done" + git fast-export \ ++ ${h_refspec:+"--refspec=$h_refspec"} \ ++ ${t_refspec:+"--refspec=$t_refspec"} \ + ${testgitmarks:+"--import-marks=$testgitmarks"} \ + ${testgitmarks:+"--export-marks=$testgitmarks"} \ - $refs | - sed -e "s#refs/heads/#${prefix}/heads/#g" ++ $refs + echo "done" + ;; + export) + if test -n "$GIT_REMOTE_TESTGIT_FAILURE" + then + # consume input so fast-export doesn't get SIGPIPE; + # git would also notice that case, but we want + # to make sure we are exercising the later + # error checks + while read line; do + test "done" = "$line" && break + done + exit 1 + fi + + before=$(git for-each-ref --format=' %(refname) %(objectname) ') + + git fast-import \ + ${force:+--force} \ + ${testgitmarks:+"--import-marks=$testgitmarks"} \ + ${testgitmarks:+"--export-marks=$testgitmarks"} \ + --quiet + + # figure out which refs were updated + git for-each-ref --format='%(refname) %(objectname)' | + while read ref a + do + case "$before" in + *" $ref $a "*) + continue ;; # unchanged + esac + if test -z "$GIT_REMOTE_TESTGIT_PUSH_ERROR" + then + echo "ok $ref" + else + echo "error $ref $GIT_REMOTE_TESTGIT_PUSH_ERROR" + fi + done + + echo + ;; + option\ *) + read cmd opt val <<-EOF + $line + EOF + case $opt in + force) + test $val = "true" && force="true" || force= + echo "ok" + ;; + *) + echo "unsupported" + ;; + esac + ;; + '') + exit + ;; + esac +done