git remote rename problem with trailing \\ for remote.url config entries (on Windows)

git remote rename problem with trailing \\ for remote.url config entries (on Windows)

[Marc Strapetz]

One of our users has just reported that:

$ git remote rename origin origin2

will turn following remote entry:

[remote "origin"]
	url = c:\\repo\\
	fetch = +refs/heads/*:refs/remotes/origin/*

into following entry for which the url is skipped:

[remote "origin2"]
[remote "origin2"]
	fetch = +refs/heads/*:refs/remotes/origin2/*

I understand that this is caused by the trailing \\ and it's easy to 
fix, but 'git push' and 'git pull' work properly with such URLs and a

$ git clone c:\repo\

will even result in the problematic remote-entry. So I guess some kind 
of validation could be helpful.

Tested with git version 2.11.0.windows.1

-Marc

Re: git remote rename problem with trailing \\ for remote.url config entries (on Windows)

[Junio C Hamano]

Marc Strapetz <marc.strapetz@syntevo.com> writes:

> One of our users has just reported that:
>
> $ git remote rename origin origin2
>
> will turn following remote entry:
>
> [remote "origin"]
> 	url = c:\\repo\\
> 	fetch = +refs/heads/*:refs/remotes/origin/*
>
> into following entry for which the url is skipped:
>
> [remote "origin2"]
> [remote "origin2"]
> 	fetch = +refs/heads/*:refs/remotes/origin2/*
>
> I understand that this is caused by the trailing \\ and it's easy to
> fix, but 'git push' and 'git pull' work properly with such URLs and a
>
> $ git clone c:\repo\
>
> will even result in the problematic remote-entry. So I guess some kind
> of validation could be helpful.

If you change the original definition of the "origin" to

        [remote "origin"]
               url = "c:\\repo\\"
               fetch = +refs/heads/*:refs/remotes/origin/*

or

        [remote "origin"]
               url = c:\\repo\\ # ends with bs
               fetch = +refs/heads/*:refs/remotes/origin/*

it seems to give me a better result.  I didn't dig to determine
where things go wrong in "remote rename", and it is possible that
the problem is isolated to that command, or the same problem exists
with any value that ends with a backslash.  If the latter, "git clone"
and anything that writes to configuration such a value needs to be
taught about this glitch and learn a workaround, I would think.

Dscho Cc'ed, not for Windows expertise, but as somebody who has done
a lot in <config.c>.

Re: git remote rename problem with trailing \\ for remote.url config entries (on Windows)

[Johannes Schindelin]

Hi,

[I know, blast from the past...]

On Mon, 13 Feb 2017, Junio C Hamano wrote:

> Marc Strapetz <marc.strapetz@syntevo.com> writes:
> 
> > One of our users has just reported that:
> >
> > $ git remote rename origin origin2
> >
> > will turn following remote entry:
> >
> > [remote "origin"]
> > 	url = c:\\repo\\
> > 	fetch = +refs/heads/*:refs/remotes/origin/*
> >
> > into following entry for which the url is skipped:
> >
> > [remote "origin2"]
> > [remote "origin2"]
> > 	fetch = +refs/heads/*:refs/remotes/origin2/*
> >
> > I understand that this is caused by the trailing \\ and it's easy to
> > fix, but 'git push' and 'git pull' work properly with such URLs and a
> >
> > $ git clone c:\repo\
> >
> > will even result in the problematic remote-entry. So I guess some kind
> > of validation could be helpful.
> 
> If you change the original definition of the "origin" to
> 
>         [remote "origin"]
>                url = "c:\\repo\\"
>                fetch = +refs/heads/*:refs/remotes/origin/*
> 
> or
> 
>         [remote "origin"]
>                url = c:\\repo\\ # ends with bs
>                fetch = +refs/heads/*:refs/remotes/origin/*
> 
> it seems to give me a better result.  I didn't dig to determine
> where things go wrong in "remote rename", and it is possible that
> the problem is isolated to that command, or the same problem exists
> with any value that ends with a backslash.  If the latter, "git clone"
> and anything that writes to configuration such a value needs to be
> taught about this glitch and learn a workaround, I would think.
> 
> Dscho Cc'ed, not for Windows expertise, but as somebody who has done
> a lot in <config.c>.

So... I finally had a look at this, and while I agree that the quoted
version is better, I also agree that the backslash is mistaken for a
continuation character (which is not even allowed in section headers).

A quick test with my `empty-config-section` patch series shows that it
addresses this issue. A quick bisec confirms that the patch with the
online "git_config_set: make use of the config parser's event stream" is
responsible for this fix.

At first, I was puzzled by this, because I expected `git remote rename` to
be backed by the `git_config_copy_or_rename_section_in_file()` function
(which my patch series does not touch).

But then I looked at the code of the `mv()` function in builtin/remote.c
and it uses `git_config_set_multivar()` and `git_config_set()`. And those
functions were indeed touched (and fixed) by above-mentioned commit.

Ciao,
Johannes

Re: git remote rename problem with trailing \\ for remote.url config entries (on Windows)

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> A quick test with my `empty-config-section` patch series shows that it
> addresses this issue. A quick bisec confirms that the patch with the
> online "git_config_set: make use of the config parser's event stream" is
> responsible for this fix.
>
> At first, I was puzzled by this, because I expected `git remote rename` to
> be backed by the `git_config_copy_or_rename_section_in_file()` function
> (which my patch series does not touch).

I played around with this test data:

        [sec "tio"]
                val = a\\
                ue = b

and saw "git config --rename-section sec.tio sec.tion" just work
fine (without the event stream change).  Without the event stream
change, "git config --unset sec.tio.ue" loses "sec.tio.val" but with
it we see we only lose the last line, which is the correct thing to
happen.

Nice.