# [PATCH 0/1] Fix test bug with spaces in file owner 4 messages from 2019-07-01 to 2019-07-02. Participants: Derrick Stolee via GitGitGadget, Johannes Schindelin, Junio C Hamano. Thread: https://gitlist.dev/t/51411 ## Derrick Stolee via GitGitGadget, 2019-07-01 13:16 Subject: [PATCH 0/1] Fix test bug with spaces in file owner Message-ID: URL: https://gitlist.dev/e/pull.280.git.gitgitgadget%40gmail.com ``` 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 ``` ## Derrick Stolee via GitGitGadget, 2019-07-01 13:16 Subject: [PATCH 1/1] t5319: use 'test-tool path-utils' instead of 'ls -l' Message-ID: URL: https://gitlist.dev/e/fec75453eb3e4c286f84c538c142c3da02f75826.1561986978.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Derrick Stolee 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 Helped-by: Johannes Schindelin Signed-off-by: Derrick Stolee --- 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 ``` ## Johannes Schindelin, 2019-07-02 12:06 Subject: Re: [PATCH 1/1] t5319: use 'test-tool path-utils' instead of 'ls -l' Message-ID: URL: https://gitlist.dev/e/nycvar.QRO.7.76.6.1907021202000.48%40tvgsbejvaqbjf.bet In-Reply-To: ``` Hi Stolee, On Mon, 1 Jul 2019, Derrick Stolee via GitGitGadget wrote: > From: Derrick Stolee > > 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 > ``` ## Junio C Hamano, 2019-07-02 17:25 Subject: Re: [PATCH 1/1] t5319: use 'test-tool path-utils' instead of 'ls -l' Message-ID: URL: https://gitlist.dev/e/xmqqimsk4adg.fsf%40gitster-ct.c.googlers.com In-Reply-To: ``` Johannes Schindelin writes: > Hi Stolee, > > On Mon, 1 Jul 2019, Derrick Stolee via GitGitGadget wrote: > >> From: Derrick Stolee >> >> 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. ```