| Message ID | d4a3f56b-5eaa-1325-f8c1-be6797a9ac03@kdbg.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | t5319: don't trip over a user name with whitespace | expand |
On Sun, Jun 30, 2019 at 2:57 PM Johannes Sixt <j6t@kdbg.org> wrote: > On my Windows system where the POSIX commands are provided by MSYS2, > I observe this output: > > $ ls -l Makefile > -rw-r--r-- 1 Johannes Sixt 197121 101780 Jun 30 09:33 Makefile > > Notice the blank in the user name. Obviously, extracting the size > of a file by counting columns won't work. But two tests in t5319 > do that. Use the stat command to print just the file size. > > Signed-off-by: Johannes Sixt <j6t@kdbg.org> > --- > diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh > @@ -443,7 +443,7 @@ test_expect_success 'repack with minimum size does not alter existing packs' ' > - MINSIZE=$(ls -l .git/objects/pack/*pack | awk "{print \$5;}" | sort -n | head -n 1) && > + MINSIZE=$(stat -c %s .git/objects/pack/*pack | sort -n | head -n 1) && Unfortunately, this is not portable. While "stat -c %s" works on Linux and MSYS2, neither that option nor the format directive are recognized on BSD-like platforms (I tested Mac OS and FreeBSD), which instead need "stat -f %z".
Am 30.06.19 um 21:48 schrieb Eric Sunshine: > On Sun, Jun 30, 2019 at 2:57 PM Johannes Sixt <j6t@kdbg.org> wrote: >> diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh >> @@ -443,7 +443,7 @@ test_expect_success 'repack with minimum size does not alter existing packs' ' >> - MINSIZE=$(ls -l .git/objects/pack/*pack | awk "{print \$5;}" | sort -n | head -n 1) && >> + MINSIZE=$(stat -c %s .git/objects/pack/*pack | sort -n | head -n 1) && > > Unfortunately, this is not portable. While "stat -c %s" works on Linux > and MSYS2, neither that option nor the format directive are recognized > on BSD-like platforms (I tested Mac OS and FreeBSD), which instead > need "stat -f %z". Ouch! I did notice that stat(1) is not in POSIX, but hoped that it was sufficiently portable. I need a new idea... -- Hannes
On Sun, Jun 30, 2019 at 10:59:34PM +0200, Johannes Sixt wrote: > Am 30.06.19 um 21:48 schrieb Eric Sunshine: > > On Sun, Jun 30, 2019 at 2:57 PM Johannes Sixt <j6t@kdbg.org> wrote: > >> diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh > >> @@ -443,7 +443,7 @@ test_expect_success 'repack with minimum size does not alter existing packs' ' > >> - MINSIZE=$(ls -l .git/objects/pack/*pack | awk "{print \$5;}" | sort -n | head -n 1) && > >> + MINSIZE=$(stat -c %s .git/objects/pack/*pack | sort -n | head -n 1) && > > > > Unfortunately, this is not portable. While "stat -c %s" works on Linux > > and MSYS2, neither that option nor the format directive are recognized > > on BSD-like platforms (I tested Mac OS and FreeBSD), which instead > > need "stat -f %z". > > Ouch! I did notice that stat(1) is not in POSIX, but hoped that it was > sufficiently portable. I need a new idea... If we are OK relying on rudimentary perl[1], then: perl -le "print((stat)[7]) for @ARGV" works. If you want it more readable, then maybe: perl -MFile::stat -le "print stat(\$_)->size for @ARGV" Both of those should work with even antique perl versions. -Peff [1] I'd also be fine with implementing a test-tool helper in C that behaves like stat(1). Or we could punt on that until somebody feels like trying to eradicate perl (because this is far from the first perl one-liner in the test suite).
Am 01.07.19 um 00:25 schrieb Jeff King: > On Sun, Jun 30, 2019 at 10:59:34PM +0200, Johannes Sixt wrote: > >> Am 30.06.19 um 21:48 schrieb Eric Sunshine: >>> On Sun, Jun 30, 2019 at 2:57 PM Johannes Sixt <j6t@kdbg.org> wrote: >>>> diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh >>>> @@ -443,7 +443,7 @@ test_expect_success 'repack with minimum size does not alter existing packs' ' >>>> - MINSIZE=$(ls -l .git/objects/pack/*pack | awk "{print \$5;}" | sort -n | head -n 1) && >>>> + MINSIZE=$(stat -c %s .git/objects/pack/*pack | sort -n | head -n 1) && >>> >>> Unfortunately, this is not portable. While "stat -c %s" works on Linux >>> and MSYS2, neither that option nor the format directive are recognized >>> on BSD-like platforms (I tested Mac OS and FreeBSD), which instead >>> need "stat -f %z". >> >> Ouch! I did notice that stat(1) is not in POSIX, but hoped that it was >> sufficiently portable. I need a new idea... > > If we are OK relying on rudimentary perl[1], then: > > perl -le "print((stat)[7]) for @ARGV" I'm fine with that. But then we should do the sort + head -n 1 at the same time. I can do that with a small script, but I'm sure it's possible in a one-liner... -- Hannes
On Sun, Jun 30, 2019 at 10:59:34PM +0200, Johannes Sixt wrote: > Am 30.06.19 um 21:48 schrieb Eric Sunshine: > > On Sun, Jun 30, 2019 at 2:57 PM Johannes Sixt <j6t@kdbg.org> wrote: > >> diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh > >> @@ -443,7 +443,7 @@ test_expect_success 'repack with minimum size does not alter existing packs' ' > >> - MINSIZE=$(ls -l .git/objects/pack/*pack | awk "{print \$5;}" | sort -n | head -n 1) && > >> + MINSIZE=$(stat -c %s .git/objects/pack/*pack | sort -n | head -n 1) && > > > > Unfortunately, this is not portable. While "stat -c %s" works on Linux > > and MSYS2, neither that option nor the format directive are recognized > > on BSD-like platforms (I tested Mac OS and FreeBSD), which instead > > need "stat -f %z". > > Ouch! I did notice that stat(1) is not in POSIX, but hoped that it was > sufficiently portable. I need a new idea... 'wc -c' perhaps? We already use it in a couple of places in the test suite to get file size. diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh index 79bfaeafa9..ddba862114 100755 --- a/t/t5319-multi-pack-index.sh +++ b/t/t5319-multi-pack-index.sh @@ -443,7 +443,7 @@ test_expect_success 'repack with minimum size does not alter existing packs' ' touch -m -t 201901010002 .git/objects/pack/pack-B* && touch -m -t 201901010003 .git/objects/pack/pack-A* && ls .git/objects/pack >expect && - MINSIZE=$(ls -l .git/objects/pack/*pack | awk "{print \$5;}" | sort -n | head -n 1) && + MINSIZE=$(wc -c .git/objects/pack/*pack | sort -n | sed -n -e "s/^ *//" -e "1 s/[^ ]*$//p") && git multi-pack-index repack --batch-size=$MINSIZE && ls .git/objects/pack >actual && test_cmp expect actual @@ -455,7 +455,7 @@ test_expect_success 'repack creates a new pack' ' cd dup && ls .git/objects/pack/*idx >idx-list && test_line_count = 5 idx-list && - THIRD_SMALLEST_SIZE=$(ls -l .git/objects/pack/*pack | awk "{print \$5;}" | sort -n | head -n 3 | tail -n 1) && + THIRD_SMALLEST_SIZE=$(wc -c .git/objects/pack/*pack | sort -n | sed -n -e "s/^ *//" -e "3 s/[^ ]*$//p") && BATCH_SIZE=$(($THIRD_SMALLEST_SIZE + 1)) && git multi-pack-index repack --batch-size=$BATCH_SIZE && ls .git/objects/pack/*idx >idx-list && Note that 'wc -c' prints the numbers in its output left padded, hence the 'sed "s/^ *//"' above, and once 'sed' is already up and running, it might as well take over the role of that 'head -n 3 | tail -n 1'.
On Mon, Jul 01, 2019 at 08:33:18AM +0200, Johannes Sixt wrote: > > If we are OK relying on rudimentary perl[1], then: > > > > perl -le "print((stat)[7]) for @ARGV" > > I'm fine with that. But then we should do the sort + head -n 1 at the > same time. I can do that with a small script, but I'm sure it's possible > in a one-liner... Something like: perl -le 'print((sort { $a <=> $b } map { (stat)[7] } @ARGV)[0])' but that is getting pretty unreadable. If we rely on List::Util, it's a bit nicer: perl -MList::Util=min -e 'print min(map { (stat)[7] } @ARGV)' but that implies perl v5.7.3. Which is from 2002, and older than our usual minimum-perl version, but we've typically been very conservative with these one-liners, since they do not respect NO_PERL. Probably writing it out like: perl -le ' my @sizes = map { (stat)[7] } @ARGV; @sizes = sort { $a <=> $b } @sizes; print $size[0]; ' would be better (and makes "3rd-smallest" a trivial change). I see Gábor suggested using "wc -c" elsewhere in the thread. That would be fine with me, too, though I think the required sed there may be getting pretty unreadable, too. :) -Peff
On Mon, Jul 01, 2019 at 05:16:02AM -0400, Jeff King wrote: > I see Gábor suggested using "wc -c" elsewhere in the thread. That would > be fine with me, too, though I think the required sed there may be > getting pretty unreadable, too. :) It could be done even without 'sed', though at the expense of running a coupe more 'wc -c's in a loop: diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh index 79bfaeafa9..bacec5e2e4 100755 --- a/t/t5319-multi-pack-index.sh +++ b/t/t5319-multi-pack-index.sh @@ -443,7 +443,12 @@ test_expect_success 'repack with minimum size does not alter existing packs' ' touch -m -t 201901010002 .git/objects/pack/pack-B* && touch -m -t 201901010003 .git/objects/pack/pack-A* && ls .git/objects/pack >expect && - MINSIZE=$(ls -l .git/objects/pack/*pack | awk "{print \$5;}" | sort -n | head -n 1) && + MINSIZE=$( + for pack in .git/objects/pack/*pack + do + wc -c <"$pack" + done | sort -n | head -n 1 + ) && git multi-pack-index repack --batch-size=$MINSIZE && ls .git/objects/pack >actual && test_cmp expect actual @@ -455,7 +460,12 @@ test_expect_success 'repack creates a new pack' ' cd dup && ls .git/objects/pack/*idx >idx-list && test_line_count = 5 idx-list && - THIRD_SMALLEST_SIZE=$(ls -l .git/objects/pack/*pack | awk "{print \$5;}" | sort -n | head -n 3 | tail -n 1) && + THIRD_SMALLEST_SIZE=$( + for pack in .git/objects/pack/*pack + do + wc -c <"$pack" + done | sort -n | head -n 3 | tail -n 1 + ) && BATCH_SIZE=$(($THIRD_SMALLEST_SIZE + 1)) && git multi-pack-index repack --batch-size=$BATCH_SIZE && ls .git/objects/pack/*idx >idx-list && Is it really better? Dunno, but at least there is no subtlety with the leading padding spaces.
Sorry I'm late to the thread. Thanks for reporting this issue, Johannes. On 7/1/2019 7:33 AM, SZEDER Gábor wrote: > On Mon, Jul 01, 2019 at 05:16:02AM -0400, Jeff King wrote: >> I see Gábor suggested using "wc -c" elsewhere in the thread. That would >> be fine with me, too, though I think the required sed there may be >> getting pretty unreadable, too. :) > > It could be done even without 'sed', though at the expense of running > a coupe more 'wc -c's in a loop:> > diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh > index 79bfaeafa9..bacec5e2e4 100755 > --- a/t/t5319-multi-pack-index.sh > +++ b/t/t5319-multi-pack-index.sh > @@ -443,7 +443,12 @@ test_expect_success 'repack with minimum size does not alter existing packs' ' > touch -m -t 201901010002 .git/objects/pack/pack-B* && > touch -m -t 201901010003 .git/objects/pack/pack-A* && > ls .git/objects/pack >expect && > - MINSIZE=$(ls -l .git/objects/pack/*pack | awk "{print \$5;}" | sort -n | head -n 1) && > + MINSIZE=$( > + for pack in .git/objects/pack/*pack > + do > + wc -c <"$pack" > + done | sort -n | head -n 1 > + ) && > git multi-pack-index repack --batch-size=$MINSIZE && > ls .git/objects/pack >actual && > test_cmp expect actual > @@ -455,7 +460,12 @@ test_expect_success 'repack creates a new pack' ' > cd dup && > ls .git/objects/pack/*idx >idx-list && > test_line_count = 5 idx-list && > - THIRD_SMALLEST_SIZE=$(ls -l .git/objects/pack/*pack | awk "{print \$5;}" | sort -n | head -n 3 | tail -n 1) && > + THIRD_SMALLEST_SIZE=$( > + for pack in .git/objects/pack/*pack > + do > + wc -c <"$pack" > + done | sort -n | head -n 3 | tail -n 1 > + ) && > BATCH_SIZE=$(($THIRD_SMALLEST_SIZE + 1)) && > git multi-pack-index repack --batch-size=$BATCH_SIZE && > ls .git/objects/pack/*idx >idx-list && > > Is it really better? Dunno, but at least there is no subtlety with > the leading padding spaces. Your change is certainly more straight-forward, and avoids all issues around Perl compatibility. It does have the issue of more 'wc' processes, which is a downside. The count here is not too bad, but if we need to duplicate this pattern elsewhere we may be better off creating a stat(1) replacement in test-lib.sh. Thanks, -Stolee
Hi Peff, On Sun, 30 Jun 2019, Jeff King wrote: > On Sun, Jun 30, 2019 at 10:59:34PM +0200, Johannes Sixt wrote: > > > Am 30.06.19 um 21:48 schrieb Eric Sunshine: > > > On Sun, Jun 30, 2019 at 2:57 PM Johannes Sixt <j6t@kdbg.org> wrote: > > >> diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh > > >> @@ -443,7 +443,7 @@ test_expect_success 'repack with minimum size does not alter existing packs' ' > > >> - MINSIZE=$(ls -l .git/objects/pack/*pack | awk "{print \$5;}" | sort -n | head -n 1) && > > >> + MINSIZE=$(stat -c %s .git/objects/pack/*pack | sort -n | head -n 1) && > > > > > > Unfortunately, this is not portable. While "stat -c %s" works on Linux > > > and MSYS2, neither that option nor the format directive are recognized > > > on BSD-like platforms (I tested Mac OS and FreeBSD), which instead > > > need "stat -f %z". > > > > Ouch! I did notice that stat(1) is not in POSIX, but hoped that it was > > sufficiently portable. I need a new idea... > > If we are OK relying on rudimentary perl[1], then: > > perl -le "print((stat)[7]) for @ARGV" > > works. If you want it more readable, then maybe: > > perl -MFile::stat -le "print stat(\$_)->size for @ARGV" Or we stop introducing new Perl calls, and use the perfectly fine `test-tool path-utils file-size` command: https://github.com/git/git/blob/v2.22.0/t/helper/test-path-utils.c#L302-L312 This solves not only portability problems but also avoids yet another obstacle into making a `NO_PERL` test suite run really work without Perl. Ciao, Dscho
On 7/1/2019 8:11 AM, Johannes Schindelin wrote: > Hi Peff, > > On Sun, 30 Jun 2019, Jeff King wrote: > >> On Sun, Jun 30, 2019 at 10:59:34PM +0200, Johannes Sixt wrote: >> >>> Am 30.06.19 um 21:48 schrieb Eric Sunshine: >>>> On Sun, Jun 30, 2019 at 2:57 PM Johannes Sixt <j6t@kdbg.org> wrote: >>>>> diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh >>>>> @@ -443,7 +443,7 @@ test_expect_success 'repack with minimum size does not alter existing packs' ' >>>>> - MINSIZE=$(ls -l .git/objects/pack/*pack | awk "{print \$5;}" | sort -n | head -n 1) && >>>>> + MINSIZE=$(stat -c %s .git/objects/pack/*pack | sort -n | head -n 1) && >>>> >>>> Unfortunately, this is not portable. While "stat -c %s" works on Linux >>>> and MSYS2, neither that option nor the format directive are recognized >>>> on BSD-like platforms (I tested Mac OS and FreeBSD), which instead >>>> need "stat -f %z". >>> >>> Ouch! I did notice that stat(1) is not in POSIX, but hoped that it was >>> sufficiently portable. I need a new idea... >> >> If we are OK relying on rudimentary perl[1], then: >> >> perl -le "print((stat)[7]) for @ARGV" >> >> works. If you want it more readable, then maybe: >> >> perl -MFile::stat -le "print stat(\$_)->size for @ARGV" > > Or we stop introducing new Perl calls, and use the perfectly fine > `test-tool path-utils file-size` command: > > https://github.com/git/git/blob/v2.22.0/t/helper/test-path-utils.c#L302-L312 > > This solves not only portability problems but also avoids yet another > obstacle into making a `NO_PERL` test suite run really work without Perl. Thanks! This does seem like the best option. Thanks for bringing this to our attention. Here is a diff, and I'll prepare a full patch: diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh index dd6083e61a2..5379e59168a 100755 --- a/t/t5319-multi-pack-index.sh +++ b/t/t5319-multi-pack-index.sh @@ -447,7 +447,7 @@ test_expect_success 'repack with minimum size does not alter existing packs' ' touch -m -t 201901010002 .git/objects/pack/pack-B* && touch -m -t 201901010003 .git/objects/pack/pack-A* && ls .git/objects/pack >expect && - MINSIZE=$(ls -l .git/objects/pack/*pack | awk "{print \$5;}" | sort -n | head -n 1) && + MINSIZE=$(test-tool path-utils file-size .git/objects/pack/*pack | sort -n | head -n 1) && git multi-pack-index repack --batch-size=$MINSIZE && ls .git/objects/pack >actual && test_cmp expect actual @@ -459,7 +459,7 @@ test_expect_success 'repack creates a new pack' ' cd dup && ls .git/objects/pack/*idx >idx-list && test_line_count = 5 idx-list && - THIRD_SMALLEST_SIZE=$(ls -l .git/objects/pack/*pack | awk "{print \$5;}" | sort -n | head -n 3 | tail -n 1) && + THIRD_SMALLEST_SIZE=$(test-tool path-utils file-size .git/objects/pack/*pack | sort -n | head -n 3 | tail -n 1) && BATCH_SIZE=$(($THIRD_SMALLEST_SIZE + 1)) && git multi-pack-index repack --batch-size=$BATCH_SIZE && ls .git/objects/pack/*idx >idx-list && Thanks, -Stolee
On Mon, Jul 01, 2019 at 02:11:43PM +0200, Johannes Schindelin wrote: > Or we stop introducing new Perl calls, and use the perfectly fine > `test-tool path-utils file-size` command: > > https://github.com/git/git/blob/v2.22.0/t/helper/test-path-utils.c#L302-L312 Ah, thanks, I missed that we had already added it. Certainly that seems like the right solution then. -Peff
On Jun 30 2019, Johannes Sixt <j6t@kdbg.org> wrote: > diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh > index 79bfaeafa9..4b4d06a1c8 100755 > --- a/t/t5319-multi-pack-index.sh > +++ b/t/t5319-multi-pack-index.sh > @@ -443,7 +443,7 @@ test_expect_success 'repack with minimum size does not alter existing packs' ' > touch -m -t 201901010002 .git/objects/pack/pack-B* && > touch -m -t 201901010003 .git/objects/pack/pack-A* && > ls .git/objects/pack >expect && > - MINSIZE=$(ls -l .git/objects/pack/*pack | awk "{print \$5;}" | sort -n | head -n 1) && > + MINSIZE=$(stat -c %s .git/objects/pack/*pack | sort -n | head -n 1) && You could also use ls -n. Andreas.
Am 01.07.19 um 14:30 schrieb Derrick Stolee: > On 7/1/2019 8:11 AM, Johannes Schindelin wrote: >> Or we stop introducing new Perl calls, and use the perfectly fine >> `test-tool path-utils file-size` command: >> >> https://github.com/git/git/blob/v2.22.0/t/helper/test-path-utils.c#L302-L312 >> >> This solves not only portability problems but also avoids yet another >> obstacle into making a `NO_PERL` test suite run really work without Perl. > Thanks! This does seem like the best option. Thanks for bringing this to our > attention. Here is a diff, and I'll prepare a full patch: Thanks. Please also explain why the first of the two tests does not fail with a large --batch-size (unless it is obvious for people who know a bit about multi-pack-index, of course). -- Hannes
On 7/1/2019 2:22 PM, Johannes Sixt wrote: > Am 01.07.19 um 14:30 schrieb Derrick Stolee: >> On 7/1/2019 8:11 AM, Johannes Schindelin wrote: >>> Or we stop introducing new Perl calls, and use the perfectly fine >>> `test-tool path-utils file-size` command: >>> >>> https://github.com/git/git/blob/v2.22.0/t/helper/test-path-utils.c#L302-L312 >>> >>> This solves not only portability problems but also avoids yet another >>> obstacle into making a `NO_PERL` test suite run really work without Perl. >> Thanks! This does seem like the best option. Thanks for bringing this to our >> attention. Here is a diff, and I'll prepare a full patch: > > Thanks. Please also explain why the first of the two tests does not fail > with a large --batch-size (unless it is obvious for people who know a > bit about multi-pack-index, of course). Sorry about missing that. Here is an explanation: The --batch-size=X option determines how much data we want to move in a single operation. Based on this value, we select packs greedily as follows: 1. If a pack has size at most X, include it. 2. If our total pack size is greater than X, stop. At the end of this process, if we have zero or one packs we do nothing. That is why a small size does nothing. Also, if our total pack size is smaller than X, we do nothing. Our batch is not "full" enough to merit the work. This is why a large size does nothing. Thanks, -Stolee
Andreas Schwab <schwab@linux-m68k.org> writes: > On Jun 30 2019, Johannes Sixt <j6t@kdbg.org> wrote: > >> diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh >> index 79bfaeafa9..4b4d06a1c8 100755 >> --- a/t/t5319-multi-pack-index.sh >> +++ b/t/t5319-multi-pack-index.sh >> @@ -443,7 +443,7 @@ test_expect_success 'repack with minimum size does not alter existing packs' ' >> touch -m -t 201901010002 .git/objects/pack/pack-B* && >> touch -m -t 201901010003 .git/objects/pack/pack-A* && >> ls .git/objects/pack >expect && >> - MINSIZE=$(ls -l .git/objects/pack/*pack | awk "{print \$5;}" | sort -n | head -n 1) && >> + MINSIZE=$(stat -c %s .git/objects/pack/*pack | sort -n | head -n 1) && > > You could also use ls -n. Nice ;-)
diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh index 79bfaeafa9..4b4d06a1c8 100755 --- a/t/t5319-multi-pack-index.sh +++ b/t/t5319-multi-pack-index.sh @@ -443,7 +443,7 @@ test_expect_success 'repack with minimum size does not alter existing packs' ' touch -m -t 201901010002 .git/objects/pack/pack-B* && touch -m -t 201901010003 .git/objects/pack/pack-A* && ls .git/objects/pack >expect && - MINSIZE=$(ls -l .git/objects/pack/*pack | awk "{print \$5;}" | sort -n | head -n 1) && + MINSIZE=$(stat -c %s .git/objects/pack/*pack | sort -n | head -n 1) && git multi-pack-index repack --batch-size=$MINSIZE && ls .git/objects/pack >actual && test_cmp expect actual @@ -455,7 +455,7 @@ test_expect_success 'repack creates a new pack' ' cd dup && ls .git/objects/pack/*idx >idx-list && test_line_count = 5 idx-list && - THIRD_SMALLEST_SIZE=$(ls -l .git/objects/pack/*pack | awk "{print \$5;}" | sort -n | head -n 3 | tail -n 1) && + THIRD_SMALLEST_SIZE=$(stat -c %s .git/objects/pack/*pack | sort -n | head -n 3 | tail -n 1) && BATCH_SIZE=$(($THIRD_SMALLEST_SIZE + 1)) && git multi-pack-index repack --batch-size=$BATCH_SIZE && ls .git/objects/pack/*idx >idx-list &&
On my Windows system where the POSIX commands are provided by MSYS2, I observe this output: $ ls -l Makefile -rw-r--r-- 1 Johannes Sixt 197121 101780 Jun 30 09:33 Makefile Notice the blank in the user name. Obviously, extracting the size of a file by counting columns won't work. But two tests in t5319 do that. Use the stat command to print just the file size. Signed-off-by: Johannes Sixt <j6t@kdbg.org> --- Notice further that the group ID is presented numerically. The actual output used in the tests is: -r--r--r-- 1 Johannes Sixt 197121 2239 Jan 1 01:03 .git/objects/pack/pack-A-40289997979ee9f2d7198a5b4ac9fe14e27cbe4b.pack -r--r--r-- 1 Johannes Sixt 197121 1385 Jan 1 01:02 .git/objects/pack/pack-B-926e20b0e641003b2f6bcddd6ccf53e7741c8642.pack -r--r--r-- 1 Johannes Sixt 197121 838 Jan 1 01:01 .git/objects/pack/pack-C-b33c048d7202659249bb0cea7bbc997ce448c75d.pack -r--r--r-- 1 Johannes Sixt 197121 560 Jan 1 01:00 .git/objects/pack/pack-D-49ca36507a52bbba6d1768e28666eb0f4e5f7d95.pack -r--r--r-- 1 Johannes Sixt 197121 4587 Jun 30 16:17 .git/objects/pack/pack-E-31ea0966bb15206fbfd1b4c2f5be4b24a03050f5.pack In both tests, this group ID was passed as --batch-size, but actually the first of the two tests does not fail. Now I have to wonder: Why that? The test carefully attempts to extract the smallest pack size to pass as --batch-size argument. But when it passes some large number, the test still passes. Is this the intent? That is, both a small and a large --batch-size satisfy 'does not alter existing packs', but some medium sized --batch-size satisfy the next test 'repack creates a new pack'. That is very curious. Disclaimer: I've no clue about this multi-pack-index thing. t/t5319-multi-pack-index.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)