From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Stefan Beller <sbeller@google.com>
Cc: git <git@vger.kernel.org>, Junio C Hamano <gitster@pobox.com>,
Brandon Williams <bmwill@google.com>
Subject: Re: [PATCH] submodule: add more exhaustive up-path testing
Date: Tue, 14 Aug 2018 23:05:47 +0200 [thread overview]
Message-ID: <87d0ukhd5g.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <CAGZ79kaK8Wt0TGvo-PyDZRLWr9PU0BRo4=DiYUXvv8c8nZ+M8A@mail.gmail.com>
On Tue, Aug 14 2018, Stefan Beller wrote:
> 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...
> 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.
next prev parent reply other threads:[~2018-08-14 21:05 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-08-14 18:59 [PATCH] submodule: add more exhaustive up-path testing Ævar Arnfjörð Bjarmason
2018-08-14 19:10 ` Stefan Beller
2018-08-14 19:54 ` Junio C Hamano
2018-08-14 21:05 ` Ævar Arnfjörð Bjarmason [this message]
2018-08-14 21:10 ` Stefan Beller
2018-08-14 21:16 ` Ævar Arnfjörð Bjarmason
2018-08-14 21:32 ` Stefan Beller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87d0ukhd5g.fsf@evledraar.gmail.com \
--to=avarab@gmail.com \
--cc=bmwill@google.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=sbeller@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.