"Derrick Stolee via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> Second, there are two tests in t5616-partial-clone.sh that I believe are
> actually broken scenarios.
I took a look...this is a tricky one.
> While the client is set up to clone the
> 'promisor-server' repo via a treeless partial clone filter (tree:0),
> that filter does not translate to the 'server' repo. Thus, fetching from
> these repos causes the server to think that the client has all reachable
> trees and blobs from the commits advertised as 'haves'. This leads the
> server to providing a thin pack assuming those objects as delta bases.
It is expected that the server sometimes sends deltas based on objects
that the client doesn't have. In fact, this test tests the ability of
Git to lazy-fetch delta bases.
> Changing the name-hash algorithm presents new delta bases and thus
> breaks the expectations of these tests.
To be precise, the change resulted in no deltas being sent (before this
change, one delta was sent). Here's what is meant to happen. The server has:
commitB - treeB - file1 ("make the tree big\nanother line\n"), file2...file100
|
commitA - treeA - file1...file100 ("make the tree big\n")
The client only has commitA. (The client does not have treeA or any
blob, since it was cloned with --filter=tree:0.)
When GIT_TEST_FULL_NAME_HASH=0 (matching the current behavior), the
server sends a non-delta commitB, a delta treeB (with base treeA), and
a non-delta blob "make the tree big\nanother line\n". This triggers a
lazy fetch of treeA, and thus treeB is inflated successfully. During
the subsequent connectivity check (with --exclude-promisor-objects,
see connected.c), it is noticed that the "make the tree big\n" blob is
missing, but since it is a promisor object (referenced by treeA, which
was fetched from the promisor remote), the connectivity check since
passes.
When GIT_TEST_FULL_NAME_HASH=1, the server sends a non-delta commitB,
a non-delta treeB, and a non-delta blob "make the tree big\nanother
line\n". No lazy fetch is triggered. During the subsequent connectivity
check, the "make the tree big\n" blob (referenced by treeB) is missing.
There is nothing that can vouch for it (the client does not have treeA,
remember) so the client does not consider it a promisor object, and thus
the connectivity check fails.
Investigating this was made a bit harder due to a missing "git -C
promisor-remote config --local uploadpack.allowfilter 1" in the test.
The above behavior is after this is included in the test.
I think the solution is to have an algorithm that preserves the property
that treeB is sent as a delta object - if not, we need to find another
way to test the lazy-fetch of delta bases. My proposal in [1] does do
that.
[1] https://lore.kernel.org/git/20241121235014.2554033-1-jonathantanmy@xxxxxxxxxx/
