On Wed, 2008-11-05 at 10:05 +0100, Stefan Sperling wrote:
> On Tue, Nov 04, 2008 at 06:20:59PM -0800, neels_at_tigris.org wrote:
> > Author: neels
> > Date: Tue Nov 4 18:20:59 2008
> > New Revision: 34060
> >
> > Log:
> > Fix default return values for *TREE_CONFLICTED in the merge callbacks.
> > Partly fix merge_props_changed() w.r.t. tree-conflicts.
> >
> > * subversion/libsvn_client/merge.c
> > (merge_props_changed):
> > Set *TREE_CONFLICTED to FALSE by default (was missing). Report one
> > new tree-conflict. Add a TODO comment to detect other tree-conflicts.
>
> Have we discussed yet how property changes and tree conflict changes
> should interact in detail (for all steps including revert)? I'm not
> saying this is not a tree conflict, but see below.
It seems fairly clear to me. From the point of view of tree conflicts, a
property change is just like a text change on a file: it has the same
importance and the same consequences. If a text change on a nonexistent
file causes a tree conflict, then so does a prop change, and so on. On a
directory, treat prop changes the same as we do on a file. (I know
that's vague.)
The definition of "revert" is that all local modifications of the target
are thrown away and conflicts are marked as "resolved". I interpret that
as meaning that any tree conflict in which TARGET is the victim shall be
marked as resolved. If TARGET is a directory that stores information
about tree conflicts in which its children are victims, those conflicts
shall not be marked as resolved.
> > (merge_file_changed):
> > Properly propagate possible tree-conflicts from merge_props_changed().
>
> > @@ -1010,8 +1016,22 @@ merge_file_changed(svn_wc_adm_access_t *
> > /* Do property merge before text merge so that keyword expansion takes
> > into account the new property values. */
> > if (prop_changes->nelts > 0)
> > - SVN_ERR(merge_props_changed(adm_access, prop_state, tree_conflicted,
> > - mine, prop_changes, original_props, baton));
> > + {
> > + svn_boolean_t tree_conflicted2;
> > +
> > + SVN_ERR(merge_props_changed(adm_access, prop_state, &tree_conflicted2,
> > + mine, prop_changes, original_props, baton));
> > +
> > + /* If the prop change caused a tree-conflict, just bail. */
> > + if (tree_conflicted2)
> > + {
> > + if (tree_conflicted != NULL)
> > + *tree_conflicted = TRUE;
> > +
> > + svn_pool_destroy(subpool);
> > + return SVN_NO_ERROR;
> > + }
> > + }
> > else
> > if (prop_state)
> > *prop_state = svn_wc_notify_state_unchanged;
>
> We now bail if a prop change could not be applied to a missing file.
> How is a user supposed to resolve this situation? Can the necessary
> operations for resolving still be carried out, or do we happen to
> disallow them because there's now a tree conflict flagged on the file?
OK, I guess we're not so clear on what we're trying to block (apart from
commits). I have said:
* We can't allow update/switch/merge into a tree-conflicted node
because we can't cope with the fact that it might conflict again;
* We can't allow update/switch/merge into a tree-conflicted node
because that's a silly thing to attempt: the user should resolve the
conflict first.
We said we need an "svn merge --allow-into-conflicted" option, and I
started on a patch for that, but I haven't got beyond passing the option
down to the merge code. And I am still not really thrilled with it as a
solution.
I wonder if in fact we should not block update/switch/merge from trying
to merge into a tree-conflicted node, but instead let it try, and then
fail if it tries to raise another conflict on the same node. (I would
not let it raise a tree conflict if there is already a text or prop
conflict, or vice versa.)
- Julian
> (These question may sound rather silly, but AFAIK we have not yet
> thought this through. As the problem markphip reported a while ago
> shows, raising tree conflicts is only half of the picture.)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-11-07 19:04:13 CET