fetch-pack: in protocol v2, reset in_vain upon ACK
In the function process_acks() in fetch-pack.c, the variable received_ack is meant to track that an ACK was received, but it was never set. This results in negotiation terminating prematurely through the in_vain counter, when the counter should have been reset upon every ACK. Therefore, reset the in_vain counter upon every ACK. Helped-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>maint
							parent
							
								
									4fa3f00abb
								
							
						
					
					
						commit
						2f0a093dd6
					
				|  | @ -1307,6 +1307,7 @@ static enum common_found process_acks(struct fetch_negotiator *negotiator, | ||||||
|  |  | ||||||
| 		if (skip_prefix(reader->line, "ACK ", &arg)) { | 		if (skip_prefix(reader->line, "ACK ", &arg)) { | ||||||
| 			struct object_id oid; | 			struct object_id oid; | ||||||
|  | 			received_ack = 1; | ||||||
| 			if (!get_oid_hex(arg, &oid)) { | 			if (!get_oid_hex(arg, &oid)) { | ||||||
| 				struct commit *commit; | 				struct commit *commit; | ||||||
| 				oidset_insert(common, &oid); | 				oidset_insert(common, &oid); | ||||||
|  |  | ||||||
|  | @ -403,6 +403,36 @@ test_expect_success 'in_vain not triggered before first ACK' ' | ||||||
| 	test_i18ngrep "Total 3 " trace | 	test_i18ngrep "Total 3 " trace | ||||||
| ' | ' | ||||||
|  |  | ||||||
|  | test_expect_success 'in_vain resetted upon ACK' ' | ||||||
|  | 	rm -rf myserver myclient trace && | ||||||
|  | 	git init myserver && | ||||||
|  |  | ||||||
|  | 	# Linked list of commits on master. The first is common; the rest are | ||||||
|  | 	# not. | ||||||
|  | 	test_commit -C myserver first_master_commit && | ||||||
|  | 	git clone "file://$(pwd)/myserver" myclient && | ||||||
|  | 	test_commit_bulk -C myclient 255 && | ||||||
|  |  | ||||||
|  | 	# Another linked list of commits on anotherbranch with no connection to | ||||||
|  | 	# master. The first is common; the rest are not. | ||||||
|  | 	git -C myserver checkout --orphan anotherbranch && | ||||||
|  | 	test_commit -C myserver first_anotherbranch_commit && | ||||||
|  | 	git -C myclient fetch origin anotherbranch:refs/heads/anotherbranch && | ||||||
|  | 	git -C myclient checkout anotherbranch && | ||||||
|  | 	test_commit_bulk -C myclient 255 && | ||||||
|  |  | ||||||
|  | 	# The new commit that the client wants to fetch. | ||||||
|  | 	git -C myserver checkout master && | ||||||
|  | 	test_commit -C myserver to_fetch && | ||||||
|  |  | ||||||
|  | 	# The client will send (as "have"s) all 256 commits in anotherbranch | ||||||
|  | 	# first. The 256th commit is common between the client and the server, | ||||||
|  | 	# and should reset in_vain. This allows negotiation to continue until | ||||||
|  | 	# the client reports that first_anotherbranch_commit is common. | ||||||
|  | 	GIT_TRACE_PACKET="$(pwd)/trace" git -C myclient fetch --progress origin master && | ||||||
|  | 	test_i18ngrep "Total 3 " trace | ||||||
|  | ' | ||||||
|  |  | ||||||
| test_expect_success 'fetch in shallow repo unreachable shallow objects' ' | test_expect_success 'fetch in shallow repo unreachable shallow objects' ' | ||||||
| 	( | 	( | ||||||
| 		git clone --bare --branch B --single-branch "file://$(pwd)/." no-reflog && | 		git clone --bare --branch B --single-branch "file://$(pwd)/." no-reflog && | ||||||
|  |  | ||||||
		Loading…
	
		Reference in New Issue
	
	 Jonathan Tan
						Jonathan Tan