On Thu, Dec 17, 2009 at 10:10 PM, Branko Čibej <brane_at_xbc.nu> wrote:
> Paul Burba wrote:
>> On Thu, Dec 17, 2009 at 5:53 PM, Branko Čibej <brane_at_xbc.nu> wrote:
>>
>>> Paul Burba wrote:
>>>
>>>> Regardless, I don't think this is a true regression, just a
>>>> new bug, and as such, not something that should hold up 1.6.7.
>>>>
>>>>
>>> Sorry, what? Subversion will suddenly error out where it previously did
>>> not. No matter /how/ that mergeinfo got into the repository, if we don't
>>> fix this before releasing 1.6.7, we'll likely get a whole bunch of error
>>> reports. Telling people that "it's not really a bug" isn't going to be
>>> very productive, given that it effectively stops them from using svn
>>> merge in this situation.
>>>
>>> Can you be sure that the circumstances that lead to this error are rare
>>> enough to address in 1.6.8?
>>>
>>
>> Hi Brane,
>>
>> I haven't explained this clearly, sorry. What has happened here is a
>> semi-complicated dance among several issues.
>>
>> Let me try to explain again:
>>
>> The reintegrate bug Hyrum encountered is not IMO a regression because
>> it *only* worked in 1.6.6 because another bug hid it.
>>
>> That other bug is issue #3547, which converted all the mergeinfo in
>> our repository from absolute paths to relative paths. The mergeinfo
>> APIs don't expect relative source paths in mergeinfo, so when it is
>> encountered the results are unpredictable (and usually wrong).
>>
>> In r890993 I made fixes so the relative pathed mergeinfo are
>> interpreted as absolute, and so the various svn_mergeinfo_* APIs don't
>> choke on the relative paths. This was nominated and backported to
>> 1.6.7.
>>
>> Along comes Hyrum, who, using the latest 1.6.7 with r890993 included,
>> tries to reintegrate a backport branch to 1.6.x. It fails, a bug!
>>
>> I try the same merge with 1.6.6 and it works. Regression right?
>> Actually no. It only worked because without the r890993 fix, the
>> relative pathed mergeinfo caused by issue #3547 was essentially
>> invisible to 1.6.6. It was just dumb luck that it actually
>> *prevented* an error. It most cases it would cause errors, as we
>> discussed here: http://svn.haxx.se/dev/archive-2009-12/0222.shtml.
>>
>> ~~~~~
>>
>> This may all be moot because I fixed the bug in r892085 and nominated
>> this for backport to 1.6.7. I also nominated a test which reproduces
>> the bug Hyrum found. This test fails across the board on trunk,
>> 1.6.x, and 1.6.6 without the the r892085 fix in place.
>>
>> I hope that makes things clearer. If not let me know or ping me in
>> IRC tomorrow.
>>
>> Thanks,
>>
>
> Oh, I think I see. So the no-leading-slash in svnadmin load caused some
> merge info to be ignored, and when you made the merge logic more
> flexible WRT the svn:mergeinfo format, this new bug (not ignoring
> irrelevant revisions in svn:mergeinfo?) raised its head.
It raised it's head for us yes, but it is not new, it's been there for
some time. I'd go as far to say that there are no new bugs here, if
by "new" we mean a regression against 1.6.6.
> OK ... I gather from other posts that we don't really know when this
> invalid mergeinfo got into the repository, but we can assume that it was
> caused by another bug (possibly even in 1.5) which has hopefully since
> been fixed.
Right, I'm not sure how the self-referential mergeinfo got in, and I
can't reproduce it. Waiting on Stephen Butler to maybe shed some
light on what version he was using.
> The question remains, though, can this situation really only be
> reproduced with the complex chain of events we saw here? In other words,
> if we had not migrated the repository to svn.apache.org and still had
> invalid mergeinfo in the repository -- would not the merge have failed
> even without your recent change in mergeinfo handling?
Right, it would have failed regardless of my change to handle relative
pathed mergeinfo. The new merge_tests.py 141 "source has target's
history as explicit mergeinfo" demonstrates the underlying reintegrate
bug. Run that test with 1.6.6 and it still fails.
> I'm trying to wrap my head around this because it doesn't seem
> reasonable that irrelevant revisions in mergeinfo would only cause
> problems in combination with your changes.
I'm not entirely sure which changes you are referring to here(?).
> Because if that's the case,
> then there's either a bug in your "flexibilification" of mergeinfo
> paths, or there's a more fundamental bug in mergeinfo processing ...
> sorry if all this isn't crystal clear, but this is a bit of a muddle. :)
Absolutely there is another bug, the one I mentioned here:
"This may all be moot because I fixed the bug in r892085 and nominated
this for backport to 1.6.7. I also nominated a test which reproduces
the bug Hyrum found. This test fails across the board on trunk,
1.6.x, and 1.6.6 without the the r892085 fix in place."
> To illustrate what I'm getting at ... say a user has a file ^/foo
Assuming you mean ^/foo is a directory here.
> in her
> repository that was created in r10. He creates a branch, "svn cp ^/foo
> ^/bar" and creates r11. in r12, she modifies ^/foo, and in r13 merges
> the change to ^/bar. One would expect ^/bar's mergeinfo to be:
> /foo:10-12,
Assuming you didn't specify any revisions and merged everything available, i.e.
svn merge ^/foo bar-wc
The mergeinfo should be
/foo:11-12
But that is a minor point.
> yes? Now let's imagine for a moment that for some reason
> this was all done with a broken client/server combo, and the actual
> mergeinfo is: /foo:2-12. Would Hyrum's reintegrate merge fail in the
> same way in this case? If not, why not?
No, that would work. The bug I fixed in r892085 only comes into play
when reintegrating branches that were never merged to earlier from the
source (like Hyrum was doing).
Paul
Received on 2009-12-18 16:30:42 CET