On Tue, 2008-10-28 at 05:36 +0100, Neels J. Hofmeyr wrote:
[...]
> This is the tiny tip of an iceberg that will require the diff callbacks to
> change (but a different change than what diff --summarize needs). The merge
> code needs to be tree-conflict aware. The relevant action takes place in the
> diff_callbacks, and it is kind of difficult to get the information out of
> there: diff callbacks return each conflict state (text, prop) separately via
> a pointer argument passed to the callback function. So we need another
> pointer argument to return a tree-conflicted state to the diff callbacks'
> notification layer (I'm saying that the callbacks signatures need to have
> another argument added). BTW, I really don't like the design of the diff
> callbacks in that respect. It takes lots of zombie typing to refactor.
Yes. The callbacks don't pass around a baton that they could use to
collect this info and process it at (e.g.) dir_close() time.
That's another argument to move to using "editor" callbacks.
[...]
> BTW, the branch tree-conflicts-notify can take no more of this. It should go
> back to trunk -- *now*.
Yes.
[Test failures ...]
> Now, do we have a design problem here? Maybe it *is* necessary to block a
> recursive commit on a directory that has *missing* items that remain in
> tree-conflict, and thus we *do* need to block a commit in a directory that
> *contains* certain tree conflicts? Repeat:
> - A commit should only be blocked when the whole directory is committed
> recursively. This already happens for nodes that are actually present, but
> missing ones currently don't get checked.
> - When committing non-conflicted siblings of a tree-conflict victim
> explicitly, then that shouldn't block.
> So maybe we *do* still need the HAS_TREE_CONFLICTED_CHILDREN as an optional
> return value in svn_wc_conflicted_p2().
>
> This is slowly turning into quite a landscape of numerous different levels
> of tree-conflicts:
>
> tree-conflicts of relevance on a given node:
> - just been newly raised on this node.
> - just been newly raised on an ancestor while crawling.
> - persisting on this node (from a previous operation).
> - persisting on any ancestors.
> - persisting on a *missing* child node, when recursing a dir.
>
>
> Hey, what if I have
> svn up
> C A
> but I do
> cd A/sub/dir
> svn ci
> -- will the commit fail to block because we're not crawling up into the
> ancestors of the current working copy directory? Do we have to persist the
> complete subtree of a tree-conflicted WC dir as conflicted, even though not
> notifying or info-ing about it? There could be a flag saying "yeah, you
> can't commit me, but it's one of my ancestors' fault, so you have to fix
> stuff further up". (Or is this already accounted for?)
>
> Wow, tree-conflicts are really complicated. There's, like, whole *layers* of
> stuff. ;)
Urg. We are way out of our depth.
> > Aside from sorting out these tests, we also need to make some new
> > tests for detection of existing conflicts, following the general
> > scheme that you outline below. Oops, guess I just volunteered. :-o
>
> I thought of kind of changing the DeepTreesTestCase running code to just
> repeat whatever it did to raise its tree-conflicts and try to evaluate
> (maybe we need to pass in more expected_stuff or maybe there's a simple
> generic way of checking whether a persistent tree-conflict is handled
> correctly). But I'd be happy to leave this to you or anyone else, as long as
> it's not me. ;)
>
> >> I hope it's OK if I do a catch-up merge on your branch. Last Friday (I
>
> Yes, go ahead. :)
Done. r33925.
[...]
> >> Now, stages of detection...
> >>
> >> We could be clearer on exactly what Subversion should be doing during
> >> "svn update/switch", with respect to conflict detection and skipping of
> >> obstructions.
> >>
> >> One of you mentioned before that a note about this would make a good
> >> code comment in update_editor.c, and I agree.
>
> I was actually meaning that exact structure I saw there. I find below
> enumeration quite difficult to filter... Let me see, originally, it was this:
>
> if target was already in conflict:
> skip this action
> else
> if this action will cause a tree conflict:
> record the tree conflict
> skip this action
> else:
> do this action
OK, I'll include that brief version too.
> > I think we should be completely silent for the rest of a conflicted
> > tree (1a), to avoid burying the user in lots of "Skipped" messages.
> > Too much output would distract the user from the initial message,
> > which describes where the tree conflict is. After resolving the tree
> > conflict, the user can check status or resume update/switch, and find
> > any other conflicts in the tree.
>
> So here we're talking about the tree-conflicts that persist from a previous
> operation? Then -1, I'd still like to see (1a) notifications, or at least a
> skip notification for the topmost items of (1a)
Sure. Note that Steve said "silent for the REST of a ...", meaning
exactly one notification inside each conflicted subtree. At least that's
how I think.
> (I mean that it says
> "skipped this whole tree because it is inside a tree-conflicted dir", but
> would have to report scattered files separately, probably). Otherwise some
> possible scenarios would just return without ANY notification at all, not
> indicating that it is skipping something due to a persisting tree-conflict.
>
> $ svn <operation1>
> C my/dir
> $ svn <operation2> (wants to add file `my/dir/subtree/addition')
> Skipped my/dir/subtree/addition, because my/dir remains in conflict.
>
> Hm, or maybe it should say something like
> $ svn <operation2>
> Skipped my/dir, because it remains in conflict.
> even though my/dir was *no* explicit target. This could become very
> confusing when stepping into my/dir/subtree, calling <operation2> and
> reading something about "Skipped my/dir" although it is not below, but above
> this directory. Run for cover! Worms!!
Urg.
> >> *
> >> * 2. if # Incoming change affects an item that's "obstructed":
> >> * on-disk node kind doesn't match recorded Working node kind,
> >> * including an absence/presence mis-match
> >> * then # Skip this change [2]:
> >> * do not update the Base nor the Working
> >> * notify "skipped because obstructed"
> >> *
> >> * 3. if # Incoming change raises a tree conflict:
> >> * tree/text/prop change to node beneath tree-scheduled[3] dir, or
> >> * tree/text/prop change to tree-scheduled[3] dir/file, or
> >> * text/prop change to tree-scheduled dir/file,
> >> * then # Skip this change:
> >> * do not update the Base nor the Working [4]
> >> * notify "tree conflict"
>
> Here, in contrast, a *newly raised* tree-conflict in a directory should
> cause all "skip" messages of sub-nodes to be omitted.
>
> >> *
> >> * 4. Apply the change:
> >> * update the Base
> >> * update the Working, possibly raising text/prop conflicts
> >> * notify as in v1.5
>
> So this is the final "else", the "default:" case, the "what we do if nothing
> above went wrong", right?
Yes.
[...]
> > Here's my basic implementation plan:
> >
> > The following update callbacks need to check for existing conflicts:
> >
> > open_directory
> > open_file
> > add_directory
> > add_file
> > delete_entry
> > absent_file
> > absent_directory
> >
> > In each callback, we'll make the following checks/calls.
> >
> > (eb->inside_a_tree_conflict)
> >
> > True if we're walking a tree-conflicted tree. Works even if the
> > target and its parent dir do not exist (e.g., we've skipped adding
> > the parent dir due to the tree conflict). Skip target silently if
> > true.
>
> Nice. We could actually add an eb->inside_a_tree_conflict_was_notified or
> something, which the topmost conflict cause sets to TRUE if it notified
> about the conflict. Then, when having a subtree as target, this is FALSE and
> indicates that we should still notify about a skip.
>
> >
> > svn_wc_conflicted_p2()
> >
> > Check for existing conflicts. Requires a versioned parent dir for
> > a tree conflict. Requires a versioned parent dir and versioned
> > target for text or prop conflicts. Skip target with notification
> > if conflicts are found.
> >
> > check_tree_conflict(&tree_conflict, eb, ...)
> >
> > Returns a fresh tree conflict if the target is a victim. Requires
> > a versioned parent dir. If a conflict is found, record it
> > persistently.
>
> So, this may eventually split into detect_- and raise_tree_conflict().
> Although I'd rather call that persist_tree_conflict() instead of raise_,
> raise sounds too much like "notify and bail out", both of which it doesn't.
Agreed.
[...]
> Thanks guys for the good collaboration, I'm benefiting greatly from your posts.
Oh, cool. I'm feeling overwhelmed today by the number of long emails
with lots of hard stuff in each.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-10-28 15:41:55 CET