You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Paul Burba <pt...@gmail.com> on 2011/02/10 18:10:11 UTC

What should this tree conflict test expect? (was: svn commit: r879162)

Hi Neels (or any other tree conflict gurus),

I know you don't have much time for Subversion these days, but if you
find some time in the near future could you take a look at the tests
you added in r879162 and combined in r879206 (this test currently
resides at merge_tree_conflict_tests.py 23 'replace vs. delete
tree-conflicts').

I made a few tweaks to the test in r1069145 and r1069158 and it now
reaches the end of the test and fails.

Currently the test does the following:

1) Merge a directory replacement onto a locally deleted directory

2) Merge a directory-replaced-with-a-file onto a locally deleted directory

3) Repeat the merge of a file-replaced-with-a-directory onto a locally
deleted directory done in #2, but this time target a subtree of #2's
target.  The only effect this has is the change the resulting TC type
from 'local delete, incoming delete upon merge' to 'local delete,
incoming replace upon merge'.

I'm a bit foggy on what we should expect here and the test doesn't
have an associated issue (I was looking at this in hopes of adding
one).

Here's what we are currently seeing on trunk:

Case #1:

  >svn st -v A\B\E
  RM +  C          -        4  ?           A\B\E
        >   local delete, incoming delete upon merge
  D                3        1 jrandom      A\B\E\alpha
  D                3        1 jrandom      A\B\E\beta
  Summary of conflicts:
    Tree conflicts: 1

Case #2:

  >svn st -v A\D\H
  D     C          3        1 jrandom      A\D\H
        >   local delete, incoming replace upon merge
  D                3        1 jrandom      A\D\H\chi
  D                3        1 jrandom      A\D\H\omega
  D                3        1 jrandom      A\D\H\psi
  Summary of conflicts:
    Tree conflicts: 1

Case #3:

  >svn st -v A\D\G
   M               3        1 jrandom      A\D\G
  RM +  C          -        4  ?           A\D\G\pi
        >   local delete, incoming delete upon merge
                   3        1 jrandom      A\D\G\rho
                   3        1 jrandom      A\D\G\tau
  Summary of conflicts:
    Tree conflicts: 1

I was half expecting that the incoming delete of the replacement be
handled first and all of these be of the 'local delete, incoming
delete upon merge' variety and have a local status of 'D ', but that
is neither what the test expects or (obviously) what the results are.

Any insight you have into this is appreciated.  If you don't have time
to look at this, are there any threads where this was discussed or
particular docs I should be looking at?  The only relevant thing I
could find was the 'Renames and Replacements' section of
https://svn.apache.org/repos/asf/subversion/trunk/notes/tree-conflicts/resolution.txt,
but that is somewhat inconclusive.

Thanks,

Paul


