From: "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: jonathantanmy@google.com, me@ttaylorr.com,
Derrick Stolee <dstolee@microsoft.com>,
Derrick Stolee <dstolee@microsoft.com>
Subject: [PATCH] connected.c: reprepare packs for corner cases
Date: Thu, 12 Mar 2020 15:36:43 +0000 [thread overview]
Message-ID: <pull.579.git.1584027403779.gitgitgadget@gmail.com> (raw)
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 reply other threads:[~2020-03-12 15:36 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-12 15:36 Derrick Stolee via GitGitGadget [this message]
2020-03-12 16:39 ` [PATCH] connected.c: reprepare packs for corner cases Jonathan Tan
2020-03-12 17:34 ` Derrick Stolee
2020-03-12 20:42 ` Junio C Hamano
2020-03-12 21:16 ` Jeff King
2020-03-12 21:26 ` Jeff King
2020-03-13 0:54 ` Derrick Stolee
2020-03-13 1:14 ` Junio C Hamano
2020-03-13 2:30 ` Jeff King
2020-03-13 2:34 ` Jeff King
2020-03-13 12:43 ` Derrick Stolee
2020-03-13 21:11 ` [PATCH v2] " Derrick Stolee via GitGitGadget
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=pull.579.git.1584027403779.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail.com \
--cc=dstolee@microsoft.com \
--cc=git@vger.kernel.org \
--cc=jonathantanmy@google.com \
--cc=me@ttaylorr.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.