Paul,
I'm reviewing for back-port to 1.5.2.
> r32706 | pburba | 2008-08-25 21:27:54 +0100 (Mon, 25 Aug 2008) | 7 lines
> [[[
> Add yet another issue #3067 merge test.
>
> * subversion/tests/cmdline/merge_tests.py
> (merge_target_and_subtrees_need_nonintersecting_ranges): New.
> (test_list): Add merge_target_and_subtrees_need_nonintersecting_ranges
> and mark as XFail.
> ]]]
> +# Test for yet another variant of issue #3067.
> +def merge_target_and_subtrees_need_nonintersecting_ranges(sbox):
> + "target and subtrees need nonintersecting revs"
> +
> + sbox.build()
> + wc_dir = sbox.wc_dir
> +
> + # Some paths we'll care about
> + nu_path = os.path.join(wc_dir, "A", "D", "G", "nu")
> + A_COPY_path = os.path.join(wc_dir, "A_COPY")
> + nu_COPY_path = os.path.join(wc_dir, "A_COPY", "D", "G", "nu")
> + omega_COPY_path = os.path.join(wc_dir, "A_COPY", "D", "H", "omega")
> + beta_COPY_path = os.path.join(wc_dir, "A_COPY", "B", "E", "beta")
> + beta_COPY_path = os.path.join(wc_dir, "A_COPY", "B", "E", "beta")
> + rho_COPY_path = os.path.join(wc_dir, "A_COPY", "D", "G", "rho")
> + omega_COPY_path = os.path.join(wc_dir, "A_COPY", "D", "H", "omega")
> + psi_COPY_path = os.path.join(wc_dir, "A_COPY", "D", "H", "psi")
omega_COPY_path and beta_COPY_path are each defined twice there. No
harm, AFAIK, just odd.
> +
> + # Make a branch to merge to.
> + wc_disk, wc_status = set_up_branch(sbox, False, 1)
> +
> + # Add file A/D/G/nu in r7.
> + svntest.main.file_write(nu_path, "This is the file 'nu'.\n")
> + svntest.actions.run_and_verify_svn(None, None, [], 'add', nu_path)
> + expected_output = wc.State(wc_dir, {'A/D/G/nu' : Item(verb='Adding')})
> + wc_status.add({'A/D/G/nu' : Item(status=' ', wc_rev=7)})
> + svntest.actions.run_and_verify_commit(wc_dir, expected_output,
> + wc_status, None, wc_dir)
> +
> + # Make a text mod to A/D/G/nu in r8.
> + svntest.main.file_write(nu_path, "New content")
> + expected_output = wc.State(wc_dir, {'A/D/G/nu' : Item(verb='Sending')})
> + wc_status.tweak('A/D/G/nu', wc_rev=8)
> + svntest.actions.run_and_verify_commit(wc_dir, expected_output,
> + wc_status, None, wc_dir)
> +
> + # Do several merges to setup a situation where a subtree needs the merge
> + # target and two of its subtrees all need non-intersecting ranges
Are there a few superfluous words there? (s/a subtree needs //)
> + # merged when doing a synch (a.k.a. cherry harvest) merge.
> + #
> + # 1) Merge -r0:7 from A to A_COPY.
> + #
> + # 2) Merge -c8 from A/D/G/nu to A_COPY/D/G/nu.
> + #
> + # 3) Merge -c-6 from A/D/H/omega to A_COPY/D/H/omega.
> + #
> + # Commit this group of merges as r9. Since we already test these type
> + # of merges to death we don't use run_and_verify_merge() on these
> + # intermediate merges.
> + svntest.actions.run_and_verify_svn(
> + None, expected_merge_output([[2,7]],
> + ['U ' + beta_COPY_path + '\n',
> + 'A ' + nu_COPY_path + '\n',
> + 'U ' + rho_COPY_path + '\n',
> + 'U ' + omega_COPY_path + '\n',
> + 'U ' + psi_COPY_path + '\n']
> + ),
> + [], 'merge', '-r0:7', sbox.repo_url + '/A', A_COPY_path)
> + svntest.actions.run_and_verify_svn(
> + None, expected_merge_output([[8]], 'U ' + nu_COPY_path + '\n'),
> + [], 'merge', '-c8', sbox.repo_url + '/A/D/G/nu', nu_COPY_path)
> + svntest.actions.run_and_verify_svn(
> + None, expected_merge_output([[-6]], 'G ' + omega_COPY_path + '\n'),
> + [], 'merge', '-c-6', sbox.repo_url + '/A/D/H/omega', omega_COPY_path)
> + expected_output = wc.State(wc_dir, {'A/D/G/rho' : Item(verb='Sending')})
Redundant line: you overwrite expected_output with a different value 8
lines later.
> + wc_status.add({'A_COPY/D/G/nu' : Item(status=' ', wc_rev=9)})
> + wc_status.tweak('A_COPY',
> + 'A_COPY/B/E/beta',
> + 'A_COPY/D/G/rho',
> + 'A_COPY/D/H/omega',
> + 'A_COPY/D/H/psi',
> + wc_rev=9)
> + expected_output = wc.State(wc_dir, {
> + 'A_COPY' : Item(verb='Sending'),
> + 'A_COPY/B/E/beta' : Item(verb='Sending'),
> + 'A_COPY/D/G/rho' : Item(verb='Sending'),
> + 'A_COPY/D/G/nu' : Item(verb='Adding'),
> + 'A_COPY/D/H/omega' : Item(verb='Sending'),
> + 'A_COPY/D/H/psi' : Item(verb='Sending'),
> + })
> + svntest.actions.run_and_verify_commit(wc_dir, expected_output, wc_status,
> + None, wc_dir)
> +
> + # Update the WC to allow full mergeinfo inheritance and elision.
> + svntest.actions.run_and_verify_svn(None, ["At revision 9.\n"], [], 'up',
> + wc_dir)
> +
> + # Merge all available revisions from A to A_COPY, the merge logic
> + # should handle this situation (no "svn: Working copy path 'D/G/nu'
> + # does not exist in repository" errors!) and after elision only
> + # A_COPY should have explicit mergeinfo.
> + short_A_COPY_path = shorten_path_kludge(A_COPY_path)
> + expected_output = wc.State(short_A_COPY_path, {
> + 'D/H/omega': Item(status='U '),
> + })
> + expected_status = wc.State(short_A_COPY_path, {
> + '' : Item(status=' M', wc_rev=9),
> + 'B' : Item(status=' ', wc_rev=9),
> + 'mu' : Item(status=' ', wc_rev=9),
> + 'B/E' : Item(status=' ', wc_rev=9),
> + 'B/E/alpha' : Item(status=' ', wc_rev=9),
> + 'B/E/beta' : Item(status=' ', wc_rev=9),
> + 'B/lambda' : Item(status=' ', wc_rev=9),
> + 'B/F' : Item(status=' ', wc_rev=9),
> + 'C' : Item(status=' ', wc_rev=9),
> + 'D' : Item(status=' ', wc_rev=9),
> + 'D/G' : Item(status=' ', wc_rev=9),
> + 'D/G/pi' : Item(status=' ', wc_rev=9),
> + 'D/G/rho' : Item(status=' ', wc_rev=9),
> + 'D/G/tau' : Item(status=' ', wc_rev=9),
> + 'D/G/nu' : Item(status=' M', wc_rev=9),
> + 'D/gamma' : Item(status=' ', wc_rev=9),
> + 'D/H' : Item(status=' ', wc_rev=9),
> + 'D/H/chi' : Item(status=' ', wc_rev=9),
> + 'D/H/psi' : Item(status=' ', wc_rev=9),
> + 'D/H/omega' : Item(status='MM', wc_rev=9),
> + })
> + expected_disk = wc.State('', {
> + '' : Item(props={SVN_PROP_MERGEINFO : '/A:2-9'}),
> + 'B' : Item(),
> + 'mu' : Item("This is the file 'mu'.\n"),
> + 'B/E' : Item(),
> + 'B/E/alpha' : Item("This is the file 'alpha'.\n"),
> + 'B/E/beta' : Item("New content"),
> + 'B/lambda' : Item("This is the file 'lambda'.\n"),
> + 'B/F' : Item(),
> + 'C' : Item(),
> + 'D' : Item(),
> + 'D/G' : Item(),
> + 'D/G/pi' : Item("This is the file 'pi'.\n"),
> + 'D/G/rho' : Item("New content"),
> + 'D/G/tau' : Item("This is the file 'tau'.\n"),
> + 'D/G/nu' : Item("New content"),
> + 'D/gamma' : Item("This is the file 'gamma'.\n"),
> + 'D/H' : Item(),
> + 'D/H/chi' : Item("This is the file 'chi'.\n"),
> + 'D/H/psi' : Item("New content"),
> + 'D/H/omega' : Item("New content"),
> + })
> + expected_skip = wc.State(short_A_COPY_path, { })
> + saved_cwd = os.getcwd()
> + os.chdir(svntest.main.work_dir)
> + svntest.actions.run_and_verify_merge(short_A_COPY_path, None, None,
> + sbox.repo_url + \
> + '/A',
> + expected_output,
> + expected_disk,
> + expected_status,
> + expected_skip,
> + None, None, None, None,
> + None, 1)
> + os.chdir(saved_cwd)
> +
OK, nothing in the test that prevents a successful review. +1 to r32706.
> ------------------------------------------------------------------------
> r32707 | pburba | 2008-08-25 22:01:28 +0100 (Mon, 25 Aug 2008) | 18 lines
> [[[
> Fix yet another issue #3067 bug.
>
> When driving the merge report editor and the merge target has subtrees that
> need different ranges applied, there was a case we didn't handle: A subtree
> may need a range merged that doesn't intersect with the range its nearest
> parent requires *nor* that any other subtree requires. In some cases this
> could cause us to describe path/revisions to the reporter that don't exist
> resulting in the dreaded "svn: Working copy path 'blah' does not exist in
> repository" errors.
>
> * subversion/libsvn_client/merge.c
> (drive_merge_report_editor):
It would be calming to see something written after the colon! Not
necessary for successful review, though.
> * subversion/tests/cmdline/merge_tests.py
> (test_list): Remove XFail from
> merge_target_and_subtrees_need_nonintersecting_ranges.
> ]]]
> /* If a subtree needs the same range applied as it's nearest parent
> - with mergeinfo, then we don't need to describe the subtree
> - separately. */
> + with mergeinfo or neither the subtree nor its nearest parent need
> + REVISION1:REVISION2 merged, then we don't need to describe the
> + subtree separately. In the latter case this could break the
> + editor if child->path didn't exist at REVISION2 and we attempt
> + to describe it via a reporter set_path call. */
> if (child->remaining_ranges->nelts)
> {
> range = APR_ARRAY_IDX(child->remaining_ranges, 0,
> svn_merge_range_t *);
> - if (parent->remaining_ranges->nelts)
> + if ((!is_rollback && range->start > revision2)
> + || (is_rollback && range->start < revision2))
> + {
> + /* Neither subtree nor parent need any
> + part of REVISION1:REVISION2. */
> + continue;
> + }
> + else if (parent->remaining_ranges->nelts)
> {
> svn_merge_range_t *parent_range =
> APR_ARRAY_IDX(parent->remaining_ranges, 0,
> @@ -2963,7 +2973,7 @@
> APR_ARRAY_IDX(child->remaining_ranges, 0,
> svn_merge_range_t *);
> if (parent_range->start == child_range->start)
> - continue; /* Same as parent. */
> + continue; /* Subtree needs same range as parent. */
> }
> }
> else /* child->remaining_ranges->nelts == 0*/
I'm still studying the implementation.
r32730: It's so trivial I won't even mention... Oh, whups, I did.
- Julian
The minor points, in the test, that I mentioned above, as a diff:
[[[
> Index: subversion/tests/cmdline/merge_tests.py
> ===================================================================
> --- subversion/tests/cmdline/merge_tests.py (revision 32738)
> +++ subversion/tests/cmdline/merge_tests.py (working copy)
> @@ -12404,9 +12404,7 @@
> nu_COPY_path = os.path.join(wc_dir, "A_COPY", "D", "G", "nu")
> omega_COPY_path = os.path.join(wc_dir, "A_COPY", "D", "H", "omega")
> beta_COPY_path = os.path.join(wc_dir, "A_COPY", "B", "E", "beta")
> - beta_COPY_path = os.path.join(wc_dir, "A_COPY", "B", "E", "beta")
> rho_COPY_path = os.path.join(wc_dir, "A_COPY", "D", "G", "rho")
> - omega_COPY_path = os.path.join(wc_dir, "A_COPY", "D", "H", "omega")
> psi_COPY_path = os.path.join(wc_dir, "A_COPY", "D", "H", "psi")
>
> # Make a branch to merge to.
> @@ -12427,7 +12425,7 @@
> svntest.actions.run_and_verify_commit(wc_dir, expected_output,
> wc_status, None, wc_dir)
>
> - # Do several merges to setup a situation where a subtree needs the merge
> + # Do several merges to setup a situation where the merge
> # target and two of its subtrees all need non-intersecting ranges
> # merged when doing a synch (a.k.a. cherry harvest) merge.
> #
> @@ -12455,7 +12453,6 @@
> svntest.actions.run_and_verify_svn(
> None, expected_merge_output([[-6]], 'G ' + omega_COPY_path + '\n'),
> [], 'merge', '-c-6', sbox.repo_url + '/A/D/H/omega', omega_COPY_path)
> - expected_output = wc.State(wc_dir, {'A/D/G/rho' : Item(verb='Sending')})
> wc_status.add({'A_COPY/D/G/nu' : Item(status=' ', wc_rev=9)})
> wc_status.tweak('A_COPY',
> 'A_COPY/B/E/beta',
]]]
---------------------------------------------------------------------
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-27 01:16:14 CEST