Re: [PATCH] subdirectory tests: code cleanup, uncomment test

From: Stefan Beller <sbeller@google.com>
Date: May 18, 2015, 21:29 UTC
Message-ID: <CAGZ79kZ9h=Oev_mymFBRNyAGokAe8-vWh_D12VZYHTUv20Djvg@mail.gmail.com>
In-Reply-To: <xmqqvbfpa8ff.fsf@gitster.dls.corp.google.com>

On Mon, May 18, 2015 at 2:21 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 29 quoted lines

> Stefan Beller <sbeller@google.com> writes:
>
>> Originally the test in t1020 was meant to not include setting the GIT_DIR
>> when testing inside a bare repository as it did not work without setting
>> GIT_DIR explicitly.
>>
>> Nowadays the test as originally intended works, so add it to the test
>> suite. We'll keep the test, which has been run through all years as another
>> test for finding regressions.
>>
>> Signed-off-by: Stefan Beller <sbeller@google.com>
>> ---
>>
>> Junio, thanks for providing the context!
>>
>> I tried tracking down when this changes via bisect, though I messed up.
>> By looking through the code I find these commits most promising to have
>> fixed the underlying issue (I am no expert on subdirectory treatment)
>> 337e51c (Use git_config_early() instead of git_config() during repo setup, 2010-11-26)
>> 72183cb (Fix gitdir detection when in subdir of gitdir, 2009-01-16)
>> 9951d3b (setup: clean up setup_discovered_git_dir(), 2010-11-26)
>
> Thanks for digging.
>
> I personally do not think we would need to say "historic" (as it
> makes it sound as if we do not care if the use case is deprecated
> and dropped in the future) but I do not offhand think of a better
> label for that test (other than doing the cop-out "test (1)" vs
> "test (2)"), so let's queue this as-is.

At first I was in the mood of labeling that test

    no file/rev ambiguity check inside a bare repo with GIT_DIR help
for older Gits

as in nursing homes for elder people, but I refrained from doing so as it sounded derogatory in my mind and it broke the line limit.

I am not happy with (historic) either, maybe "(explicit GIT_DIR)" is describing the test better without giving the reader the thoughts as you raised here?

Show 42 quoted lines

>
>
>>
>>  t/t1020-subdirectory.sh | 11 +++++++----
>>  1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/t/t1020-subdirectory.sh b/t/t1020-subdirectory.sh
>> index 2edb4f2..022641d 100755
>> --- a/t/t1020-subdirectory.sh
>> +++ b/t/t1020-subdirectory.sh
>> @@ -162,16 +162,20 @@ test_expect_success 'no file/rev ambiguity check inside .git' '
>>       )
>>  '
>>
>> -test_expect_success 'no file/rev ambiguity check inside a bare repo' '
>> +test_expect_success '(historic) no file/rev ambiguity check inside a bare repo' '
>> +     test_when_finished "rm -fr foo.git" &&
>>       git clone -s --bare .git foo.git &&
>>       (
>>               cd foo.git &&
>> +             # older Git needed help by exporting GIT_DIR=.
>> +             # to realize that it is inside a bare repository.
>> +             # We keep this test around for regression testing.
>>               GIT_DIR=. git show -s HEAD
>>       )
>>  '
>>
>> -# This still does not work as it should...
>> -: test_expect_success 'no file/rev ambiguity check inside a bare repo' '
>> +test_expect_success 'no file/rev ambiguity check inside a bare repo' '
>> +     test_when_finished "rm -fr foo.git" &&
>>       git clone -s --bare .git foo.git &&
>>       (
>>               cd foo.git &&
>> @@ -180,7 +184,6 @@ test_expect_success 'no file/rev ambiguity check inside a bare repo' '
>>  '
>>
>>  test_expect_success SYMLINKS 'detection should not be fooled by a symlink' '
>> -     rm -fr foo.git &&
>>       git clone -s .git another &&
>>       ln -s another yetanother &&
>>       (