git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 2/3] upload-pack: make sure "want" objects are parsed

From
Jeff King <peff@peff.net>
Date
Mar 16, 2013, 10:27 UTC
Message-ID
<20130316102701.GB29533@sigill.intra.peff.net>
In-Reply-To
<20130316102428.GA29358@sigill.intra.peff.net>

When upload-pack receives a "want" line from the client, it adds it to an object array. We call lookup_object to find the actual object, which will only check for objects already in memory. This works because we are expecting to find objects that we already loaded during the ref advertisement.

We use the resulting object structs for a variety of purposes. Some of them care only about the object flags, but others care about the type of the object (e.g., ok_to_give_up), or even feed them to the revision parser (when --depth is used), which assumes that objects it receives are fully parsed.

Once upon a time, this was OK; any object we loaded into memory would also have been parsed. But since 435c833 (upload-pack: use peel_ref for ref advertisements, 2012-10-04), we try to avoid parsing objects during the ref advertisement. This means that lookup_object may return an object with a type of OBJ_NONE. The resulting mess depends on the exact set of objects, but can include the revision parser barfing, or the shallow code sending the wrong set of objects.

This patch teaches upload-pack to parse each "want" object as we receive it. We do not replace the lookup_object call with parse_object, as the current code is careful not to let just any object appear on a "want" line, but rather only one we have previously advertised (whereas parse_object would actually load any arbitrary object from disk).

Signed-off-by: Jeff King <peff@peff.net>
---
 t/t5500-fetch-pack.sh | 9 +++++++++
 upload-pack.c         | 2 +-
 2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 354d32c..d574085 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -364,6 +364,15 @@ EOF
 	test_cmp count7.expected count7.actual
 '
 
+test_expect_success 'clone shallow with packed refs' '
+	git pack-refs --all &&
+	git clone --depth 1 --branch A "file://$(pwd)/." shallow8 &&
+	echo "in-pack: 4" > count8.expected &&
+	GIT_DIR=shallow8/.git git count-objects -v |
+		grep "^in-pack" > count8.actual &&
+	test_cmp count8.expected count8.actual
+'
+
 test_expect_success 'setup tests for the --stdin parameter' '
 	for head in C D E F
 	do
diff --git a/upload-pack.c b/upload-pack.c
index 6bf81aa..41736ec 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -639,7 +639,7 @@ static void receive_needs(void)
 			use_include_tag = 1;
 
 		o = lookup_object(sha1_buf);
-		if (!o)
+		if (!o || !parse_object(o->sha1))
 			die("git upload-pack: not our ref %s",
 			    sha1_to_hex(sha1_buf));
 		if (!(o->flags & WANTED)) {
-- 
1.8.2.rc2.7.gef06216
Previous: Jeff KingNext: Jeff King
Message 3 of 9 in “fix unparsed object access in upload-pack”
  1. 0/3 fix unparsed object access in upload-packJeff King, Mar 16, 2013
  2. 1/3 upload-pack: drop lookup-before-parse optimizationJeff King, Mar 16, 2013
  3. 2/3 upload-pack: make sure "want" objects are parsedJeff King, Mar 16, 2013
  4. 3/3 upload-pack: load non-tip "want" objects from diskJeff King, Mar 16, 2013
  5. Junio C HamanoMar 17, 2013
  6. Jeff KingMar 17, 2013
  7. Junio C HamanoMar 17, 2013
  8. Jeff KingMar 17, 2013
  9. René ScharfeMar 17, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.