On 2011-09-27 09:07, Junio C Hamano wrote:
> Benny Halevy <bhalevy@xxxxxxxxxx> writes:
>
>> On 2011-09-26 21:04, Junio C Hamano wrote:
>>> Benny Halevy <benny@xxxxxxxxxx> 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.
>> 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
>
> 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 ;-)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html