Re: [PATCH] submodule: add more exhaustive up-path testing

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Aug 14, 2018, 21:05 UTC
Message-ID: <87d0ukhd5g.fsf@evledraar.gmail.com>
In-Reply-To: <CAGZ79kaK8Wt0TGvo-PyDZRLWr9PU0BRo4=DiYUXvv8c8nZ+M8A@mail.gmail.com>

On Tue, Aug 14 2018, Stefan Beller wrote:

Show 17 quoted lines

> On Tue, Aug 14, 2018 at 11:59 AM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>>
>> The tests added in 63e95beb08 ("submodule: port resolve_relative_url
>> from shell to C", 2016-04-15) didn't do a good job of testing various
>> up-path invocations where the up-path would bring us beyond even the
>> URL in question without emitting an error.
>>
>> These results look nonsensical, but it's worth exhaustively testing
>> them before fixing any of this code, so we can see which of these
>> cases were changed.
>
> Yeah. Please look at the comment in builtin/submodule--helper.c
> in that commit, where I described my expectations.
>
> I should have put them into tests instead with the expectations
> spelled out there.

I'll check that out thanks. I saw that comment, but have been skimming most of this code...

Show 18 quoted lines

> Thanks for this patch!
> Stefan
>
>>
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> ---
>
>
>> So I think these tests are worthwihle in themselves,
>
> The reason I put it in the comment instead of tests was the
> ease of spelling out both the status quo and expectations.
>
>> but would like
>> some advice on how to proceed with that from someone more familiar
>> with submodules.
>
> So ideally we'd also error out as soon as the host name is touched?

Do we have some utility function that'll take whatever we have in remote.<name>.url and spew out the username / host / path? We must, since the clone protocol needs it, but I haven't found it.