> -----Original Message----- > From: Stefan Sperling [mailto:s...@elego.de] > Sent: vrijdag 22 augustus 2014 11:51 > To: dev@subversion.apache.org > Subject: Re: svn commit: r1619717 - > /subversion/trunk/subversion/libsvn_wc/conflicts.c > > On Fri, Aug 22, 2014 at 09:29:27AM -0000, rhuij...@apache.org wrote: > > Author: rhuijben > > Date: Fri Aug 22 09:29:27 2014 > > New Revision: 1619717 > > > > URL: http://svn.apache.org/r1619717 > > > * subversion/libsvn_wc/conflicts.c > > (read_tree_conflict_desc): > > Don't look at deleted nodes > > when merging, as those nodes can be completely replaced by the merge. > > Given a merge that replaces a locally (uncommitted) deleted file, with > something of a different node kind, say, a directory, then:
Are we talking about merging a replace, or about adding something new with a merge (that happens to replace something else). In other words: * are we merging a delete + and add (and calling it a replace... or 'merging a replace') * Or are we merging an add over something that was locally deleted before the merge. This difference makes all the difference in answering your further questions > > The victim's node kind is 'file'. (local file delete ...) What victim? Whatever was locally deleted? Or the thing to be deleted by the merge? Or the thing to be added by the merge? (I don't think we are talking about the thing to be updated by the merge) All of these are victims... I don't see a single 'victim kind', I see many possible kinds. And potentially (in the working copy) there could be many op-depth layers below all this, which in some cases might also be called victims... but luckily we consistently ignore these in merge scenarios. With 1.7 we moved most things to the notion that whenever something is locally deleted, it is no longer a directory, nor a file... It is a deleted node. (In <= 1.6 a deleted directory/file could only be replaced by something of the same kind... Especially since svn_wc_entry_t only had a single kind field) Once the delete is really a relevant part of the tree conflict, we still report it... (e.g. when updating, where we do look at the BASE tree), but in most cases the fact that there is something deleted is far more interesting than what was deleted. > > The merge-left node kind could be any of 'none' (the directory was > added in the merge source), or 'file' or 'dir' (the directory replaced > a file or directory in the merge source.) > > The merge-right node kind is 'directory'. (... incoming directory > add/replace upon merge) > > It looks like we call the victim a 'directory'... > > $ cd svn-sandbox/trunk > $ svn rm alpha # alpha is a file > D alpha > $ cd ../branch/ > $ svn rm alpha > D alpha > $ svn mkdir alpha > A alpha > $ svn ci -mm > Replacing alpha > Committing transaction... > Committed revision 3. > $ cd ../trunk > $ svn merge ^/branch > --- Merging r2 through r3 into '.': > C alpha > A alpha > --- Recording mergeinfo for merge of r2 through r3 into '.': > U . > Summary of conflicts: > Tree conflicts: 1 > Tree conflict on 'alpha' > > local dir delete, incoming dir replace upon merge > Select: (r) mark resolved, (p) postpone, (q) quit resolution, (h) help: > > I'm not sure this should even be flagged as a conflict. > However, the victim node kind is wrong. I think it should say local file delete, ... (but local, left-src and right-src are all victims) I'm not sure in which state the working copy is here now. I wouldn't be surprised if we already merged the add part of the replacement here... (to keep as much information as possible) In that case looking in the working copy or on disk is what makes us report the wrong node kind. On a local delete we should really look one layer below the current op-depth to see what the original node kind is/was, before the local delete/replacement. Bert > > > --- subversion/trunk/subversion/libsvn_wc/conflicts.c (original) > > +++ subversion/trunk/subversion/libsvn_wc/conflicts.c Fri Aug 22 09:29:27 > 2014 > > @@ -1813,11 +1813,14 @@ read_tree_conflict_desc(svn_wc_conflict_ > > SVN_ERR(svn_io_check_path(local_abspath, &local_kind, > scratch_pool)); > > else if (operation == svn_wc_operation_merge) > > { > > - /* Read the tree conflict victim's node kind from the working > > - * or base tree. */ > > + /* Read the tree conflict victim's node kind from the working copy, > > + or if it doesn't exist directly from disk. */ > > SVN_ERR(svn_wc__db_read_kind(&local_kind, db, local_abspath, > > - TRUE, TRUE, TRUE, scratch_pool)); > > - if (local_kind == svn_node_unknown) > > + TRUE /* allow missing */, > > + FALSE /* show deleted */, > > + FALSE /* show hidden */, scratch_pool)); > > + > > + if (local_kind == svn_node_unknown || local_kind == > svn_node_none) > > SVN_ERR(svn_io_check_path(local_abspath, &local_kind, > scratch_pool)); > > } > > else if (operation == svn_wc_operation_update ||