Skip to content

Fix test bug with spaces in file owner #280

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Fix test bug with spaces in file owner #280

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions t/t5319-multi-pack-index.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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* &&
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Johannes Schindelin wrote (reply to this):

Hi Stolee,

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

> From: Derrick Stolee <[email protected]>
>
> 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 n=
ot 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=3D$(ls -l .git/objects/pack/*pack | awk "{print \$5;}" | sort=
 -n | head -n 1) &&
> +		MINSIZE=3D$(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 =3D 5 idx-list &&
> -		THIRD_SMALLEST_SIZE=3D$(ls -l .git/objects/pack/*pack | awk "{print \=
$5;}" | sort -n | head -n 3 | tail -n 1) &&
> +		THIRD_SMALLEST_SIZE=3D$(test-tool path-utils file-size .git/objects/p=
ack/*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=3D$(($THIRD_SMALLEST_SIZE + 1)) &&
>  		git multi-pack-index repack --batch-size=3D$BATCH_SIZE &&
>  		ls .git/objects/pack/*idx >idx-list &&
> --
> gitgitgadget
>

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Junio C Hamano wrote (reply to this):

Johannes Schindelin <[email protected]> writes:

> Hi Stolee,
>
> On Mon, 1 Jul 2019, Derrick Stolee via GitGitGadget wrote:
>
>> From: Derrick Stolee <[email protected]>
>>
>> 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.

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
Expand All @@ -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 &&
Expand Down