> Author: neels
> Date: Wed Sep 2 17:32:50 2009
> New Revision: 879162
>
> * subversion/tests/cmdline/merge_tests.py
>  (merge_replace_causes_tree_conflict2, merge_replace_causes_tree_conflict3):
>   Add two tests, related to tree-conflicts during replace,
>   both marked XFail.
>
> --- subversion/trunk/subversion/tests/cmdline/merge_tests.py    2009/09/02
> 17:30:11        879161
> +++ subversion/trunk/subversion/tests/cmdline/merge_tests.py    2009/09/02
> 17:32:50        879162
> @@ -16228,6 +16228,437 @@ def copy_then_replace_via_merge(sbox):
>     })
>   actions.run_and_verify_status(branch_J, expected_status)
>
> +#----------------------------------------------------------------------
> +
> +def merge_replace_causes_tree_conflict2(sbox):
> +  "replace that causes a tree-conflict 2"
> +
> +  #  svntest.factory.make(sbox,r"""
> +  #      # make a branch of A
> +  #      svn cp $URL/A $URL/branch
> +  #      svn up
> +  #      # ACTIONS ON THE MERGE SOURCE (branch)
> +  #      # various deletes of files and dirs
> +  #      svn delete branch/mu branch/B/E branch/D/H
> +  #      svn ci -m "'delete' parts of the replace actions"
> +  #      svn up
> +  #
> +  #      # replacements.
> +  #      # file-with-file
> +  #      echo "replacement for mu" > branch/mu
> +  #      svn add branch/mu
> +  #      # dir-with-dir
> +  #      svn mkdir branch/B/E
> +  #      svn ps propname propval branch/B/E
> +  #      # (file-with-dir omitted, see next test)
> +  #      # dir-with-file
> +  #      echo "replacement for H" > branch/D/H
> +  #      svn add branch/D/H
> +  #      svn ci -m "'add' parts of the replace actions"
> +  #
> +  #      # ACTIONS ON THE MERGE TARGET (A)
> +  #      # local mods to conflict with merge source
> +  #      svn delete A/mu A/B/E A/D/H
> +  #      svn merge $URL/A $URL/branch A
> +  #      svn st
> +  #      """)
> +
> +  sbox.build()
> +  wc_dir = sbox.wc_dir
> +  url = sbox.repo_url
> +
> +  A = os.path.join(wc_dir, 'A')
> +  A_B_E = os.path.join(wc_dir, 'A', 'B', 'E')
> +  A_D_H = os.path.join(wc_dir, 'A', 'D', 'H')
> +  A_mu = os.path.join(wc_dir, 'A', 'mu')
> +  branch_B_E = os.path.join(wc_dir, 'branch', 'B', 'E')
> +  branch_D_H = os.path.join(wc_dir, 'branch', 'D', 'H')
> +  branch_mu = os.path.join(wc_dir, 'branch', 'mu')
> +  url_A = url + '/A'
> +  url_branch = url + '/branch'
> +
> +  # make a branch of A
> +  # svn cp $URL/A $URL/branch
> +  expected_stdout = verify.UnorderedOutput([
> +    '\n',
> +    'Committed revision 2.\n',
> +  ])
> +
> +  actions.run_and_verify_svn2('OUTPUT', expected_stdout, [], 0, 'cp', url_A,
> +    url_branch, '-m', 'copy log')
> +
> +  # svn up
> +  expected_output = svntest.wc.State(wc_dir, {
> +    'branch'            : Item(status='A '),
> +    'branch/D'          : Item(status='A '),
> +    'branch/D/G'        : Item(status='A '),
> +    'branch/D/G/rho'    : Item(status='A '),
> +    'branch/D/G/tau'    : Item(status='A '),
> +    'branch/D/G/pi'     : Item(status='A '),
> +    'branch/D/H'        : Item(status='A '),
> +    'branch/D/H/chi'    : Item(status='A '),
> +    'branch/D/H/psi'    : Item(status='A '),
> +    'branch/D/H/omega'  : Item(status='A '),
> +    'branch/D/gamma'    : Item(status='A '),
> +    'branch/B'          : Item(status='A '),
> +    'branch/B/E'        : Item(status='A '),
> +    'branch/B/E/alpha'  : Item(status='A '),
> +    'branch/B/E/beta'   : Item(status='A '),
> +    'branch/B/F'        : Item(status='A '),
> +    'branch/B/lambda'   : Item(status='A '),
> +    'branch/mu'         : Item(status='A '),
> +    'branch/C'          : Item(status='A '),
> +  })
> +
> +  expected_disk = svntest.main.greek_state.copy()
> +  expected_disk.add({
> +    'branch'            : Item(),
> +    'branch/D'          : Item(),
> +    'branch/D/G'        : Item(),
> +    'branch/D/G/rho'    : Item(contents="This is the file 'rho'.\n"),
> +    'branch/D/G/tau'    : Item(contents="This is the file 'tau'.\n"),
> +    'branch/D/G/pi'     : Item(contents="This is the file 'pi'.\n"),
> +    'branch/D/H'        : Item(),
> +    'branch/D/H/omega'  : Item(contents="This is the file 'omega'.\n"),
> +    'branch/D/H/chi'    : Item(contents="This is the file 'chi'.\n"),
> +    'branch/D/H/psi'    : Item(contents="This is the file 'psi'.\n"),
> +    'branch/D/gamma'    : Item(contents="This is the file 'gamma'.\n"),
> +    'branch/B'          : Item(),
> +    'branch/B/E'        : Item(),
> +    'branch/B/E/alpha'  : Item(contents="This is the file 'alpha'.\n"),
> +    'branch/B/E/beta'   : Item(contents="This is the file 'beta'.\n"),
> +    'branch/B/F'        : Item(),
> +    'branch/B/lambda'   : Item(contents="This is the file 'lambda'.\n"),
> +    'branch/mu'         : Item(contents="This is the file 'mu'.\n"),
> +    'branch/C'          : Item(),
> +  })
> +
> +  expected_status = actions.get_virginal_state(wc_dir, 2)
> +  expected_status.add({
> +    'branch'            : Item(status='  ', wc_rev='2'),
> +    'branch/B'          : Item(status='  ', wc_rev='2'),
> +    'branch/B/E'        : Item(status='  ', wc_rev='2'),
> +    'branch/B/E/beta'   : Item(status='  ', wc_rev='2'),
> +    'branch/B/E/alpha'  : Item(status='  ', wc_rev='2'),
> +    'branch/B/lambda'   : Item(status='  ', wc_rev='2'),
> +    'branch/B/F'        : Item(status='  ', wc_rev='2'),
> +    'branch/D'          : Item(status='  ', wc_rev='2'),
> +    'branch/D/G'        : Item(status='  ', wc_rev='2'),
> +    'branch/D/G/pi'     : Item(status='  ', wc_rev='2'),
> +    'branch/D/G/tau'    : Item(status='  ', wc_rev='2'),
> +    'branch/D/G/rho'    : Item(status='  ', wc_rev='2'),
> +    'branch/D/gamma'    : Item(status='  ', wc_rev='2'),
> +    'branch/D/H'        : Item(status='  ', wc_rev='2'),
> +    'branch/D/H/psi'    : Item(status='  ', wc_rev='2'),
> +    'branch/D/H/chi'    : Item(status='  ', wc_rev='2'),
> +    'branch/D/H/omega'  : Item(status='  ', wc_rev='2'),
> +    'branch/mu'         : Item(status='  ', wc_rev='2'),
> +    'branch/C'          : Item(status='  ', wc_rev='2'),
> +  })
> +
> +  actions.run_and_verify_update(wc_dir, expected_output, expected_disk,
> +    expected_status, None, None, None, None, None, False, wc_dir)
> +
> +  # ACTIONS ON THE MERGE SOURCE (branch)
> +  # various deletes of files and dirs
> +  # svn delete branch/mu branch/B/E branch/D/H
> +  expected_stdout = verify.UnorderedOutput([
> +    'D         ' + branch_mu + '\n',
> +    'D         ' + branch_B_E + '/alpha\n',
> +    'D         ' + branch_B_E + '/beta\n',
> +    'D         ' + branch_B_E + '\n',
> +    'D         ' + branch_D_H + '/chi\n',
> +    'D         ' + branch_D_H + '/omega\n',
> +    'D         ' + branch_D_H + '/psi\n',
> +    'D         ' + branch_D_H + '\n',
> +  ])
> +
> +  actions.run_and_verify_svn2('OUTPUT', expected_stdout, [], 0, 'delete',
> +    branch_mu, branch_B_E, branch_D_H)
> +
> +  # svn ci -m "'delete' parts of the replace actions"
> +  expected_output = svntest.wc.State(wc_dir, {
> +    'branch/D/H'        : Item(verb='Deleting'),
> +    'branch/B/E'        : Item(verb='Deleting'),
> +    'branch/mu'         : Item(verb='Deleting'),
> +  })
> +
> +  expected_status.remove('branch/mu', 'branch/D/H', 'branch/D/H/psi',
> +    'branch/D/H/chi', 'branch/D/H/omega', 'branch/B/E', 'branch/B/E/beta',
> +    'branch/B/E/alpha')
> +
> +  actions.run_and_verify_commit(wc_dir, expected_output, expected_status,
> +    None, wc_dir)
> +
> +  # svn up
> +  expected_output = svntest.wc.State(wc_dir, {})
> +
> +  expected_disk.remove('branch/mu', 'branch/D/H', 'branch/D/H/omega',
> +    'branch/D/H/chi', 'branch/D/H/psi', 'branch/B/E', 'branch/B/E/alpha',
> +    'branch/B/E/beta')
> +
> +  expected_status.tweak(wc_rev='3')
> +
> +  actions.run_and_verify_update(wc_dir, expected_output, expected_disk,
> +    expected_status, None, None, None, None, None, False, wc_dir)
> +
> +  # replacements.
> +  # file-with-file
> +  # echo "replacement for mu" > branch/mu
> +  main.file_write(branch_mu, 'replacement for mu')
> +
> +  # svn add branch/mu
> +  expected_stdout = ['A         ' + branch_mu + '\n']
> +
> +  actions.run_and_verify_svn2('OUTPUT', expected_stdout, [], 0, 'add',
> +    branch_mu)
> +
> +  # dir-with-dir
> +  # svn mkdir branch/B/E
> +  expected_stdout = ['A         ' + branch_B_E + '\n']
> +
> +  actions.run_and_verify_svn2('OUTPUT', expected_stdout, [], 0, 'mkdir',
> +    branch_B_E)
> +
> +  # svn ps propname propval branch/B/E
> +  expected_stdout = ["property 'propname' set on '" + branch_B_E + "'\n"]
> +
> +  actions.run_and_verify_svn2('OUTPUT', expected_stdout, [], 0, 'ps',
> +    'propname', 'propval', branch_B_E)
> +
> +  # (file-with-dir omitted, see next test)
> +  # dir-with-file
> +  # echo "replacement for H" > branch/D/H
> +  main.file_write(branch_D_H, 'replacement for H')
> +
> +  # svn add branch/D/H
> +  expected_stdout = ['A         ' + branch_D_H + '\n']
> +
> +  actions.run_and_verify_svn2('OUTPUT', expected_stdout, [], 0, 'add',
> +    branch_D_H)
> +
> +  # svn ci -m "'add' parts of the replace actions"
> +  expected_output = svntest.wc.State(wc_dir, {
> +    'branch/D/H'        : Item(verb='Adding'),
> +    'branch/B/E'        : Item(verb='Adding'),
> +    'branch/mu'         : Item(verb='Adding'),
> +  })
> +
> +  expected_status.add({
> +    'branch/D/H'        : Item(status='  ', wc_rev='4'),
> +    'branch/B/E'        : Item(status='  ', wc_rev='4'),
> +    'branch/mu'         : Item(status='  ', wc_rev='4'),
> +  })
> +
> +  actions.run_and_verify_commit(wc_dir, expected_output, expected_status,
> +    None, wc_dir)
> +
> +  # ACTIONS ON THE MERGE TARGET (A)
> +  # local mods to conflict with merge source
> +  # svn delete A/mu A/B/E A/D/H
> +  expected_stdout = verify.UnorderedOutput([
> +    'D         ' + A_mu + '\n',
> +    'D         ' + A_B_E + '/alpha\n',
> +    'D         ' + A_B_E + '/beta\n',
> +    'D         ' + A_B_E + '\n',
> +    'D         ' + A_D_H + '/chi\n',
> +    'D         ' + A_D_H + '/omega\n',
> +    'D         ' + A_D_H + '/psi\n',
> +    'D         ' + A_D_H + '\n',
> +  ])
> +
> +  actions.run_and_verify_svn2('OUTPUT', expected_stdout, [], 0, 'delete',
> +    A_mu, A_B_E, A_D_H)
> +
> +  # svn merge $URL/A $URL/branch A
> +  expected_stdout = verify.UnorderedOutput([
> +    "--- Merging differences between repository URLs into '" + A + "':\n",
> +    '   C ' + A_B_E + '\n',
> +    '   C ' + A_mu + '\n',
> +    '   C ' + A_D_H + '\n',
> +    'Summary of conflicts:\n',
> +    '  Tree conflicts: 3\n',
> +  ])
> +
> +  actions.run_and_verify_svn2('OUTPUT', expected_stdout, [], 0, 'merge',
> +    url_A, url_branch, A)
> +
> +  # svn st
> +  expected_status.tweak('A', status=' M')
> +  expected_status.tweak('A/mu', 'A/B/E', 'A/D/H', status='D ',
> +    treeconflict='C')
> +  expected_status.tweak('A/D/H/chi', 'A/D/H/omega', 'A/D/H/psi', status='D ')
> +  expected_status.tweak('A/B/E/alpha', 'A/B/E/beta', status='D ')
> +
> +  actions.run_and_verify_status(wc_dir, expected_status)
> +
> +
> +def merge_replace_causes_tree_conflict3(sbox):
> +  "replace that causes a tree-conflict 3"
> +
> +  #  svntest.factory.make(sbox,r"""
> +  #      # make a branch of A/D/G
> +  #      svn cp $URL/A/D/G $URL/branch
> +  #      svn up
> +  #      # ACTIONS ON THE MERGE SOURCE (branch)
> +  #      # delete a file
> +  #      svn delete branch/pi
> +  #      svn ci -m "'delete' parts of the replace actions"
> +  #      svn up
> +  #
> +  #      # replace file with dir
> +  #      svn mkdir branch/pi
> +  #      svn ci -m "'add' parts of the replace actions"
> +  #
> +  #      # ACTIONS ON THE MERGE TARGET (A)
> +  #      # local mods to conflict with merge source
> +  #      svn delete A/D/G/pi
> +  #      svn ci -m del
> +  #      svn up A
> +  #      svn merge $URL/A/D/G $URL/branch A/D/G
> +  #      """)
> +
> +  sbox.build()
> +  wc_dir = sbox.wc_dir
> +  url = sbox.repo_url
> +
> +  A = os.path.join(wc_dir, 'A')
> +  A_D_G = os.path.join(wc_dir, 'A', 'D', 'G')
> +  A_D_G_pi = os.path.join(wc_dir, 'A', 'D', 'G', 'pi')
> +  branch_pi = os.path.join(wc_dir, 'branch', 'pi')
> +  url_A_D_G = url + '/A/D/G'
> +  url_branch = url + '/branch'
> +
> +  # make a branch of A/D/G
> +  # svn cp $URL/A/D/G $URL/branch
> +  expected_stdout = verify.UnorderedOutput([
> +    '\n',
> +    'Committed revision 2.\n',
> +  ])
> +
> +  actions.run_and_verify_svn2('OUTPUT', expected_stdout, [], 0, 'cp',
> +    url_A_D_G, url_branch, '-m', 'copy log')
> +
> +  # svn up
> +  expected_output = svntest.wc.State(wc_dir, {
> +    'branch'            : Item(status='A '),
> +    'branch/pi'         : Item(status='A '),
> +    'branch/tau'        : Item(status='A '),
> +    'branch/rho'        : Item(status='A '),
> +  })
> +
> +  expected_disk = svntest.main.greek_state.copy()
> +  expected_disk.add({
> +    'branch'            : Item(),
> +    'branch/pi'         : Item(contents="This is the file 'pi'.\n"),
> +    'branch/rho'        : Item(contents="This is the file 'rho'.\n"),
> +    'branch/tau'        : Item(contents="This is the file 'tau'.\n"),
> +  })
> +
> +  expected_status = actions.get_virginal_state(wc_dir, 2)
> +  expected_status.add({
> +    'branch'            : Item(status='  ', wc_rev='2'),
> +    'branch/pi'         : Item(status='  ', wc_rev='2'),
> +    'branch/tau'        : Item(status='  ', wc_rev='2'),
> +    'branch/rho'        : Item(status='  ', wc_rev='2'),
> +  })
> +
> +  actions.run_and_verify_update(wc_dir, expected_output, expected_disk,
> +    expected_status, None, None, None, None, None, False, wc_dir)
> +
> +  # ACTIONS ON THE MERGE SOURCE (branch)
> +  # delete a file
> +  # svn delete branch/pi
> +  expected_stdout = ['D         ' + branch_pi + '\n']
> +
> +  actions.run_and_verify_svn2('OUTPUT', expected_stdout, [], 0, 'delete',
> +    branch_pi)
> +
> +  # svn ci -m "'delete' parts of the replace actions"
> +  expected_output = svntest.wc.State(wc_dir, {
> +    'branch/pi'         : Item(verb='Deleting'),
> +  })
> +
> +  expected_status.remove('branch/pi')
> +
> +  actions.run_and_verify_commit(wc_dir, expected_output, expected_status,
> +    None, wc_dir)
> +
> +  # svn up
> +  expected_output = svntest.wc.State(wc_dir, {})
> +
> +  expected_disk.remove('branch/pi')
> +
> +  expected_status.tweak(wc_rev='3')
> +
> +  actions.run_and_verify_update(wc_dir, expected_output, expected_disk,
> +    expected_status, None, None, None, None, None, False, wc_dir)
> +
> +  # replace file with dir
> +  # svn mkdir branch/pi
> +  expected_stdout = ['A         ' + branch_pi + '\n']
> +
> +  actions.run_and_verify_svn2('OUTPUT', expected_stdout, [], 0, 'mkdir',
> +    branch_pi)
> +
> +  # svn ci -m "'add' parts of the replace actions"
> +  expected_output = svntest.wc.State(wc_dir, {
> +    'branch/pi'         : Item(verb='Adding'),
> +  })
> +
> +  expected_status.add({
> +    'branch/pi'         : Item(status='  ', wc_rev='4'),
> +  })
> +
> +  actions.run_and_verify_commit(wc_dir, expected_output, expected_status,
> +    None, wc_dir)
> +
> +  # ACTIONS ON THE MERGE TARGET (A)
> +  # local mods to conflict with merge source
> +  # svn delete A/D/G/pi
> +  expected_stdout = ['D         ' + A_D_G_pi + '\n']
> +
> +  actions.run_and_verify_svn2('OUTPUT', expected_stdout, [], 0, 'delete',
> +    A_D_G_pi)
> +
> +  # svn ci -m del
> +  expected_output = svntest.wc.State(wc_dir, {
> +    'A/D/G/pi'          : Item(verb='Deleting'),
> +  })
> +
> +  expected_status.remove('A/D/G/pi')
> +
> +  actions.run_and_verify_commit(wc_dir, expected_output, expected_status,
> +    None, wc_dir)
> +
> +  # svn up A
> +  expected_output = svntest.wc.State(wc_dir, {})
> +
> +  expected_disk.remove('A/D/G/pi')
> +  expected_disk.add({
> +    'branch/pi'         : Item(),
> +  })
> +
> +  expected_status.tweak(wc_rev='5')
> +  expected_status.tweak('', 'branch', 'branch/tau', 'branch/rho', 'iota',
> +    wc_rev='3')
> +  expected_status.tweak('branch/pi', wc_rev='4')
> +
> +  actions.run_and_verify_update(wc_dir, expected_output, expected_disk,
> +    expected_status, None, None, None, None, None, False, A)
> +
> +  # svn merge $URL/A/D/G $URL/branch A/D/G
> +  expected_stdout = verify.UnorderedOutput([
> +    "--- Merging differences between repository URLs into '" + A_D_G +
> +    "':\n",
> +    '   C ' + A_D_G_pi + '\n',
> +    'Summary of conflicts:\n',
> +    '  Tree conflicts: 1\n',
> +  ])
> +
> +  actions.run_and_verify_svn2('OUTPUT', expected_stdout, [], 0, 'merge',
> +    url_A_D_G, url_branch, A_D_G)
> +
>
>  ########################################################################
>  # Run the tests
> @@ -16450,6 +16881,8 @@ test_list = [ None,
>               SkipUnless(handle_gaps_in_implicit_mergeinfo,
>                          server_has_mergeinfo),
>               XFail(copy_then_replace_via_merge),
> +              XFail(merge_replace_causes_tree_conflict2),
> +              XFail(merge_replace_causes_tree_conflict3),
>              ]
>
>  if __name__ == '__main__':
>

