upload-pack: fix slow fetches from a shallow repository

PATCH/RFC In review Harald Nordgren via GitGitGadget 3 participants 5 messages Oct 17 – Nov 11

Share & tools

Harald Nordgren via GitGitGadget

author

2026-10-17 22:11 UTC #0 lore raw

From: Harald Nordgren <[email protected]>

Fetching from a shallow repository with a long history can hang. In a homebrew-core clone with 900k commits, fetching one new commit into another clone took 7 minutes. The same fetch over protocol v0 takes about a second.

Over protocol v2, the server goes through all of its history to work out which of its shallow commits to tell the client about, and a history with many merges makes it go over the same commits again and again. Protocol v0 lists all shallow commits and lets the client sort them out.

Do the same in v2 when the client does not ask for a depth. This also makes git clone --reject-shallow refuse a shallow repository over v2 in every case, as it already does over v0. For git fetch --unshallow, which still needs to go through the history, visit each commit only once.

Signed-off-by Harald Nordgren <[email protected]>

---
upload-pack: fix slow fetches from a shallow repository

Fix git fetch hanging for minutes when the remote is a shallow repository with a long history.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2455%2FHaraldNordgren%2Fupload-pack-shallow-walk-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2455/HaraldNordgren/upload-pack-shallow-walk-v1
Pull-Request: https://github.com/git/git/pull/2455

 shallow.c     | 3 ++-
 upload-pack.c | 5 ++---
 2 files changed, 4 insertions(+), 4 deletions(-)

shallow.c +2 -1

index 8e244a5669..921db5e17a 100644
@@ -209,7 +209,8 @@ static struct commit_list *get_shallows_or_depth(struct object_array *heads,
 				*depth_slot = xmalloc(sizeof(int));
 				**depth_slot = cur_depth;
 			} else {
-				if (cur_depth >= **depth_slot)
+				if (depth == INFINITE_DEPTH ||
+				    cur_depth >= **depth_slot)
 					continue;
 				**depth_slot = cur_depth;
 			}

upload-pack.c +2 -3

index 197bcfa279..844bf80575 100644
@@ -1718,9 +1718,8 @@ static void send_shallow_info(struct upload_pack_data *data)
 
 	packet_writer_write(&data->writer, "shallow-info\n");
 
-	if (!send_shallow_list(data) &&
-	    is_repository_shallow(the_repository))
-		deepen(data, INFINITE_DEPTH);
+	if (!send_shallow_list(data))
+		advertise_shallow_grafts(1);
 
 	packet_delim(1);
 }

base-commit: c51408889c3b372814bc20b9431c3b74d0d268eb

Signature · 1 line

gitgitgadget

Junio C Hamano

2026-10-19 13:27 UTC #1 lore raw

upload-pack.c static void send_shallow_info(struct upload_pack_data *data) · line 1722

Quoted 13 lines · Harald Nordgren via GitGitGadget

upload-pack.c +2 -3

index 197bcfa279..844bf80575 100644
@@ -1718,9 +1718,8 @@ static void send_shallow_info(struct upload_pack_data *data)
 
 	packet_writer_write(&data->writer, "shallow-info\n");
 
-	if (!send_shallow_list(data) &&
-	    is_repository_shallow(the_repository))
-		deepen(data, INFINITE_DEPTH);
+	if (!send_shallow_list(data))
+		advertise_shallow_grafts(1);

