Stefan Beller <sbeller@xxxxxxxxxx> writes:
> Some users may rely on this by always cloning with '/.' and having
> an additional '../' in the relative path for the submodule, and this
> patch breaks them. So why introduce this patch?
>
> The fix in c12922024 (submodule: ignore trailing slash on superproject
> URL, 2016-10-10) and prior discussion revealed, that Git and Git
> for Windows treat URLs differently, as currently Git for Windows
> strips off a trailing dot from paths when calling a Git binary
> unlike when running a shell. Which means Git for Windows is already
> doing the right thing for the case mentioned above, but it would fail
> our current tests, that test for the broken behavior and it would
> confuse users working across platforms. So we'd rather fix it
> in Git to ignore any of these trailing no ops in the path properly.
>
> We never produce the URLs with a trailing '/.' in Git ourselves,
> they come to us, because the user used it as the URL for cloning
> a superproject. Normalize these paths.
>
> Helped-by: Junio C Hamano <gitster@xxxxxxxxx>
> Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx>
> ---
>
> * reworded the commit message, taken from Junio, but added more explanation
> why we want to introduce this patch.
The additional explanation is very good.
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 260f46f..ac03cb3 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -76,6 +76,29 @@ static int chop_last_dir(char **remoteurl, int is_relative)
> return 0;
> }
>
> +static void strip_url_ending(char *url, size_t *_len)
> +{
> + size_t len = _len ? *_len : strlen(url);
Stare at our codebase and you'd notice that we avoid using names
with leading underscore deliberately and use trailing one instead
when we name a throw-away name like this. Let's do the same here.
I.e.
static void strip_url_ending(char *url, size_t *len_)
{
size_t len = len_ ? *len_ : strlen(url);
> + for (;;) {
> + if (len > 1 && is_dir_sep(url[len-2]) && url[len-1] == '.') {
> + url[len-2] = '\0';
"len-1" and "len-2" are usually spelled with SP on both sides of
binary operators.
> + len -= 2;
> + continue;
> + }
> + if (len > 0 && is_dir_sep(url[len-1])) {
> + url[len-1] = '\0';
> + len --;
And post-decrement sticks to whatever it is decrementing without SP.
> + continue;
> + }