Re: What should this tree conflict test expect? (was: svn commit: r879162)

Posted by Paul Burba <pt...@gmail.com>.
On Fri, Feb 11, 2011 at 7:07 AM, Julian Foad <ju...@wandisco.com> wrote:
> On Thu, 2011-02-10 at 12:10 -0500, Paul Burba wrote:
>> Hi Neels (or any other tree conflict gurus),
>>
>> I know you don't have much time for Subversion these days, but if you
>> find some time in the near future could you take a look at the tests
>> you added in r879162 and combined in r879206 (this test currently
>> resides at merge_tree_conflict_tests.py 23 'replace vs. delete
>> tree-conflicts').
>>
>> I made a few tweaks to the test in r1069145 and r1069158 and it now
>> reaches the end of the test and fails.
>>
>> Currently the test does the following:
>>
>> 1) Merge a directory replacement onto a locally deleted directory
>>
>> 2) Merge a directory-replaced-with-a-file onto a locally deleted directory
>>
>> 3) Repeat the merge of a file-replaced-with-a-directory onto a locally
>> deleted directory done in #2, but this time target a subtree of #2's
>> target.
>
> This doesn't repeat anything of #2, it's completely separate.

Ah yes, I missed the fact that the merge was at depth immediates.

>  #2 directory-replaced-with-a-file onto A/D/H (target A/D depth immed)
>  #3 file-replaced-with-a-directory onto A/D/G/pi (target A/D/G)
>
> I would expect to see a fourth case tested too:
>
>  #4 file-replaced-with-a-file onto mu

