diff mbox series

[v2,2/2] fetch-pack: respect --no-update-shallow in v2

Message ID 943b1cbc61dbcc498a9e34d7d39dc8617cb99195.1553622679.git.jonathantanmy@google.com (mailing list archive)
State New, archived
Headers show
Series Last big GIT_TEST_PROTOCOL_VERSION=2 fix, hopefully | expand

Commit Message

Jonathan Tan March 26, 2019, 5:53 p.m. UTC
In protocol v0, when sending "shallow" lines, the server distinguishes
between lines caused by the remote repo being shallow and lines caused
by client-specified depth settings. Unless "--update-shallow" is
specified, there is a difference in behavior: refs that reach the former
"shallow" lines, but not the latter, are rejected. But in v2, the server
does not, and the client treats all "shallow" lines like lines caused by
client-specified depth settings.

Full restoration of v0 functionality is not possible without protocol
change, but we can implement a heuristic: if we specify any depth
setting, treat all "shallow" lines like lines caused by client-specified
depth settings (that is, unaffected by "--no-update-shallow"), but
otherwise, treat them like lines caused by the remote repo being shallow
(that is, affected by "--no-update-shallow"). This restores most of v0
behavior, except in the case where a client fetches from a shallow
repository with depth settings.

This patch causes a test that previously failed with
GIT_TEST_PROTOCOL_VERSION=2 to pass.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
 fetch-pack.c | 43 +++++++++++++++++++++++++++++++++++--------
 1 file changed, 35 insertions(+), 8 deletions(-)

Comments

Jeff King March 26, 2019, 6:23 p.m. UTC | #1
On Tue, Mar 26, 2019 at 10:53:26AM -0700, Jonathan Tan wrote:

> @@ -1625,6 +1649,7 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
>  {
>  	struct ref *ref_cpy;
>  	struct shallow_info si;
> +	struct oid_array shallows_scratch = OID_ARRAY_INIT;
>  
>  	fetch_pack_setup();
>  	if (nr_sought)
> @@ -1649,10 +1674,11 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
>  		die(_("no matching remote head"));
>  	}
>  	if (version == protocol_v2) {
> -		if (shallow && shallow->nr)
> +		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 {
>  		prepare_shallow_info(&si, shallow);
> @@ -1680,6 +1706,7 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
>  	update_shallow(args, sought, nr_sought, &si);
>  cleanup:
>  	clear_shallow_info(&si);
> +	oid_array_clear(&shallows_scratch);
>  	return ref_cpy;
>  }

Yeah, this is much clearer than what I posted earlier. Thanks for
thinking it through.

The patches look good to me, modulo the timing of the NULL thing I
mentioned elsewhere.

-Peff
diff mbox series

Patch

diff --git a/fetch-pack.c b/fetch-pack.c
index a0eb268dfc..4831eb6a25 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1253,9 +1253,11 @@  static int process_acks(struct fetch_negotiator *negotiator,
 }
 
 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) {
@@ -1265,8 +1267,7 @@  static void receive_shallow_info(struct fetch_pack_args *args,
 		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)) {
@@ -1279,7 +1280,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;
+			unshallow_received = 1;
 			continue;
 		}
 		die(_("expected shallow/unshallow, got %s"), reader->line);
@@ -1289,10 +1290,31 @@  static void receive_shallow_info(struct fetch_pack_args *args,
 	    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;
 	}
@@ -1337,6 +1359,8 @@  static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
 				    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);
@@ -1411,7 +1435,7 @@  static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
 		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);
@@ -1625,6 +1649,7 @@  struct ref *fetch_pack(struct fetch_pack_args *args,
 {
 	struct ref *ref_cpy;
 	struct shallow_info si;
+	struct oid_array shallows_scratch = OID_ARRAY_INIT;
 
 	fetch_pack_setup();
 	if (nr_sought)
@@ -1649,10 +1674,11 @@  struct ref *fetch_pack(struct fetch_pack_args *args,
 		die(_("no matching remote head"));
 	}
 	if (version == protocol_v2) {
-		if (shallow && shallow->nr)
+		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 {
 		prepare_shallow_info(&si, shallow);
@@ -1680,6 +1706,7 @@  struct ref *fetch_pack(struct fetch_pack_args *args,
 	update_shallow(args, sought, nr_sought, &si);
 cleanup:
 	clear_shallow_info(&si);
+	oid_array_clear(&shallows_scratch);
 	return ref_cpy;
 }