On Tue, Mar 16, 2010 at 7:52 AM, Julian Foad <julian.foad_at_wandisco.com> wrote:
> On Mon, 2010-03-15, Paul Burba wrote:
>> On Fri, Mar 12, 2010 at 12:59 PM, Julian Foad <julian.foad_at_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-test.c@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
Received on 2010-03-16 13:21:52 CET