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.
> 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.
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?)
- Julian
Received on 2010-03-16 12:53:17 CET