On Mon, Mar 31, 2008 at 4:04 PM, Julian Foad <julianfoad_at_btopenworld.com> wrote:
> The following merge corrupts svn:mergeinfo:
>
> svn merge --ignore-ancestry --accept=mine-full -c26169 \
> subversion/tests/cmdline/merge_tests.py \
>
> subversion/tests/cmdline/merge_tests.py
>
> because (in subversion/libsvn_client/merge.c) do_file_merge() says:
>
> if (honor_mergeinfo)
> {
> ...
> calculate mergeinfo_path
> ...
> }
> ...
> if (record_mergeinfo ...)
> {
> ...
> use mergeinfo_path
> ...
> }
>
> and, in merges like this one, "record_mergeinfo" is true but "honor_mergeinfo"
> isn't.
>
> One interpretation of the bug is to say that we should never be recording
> mergeinfo when we're not honouring it, because we decided that we never would
> want to and that "ignore_ancestry" should prevent both things. In that case,
> the following should fix it:
>
> [[[
> /* Set *HONOR_MERGEINFO and *RECORD_MERGEINFO (if non-NULL)
> - appropriately for MERGE_B. */
> + appropriately for MERGE_B.
> + One rule is that we shan't record mergeinfo if we're not honoring it. */
> static APR_INLINE void
> mergeinfo_behavior(svn_boolean_t *honor_mergeinfo,
> svn_boolean_t *record_mergeinfo,
> merge_cmd_baton_t *merge_b)
> {
> if (honor_mergeinfo)
> *honor_mergeinfo = (merge_b->mergeinfo_capable
> && merge_b->sources_ancestral
> && merge_b->same_repos
> && (! merge_b->ignore_ancestry));
>
> if (record_mergeinfo)
> *record_mergeinfo = (merge_b->mergeinfo_capable
> && merge_b->sources_ancestral
> && merge_b->same_repos
> + && (! merge_b->ignore_ancestry)
> && (! merge_b->dry_run));
> }
> ]]]
>
> Paul, can you verify this is a correct fix? Can you see any knock-on effect?
> The only other code path it affects is do_directory_merge, which is structured
> a little differently and doesn't suffer the same problem.
Hi Julian,
I agree that we need to clarify the effect --ignore-ancestry has
regarding mergeinfo. Your fix is certainly correct in that it fixes
the bogus mergeinfo/segfault, but I'm not sure that it is correct in
terms of expanding the meaning of --ignore-ancestry to mean *not*
setting mergeinfo. To get back to what Karl said earlier in this
thread, maybe we need a --no-record option so we can do all of the
following:
A) Ignore mergeinfo when calculating what to merge and don't set any mergeinfo
B) Ignore mergeinfo when calculating what to merge but set mergeinfo
describing the merge
C) Consider mergeinfo when calculating what to merge but don't set any mergeinfo
D) Consider mergeinfo when calculating what to merge and set mergeinfo
describing the merge
Question is, are there valid use cases for all of these? Obviously
there is for D, that's Merge Tracking(TM)! B is what using
--ignore-ancestry today does and it seems potentially useful in cases
like those described in issue #2898 - Imagine a case like that
described in http://subversion.tigris.org/issues/show_bug.cgi?id=2898#desc7
where there are other subtrees with mergeinfo that we want
updated/elided.
But for A and C I can't come up with a valid use case...Given that B
and D are handled today, for 1.5 I'd be happy just fixing
do_file_merge() and waiting to see if we need a --no-record option to
enable A and C and putting it in 1.6.
> It certainly seems to fix the problem. I'm running "make check" now.
FWIW It will break merge_tests.py 19 and 60, but in both cases it
would just be a matter of tweaking the tests to agree with the new
meaning of --ignore-ancestry.
Paul
> (I don't
> have a specific test for this. Symptom is sometimes/on-some-systems it crashes,
> sometimes it produces bogus results...)
>
> - Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-04-01 03:50:55 CEST