The advertise_shallow_grafts() function is written for protocol v0 and uses packet_write_fmt() to write raw data directly to the named file descriptor (in this case, fd #1).

However, protocol v2 negotiation uses the packet_writer API. When sideband-all is in use, the response must be multiplexed over the sideband channel. Because the updated code fails to use the packet_writer API, 'shallow' lines are written without the '\001' band designator. The other side then interprets 's' (the first byte of 'shallow', which is 115 in decimal) as the sideband channel and dies with a protocol violation.

 t/t5702-protocol-v2.sh | 12 ++++++++++++
 1 file changed, 12 insertions(+)

t/t5702-protocol-v2.sh +12 -0

index 7e69ba7f84..df3ec8c1c9 100755
@@ -15,6 +15,18 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 start_git_daemon --export-all --enable=receive-pack
 daemon_parent=$GIT_DAEMON_DOCUMENT_ROOT_PATH/parent
 
+test_expect_success 'fetch from shallow server over v2' '
+	test_when_finished "rm -fr server shallow-server client" &&
+	git init server &&
+	test_commit -C server one &&
+	test_commit -C server two &&
+	git clone --depth=1 "file://$(pwd)/server" shallow-server &&
+	test_commit -C shallow-server three &&
+
+	git -C shallow-server config uploadpack.allowsidebandall true &&
+	git -c protocol.version=2 clone "file://$(pwd)/shallow-server" client
+'
+
 test_expect_success 'create repo to be served by git-daemon' '
 	git init "$daemon_parent" &&
 	test_commit -C "$daemon_parent" one

Harald Nordgren

2026-10-19 14:51 UTC #2 lore raw

Quoted 11 lines

The advertise_shallow_grafts() function is written for protocol v0 and uses packet_write_fmt() to write raw data directly to the named file descriptor (in this case, fd #1).
However, protocol v2 negotiation uses the packet_writer API. When sideband-all is in use, the response must be multiplexed over the sideband channel. Because the updated code fails to use the packet_writer API, 'shallow' lines are written without the '\001' band designator. The other side then interprets 's' (the first byte of 'shallow', which is 115 in decimal) as the sideband channel and dies with a protocol violation.

Thanks for pointing this out! I'll take a look!

Harald

Harald Nordgren via GitGitGadget

author

2026-10-20 13:40 UTC #3 lore raw

PATCH v2 1/1 upload-pack: fix slow fetches from a shallow repository

In reply to Harald Nordgren via GitGitGadget

From: Harald Nordgren <[email protected]>

Signed-off-by Harald Nordgren <[email protected]>

---
upload-pack: fix slow fetches from a shallow repository

Fix git fetch hanging for minutes when the remote is a shallow repository with a long history.

Changes in v2:

* Fix fetches with sideband-all by writing the shallow lines through the packet writer. Add a test that clones from a shallow repository with uploadpack.allowSidebandAll set.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2455%2FHaraldNordgren%2Fupload-pack-shallow-walk-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2455/HaraldNordgren/upload-pack-shallow-walk-v2
Pull-Request: https://github.com/git/git/pull/2455

Range-diff vs v1:

1: 4f4192120c ! 1: 8b0cb4c0e0 upload-pack: fix slow fetches from a shallow repository

     @@ shallow.c: static struct commit_list *get_shallows_or_depth(struct object_array
       				**depth_slot = cur_depth;
       			}
      
     + ## t/t5537-fetch-shallow.sh ##
     +@@ t/t5537-fetch-shallow.sh: test_expect_success 'clone from shallow clone' '
     + 	)
     + '
     + 
     ++test_expect_success 'clone from shallow clone with sideband-all' '
     ++	test_config -C shallow uploadpack.allowSidebandAll true &&
     ++	test_when_finished "rm -rf shallow-sideband-all" &&
     ++	git clone --no-local shallow shallow-sideband-all &&
     ++	(
     ++		cd shallow-sideband-all &&
     ++		git log --format=%s >actual &&
     ++		test_write_lines 4 3 >expect &&
     ++		test_cmp expect actual
     ++	)
     ++'
     ++
     + test_expect_success 'fetch from shallow clone' '
     + 	(
     + 	cd shallow &&
     +
       ## upload-pack.c ##
     -@@ upload-pack.c: static void send_shallow_info(struct upload_pack_data *data)
     +@@ upload-pack.c: static void send_wanted_ref_info(struct upload_pack_data *data)
     + 	packet_writer_delim(&data->writer);
     + }
       
     - 	packet_writer_write(&data->writer, "shallow-info\n");
     ++static int send_shallow_graft(const struct commit_graft *graft, void *cb_data)
     ++{
     ++	struct packet_writer *writer = cb_data;
     ++
     ++	if (graft->nr_parent == -1)
     ++		packet_writer_write(writer, "shallow %s\n",
     ++				    oid_to_hex(&graft->oid));
     ++	return 0;
     ++}
     ++
     + static void send_shallow_info(struct upload_pack_data *data)
     + {
     + 	/* No shallow info needs to be sent */
     +@@ upload-pack.c: static void send_shallow_info(struct upload_pack_data *data)
       
     --	if (!send_shallow_list(data) &&
     --	    is_repository_shallow(the_repository))
     + 	if (!send_shallow_list(data) &&
     + 	    is_repository_shallow(the_repository))
      -		deepen(data, INFINITE_DEPTH);
     -+	if (!send_shallow_list(data))
     -+		advertise_shallow_grafts(1);
     ++		for_each_commit_graft(send_shallow_graft, &data->writer);
       
       	packet_delim(1);
       }

 shallow.c                |  3 ++-
 t/t5537-fetch-shallow.sh | 12 ++++++++++++
 upload-pack.c            | 12 +++++++++++-
 3 files changed, 25 insertions(+), 2 deletions(-)

shallow.c +2 -1

index 8e244a5669..921db5e17a 100644
@@ -209,7 +209,8 @@ static struct commit_list *get_shallows_or_depth(struct object_array *heads,
 				*depth_slot = xmalloc(sizeof(int));
 				**depth_slot = cur_depth;
 			} else {
-				if (cur_depth >= **depth_slot)
+				if (depth == INFINITE_DEPTH ||
+				    cur_depth >= **depth_slot)
 					continue;
 				**depth_slot = cur_depth;
 			}

t/t5537-fetch-shallow.sh +12 -0

index f323ceebd2..140d5c2dba 100755
@@ -43,6 +43,18 @@ test_expect_success 'clone from shallow clone' '
 	)
 '
 
