[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

RE: svn commit: r1619717 - /subversion/trunk/subversion/libsvn_wc/conflicts.c

From: Bert Huijben <bert_at_qqmail.nl>
Date: Fri, 22 Aug 2014 12:26:12 +0200

> -----Original Message-----
> From: Stefan Sperling [mailto:stsp_at_elego.de]
> Sent: vrijdag 22 augustus 2014 11:51
> To: dev_at_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, rhuijben_at_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 ||
Received on 2014-08-22 12:26:46 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.