tree conflicts conflict with implementation of test for issue #2829

[Stefan Sperling <st...@elego.de>]

Hi again,

Going through the failing tests on the tree-conflict branch,
I've caught another test that triggers a tree conflict but
does not handle this correctly at the moment.

The test in quesion is this one:

def mergeinfo_recording_in_skipped_merge(sbox):
  "mergeinfo recording in skipped merge"

The test stems from issue #2829. The problem seems to be that
the test assumes that:

1. Creating a branch off trunk:

  # Create a WC with a single branch
    sbox.build()
    wc_dir = sbox.wc_dir
    wc_disk, wc_status = set_up_branch(sbox, True, 1)

2. Deleting a file in the branch:

  # Delete A_COPY/B/E
  svntest.actions.run_and_verify_svn(None, None, [], 'rm',
                                     A_COPY_B_E_path)

3. And then merging new revs from trunk into the branch:

  # Merge /A to /A_COPY ie., r1 to r4 
  <snip>
  svntest.actions.run_and_verify_merge(short_A_COPY, None, None,
                                       A_url,
  <snip>

... causes 'merge' to skip the deleted file.

However, on the tree-conflicts branch, this does not cause a skip
anymore, but a tree conflict.

Quoting the "USE CASE 4" section from notes/tree-conflicts/detection.txt:

  If 'svn merge' tries to modify a file that does not exist in the
  target working copy, then the target file is a tree conflict victim.

So can the test be adjusted in a way that makes the merge cause
a skip, but not a tree conflict?

One idea that comes to mind is that skipped merges should still occur
for merges with source directories that the user has no authorisation
for. If the test could somehow emulate that, it would pass again.

Anyone involved in fixing issue #2829 around for comments?
dlr? pburba? stylesen?

-- 
Stefan Sperling <st...@elego.de>                 Software Developer
elego Software Solutions GmbH                            HRB 77719
Gustav-Meyer-Allee 25, Gebaeude 12        Tel:  +49 30 23 45 86 96 
13355 Berlin                              Fax:  +49 30 23 45 86 95
http://www.elego.de                 Geschaeftsfuehrer: Olaf Wagner

Re: tree conflicts conflict with implementation of test for issue #2829

[Julian Foad <ju...@btopenworld.com>]

Stefan Sperling wrote:
> Going through the failing tests on the tree-conflict branch,
> I've caught another test that triggers a tree conflict but
> does not handle this correctly at the moment.
> 
> The test in quesion is this one:
> 
> def mergeinfo_recording_in_skipped_merge(sbox):
>   "mergeinfo recording in skipped merge"
> 
> The test stems from issue #2829.

Issue #2829's summary is, "Improve handling for skipped paths encountered 
during a merge," and the point of the issue is that "merge info should be 
recorded for the target, and recorded as empty (or with no modifications, if 
there is pre-existing merge info) for the skipped items."


> The problem seems to be that
> the test assumes that:
> 
> 1. Creating a branch off trunk:
[...]
> 2. Deleting a file in the branch:
[...]
> 3. And then merging new revs from trunk into the branch:
[...]
> 
> ... causes 'merge' to skip the deleted file.
> 
> However, on the tree-conflicts branch, this does not cause a skip
> anymore, but a tree conflict.

It is entirely correct that a conflict should be raised in this case where the 
file has been deleted from the branch, as that is no longer a case that we wish 
to handle by "skipping".


[...]
> So can the test be adjusted in a way that makes the merge cause
> a skip, but not a tree conflict?

We could decide that the purpose of this particular test is to test what 
happens in a "skip" situation, in which case, yes, it should set up a slightly 
different scenario that will cause a skip, such as your authorisation idea below.

Alternatively, we could decide that the purpose of this particular test is to 
test a tree conflict use case, in which case the test should be changed to 
expect a tree conflict.

Its original purpose was to test the "skip" behaviour, so the latter would be a 
better short-term fix. Really, we want both tests so perhaps you could split it 
into two.


> One idea that comes to mind is that skipped merges should still occur
> for merges with source directories that the user has no authorisation
> for. If the test could somehow emulate that, it would pass again.

I'm not sure if we want to continue skipping such paths. It seems to me like an 
unsafe and not very useful behaviour. It would be safer to flag a conflict so 
that the user can clearly see that the merge they requested is not going ahead 
as planned. If the user really wants to go ahead with a partial merge, they can 
either request the whole merge and then resolve the conflicts, or instead they 
could request a merge of just the path(s) that they have access to.

I think I'd better raise this subject of "skipped" targets in a separate thread.

- Julian


> Anyone involved in fixing issue #2829 around for comments?
> dlr? pburba? stylesen?

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org