I added that in r1070629 as well as rewriting the test a bit to make
it easier to follow.

> (The test starts by calling merge_replace_setup() which sets up for all
> of these four cases.)
>
>
>>   The only effect this has is the change the resulting TC type
>> from 'local delete, incoming delete upon merge' to 'local delete,
>> incoming replace upon merge'.
>
> The ideal expectation would be for all three or four cases to report
> 'local delete, incoming replace upon merge'.
>
> I don't know how close we are in implementation to being able to report
> these as incoming replacements; it might not be easy.

I'm not sure either and at the moment don't plan to go any further
than adding an associated issue for this test (issue #3806).

Thanks Julian,

Paul

>> I'm a bit foggy on what we should expect here and the test doesn't
>> have an associated issue (I was looking at this in hopes of adding
>> one).
>>
>> Here's what we are currently seeing on trunk:
>>
>> Case #1:
>>
>>   >svn st -v A\B\E
>>   RM +  C          -        4  ?           A\B\E
>>         >   local delete, incoming delete upon merge
>
> That's quite good except it should say "incoming replace".
>
>> Case #2:
>>
>>   >svn st -v A\D\H
>>   D     C          3        1 jrandom      A\D\H
>>         >   local delete, incoming replace upon merge
>
> That's quite good except it should say "R" not "D".
>
>> Case #3:
>>
>>   >svn st -v A\D\G
>>    M               3        1 jrandom      A\D\G
>>   RM +  C          -        4  ?           A\D\G\pi
>>         >   local delete, incoming delete upon merge
>
> That's quite good except it should say "incoming replace".
>
> Wierd that in all these cases the "D"/"R" is opposite to the message.
>
> (Also, I think status should never report "M" in the properties column
> of a "R"eplaced node.  But that's a separate issue, probably unrelated
> to conflicts.)
>
> - Julian
>
>
>> I was half expecting that the incoming delete of the replacement be
>> handled first and all of these be of the 'local delete, incoming
>> delete upon merge' variety and have a local status of 'D ', but that
>> is neither what the test expects or (obviously) what the results are.
>>
>> Any insight you have into this is appreciated.  If you don't have time
>> to look at this, are there any threads where this was discussed or
>> particular docs I should be looking at?  The only relevant thing I
>> could find was the 'Renames and Replacements' section of
>> https://svn.apache.org/repos/asf/subversion/trunk/notes/tree-conflicts/resolution.txt,
>> but that is somewhat inconclusive.
>>
>> Thanks,
>>
>> Paul

Re: What should this tree conflict test expect? (was: svn commit: r879162)

Posted by Julian Foad <ju...@wandisco.com>.
On Thu, 2011-02-10 at 12:10 -0500, Paul Burba wrote:
> Hi Neels (or any other tree conflict gurus),
> 
> I know you don't have much time for Subversion these days, but if you
> find some time in the near future could you take a look at the tests
> you added in r879162 and combined in r879206 (this test currently
> resides at merge_tree_conflict_tests.py 23 'replace vs. delete
> tree-conflicts').
> 
> I made a few tweaks to the test in r1069145 and r1069158 and it now
> reaches the end of the test and fails.
> 
> Currently the test does the following:
> 
> 1) Merge a directory replacement onto a locally deleted directory
> 
> 2) Merge a directory-replaced-with-a-file onto a locally deleted directory
> 
> 3) Repeat the merge of a file-replaced-with-a-directory onto a locally
> deleted directory done in #2, but this time target a subtree of #2's
> target.

