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

[PATCH] connected.c: reprepare packs for corner cases

From
Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>
Date
Mar 12, 2020, 15:36 UTC
Message-ID
<pull.579.git.1584027403779.gitgitgadget@gmail.com>
From: Derrick Stolee <dstolee@microsoft.com>

While updating the microsoft/git fork on top of v2.26.0-rc0 and consuming that build into Scalar, I noticed a corner case bug around partial clone.

The "scalar clone" command can create a Git repository with the proper config for using partial clone with the "blob:none" filter. Instead of calling "git clone", it runs "git init" then sets a few more config values before running "git fetch".

In our builds on v2.26.0-rc0, we noticed that our "git fetch" command was failing with

  error: https://github.com/microsoft/scalar did not send all necessary objects

This does not happen if you copy the config file from a repository created by "git clone --filter=blob:none <url>", but it does happen when adding the config option "core.logAllRefUpdates = true".

By debugging, I was able to see that the loop inside check_connnected() that checks if all refs are contained in promisor packs actually did not have any packfiles in the packed_git list.

I'm not sure what corner-case issues caused this config option to prevent the reprepare_packed_git() from being called at the proper spot during the fetch operation. Even worse, I have failed to create a test case to prevent a regression.

Placing a reprepare_packed_git() call inside chck_connected() before looping through the packed_git list seems like the safest way to avoid this issue in the future. While reprepare_packed_git() does another scan of the pack directory, it is not terribly expensive as long as we do not run it in a loop. We check connectivity only a few times per command, so this will not have a meaningful performance impact. In exchange, we get extra safety around this check.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
    connected.c: reprepare packs for corner cases
    
    I included how I found this (integrating v2.26.0-rc0 into Scalar), but I
    am able to reproduce it on my Linux machine using real fetches from
    github.com. I'm not sure why I was unable to reproduce the issue in test
    cases using the file:// URLs or the HTTP tests.
    
    Thanks, -Stolee
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-579%2Fderrickstolee%2Ffetch-reprepare-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-579/derrickstolee/fetch-reprepare-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/579
 connected.c | 4 ++++
 1 file changed, 4 insertions(+)
diff --git a/connected.c b/connected.c
index 7e9bd1bc622..ac52b07b474 100644
--- a/connected.c
+++ b/connected.c
@@ -61,7 +61,11 @@ int check_connected(oid_iterate_fn fn, void *cb_data,
 		 * object is a promisor object. Instead, just make sure we
 		 * received, in a promisor packfile, the objects pointed to by
 		 * each wanted ref.
+		 *
+		 * Before checking for promisor packs, be sure we have the
+		 * latest pack-files loaded into memory.
 		 */
+		reprepare_packed_git(the_repository);
 		do {
 			struct packed_git *p;
 

base-commit: b4374e96c84ed9394fed363973eb540da308ed4f
-- 
gitgitgadget
Next: Jonathan Tan
Message 1 of 12 in “connected.c: reprepare packs for corner cases”
  1. connected.c: reprepare packs for corner casesDerrick Stolee via GitGitGadget, Mar 12, 2020
  2. Jonathan TanMar 12, 2020
  3. Derrick StoleeMar 12, 2020
  4. Junio C HamanoMar 12, 2020
  5. Jeff KingMar 12, 2020
  6. Jeff KingMar 12, 2020
  7. Derrick StoleeMar 13, 2020
  8. Junio C HamanoMar 13, 2020
  9. Jeff KingMar 13, 2020
  10. Jeff KingMar 13, 2020
  11. Derrick StoleeMar 13, 2020
  12. connected.c: reprepare packs for corner casesDerrick Stolee via GitGitGadget, Mar 13, 2020

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.