fetch-pack: do not take shallow lock unnecessarily
When fetching using protocol v2, the remote may send a "shallow-info" section if the client is shallow. If so, Git as the client currently takes the shallow file lock, even if the "shallow-info" section is empty. This is not a problem except that Git does not support taking the shallow file lock after modifying the shallow file, because is_repository_shallow() stores information that is never cleared. And this take-after-modify occurs when Git does a tag-following fetch from a shallow repository on a transport that does not support tag following (since in this case, 2 fetches are performed). To solve this issue, take the shallow file lock (and perform all other shallow processing) only if the "shallow-info" section is non-empty; otherwise, behave as if it were empty. A full solution (probably, ensuring that any action of committing shallow file locks also includes clearing the information stored by is_repository_shallow()) would solve the issue without need for this patch, but this patch is independently useful (as an optimization to prevent writing a file in an unnecessary case), hence why I wrote it. I have included a NEEDSWORK outlining the full solution. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>maint
							parent
							
								
									ecbdaf0899
								
							
						
					
					
						commit
						bd0b42aed3
					
				|  | @ -1232,6 +1232,8 @@ static int process_acks(struct fetch_negotiator *negotiator, | |||
| static void receive_shallow_info(struct fetch_pack_args *args, | ||||
| 				 struct packet_reader *reader) | ||||
| { | ||||
| 	int line_received = 0; | ||||
|  | ||||
| 	process_section_header(reader, "shallow-info", 0); | ||||
| 	while (packet_reader_read(reader) == PACKET_READ_NORMAL) { | ||||
| 		const char *arg; | ||||
|  | @ -1241,6 +1243,7 @@ static void receive_shallow_info(struct fetch_pack_args *args, | |||
| 			if (get_oid_hex(arg, &oid)) | ||||
| 				die(_("invalid shallow line: %s"), reader->line); | ||||
| 			register_shallow(the_repository, &oid); | ||||
| 			line_received = 1; | ||||
| 			continue; | ||||
| 		} | ||||
| 		if (skip_prefix(reader->line, "unshallow ", &arg)) { | ||||
|  | @ -1253,6 +1256,7 @@ static void receive_shallow_info(struct fetch_pack_args *args, | |||
| 				die(_("error in object: %s"), reader->line); | ||||
| 			if (unregister_shallow(&oid)) | ||||
| 				die(_("no shallow found: %s"), reader->line); | ||||
| 			line_received = 1; | ||||
| 			continue; | ||||
| 		} | ||||
| 		die(_("expected shallow/unshallow, got %s"), reader->line); | ||||
|  | @ -1262,9 +1266,12 @@ static void receive_shallow_info(struct fetch_pack_args *args, | |||
| 	    reader->status != PACKET_READ_DELIM) | ||||
| 		die(_("error processing shallow info: %d"), reader->status); | ||||
|  | ||||
| 	setup_alternate_shallow(&shallow_lock, &alternate_shallow_file, NULL); | ||||
| 	if (line_received) { | ||||
| 		setup_alternate_shallow(&shallow_lock, &alternate_shallow_file, | ||||
| 					NULL); | ||||
| 		args->deepen = 1; | ||||
| 	} | ||||
| } | ||||
|  | ||||
| static void receive_wanted_refs(struct packet_reader *reader, | ||||
| 				struct ref **sought, int nr_sought) | ||||
|  |  | |||
|  | @ -43,6 +43,13 @@ int register_shallow(struct repository *r, const struct object_id *oid) | |||
|  | ||||
| int is_repository_shallow(struct repository *r) | ||||
| { | ||||
| 	/* | ||||
| 	 * NEEDSWORK: This function updates | ||||
| 	 * r->parsed_objects->{is_shallow,shallow_stat} as a side effect but | ||||
| 	 * there is no corresponding function to clear them when the shallow | ||||
| 	 * file is updated. | ||||
| 	 */ | ||||
|  | ||||
| 	FILE *fp; | ||||
| 	char buf[1024]; | ||||
| 	const char *path = r->parsed_objects->alternate_shallow_file; | ||||
|  |  | |||
|  | @ -471,6 +471,24 @@ test_expect_success 'upload-pack respects client shallows' ' | |||
| 	grep "fetch< version 2" trace | ||||
| ' | ||||
|  | ||||
| test_expect_success 'ensure that multiple fetches in same process from a shallow repo works' ' | ||||
| 	rm -rf server client trace && | ||||
|  | ||||
| 	test_create_repo server && | ||||
| 	test_commit -C server one && | ||||
| 	test_commit -C server two && | ||||
| 	test_commit -C server three && | ||||
| 	git clone --shallow-exclude two "file://$(pwd)/server" client && | ||||
|  | ||||
| 	git -C server tag -a -m "an annotated tag" twotag two && | ||||
|  | ||||
| 	# Triggers tag following (thus, 2 fetches in one process) | ||||
| 	GIT_TRACE_PACKET="$(pwd)/trace" git -C client -c protocol.version=2 \ | ||||
| 		fetch --shallow-exclude one origin && | ||||
| 	# Ensure that protocol v2 is used | ||||
| 	grep "fetch< version 2" trace | ||||
| ' | ||||
|  | ||||
| # Test protocol v2 with 'http://' transport | ||||
| # | ||||
| . "$TEST_DIRECTORY"/lib-httpd.sh | ||||
|  |  | |||
		Loading…
	
		Reference in New Issue
	
	 Jonathan Tan
						Jonathan Tan