git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] git-remote rename should match whole string when renaming remote ref directory

From
BHBenny Halevy <bhalevy@tonian.com>
Date
Sep 27, 2011, 09:38 UTC
Message-ID
<4E8199A6.5010804@tonian.com>
In-Reply-To
<7voby6y0vs.fsf@alter.siamese.dyndns.org>
On 2011-09-27 09:07, Junio C Hamano wrote:
Show 22 quoted lines
> Benny Halevy <bhalevy@tonian.com> writes:
> 
>> On 2011-09-26 21:04, Junio C Hamano wrote:
>>> Benny Halevy <benny@tonian.com> writes:
>>> ...
>>>> -	if (!prefixcmp(refname, buf.buf)) {
>>>> +	if (!strcmp(refname, buf.buf)) {
>>>
>>> At this point of the code, refname has "refs/remotes/test/foo" and it is
>>> queued to later rename it to "refs/remotes/test-/foo" (the next invocation
>>> of this function will see "refs/remotes/test/bar" in refname). And the
>>> strbuf buf.buf has "refs/remotes/test"; your !strcmp(refname, buf.buf)
>>> would never trigger, I suspect.
>>>
>>> Isn't 60e5eee (remote: "rename o foo" should not rename ref "origin/bar",
>>> 2011-09-01) the correct fix for this issue?...
>>
>> OK, 60e5eee solves the problem too.
> 
> Hmm, what do you mean by "too" here?  Martin's patch fixes the issue, but
> does yours, too?
> 
correct.
Show 16 quoted lines
>> FWIW, here's the test I used:
>> ...
>> cd main || fail cd main failed
>> for i in test test-2; do
>> 	$git remote add $i file://$cwd/$i || fail git remote add $i failed
>> done
>> $git remote update || fail git remote update fail
>> $git remote rename test test-
>> $git show test-2/master || fail FAILED
>> echo PASSED
> 
> Before your last "echo PASSED", add this line:
> 
> 	$git show test-/master || fail FAILED
> 
> and see what happens with your patch.

Yup, with my patch this test indeed fails, and with Martin's it passes.

Thanks!
Benny
Show 22 quoted lines
> 
> It is unfortunately a rather common trap to fall into, so I wouldn't blame
> you too much. People tend to concentrate only on an aspect of the problem
> that originally motivated them, and forget about the other issues that are
> equally important, if not more. In this case, you were too thrilled to see
> that your updated code no longer renames "test-2" mistakenly to "test--2",
> and you forgot that the primary task of the resulting code was to rename
> "test" to "test-" correctly. The additional line I gave you above is to
> test that.
> 
> When testing your own code, make it a habit to _always_ test both sides of
> the coin. It is somewhat difficult until you get used to it [*1*], but it
> is a skill that is really worth acquiring.
> 
> Thanks.
> 
> 
> [Footnote]
> 
> *1* ...and I do not claim that I myself never forget to fully enumerate
> other sides; even experienced people still overlook and embarrass
> themselves in public ;-)
Previous: Junio C Hamano
Message 5 of 5 in “git-remote rename should match whole string when renaming remote ref directory”
  1. git-remote rename should match whole string when renaming remote ref directoryBenny Halevy, Sep 26, 2011
  2. Junio C HamanoSep 26, 2011
  3. Benny HalevySep 27, 2011
  4. Junio C HamanoSep 27, 2011
  5. Benny HalevySep 27, 2011

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.