Re: [PATCH v2 3/3] t1700: make sure split-index respects core.sharedrepository

2017-06-24 Thread Christian Couder
On Sat, Jun 24, 2017 at 12:20 AM, Junio C Hamano  wrote:
> Christian Couder  writes:
>
>> Add a few tests to check that both the split-index file and the
>> shared-index file are created using the right permissions when
>> core.sharedrepository is set.
>>
>> Signed-off-by: Christian Couder 
>> ---
>>  t/t1700-split-index.sh | 17 +
>>  1 file changed, 17 insertions(+)
>>
>> diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
>> index af3ec0da5a..2c5be732e4 100755
>> --- a/t/t1700-split-index.sh
>> +++ b/t/t1700-split-index.sh
>> @@ -370,4 +370,21 @@ test_expect_success 'check splitIndex.sharedIndexExpire 
>> set to "never" and "now"
>>   test $(ls .git/sharedindex.* | wc -l) -le 2
>>  '
>>
>> +while read -r mode modebits filename; do
>
> Style.

Fixed in the version (v3) I just sent.

> Running this twice in a loop would create two .git/sharedindex.*
> files in quick succession.  I do not think we want to assume that
> the filesystem timestamp can keep up with us to allow "ls -t" to
> work reliably in the second round (if there is a leftover shared
> index from previous test, even the first round may not catch the
> latest one).

Yeah, it might be a problem on some systems.

> How about doing each iteration this way instead?  Which might be a
> better solution to work around that.
>
> - with core.sharedrepository set to false, force the index to be
>   unsplit; "index" will have the default unshared permission
>   bits (but we do not care what it is and no need to check it).
>
> - remove any leftover sharedindex.*, if any.
>
> - with core.sharedrepository set to whatever mode being tested,
>   do the adding to force split.
>
> - test the permission of index file.
>
> - test the permission of sharedindex.* file; there should be
>   only one instance, so erroring out when we see two or more is
>   also a good test.
>
> The last two steps may look like:
>
> test_modebits .git/index >actual && test_cmp expect actual &&
> shared=$(ls .git/sharedindex.*) &&
> case "$shared" in
> *" "*)
> # we have more than one???
> false ;;
> *)
> test_modebits "shared" >actual &&
> test_cmp expect actual ;;
> esac

Ok, it does what you suggest in v3.

Thanks.


Re: [PATCH v2 3/3] t1700: make sure split-index respects core.sharedrepository

2017-06-23 Thread Junio C Hamano
Christian Couder  writes:

> Add a few tests to check that both the split-index file and the
> shared-index file are created using the right permissions when
> core.sharedrepository is set.
>
> Signed-off-by: Christian Couder 
> ---
>  t/t1700-split-index.sh | 17 +
>  1 file changed, 17 insertions(+)
>
> diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
> index af3ec0da5a..2c5be732e4 100755
> --- a/t/t1700-split-index.sh
> +++ b/t/t1700-split-index.sh
> @@ -370,4 +370,21 @@ test_expect_success 'check splitIndex.sharedIndexExpire 
> set to "never" and "now"
>   test $(ls .git/sharedindex.* | wc -l) -le 2
>  '
>  
> +while read -r mode modebits filename; do

Style.

while read -r mode modebits filename
do

> + test_expect_success POSIXPERM "split index respects 
> core.sharedrepository $mode" '
> + git config core.sharedrepository "$mode" &&
> + : >"$filename" &&
> + git update-index --add "$filename" &&
> + echo "$modebits" >expect &&
> + test_modebits .git/index >actual &&
> + test_cmp expect actual &&
> + newest_shared_index=$(ls -t .git/sharedindex.* | head -1) &&
> + test_modebits "$newest_shared_index" >actual &&
> + test_cmp expect actual
> + '

Running this twice in a loop would create two .git/sharedindex.*
files in quick succession.  I do not think we want to assume that
the filesystem timestamp can keep up with us to allow "ls -t" to
work reliably in the second round (if there is a leftover shared
index from previous test, even the first round may not catch the
latest one).

How about doing each iteration this way instead?  Which might be a
better solution to work around that.

- with core.sharedrepository set to false, force the index to be
  unsplit; "index" will have the default unshared permission
  bits (but we do not care what it is and no need to check it).

- remove any leftover sharedindex.*, if any.

- with core.sharedrepository set to whatever mode being tested,
  do the adding to force split.

- test the permission of index file.

- test the permission of sharedindex.* file; there should be
  only one instance, so erroring out when we see two or more is
  also a good test.

The last two steps may look like:

test_modebits .git/index >actual && test_cmp expect actual &&
shared=$(ls .git/sharedindex.*) &&
case "$shared" in
*" "*)
# we have more than one???
false ;;
*)  
test_modebits "shared" >actual &&
test_cmp expect actual ;;
esac

> +done <<\EOF
> +0666 -rw-rw-rw- seventeen
> +0642 -rw-r---w- eightteen
> +EOF
> +
>  test_done


[PATCH v2 3/3] t1700: make sure split-index respects core.sharedrepository

2017-06-23 Thread Christian Couder
Add a few tests to check that both the split-index file and the
shared-index file are created using the right permissions when
core.sharedrepository is set.

Signed-off-by: Christian Couder 
---
 t/t1700-split-index.sh | 17 +
 1 file changed, 17 insertions(+)

diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
index af3ec0da5a..2c5be732e4 100755
--- a/t/t1700-split-index.sh
+++ b/t/t1700-split-index.sh
@@ -370,4 +370,21 @@ test_expect_success 'check splitIndex.sharedIndexExpire 
set to "never" and "now"
test $(ls .git/sharedindex.* | wc -l) -le 2
 '
 
+while read -r mode modebits filename; do
+   test_expect_success POSIXPERM "split index respects 
core.sharedrepository $mode" '
+   git config core.sharedrepository "$mode" &&
+   : >"$filename" &&
+   git update-index --add "$filename" &&
+   echo "$modebits" >expect &&
+   test_modebits .git/index >actual &&
+   test_cmp expect actual &&
+   newest_shared_index=$(ls -t .git/sharedindex.* | head -1) &&
+   test_modebits "$newest_shared_index" >actual &&
+   test_cmp expect actual
+   '
+done <<\EOF
+0666 -rw-rw-rw- seventeen
+0642 -rw-r---w- eightteen
+EOF
+
 test_done
-- 
2.13.1.519.g0a0746bea4