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

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Apr 10, 2018, 22:35 UTC
Message-ID: <nycvar.QRO.7.76.6.1804110020380.56@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz>
In-Reply-To: <xmqqo9y6dj1s.fsf@gitster.mtv.corp.google.com>

Hi,

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

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

Show 47 quoted lines

> 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