From: Junio C Hamano <gitster@pobox.com>
Date: Thu, 25 Apr 2019 07:41:23 +0000 (+0900)
Subject: Merge branch 'jk/fetch-reachability-error-fix'
X-Git-Tag: v2.22.0-rc0~42
X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/57a6b93236f3b1667a1947d3fc83f6e942d3bfca?ds=inline;hp=-c

Merge branch 'jk/fetch-reachability-error-fix'

Code clean-up and a fix for "git fetch" by an explicit object name
(as opposed to fetching refs by name).

* jk/fetch-reachability-error-fix:
fetch: do not consider peeled tags as advertised tips
remote.c: make singular free_ref() public
fetch: use free_refs()
pkt-line: prepare buffer before handling ERR packets
upload-pack: send ERR packet for non-tip objects
t5530: check protocol response for "not our ref"
t5516: drop ok=sigpipe from unreachable-want tests
---

57a6b93236f3b1667a1947d3fc83f6e942d3bfca
diff --combined fetch-pack.c
index 0f158776b0,bb8eac8126..3f24d0c8a6
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@@ -573,9 -573,14 +573,14 @@@ static void filter_refs(struct fetch_pa
  		next = ref->next;
  
  		if (starts_with(ref->name, "refs/") &&
- 		    check_refname_format(ref->name, 0))
- 			; /* trash */
- 		else {
+ 		    check_refname_format(ref->name, 0)) {
+ 			/*
+ 			 * trash or a peeled value; do not even add it to
+ 			 * unmatched list
+ 			 */
+ 			free_one_ref(ref);
+ 			continue;
+ 		} else {
  			while (i < nr_sought) {
  				int cmp = strcmp(ref->name, sought[i]->name);
  				if (cmp < 0)
@@@ -630,10 -635,7 +635,7 @@@
  	}
  
  	oidset_clear(&tip_oids);
- 	for (ref = unmatched; ref; ref = next) {
- 		next = ref->next;
- 		free(ref);
- 	}
+ 	free_refs(unmatched);
  
  	*refs = newlist;
  }
@@@ -1253,11 -1255,9 +1255,11 @@@ static int process_acks(struct fetch_ne
  }
  
  static void receive_shallow_info(struct fetch_pack_args *args,
 -				 struct packet_reader *reader)
 +				 struct packet_reader *reader,
 +				 struct oid_array *shallows,
 +				 struct shallow_info *si)
  {
 -	int line_received = 0;
 +	int unshallow_received = 0;
  
  	process_section_header(reader, "shallow-info", 0);
  	while (packet_reader_read(reader) == PACKET_READ_NORMAL) {
@@@ -1267,7 -1267,8 +1269,7 @@@
  		if (skip_prefix(reader->line, "shallow ", &arg)) {
  			if (get_oid_hex(arg, &oid))
  				die(_("invalid shallow line: %s"), reader->line);
 -			register_shallow(the_repository, &oid);
 -			line_received = 1;
 +			oid_array_append(shallows, &oid);
  			continue;
  		}
  		if (skip_prefix(reader->line, "unshallow ", &arg)) {
@@@ -1280,7 -1281,7 +1282,7 @@@
  				die(_("error in object: %s"), reader->line);
  			if (unregister_shallow(&oid))
  				die(_("no shallow found: %s"), reader->line);
 -			line_received = 1;
 +			unshallow_received = 1;
  			continue;
  		}
  		die(_("expected shallow/unshallow, got %s"), reader->line);
@@@ -1290,41 -1291,15 +1292,41 @@@
  	    reader->status != PACKET_READ_DELIM)
  		die(_("error processing shallow info: %d"), reader->status);
  
 -	if (line_received) {
 +	if (args->deepen || unshallow_received) {
 +		/*
 +		 * Treat these as shallow lines caused by our depth settings.
 +		 * In v0, these lines cannot cause refs to be rejected; do the
 +		 * same.
 +		 */
 +		int i;
 +
 +		for (i = 0; i < shallows->nr; i++)
 +			register_shallow(the_repository, &shallows->oid[i]);
  		setup_alternate_shallow(&shallow_lock, &alternate_shallow_file,
  					NULL);
  		args->deepen = 1;
 +	} else if (shallows->nr) {
 +		/*
 +		 * Treat these as shallow lines caused by the remote being
 +		 * shallow. In v0, remote refs that reach these objects are
 +		 * rejected (unless --update-shallow is set); do the same.
 +		 */
 +		prepare_shallow_info(si, shallows);
 +		if (si->nr_ours || si->nr_theirs)
 +			alternate_shallow_file =
 +				setup_temporary_shallow(si->shallow);
 +		else
 +			alternate_shallow_file = NULL;
  	} else {
  		alternate_shallow_file = NULL;
  	}
  }
  
 +static int cmp_name_ref(const void *name, const void *ref)
 +{
 +	return strcmp(name, (*(struct ref **)ref)->name);
 +}
 +
  static void receive_wanted_refs(struct packet_reader *reader,
  				struct ref **sought, int nr_sought)
  {
@@@ -1332,16 -1307,20 +1334,16 @@@
  	while (packet_reader_read(reader) == PACKET_READ_NORMAL) {
  		struct object_id oid;
  		const char *end;
 -		int i;
 +		struct ref **found;
  
  		if (parse_oid_hex(reader->line, &oid, &end) || *end++ != ' ')
  			die(_("expected wanted-ref, got '%s'"), reader->line);
  
 -		for (i = 0; i < nr_sought; i++) {
 -			if (!strcmp(end, sought[i]->name)) {
 -				oidcpy(&sought[i]->old_oid, &oid);
 -				break;
 -			}
 -		}
 -
 -		if (i == nr_sought)
 +		found = bsearch(end, sought, nr_sought, sizeof(*sought),
 +				cmp_name_ref);
 +		if (!found)
  			die(_("unexpected wanted-ref: '%s'"), reader->line);
 +		oidcpy(&(*found)->old_oid, &oid);
  	}
  
  	if (reader->status != PACKET_READ_DELIM)
@@@ -1360,8 -1339,6 +1362,8 @@@ static struct ref *do_fetch_pack_v2(str
  				    int fd[2],
  				    const struct ref *orig_ref,
  				    struct ref **sought, int nr_sought,
 +				    struct oid_array *shallows,
 +				    struct shallow_info *si,
  				    char **pack_lockfile)
  {
  	struct ref *ref = copy_ref_list(orig_ref);
@@@ -1436,7 -1413,7 +1438,7 @@@
  		case FETCH_GET_PACK:
  			/* Check for shallow-info section */
  			if (process_section_header(&reader, "shallow-info", 1))
 -				receive_shallow_info(args, &reader);
 +				receive_shallow_info(args, &reader, shallows, si);
  
  			if (process_section_header(&reader, "wanted-refs", 1))
  				receive_wanted_refs(&reader, sought, nr_sought);
@@@ -1640,8 -1617,9 +1642,8 @@@ static int iterate_ref_map(void *cb_dat
  }
  
  struct ref *fetch_pack(struct fetch_pack_args *args,
 -		       int fd[], struct child_process *conn,
 +		       int fd[],
  		       const struct ref *ref,
 -		       const char *dest,
  		       struct ref **sought, int nr_sought,
  		       struct oid_array *shallow,
  		       char **pack_lockfile,
@@@ -1649,7 -1627,6 +1651,7 @@@
  {
  	struct ref *ref_cpy;
  	struct shallow_info si;
 +	struct oid_array shallows_scratch = OID_ARRAY_INIT;
  
  	fetch_pack_setup();
  	if (nr_sought)
@@@ -1673,18 -1650,13 +1675,18 @@@
  		packet_flush(fd[1]);
  		die(_("no matching remote head"));
  	}
 -	prepare_shallow_info(&si, shallow);
 -	if (version == protocol_v2)
 +	if (version == protocol_v2) {
 +		if (shallow->nr)
 +			BUG("Protocol V2 does not provide shallows at this point in the fetch");
 +		memset(&si, 0, sizeof(si));
  		ref_cpy = do_fetch_pack_v2(args, fd, ref, sought, nr_sought,
 +					   &shallows_scratch, &si,
  					   pack_lockfile);
 -	else
 +	} else {
 +		prepare_shallow_info(&si, shallow);
  		ref_cpy = do_fetch_pack(args, fd, ref, sought, nr_sought,
  					&si, pack_lockfile);
 +	}
  	reprepare_packed_git(the_repository);
  
  	if (!args->cloning && args->deepen) {
@@@ -1706,7 -1678,6 +1708,7 @@@
  	update_shallow(args, sought, nr_sought, &si);
  cleanup:
  	clear_shallow_info(&si);
 +	oid_array_clear(&shallows_scratch);
  	return ref_cpy;
  }
  
diff --combined t/t5516-fetch-push.sh
index 49bf4280e8,4f065212b8..c81ca360ac
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@@ -1147,12 -1147,8 +1147,12 @@@ test_expect_success 'fetch exact SHA1' 
  		git prune &&
  		test_must_fail git cat-file -t $the_commit &&
  
 +		# Some protocol versions (e.g. 2) support fetching
 +		# unadvertised objects, so restrict this test to v0.
 +
  		# fetching the hidden object should fail by default
 -		test_must_fail git fetch -v ../testrepo $the_commit:refs/heads/copy 2>err &&
 +		test_must_fail env GIT_TEST_PROTOCOL_VERSION= \
 +			git fetch -v ../testrepo $the_commit:refs/heads/copy 2>err &&
  		test_i18ngrep "Server does not allow request for unadvertised object" err &&
  		test_must_fail git rev-parse --verify refs/heads/copy &&
  
@@@ -1208,10 -1204,7 +1208,10 @@@ d
  		mk_empty shallow &&
  		(
  			cd shallow &&
 -			test_must_fail git fetch --depth=1 ../testrepo/.git $SHA1 &&
 +			# Some protocol versions (e.g. 2) support fetching
 +			# unadvertised objects, so restrict this test to v0.
 +			test_must_fail env GIT_TEST_PROTOCOL_VERSION= \
 +				git fetch --depth=1 ../testrepo/.git $SHA1 &&
  			git --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&
  			git fetch --depth=1 ../testrepo/.git $SHA1 &&
  			git cat-file commit $SHA1
@@@ -1239,20 -1232,16 +1239,21 @@@
  		mk_empty shallow &&
  		(
  			cd shallow &&
 -			test_must_fail git fetch ../testrepo/.git $SHA1_3 &&
 -			test_must_fail git fetch ../testrepo/.git $SHA1_1 &&
 +			# Some protocol versions (e.g. 2) support fetching
 +			# unadvertised objects, so restrict this test to v0.
- 			test_must_fail ok=sigpipe env GIT_TEST_PROTOCOL_VERSION= \
++			test_must_fail env GIT_TEST_PROTOCOL_VERSION= \
 +				git fetch ../testrepo/.git $SHA1_3 &&
- 			test_must_fail ok=sigpipe env GIT_TEST_PROTOCOL_VERSION= \
++			test_must_fail env GIT_TEST_PROTOCOL_VERSION= \
 +				git fetch ../testrepo/.git $SHA1_1 &&
  			git --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&
  			git fetch ../testrepo/.git $SHA1_1 &&
  			git cat-file commit $SHA1_1 &&
  			test_must_fail git cat-file commit $SHA1_2 &&
  			git fetch ../testrepo/.git $SHA1_2 &&
  			git cat-file commit $SHA1_2 &&
- 			test_must_fail ok=sigpipe env GIT_TEST_PROTOCOL_VERSION= \
- 				git fetch ../testrepo/.git $SHA1_3
 -			test_must_fail git fetch ../testrepo/.git $SHA1_3 2>err &&
++			test_must_fail env GIT_TEST_PROTOCOL_VERSION= \
++				git fetch ../testrepo/.git $SHA1_3 2>err &&
+ 			test_i18ngrep "remote error:.*not our ref.*$SHA1_3\$" err
  		)
  	'
  done
@@@ -1284,6 -1273,17 +1285,17 @@@ test_expect_success 'fetch follows tag
  	test_cmp expect actual
  '
  
+ test_expect_success 'peeled advertisements are not considered ref tips' '
+ 	mk_empty testrepo &&
+ 	git -C testrepo commit --allow-empty -m one &&
+ 	git -C testrepo commit --allow-empty -m two &&
+ 	git -C testrepo tag -m foo mytag HEAD^ &&
+ 	oid=$(git -C testrepo rev-parse mytag^{commit}) &&
+ 	test_must_fail env GIT_TEST_PROTOCOL_VERSION= \
+ 		git fetch testrepo $oid 2>err &&
+ 	test_i18ngrep "Server does not allow request for unadvertised object" err
+ '
+ 
  test_expect_success 'pushing a specific ref applies remote.$name.push as refmap' '
  	mk_test testrepo heads/master &&
  	rm -fr src dst &&
@@@ -1382,7 -1382,7 +1394,7 @@@ test_expect_success 'push does not foll
  	test_cmp expect actual
  '
  
 -test_expect_success 'push --follow-tag only pushes relevant tags' '
 +test_expect_success 'push --follow-tags only pushes relevant tags' '
  	mk_test testrepo heads/master &&
  	rm -fr src dst &&
  	git init src &&
@@@ -1396,7 -1396,7 +1408,7 @@@
  		git tag -m "future" future &&
  		git checkout master &&
  		git for-each-ref refs/heads/master refs/tags/tag >../expect &&
 -		git push --follow-tag ../dst master
 +		git push --follow-tags ../dst master
  	) &&
  	(
  		cd dst &&