Re: [PATCH 1/2] remote: write correct fetch spec when renaming remote 'remote'

[Jeff King <peff@peff.net>]

On Wed, Aug 31, 2011 at 09:50:42PM -0400, Martin von Zweigbergk wrote:

> When renaming a remote whose name is contained in a configured fetch
> refspec for that remote, we currently replace the first occurrence of
> the remote name in the refspec. This is correct in most cases, but
> breaks if the remote name occurs in the fetch refspec before the
> expected place. For example, we currently change
> 
> [remote "remote"]
> 	url = git://git.kernel.org/pub/scm/git/git.git
> 	fetch = +refs/heads/*:refs/remotes/remote/*
> 
> into
> 
> [remote "origin"]
> 	url = git://git.kernel.org/pub/scm/git/git.git
> 	fetch = +refs/heads/*:refs/origins/remote/*

Oops.

> Reduce the risk of changing incorrect sections of the refspec by
> requiring the string to be matched to have leading and trailing
> slashes, i.e. match "/<name>/" instead of just "<name>".

Doesn't this just mean that:

  git remote rename remotes foo

will break in the same way?

> We could have required even a leading ":refs/remotes/", but that would
> mean that we would limit the types of refspecs we could help the user
> update.

Actually, I think it's better to be more conservative, and rename only:

  refs/remotes/$OLD/

into

  refs/remotes/$NEW/

If we are tweaking the refspecs, it's because we assume that the refspec
follows a certain naming convention (i.e., the one we set up with "git
remote add"). If somebody has tweaked this to be:

  refs/heads/$OLD/*

or even:

  refs/heads/foo/$OLD/bar

then we are just guessing about what they want. And I suspect such a
person would not use "git remote rename", anyway, but would instead edit
the config themselves. I'd rather make it safe for people using the
default config, and people who want to stray from that can deal with
updating the config themselves (since they would have to have done so to
get into that state in the first place).

Maybe we should even print a warning in that case.

-Peff