Re: [PATCH] t1006: add tests for %(objectsize:disk)
From: René Scharfe <hidden>
Date: 2023-12-21 12:20:12
Am 21.12.23 um 10:47 schrieb Jeff King:
On Tue, Dec 19, 2023 at 05:42:39PM +0100, René Scharfe wrote:quoted
quoted
This adds a packed-object function, but I doubt anybody actually calls it. If we're going to do that, it's probably worth adding some tests for "cat-file --batch-check" or similar.Yes, and I was assuming that someone else would be eager to add such tests. *ahem*:P OK, here it is. This can be its own topic, or go on top of the rs/t6300-compressed-size-fix branch.
Great, thank you!
-- >8 -- Subject: [PATCH] t1006: add tests for %(objectsize:disk) Back when we added this placeholder in a4ac106178 (cat-file: add %(objectsize:disk) format atom, 2013-07-10), there were no tests, claiming "[...]the exact numbers returned are volatile and subject to zlib and packing decisions". But we can use a little shell hackery to get the expected numbers ourselves. To a certain degree this is just re-implementing what Git is doing under the hood, but it is still worth doing. It makes sure we exercise the %(objectsize:disk) code at all, and having the two implementations agree gives us more confidence. Note that our shell code assumes that no object appears twice (either in two packs, or as both loose and packed), as then the results really are undefined. That's OK for our purposes, and the test will notice if that assumption is violated (the shell version would produce duplicate lines that Git's output does not have). Helped-by: René Scharfe [off-list ref] Signed-off-by: Jeff King <redacted> --- I stole a bit of your awk. You can tell because I'd have written it in perl. ;)
I think we can do it even in shell, especially if...
quoted hunk ↗ jump to hunk
t/t1006-cat-file.sh | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+)diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh index 271c5e4fd3..21915be308 100755 --- a/t/t1006-cat-file.sh +++ b/t/t1006-cat-file.sh@@ -1100,6 +1100,40 @@ test_expect_success 'cat-file --batch="batman" with --batch-all-objects will wor cmp expect actual ' +test_expect_success 'cat-file %(objectsize:disk) with --batch-all-objects' ' + # our state has both loose and packed objects, + # so find both for our expected output + { + find .git/objects/?? -type f | + awk -F/ "{ print \$0, \$3\$4 }" | + while read path oid + do + size=$(test_file_size "$path") && + echo "$oid $size" || + return 1 + done && + rawsz=$(test_oid rawsz) && + find .git/objects/pack -name "*.idx" | + while read idx + do + git show-index <"$idx" >idx.raw && + sort -n <idx.raw >idx.sorted && + packsz=$(test_file_size "${idx%.idx}.pack") && + end=$((packsz - rawsz)) && + awk -v end="$end" " + NR > 1 { print oid, \$1 - start } + { start = \$1; oid = \$2 } + END { print oid, end - start } + " idx.sorted ||
... we stop slicing the data against the grain. Let's reverse the order
(sort -r), then we don't need to carry the oid forward:
sort -nr <idx.raw >idx.sorted &&
packsz=$(test_file_size "${idx%.idx}.pack") &&
end=$((packsz - rawsz)) &&
awk -v end="$end" "
{ print \$2, end - \$1; end = \$1 }
" idx.sorted ||
And at that point it should be easy to use a shell loop instead of awk:
while read start oid rest
do
size=$((end - start)) &&
end=$start &&
echo "$oid $size" ||
return 1
done <idx.sorted
+ return 1 + done + } >expect.raw && + sort <expect.raw >expect &&
The reversal above becomes irrelevant with that line, so the result in expect stays the same. Should we deduplicate here, like cat-file does (i.e. use "sort -u")? Having the same object in multiple places for whatever reason would not be a cause for reporting an error in this test, I would think.
+ git cat-file --batch-all-objects \ + --batch-check="%(objectname) %(objectsize:disk)" >actual && + test_cmp expect actual +' + test_expect_success 'set up replacement object' ' orig=$(git rev-parse HEAD) && git cat-file commit $orig >orig &&
One more thing: We can do the work of the first awk invocation in the already existing loop as well:
+test_expect_success 'cat-file %(objectsize:disk) with --batch-all-objects' '
+ # our state has both loose and packed objects,
+ # so find both for our expected output
+ {
+ find .git/objects/?? -type f |
+ awk -F/ "{ print \$0, \$3\$4 }" |
+ while read path oid
+ do
+ size=$(test_file_size "$path") &&
+ echo "$oid $size" ||
+ return 1
+ done &&
... but the substitutions are a bit awkward:
find .git/objects/?? -type f |
while read path
do
basename=${path##*/} &&
dirname=${path%/$basename} &&
oid="${dirname#.git/objects/}${basename}" &&
size=$(test_file_size "$path") &&
echo "$oid $size" ||
return 1
done &&
The avoided awk invocation might be worth the trouble, though.
René