This doesn't repeat anything of #2, it's completely separate.

  #2 directory-replaced-with-a-file onto A/D/H (target A/D depth immed)
  #3 file-replaced-with-a-directory onto A/D/G/pi (target A/D/G)

I would expect to see a fourth case tested too:

  #4 file-replaced-with-a-file onto mu

(The test starts by calling merge_replace_setup() which sets up for all
of these four cases.)


>   The only effect this has is the change the resulting TC type
> from 'local delete, incoming delete upon merge' to 'local delete,
> incoming replace upon merge'.

The ideal expectation would be for all three or four cases to report
'local delete, incoming replace upon merge'.

I don't know how close we are in implementation to being able to report
these as incoming replacements; it might not be easy.


> I'm a bit foggy on what we should expect here and the test doesn't
> have an associated issue (I was looking at this in hopes of adding
> one).
> 
> Here's what we are currently seeing on trunk:
> 
> Case #1:
> 
>   >svn st -v A\B\E
>   RM +  C          -        4  ?           A\B\E
>         >   local delete, incoming delete upon merge

That's quite good except it should say "incoming replace".

> Case #2:
> 
>   >svn st -v A\D\H
>   D     C          3        1 jrandom      A\D\H
>         >   local delete, incoming replace upon merge

That's quite good except it should say "R" not "D".

> Case #3:
> 
>   >svn st -v A\D\G
>    M               3        1 jrandom      A\D\G
>   RM +  C          -        4  ?           A\D\G\pi
>         >   local delete, incoming delete upon merge

That's quite good except it should say "incoming replace".

Wierd that in all these cases the "D"/"R" is opposite to the message.

(Also, I think status should never report "M" in the properties column
of a "R"eplaced node.  But that's a separate issue, probably unrelated
to conflicts.)

- Julian


> I was half expecting that the incoming delete of the replacement be
> handled first and all of these be of the 'local delete, incoming
> delete upon merge' variety and have a local status of 'D ', but that
> is neither what the test expects or (obviously) what the results are.
> 
> Any insight you have into this is appreciated.  If you don't have time
> to look at this, are there any threads where this was discussed or
> particular docs I should be looking at?  The only relevant thing I
> could find was the 'Renames and Replacements' section of
> https://svn.apache.org/repos/asf/subversion/trunk/notes/tree-conflicts/resolution.txt,
> but that is somewhat inconclusive.
> 
> Thanks,
> 
> Paul