Yesterday in IRC Julian described a problem he was having while doing
a catch up merge of the tree-conlficts branch with trunk. The
attached file julianf.bad.catchip.merge.txt documents exactly what he
encountered -- in a nutshell he was getting a conflict on
'www/svn_1.5_releasenotes.html' which was entirely unexpected since
the only changes made to that file on the branch were via catch-up
merges from trunk.
The full mergeinfo the tree-conflicts branch @32461 is in the attached
file tree-conflicts.explicit.mergeinfo.at.r32461.txt. To save you
some pain I'll sum it up: Other than the merge target itself, there
are 10 subtrees with explicit mergeinfo:
'.':
'subversion\include\private\svn_auth_private.h':
'notes\tree-conflicts\design-overview.txt':
'subversion\libsvn_auth_kwallet\kwallet.cpp':
'tools\server-side\test_svn_server_log_parse.py':
'tools\server-side\svn_server_log_parse.py':
'www\issue-tracker.html':
'www\development.html':
'notes\tree-conflicts\requirements.txt':
'www\tasks.html':
'subversion\tests\cmdline\tree_conflict_tests.txt':
Note: The root of the tree-conflicts branch has mergeinfo describing
some of it's natural history, specifically '/trunk:1-31822' (the tree
conflicts branch wasn't created until r28217, so 1-28216 describes the
branch's natural history). This was probably due to issue #3157 which
has been fixed. While having natural history in explicit mergeinfo is
not necessary, it should be harmless, so we can safely ignore it here.
Julian was trying to merge -r0:32172 into a tree-conflicts WC @32478.
At the start of the merge these are the ranges that require merging
for each subtree (this is simply the remaining_ranges field for each
child in the children_with_mergeinfo array):
.
31823-32172
notes/tree-conflicts/design-overview.txt
31498-31715,31823-32172
notes/tree-conflicts/requirements.txt
31498-31715,31823-32172
subversion/include/private/svn_auth_private.h
31498-31715,31823-32172
subversion/libsvn_auth_kwallet/kwallet.cpp
31498-31715,31823-32172
tools/server-side/svn_server_log_parse.py
31228-31497,31498-31715,31823-32172
tools/server-side/test_svn_server_log_parse.py
31228-31497,31498-31715,31823-32172
www/development.html
31823-32172
www/issue-tracker.html
31823-32172
www/tasks.html
31823-32172
Note that 'subversion\tests\cmdline\tree_conflict_tests.txt' is not in
this list as it was copied individually from trunk to the
tree-conflicts branch in r31225 then deleted from trunk in r31226 so
there is no merge source for this path.
Note also that the 31228-31497 and 31498-31715 ranges are contiguous,
I only broke them out to highlight which revisions are common to which
subtrees. Since there are two contiguous ranges there are two editor
drives (i.e. calls to merge.c:drive_merge_report_editor()):
1) r31228-31715
2) r31823-32172
The first drive is where we get into trouble. For that first editor drive we:
A) Call svn_client__get_diff_editor() - get a svn_delta_editor_t *diff
editor and edit baton with start revision 31227 and path '.' (i.e. the
merge target, the root of the tree-conflicts WC).
B) Call svn_ra_do_diff3() - using the diff editor and baton get a
svn_ra_reporter3_t *reporter and report baton
C) Make a reporter->set_path() call describing '.' at a revision such
that we don't repeat the merge of r31228-31715 into it.
The problem with the existing code is that in C) we use the start
range of '.'s remaining revisions, r31823, a revision outside of the
start revision we passed to svn_client__get_diff_editor() and the
revision we passed to svn_ra_do_diff3().
Looking at the documentation for svn_ra_reporter3_t and
svn_ra_do_diff3 I confess it is not crystal clear exactly what we
would expect this to do, but it would seem to make more sense to pass
svn_ra_do_diff3() r31715. Since this editor drive is merging
r31228-31715 and '.' already has that range merged to it, it's only
the subtrees we need to merge, so describing '.' as at r31715 seems to
ok -- dont merge anything to the target in this drive. The attached
patch does just that and seems to solve the problems Julian was
seeing. Does my analysis seem correct? The thing that bugs me is
that it feels like it should be easy to write a new merge test to
expose this problem, but I can't seem to do that, which makes we
wonder if I got it all straight :-\
Paul
P.S. The patch passes all tests.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-08-14 20:26:01 CEST