On Tue, 2008-11-04 at 00:26 +0100, Stephen Butler wrote:
> Quoting Julian Foad <julianfoad_at_btopenworld.com>:
>
> > On Mon, 2008-11-03 at 18:45 +0100, Stephen Butler wrote:
> >> Hello tree conflict fans,
> >>
> >> While hacking on svn resolve, I made a list of tree conflict types
Ahh, a light goes on, after I wrote everything in this reply below. I
think I see now why you are concerned about how to resolve obstructions:
it's because you're writing code to handle the cases that can be
represented by the conflict description, and the 'reason' field can have
the value 'obstructed'.
> >> distinguished by incoming "action" and local "reason", printed below
> >> as "(action:reason)". For each type, I describe what I think the
> >> effect of each --accept option should be. Operation-specific parts
> >> are in square brackets.
> >
> > Thanks, Steve.
> >
> > Have you read my thoughts in notes/tree-conflicts/resolution.txt? It's
> > based on what I thought we should do rather than what Subversion
> > currently does, so it's not a direct answer. It attempts to show the
> > reasons for defining each action in a particular way.
>
> Yes, sorry I forgot to mention your doc up front. It's required reading
> for this topic. It includes issues that I gloss over, such as the
> likely meaning of each situation from the user's point of view.
>
> >
> >> Please have a look and make corrections/comments inline.
> >
> > OK. First, what do you assume is the starting point for the state of the
> > "working" files and directories in the WC? ("Whatever state Subversion
> > trunk_at_34033 leaves it in"? "Not touched by the attempted incoming
> > change"?) I assume the latter: not touched.
>
> Yes, I assume we'll go back to the general plan of skipping all tree
> conflict victims. My quick hack of allowing deletions to continue
> ought to be rolled back as soon as we tweak svn add and svn remove
> to "do the right thing" to a tree conflict victim, allowing the
> user to recover from the skipped deletion.
And then there's the question of whether the base update is done at
detection time, or at resolve time, or neither (but by a subsequent
update after resolve).
The best thing, I think, would be to update the base at detection time.
That is consistent with text and property conflict handling.
I can't see any benefit to delaying the base update, and there is a cost
to doing so: we must either store the new base at detection time, or
contact the repository again to update it (at resolve time or by a
second "svn update").
> > I think we can safely say that mention of "update" means "update or
> > switch".
>
> Yes, and checkout, too. ;-)
>
> >
> > I assume that "theirs-full" means "I want my victim to end up like the
> > final state of the incoming change, otherwise known as the merge-right
> > source", and "mine-full" means "I want my victim to end up like I had it
> > before I tried this operation that raised a conflict". Such definitions
> > are in line with the meanings they have for a text conflict. See also
> > the section 'Meaning of "Choose Theirs" and "Choose Mine"' in
> > notes/tree-conflicts/resolution.txt.
> >
> > I assume "accept=base" means "Like the merge-left source". My reasons:
> > For an update, the pristine base of the WC before the update is the same
> > as the merge-left source. The only other option I can think of is the
> > pristine base of the WC after the update, but that would be identical to
> > "accept=theirs" and identical to "revert". For a merge, it could mean
> > "Like the merge-left source" or "Like the pristine base of the WC". The
> > former is directly related to the merge; the latter (which means the
> > same as "svn revert") is not one of the four versions involved in the
> > merge (merge-left source, merge-right source, initial target, final
> > target).
>
> I had assumed that "base" always meant the WC BASE before the operation.
> But I suppose 'svn resolve --accept base' is more useful for merge if
> it does something different than 'svn revert' does.
>
> >
> >> Thanks,
> >> Steve
> >>
> >>
> >> (*:*)
> >>
> >> working: Do nothing (aside from removing the item's tree conflict
> >> status).
> >>
> >>
> >> (edit:delete)
> >
> > Reason "delete" meaning "object is already schedule-delete", to quote
> > from svn_wc_conflict_reason_deleted.
> >
> >> base: Revert the deletion of the victim_at_BASE.
> >
> > OK.
> >
> >> mine-full: [Update: Apply incoming chagnes to the victim.] [Merge:
> >> Do nothing.]
> >
> > Update: you mean "Revert the scheduled deletion, and then apply incoming
> > changes"? No, I disagree. "My" state is that the victim is deleted. To
> > keep "my version", I want the incoming changes ignored, the BASE updated
> > to the new incoming base, and the schedule left as "delete".
>
> Yes, I agree we should apply the incoming changes to the victim's
> hidden BASE only.
>
> >
> > Merge: assuming the victim hasn't already been touched at detection
> > time, I agree.
> >
> >> theirs-full: Undo the deletion and update the victim (without
> >> triggering another tree conflict on the victim).
> >
> > Update: yes.
> >
> > Merge: No. What you describe is for the meaningwould result in the
> > version that last existed on the TARGET plus incoming changes, whereas
> > "their" version is what previously existed in the SOURCE plus its
> > incoming changes.
>
> I think I understand now. This is use case #5 in issue 2282, where
> the tree conflict is created because the SOURCE-LEFT differs from
> the TARGET. If the user chooses theirs-full, SOURCE-LEFT should be
> the starting point.
>
> >
> >> (edit:missing)
> >
> > This sounds like an "obstruction": a mismatch between metadata and
> > what's on disk, produced by a mis-use of "svn add" and "svn delete".
> >
> > We had an idea that we could handle obstructions like tree conflicts,
> > because they share some things in common. However, I now think they are
> > sufficiently different that it would make unnecessary complexity.
> >
> > Therefore I don't care about (edit:missing). I want it to be caught as
> > an error before tree conflict detection code gets to look at it.
>
> Update throws an error for an obstruction, as it has in 1.5 and
> earlier, which has the effect of blocking commit. But merge has
> to raise a tree conflict.
No, merge does not have to raise a conflict. It can and should bail out
or skip the item: "Sorry, you can't merge into this target because it's
broken. Fix your obstruction." (Bailing out is less than ideal because
there's no clean way to revert or re-start a merge into a non-pristine
WC, but that is a problem for another day. Skipping is less than ideal
because we don't record that information. Raising a tree conflict is
less than ideal because we don't have a sane idea of what is 'mine' -
what's its EOL style, what's its schedule? What's its node kind, even?)
> The pre-1.6 merge behavior is to notify
> the user of the obstruction without blocking commit. I believe
> we're against that. ;-)
Specifically we're against the fact that it doesn't record what it did
so there's no way to check afterwards whether there was such a problem
(unless you capture the notification output text).
Raising a conflict seems like one way to persist the information.
However, there are significant differences between an obstruction and a
normal tree conflict, as I mentioned above, so the WC has no idea how to
convert from 'working' text to 'base' text in repository-normal format,
for example. Therefore we cannot do anything as simple as 'choose mine'
to resolve it. Basically, as I see it, there is no point in trying to
fit obstructions into a design which is already packed with enough
trouble to overwhelm us! Obstructions don't come from proper use of
Subversion, and there's already a mechanism for reporting them (svn
status), and the user already knows how to deal with them.
Another angle is that I don't want the "conflict description" to just
say "it's a conflict, reason = deleted", but in future, "it's a
conflict, base=(URL_at_REV), theirs=(URL_at_REV),
mine=(WCPATH,PROPS,SCHEDULE,etc.)". That kind of detailed info becomes
much too complex to use if we have to cope with an inconsistent "mine"
as "mine.metadata=(KIND,PROPS,SCHEDULE,etc.), mine.disk=(KIND,WCPATH)".
> Recovering gracefully from obstructions is difficult and probably
> can't be automated, as is obvious from the radical steps that would
> be needed for the (*:obstructed) items at the end of this list.
> I've deleted those items, since we've no hope of implementing them.
Great.
> I think we should mark merge obstructions as tree conflicts, and
> support only --accept=working when resolving them.
Argh, no!!!
OK, let me put that another way: Be my guest, after we've got all normal
(non-obstruction) tree conflicts working fully.
I do keep wondering if we've got different definitions of "obstruction".
I don't include the case where a merge wants to edit a file 'foo', but
the WC has a directory 'foo' instead of a file. As long as the WC's
metadata says 'foo' is a dir and 'foo' is also a dir on disk, then
there's no obstruction, just a tree conflict of 'file edit onto
non-file'. To me, "obstruction" means "WC metadata doesn't match WC
disk". Agreed?
Here is the chart, updated with our comments so far, including me
removing all the remaining "obstruction" cases:
-----------------------------------------------------------------
Assumptions:
* This is for files. Some of it should apply to directories too.
* The base is not updated at detection time.
(*:*)
working:
Do nothing (aside from removing the item's tree conflict status).
[Update: Update the base, if we decided to skip it when the
conflict was raised.]
(edit:delete)
base:
Set the schedule to 'normal'.
[Update: Create the 'working' file from the old base.
Update the base.]
[Merge: Create the 'working' file from the merge-left source.]
mine-full:
[Update: Update the base.]
[Merge: Do nothing.]
(Leave the schedule as "delete".)
theirs-full:
Set the schedule to 'normal'.
[Update: Update the base.
Create a 'working' file from the new base.]
[Merge: Create a 'working' file from the merge-right source.]
(edit:unknown) [occurs during merge only]
base:
[Merge: Add-with-history the SOURCE-LEFT item]
mine-full:
Do nothing.
theirs-full:
Add-with-history the merge-right counterpart of the victim.
(delete:edit)
base:
[Update: Finish deleting the victim.
Add-with-history the victim_at_BASE without local mods.]
[Merge: Delete the victim.
Add-with-history the SOURCE-LEFT item]
mine-full:
[Update: Finish deleting the victim.
Add-with-history the victim_at_BASE including local mods.]
[Merge: Do nothing.]
theirs-full:
[Update: Finish deleting the victim.
"rm" the unversioned victim.]
[Merge: Delete the victim.]
(delete:delete)
(delete:unknown) [occurs during merge only]
base:
[Update: Add-with-history the victim_at_BASE.]
[Merge: Add-with-history the SOURCE-LEFT item]
mine-full:
[Update: Finish deleting the victim.]
theirs-full:
[Update: Finish deleting the victim.]
(add:add) [occurs only if victim is locally added with history]
base:
Revert the victim and "rm" the unversioned item.
Finish adding the new item, and then delete it.
mine-full:
[Update: Finish updating the victim, resolving any text or
prop conflicts with '--mine-full'. Ideally, we would also
delete any subtrees newly added by the operation.]
[Merge: Do nothing.]
theirs-full:
Revert the victim and "rm" the unversioned item.
[Update: Finish adding the new item.]
[Merge: Add-with-history the merge-right counterpart of the victim.]
-----------------------------------------------------------------
- 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-11-04 14:44:02 CET