On 5/2/25 7:34 PM, Taylor Blau wrote:
On Mon, Mar 24, 2025 at 03:22:42PM +0000, Derrick Stolee via GitGitGadget wrote:
From: Derrick Stolee <stolee@xxxxxxxxx>
It can be notoriously difficult to detect if delta bases are being
computed properly during 'git push'. Construct an example where it will
make a kilobyte worth of difference when a delta base is not found. We
can then use the progress indicators to distinguish between bytes and
KiB depending on whether the delta base is found and used.
Signed-off-by: Derrick Stolee <stolee@xxxxxxxxx>
---
t/t5538-push-shallow.sh | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/t/t5538-push-shallow.sh b/t/t5538-push-shallow.sh
index e91fcc173e8..11b85cca9e8 100755
--- a/t/t5538-push-shallow.sh
+++ b/t/t5538-push-shallow.sh
@@ -123,4 +123,38 @@ EOF
git cat-file blob $(echo 1|git hash-object --stdin) >/dev/null
)
'
+
+test_expect_success 'push new commit from shallow clone has correct object count' '
+ git init origin &&
+ test_commit -C origin a &&
+ test_commit -C origin b &&
+
+ git clone --depth=1 "file://$(pwd)/origin" client &&
+ git -C client checkout -b topic &&
+ git -C client commit --allow-empty -m "empty" &&
+ GIT_PROGRESS_DELAY=0 git -C client push --progress origin topic 2>err &&
+ test_grep "Enumerating objects: 1, done." err
Why is a full 'git push' necessary here? Could you instead directly
invoke pack-objects as git push / send-pack does? That test_grep to
assert on the size of the pack seems very fragile to me.
Here, I think we _need_ to test at the 'git push' level, since that is the
user behavior we want to protect. Earlier testing using 'git pack-objects'
directly had not discovered this subtle issue in how it was used in the
shallow case.
It does make the test more fragile, but it's testing "the right thing"
instead of locking in an internal detail.
+
+test_expect_success 'push new commit from shallow clone has good deltas' '
+ git init base &&
+ test_seq 1 999 >base/a &&
+ test_commit -C base initial &&
+ git -C base add a &&
+ git -C base commit -m "big a" &&
I don't think it really matters, but you may want to write a test_tick
here before committing.
I don't understand why. Is the point to get a different commit time?
+ test_grep "Enumerating objects: 5, done" err &&
+
+ # If the delta base is found, then this message uses "bytes".
+ # If the delta base is not found, then this message uses "KiB".
+ test_grep "Writing objects: .* bytes" err
If we had the raw pack that was generated, could you use verify-pack
instead to check that the desired delta/base relationship was
discovered?
True, we could do that with a lot of effort, but what we really care
about is "the push was small" not "this specific delta base was
picked" (though, the delta base is how we got a small packfile).
Thanks,
-Stolee