+test_expect_success 'clone from shallow clone with sideband-all' '
+	test_config -C shallow uploadpack.allowSidebandAll true &&
+	test_when_finished "rm -rf shallow-sideband-all" &&
+	git clone --no-local shallow shallow-sideband-all &&
+	(
+		cd shallow-sideband-all &&
+		git log --format=%s >actual &&
+		test_write_lines 4 3 >expect &&
+		test_cmp expect actual
+	)
+'
+
 test_expect_success 'fetch from shallow clone' '
 	(
 	cd shallow &&

upload-pack.c +11 -1

index 197bcfa279..e6dc19526c 100644
@@ -1709,6 +1709,16 @@ static void send_wanted_ref_info(struct upload_pack_data *data)
 	packet_writer_delim(&data->writer);
 }
 
+static int send_shallow_graft(const struct commit_graft *graft, void *cb_data)
+{
+	struct packet_writer *writer = cb_data;
+
+	if (graft->nr_parent == -1)
+		packet_writer_write(writer, "shallow %s\n",
+				    oid_to_hex(&graft->oid));
+	return 0;
+}
+
 static void send_shallow_info(struct upload_pack_data *data)
 {
 	/* No shallow info needs to be sent */

@@ -1720,7 +1730,7 @@ static void send_shallow_info(struct upload_pack_data *data)
 
 	if (!send_shallow_list(data) &&
 	    is_repository_shallow(the_repository))
-		deepen(data, INFINITE_DEPTH);
+		for_each_commit_graft(send_shallow_graft, &data->writer);
 
 	packet_delim(1);
 }

base-commit: c51408889c3b372814bc20b9431c3b74d0d268eb

Signature · 1 line

gitgitgadget

Harald Nordgren

2026-11-11 08:28 UTC #4 lore raw

Hi Junio!

Would you have time to take another look here?

Harald