Re: [PATCH] Follow up to r27817

On Thu, 15 Nov 2007, Senthil Kumaran S wrote:
...
> I am attaching a patch along with this email which is a follow up to
> r27817, based on glasser's review comments.
>
> [[[
> Follow up to r27817.
>
> Improve r27817 based on glasser's comments.
>
> * subversion/svndumpfilter/main.c
...
> (skip_missing_merge_source_or_renumber_revs): Function renamed from
> renumber_merge_source_rev_range. Fix logical bug in include/exclude
> prefix
> path skipping and avoid possible crashes in dereferencing revmap_start
> and
> revmap_end
> (set_node_property): Update call to renumber_merge_source_rev_range, to
> the
> renamed function skip_missing_merge_source_or_renumber_revs

Senthil, I committed the remainder of these changes, heavily modified,
in r27845. Please review them to, and make sure I didn't screw
anything up. :P

I'm also a little leary of this new --skip-missing-merge-sources
option for 'svndumpfilter', because it's somewhat inconsistent with
what we did in libsvn_repos/load.c (for merge source path prefix
adjustment), where we make the assumption that all merge source paths
are present in the dump file that you're loading. The use case is
slightly different here, since we are explicitly filtering out part of
the dump file, but I did want to point out the inconsistency (which
perhaps we're okay with).

Also, it would be fantastic if you could introduce a new test suite
(svndumpfilter_tests.py) for this issue, and for issue #3020. I
noticed that you've already got a dump file for the latter; you can
incorporate it using a svndumpfilter_tests_data/ directory, like you
see elsewhere in the command-line test suite.