On Tue, Mar 16, 2010 at 8:21 AM, Paul Burba <ptbu...@gmail.com> wrote:
> On Tue, Mar 16, 2010 at 7:52 AM, Julian Foad <julian.f...@wandisco.com> wrote:
>> On Mon, 2010-03-15, Paul Burba wrote:
>>> On Fri, Mar 12, 2010 at 12:59 PM, Julian Foad <julian.f...@wandisco.com>
>>> wrote:
>>> > Hi Paul.
>>> >
>>> > I think we can tighten the validation of svn_merge_range_t to exclude
>>> > change number "r0" (RANGE->start == -1) as in the following patch.
>>> >
>>> > My reasoning is that a change numbered "r0" is not a valid concept in
>>> > any Subversion system because the state (tree-snapshot) numbered r0 is
>>> > by definition the beginning. (It also happens to be empty by
>>> > definition, but that's not so relevant.) We can say the same in a
>>> > different way: change "r0" would mean the change from "r(-1)" to "r0",
>>> > and "r(-1)" is not a valid concept.
>>> >
>>> > Makes sense?
>>>
>>> Hi Julian,
>>>
>>> It does make sense, but I can't apply this patch, seems to have a few
>>> problems, see below.
>>
>> [...]
>>> > @@ -877,24 +877,27 @@ test_remove_rangelist(apr_pool_t *pool)
>>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>> Why is that all on one line? Those offsets are supposed to be on
>>> their own line no?
>>
>> I don't think this is the problem. This is the diff format produced by
>> the "--show-c-function" ("-p") option to GNU diff or "svn diff".
>>
>>> Also, test_remove_rangelist(apr_pool_t *pool) looks to be on line 706
>>> in
>>> https://svn.apache.org/repos/asf/subversion/trunk/subversion/tests/libsvn_subr/mergeinfo-tes...@922300,
>>> so it seems your from-to headers are off.
>>
>> That's just because the "--show-c-function" option is very dumb and just
>> intended as a guess. It just shows (part of) the last line it saw
>> before the current hunk that looked like a line introducing a function.
>>
>> [...]
>>> > @@ -943,12 +946,15 @@ test_rangelist_remove_randomly(apr_pool_
>>> ^^^
>>> Again, all on one line and this time the line is truncated.
>>
>> Yup, that's normal.
>
> Ok, didn't realize that was all normal.
>
>>> Anyhow, could try attaching the patch again?
>>
>> Certainly. Attached v2, which in addition has a fix for one of the
>> places that generated an "r0" merge-range.
>
> Great, the applies cleanly.
>
>> It's possible your patch application was marred by line-endings, or
>> maybe can't cope with 6 context lines (the default is 3), so I've
>> attached the patch this time, and cut the context lines back to 3.
>> (What patch-application program was it?)
>
> It's TortoiseSVN's built-in patch tool.
>
> I have to prep for a meeting right now, but I'll look at this later today.
>
> Thanks,
>
> Paul
Julian,
Your patch looks good to me. It passes all tests [fsfs x ra_local]
and I can't see that it will introduce any problems. I assume the
mergeinfo_tests.py 2 failure you mentioned at the start of the thread
was solved in v2?
Paul
