On Mon, Nov 28 2022, Jiang Xin wrote:
> On Mon, Nov 28, 2022 at 9:28 PM Ævar Arnfjörð Bjarmason
> <avarab@xxxxxxxxx> wrote:
>>
>>
>> On Mon, Nov 28 2022, Jiang Xin wrote:
>>
>> > From: Jiang Xin <zhiyou.jx@xxxxxxxxxxxxxxx>
>> >
>> > The template dir prepared in test case "forced modes" is not used as
>> > expected because a wrong template dir is provided to "git init". This is
>> > because the $CWD for "git-init" command is a sibling directory alongside
>> > the template directory. Change it to the right template directory and
>> > add a protection test using "test_path_is_file".
>> >
>> > The wrong template directory was introduced by mistake in commit
>> > e1df7fe43f (init: make --template path relative to $CWD, 2019-05-10).
>> >
>> > Signed-off-by: Jiang Xin <zhiyou.jx@xxxxxxxxxxxxxxx>
>> > ---
>> > t/t1301-shared-repo.sh | 3 ++-
>> > 1 file changed, 2 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh
>> > index 93a2f91f8a..7578e75d77 100755
>> > --- a/t/t1301-shared-repo.sh
>> > +++ b/t/t1301-shared-repo.sh
>> > @@ -140,7 +140,8 @@ test_expect_success POSIXPERM 'forced modes' '
>> > (
>> > cd new &&
>> > umask 002 &&
>> > - git init --shared=0660 --template=templates &&
>> > + git init --shared=0660 --template=../templates &&
>> > + test_path_is_file .git/hooks/post-update &&
>> > >frotz &&
>> > git add frotz &&
>> > git commit -a -m initial &&
>>
>> This fix looks fishy to me. The code you're changing looks like it was
>> buggy, but this looks like it's sweeping under the rug the fact that
>> "templates" never did anything at this point.
>>
>> So I'm not saying you should squash this in, but if you do so you'll see
>> that we only ever used this later.
>>
>> diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh
>> index d4315b5ef5a..106ccc5704e 100755
>> --- a/t/t1301-shared-repo.sh
>> +++ b/t/t1301-shared-repo.sh
>> @@ -129,15 +129,12 @@ test_expect_success POSIXPERM 'git reflog expire honors core.sharedRepository' '
>> '
>>
>> test_expect_success POSIXPERM 'forced modes' '
>> - mkdir -p templates/hooks &&
>> - echo update-server-info >templates/hooks/post-update &&
>> - chmod +x templates/hooks/post-update &&
>
> The "post-update" is used in this test case. A wrong template dir
> leads to an empty hooks dir in "new/", that cause the test at the
> end of this test case passed by accident.
>
> # post-update hook must be 0770
> test -z "$(sed -n -e "/post-update/{
> /^-rwxrwx---/d
> p
> }" actual)" &&
Yes exactly, which is what I was pointing out with "isn't the real
fix...?". I.e. this does seem to improve things, but as this shows this
test is really fragile already.
So I think if we're poking at this it makes sense to change that "test
-z" to use "test_modebits" or something instead, so it'll actually do
what we expect, and not just silently pass if the hook is missing...
