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

From: Jeff King <peff@peff.net>
Date: Sep 2, 2011, 16:03 UTC
Message-ID: <20110902160333.GB19690@sigill.intra.peff.net>
In-Reply-To: <1314924634-12235-1-git-send-email-martin.von.zweigbergk@gmail.com>

On Thu, Sep 01, 2011 at 08:50:33PM -0400, Martin von Zweigbergk wrote:

Show 17 quoted lines

> +	strbuf_addf(&old_remote_context, ":refs/remotes/%s/", rename.old);
>  	for (i = 0; i < oldremote->fetch_refspec_nr; i++) {
>  		char *ptr;
>  
>  		strbuf_reset(&buf2);
>  		strbuf_addstr(&buf2, oldremote->fetch_refspec[i]);
> -		ptr = strstr(buf2.buf, rename.old);
> +		ptr = strstr(buf2.buf, old_remote_context.buf);
>  		if (ptr)
> -			strbuf_splice(&buf2, ptr-buf2.buf, strlen(rename.old),
> -					rename.new, strlen(rename.new));
> +			strbuf_splice(&buf2,
> +				      ptr-buf2.buf + strlen(":refs/remotes/"),
> +				      strlen(rename.old), rename.new,
> +				      strlen(rename.new));
>  		if (git_config_set_multivar(buf.buf, buf2.buf, "^$", 0))
>  			return error("Could not append '%s'", buf.buf);

If we find a refspec that does not match this pattern, what should we do? This code silently ignores it.

At the very least, I'd like to see a warning indicating that one or more fetch refspecs were not rewritten, and that the user should update the config manually.

What should happen if there are multiple refspecs, and one doesn't match? Should we rewrite the ones we can, or should we abort and not rewrite anything at all? I think the latter is probably less confusing.

These are corner cases, obviously, and most people will just have the stock refspecs or something that follows their pattern.

-Peff