[PATCH 0/1] Fix test bug with spaces in file owner

[PATCH 0/1] Fix test bug with spaces in file owner

[Derrick Stolee via GitGitGadget]

Thanks to Johannes Sixt for reporting this bug and Dscho for presenting the
correct test-tool to use.

This applies on ds/midx-expire-repack. If we instead want to squash this
into that branch, the two hunks will need to be squashed into these commits:

THIRD_SMALLEST_SIZE applies to ce1e4a105b4 (midx: implement midx_repack(),
2019-06-10).

MINSIZE applies to 2af890bb280 (multi-pack-index: prepare 'repack'
subcommand, 2019-06-10).

Thanks, -Stolee

Derrick Stolee (1):
  t5319: use 'test-tool path-utils' instead of 'ls -l'

 t/t5319-multi-pack-index.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)


base-commit: b526d8cbbb8740faa10411caa757c6586395fcab
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-280%2Fderrickstolee%2Fmidx-test-fix-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-280/derrickstolee/midx-test-fix-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/280
-- 
gitgitgadget

[PATCH 1/1] t5319: use 'test-tool path-utils' instead of 'ls -l'

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <dstolee@microsoft.com>

Using 'ls -l' and parsing the columns to find file sizes is
problematic when the platform could report the owner as a name
with spaces. Instead, use the 'test-tool path-utils file-size'
command to list only the sizes.

Reported-by: Johannes Sixt <j6t@kdbg.org>
Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 t/t5319-multi-pack-index.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh
index 79bfaeafa9..c72ca04399 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=$(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
@@ -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=$(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 &&
-- 
gitgitgadget

Re: [PATCH 1/1] t5319: use 'test-tool path-utils' instead of 'ls -l'

[Johannes Schindelin]

Hi Stolee,

On Mon, 1 Jul 2019, Derrick Stolee via GitGitGadget wrote:

> From: Derrick Stolee <dstolee@microsoft.com>
>
> Using 'ls -l' and parsing the columns to find file sizes is
> problematic when the platform could report the owner as a name
> with spaces. Instead, use the 'test-tool path-utils file-size'
> command to list only the sizes.

Thank you!

> diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh
> index 79bfaeafa9..c72ca04399 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=$(test-tool path-utils file-size .git/objects/pack/*pack | sort -n | head -n 1) &&

You know, we could also add a `--show-minimum` option...

> @@ -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=$(test-tool path-utils file-size .git/objects/pack/*pack | sort -n | head -n 3 | tail -n 1) &&

I guess the `--show-minimum` option is no longer such a smart idea ;-)

Patch looks good, thanks for working on this,
Dscho

>  		BATCH_SIZE=$(($THIRD_SMALLEST_SIZE + 1)) &&
>  		git multi-pack-index repack --batch-size=$BATCH_SIZE &&
>  		ls .git/objects/pack/*idx >idx-list &&
> --
> gitgitgadget
>

Re: [PATCH 1/1] t5319: use 'test-tool path-utils' instead of 'ls -l'

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Hi Stolee,
>
> On Mon, 1 Jul 2019, Derrick Stolee via GitGitGadget wrote:
>
>> From: Derrick Stolee <dstolee@microsoft.com>
>>
>> Using 'ls -l' and parsing the columns to find file sizes is
>> problematic when the platform could report the owner as a name
>> with spaces. Instead, use the 'test-tool path-utils file-size'
>> command to list only the sizes.
>
> Thank you!

Yup.  Sounds good.