Hi Neels!
Thanks for all your feedback! The use of the libsvn_wc terms BASE,
WORKING and ACTUAL will be replaced by your suggested (or was it
Julians?) checked-in state and checked-out state in the next version of
the RFC.
Find further comments inline.
On Tue, Feb 09, 2010 at 03:20:57PM +0100, Neels J Hofmeyr wrote:
> Daniel Näslund wrote:
> > Design spec for tree conflict resolution in the commandline client
> > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > The hard part is figuring out what state the wc is in during the
> > different user cases.
>
> Actually, wc-ng should make that part easy. The hard part is making the
> conflict resolution conform with adjacent WC states. (attention, high degree
> of meta language. No need to understand me :P )
After a read through of last weeks discussions on pristine and base, the
hard part is probably to understand what the terminology actually means! :)
[...]
> > Contents
> > =========
> > Problem definition
> > Requirements
> > Terminology
> > Use cases update/switch
> > Use cases merge
> > API changes
> > User interface
> >
> > Problem definition
> > =========================
> > Users are having problems understanding how to resolve tree conflicts.
> > For some operations they may not know how to get back to a previous
> > state. They don't always know how to view the changes causing a
> > conflict.
>
> Agreed!! :)
The spec has really been about resolving conflicts and defining the
states of the wc. We haven't touched on methods for viewing the changes.
A diff is good for a text conflict but of limited value for displaying a
tree conflict. More on that later.
> > Requirements
> > =====================
> > It should be easy for the user to understand why the conflict has
> > happened and how to resolv it.
> +1!!
>
> >
> > Update, switch and merge should be reversible. That is; going back to
> > the former revision in the wc should restore the contents to the
> > original.
>
> Need some finer grain here: "going back"?
> 'svn revert' should be able to undo all local changes, be they from merge or
> manually inflicted. After 'revert', any tree-conflict should be gone, and
> the node should reflect a state achievable with 'svn checkout'.
>
> We could have a desire to revert only the last steps of current
> modifications (i.e. only revert the last three merges that I did on top of a
> locally modified file, the last of which caused a tree-conflict) -- but this
> enters a scope far beyond tree conflict resolution. It would surely be nice
> nevertheless, and it might be implemented by layers of THEIRS information...
> in a different RFC.
> > The resolver should not handle moves since we have no way to track
> > those. When I say handle moves I mean "do something about the other end
> > not affected by this conflict". We will apply give the option to apply
> > changes elswehere and do renames but we will leave some files behind for
> > the user to clean up.
>
> (on the long run, with editor v2, we may be able to track moves
> satisfactorily. We might want to design this future behavior now so we can
> prepare for it and don't get ourselves in a big mess later.)
Noted. My idea so far has been to substitute detection functionality by
editor-v2 with input from the user, e.g. the user has the responsibility
to detect moves and copies. When we have editor-v2, those parts should
be easily replaced.
> > ### There should be a good way to view what has caused a conflict.
> > ### Perhaps some info from 'svn info'.
>
> Currently, 'svn info' on a tree-conflicted node says something like
> [[[
> Tree conflict: local edit, incoming delete upon merge.
> Source left: (kind) URL_at_rev
> Source right: (kind) URL_at_rev
> ]]]
>
> Are you saying that there should be more info? Which in particular?
I'm talking about the options given when running the interactive tree
conflict resolver (In case you thought I meant adding more info to svn
info). If I got a tree conflict running without the resolver I would
want to know :
* What is the content of theirs? A dir listing or file content.
* what is the content of mine? A dir listing or file content.
* Was the target copied [#]?
* Was the target moved? [#]
* If the target was copied or moved in theirs, where did it get
copied/moved from? We can't tell this automatically (until you know
what...) but we can help the user make a pretty good guess.
* For merges, we probably would want to check the svn:mergeinfo in some
cases.
[#] I have a weak understanding of the copy-from and moved_here
concepts. As I understand it, we can detect locally moved or copied
targets. We can't tell what has happened on the server side but we know
what has happened locally.
To exemplify: For a locally copied, incoming move of the file alpha2 I would do
something like this:
[[[
svn st -> shows me a local add, incoming add upon update TC on alpha2
svn info alpha2 -> shows that alpha2 was locally copied from alpha
svn info ^/trunk/alpha2 -> shows me
svn log -v ^/trunk/alpha2 -> shows that alpha2 was moved from alpha in r2
]]]
If I was dealing with a TC on a dir I could get some additional
information with svn ls -R ^/trunk/dir.
In the end it boils down to: 1) We need a way to show the structural
changes to the user in a better way than a diff. 2) We need a way to
help the user decide on what has happened during a copy or move. The
problem lies in the nature of commandline program. It doesn't cope that
well with interactively. Are we expecting the user to have another
terminal open and run those commands there?
If I was doing a resolver for Subclipse I would present a graph
illustrating the changes in two file trees. It would look so _good_. I
could start writing something to graphically represent the changes in
two trees for the commandline client but text mode is text mode.
(Although git log --graph is a useful thing and the standalone tree
command is useful too). Just thinking out loud here.
> > The tree conflict resolver interface should be consistent with the
> > existing resolver. It should provide
> > {postpone,theirs,mine,diff}-options.
>
> :s/\(theirs\|mine\)/&-tree,&-tree/g ?
> Whatever, there's much more detail further below.
>
> >
> > ### The user should be able to run the conflict resolver at any time.
> > ### We have to fix libsvn_wc/conflicts.c first. Not really
> > ### specific to this feature.
>
> As far as I've understood, we should teach 'svn resolve --accept=*' to do
> interactive tree-conflict resolution. Is there anything blocking that?
Bert told me on IRC (slightly rewritten):
<@Bert> dannas: Currently we don't store all the information available
in the interactive conflict resolver for future use E.g. In the
interactive conflict resolver for properties you have the older, left
and right values (actual value).. but once postponed you just have a
textfile containing a list of information that might have been modified
or which might be in- or overcomplete My idea is to commit the conflict
to the working copy before running the interactive resolver (including
all the data now not stored yet).. and then run the resolver on the WC.
And then continue. A 'svn resolve' invocation can then just re-use that
code path. Currently we sometimes run the interactive resolver on a
half modified working copy. And then use the result of the resolver to
complete it in one way or antoher
> > Terminology
> > ============
> > In this document, WORKING means the user's version, which possibly has
> > text, property and/or tree modifications relative to the BASE; it does
> > not mean the WC-NG database concept that is known as WORKING.
>
> argh, well, ok...
> IMHO it would be better not to overload the word "WORKING" in this RFC...
> Below, when I say 'BASE tree' or 'WORKING tree', I mean the wc-ng metadata
> store. When I say 'actual WC file system' I mean those files editable by the
> user in her working copy, not the metadata.
> (Hm, I don't say that much about props though, expect some remarks about
> props to be missing below.)
Too bad, I had hoped that BASE, WORKING and ACTUAL could be used at this
abstraction level too. I saw you suggestions in the cheat sheet about
using checked-in state and checked-out state. I'll think about it some
more and probably come to the conclusion that those terms works. :)
> > Use cases update/switch
> > ========================
[...]
> > Local add, incoming add
> > -------------------------
> > THEIRS: Put new BASE file/dir in WORKING.
> What should happen here is that the incoming add is pulled into the BASE
> tree node, while the wc-ng WORKING tree node still reflects the local add,
> and the file in the actual WC file system stays unchanged. So, the local
> status becomes 'replaced', and a 'revert' would remove the WORKING tree node
> and change the actual WC file to the pristine contents of its BASE tree
> node, so that the WC looks as if no local add had ever happened, and as if
> the incoming add was completed successfully.
>
> > MINE: Keep current WORKING file/dir.
> Yes. Only remove the conflict marker, nothing else to be done.
>
> > RENAME-MINE:
> > Move WORKING file/dir to <user-suggest>. Replace WORKING
> > file/dir with the BASE-TARGET file/dir.
>
> Oh you mean the user gives her added node a different name to avoid the
> conflict ... makes sense
> Schedule a local add at <user-suggest> with the current actual WC content of
> 'this' path, then revert 'this' path, leaving behind the result of the
> incoming add (in the BASE tree), and updating the actual WC filesystem to
> the BASE tree node's pristine contents.
>
>
> > MERGE Merge BASE file1 onto WORKING file2.
> <groan> ;)
> Hey, wait a minute, you said "Use cases update/switch" above. What *about*
> merge with update/switch??
> Oh you mean, this is what happens when the user says --accept=merge... ok
>
> > If any copyfrom info is present (i.e. at least one of the files
> > was copied), the user needs to select a copyfrom source:
> >
> > WORKING file | BASE file | Options
> > --------------------------------------
> > has copyfrom | has copyfrom |
> ---------------------------------------
> > Yes Yes Pick one copyfrom of 2, or none.
> > Yes No Use WORKING copyfrom, or none.
> > No Yes Use BASE copyfrom, or none.
>
> So the user has to *always* choose which history/copyfrom she wants to keep
> attached to the node. Good, haven't thought of that yet :)
As I was saying earlier, I don't really understand copyfrom. Can we have
copyfrom information on a BASE file (or checked-out file to use your
suggested terminology) without editor-v2? The checked-out file has no
local modifications so the copy could only have happened on an incoming
revision and we can't track copies and moves on those, right?
> > Local del, incoming del
> > -------------------------
> > THEIRS: Nothing to do.
> > MINE: Nothing to do.
>
> Yes. When the user says --accept=theirs/mine, nothing needs to be done
> except removing the conflict marker.
>
> > RENAME: Merge BASE-TARGET <moved>
> > onto WORKING <moved>.
> > ### We need the user to tell us there was a rename until
> > ### editor-v2 is here. Until then <moved> must be a user
> > ### suggestion.
>
> I think this needs another table that lists combos of 'locally moved' or not
> vs. 'incoming move' or not.
yeah.
> >
> > Local del, incoming edit
> > -------------------------
> > THEIRS: Replace the deleted WORKING file/dir with edited BASE file/dir.
> > MINE: Keep current WORKING file/dir.
>
> You mean locally schedule the node for deletion.
Yes.
> > ELSEWHERE:
> (comment from the future:)
> MERGE-ELSEWHERE:
>
> > Merge BASE file/dir onto WORKING <user-suggest>.
> > ### editor-v2 will automatically find a move. No need for this
> > ### option then?
> There will still be a tree-conflict reported, and the user should have the
> option of applying the incoming edit to where she moved the local file. Need
> this option. (but IMHO give it a better name)
>
> > ### stsp: I hope to get the local move case working
> > ### automatically before 1.7 release
>
> Weey! :D How're ya gonna do that? Iterate all local adds and see if they
> have copy_from info? Introduce copied_to info?
>
> > Local edit, incoming del
> > --------------------------
> > THEIRS: Delete the file/dir from WORKING and ACTUAL.
>
> Hey!! Now you're changing your terminology! That's not fair!
> To clarify: remove all nodes from all trees including the actual working
> copy file system. The file is now officially gone. (no status, no need to
> commit for the delete to take effect)
Perhaps this is wrong. We should just do scheduling. It's up to the user
to do the final decision to remove the physical file.
> > MINE: Keep current WORKING file/dir.
>
> The BASE tree node has been removed by the update that flagged the tree
> conflict. Create a WORKING tree node, i.e. schedule the file as locally
> added! The repos thinks this node is deleted, so we need to re-add it at the
> next commit.
Or keep the scheduled file as locally added. It's been re-added before
we run the resolver.
> > MOVE-MY-MODS:
> > Schedule BASE add-half for addition. Merge WORKING file/dir to
> > add-half. (Must be suggested by user. We can't track add-halfs
> > right now.
>
> Maybe rather --accept=THEIR-RENAME?
The names of the options is totally open for suggestions.
> The add part of "their" rename has been recorded in the BASE tree (at the
> move-target path) by the update that flagged the tree conflict -- it is now
> known as officially existent. Simply modify the file in the actual WC file
> system, i.e. schedule the file *modified*.
Doh, my mistake.
Update/switch was the easy part, now it's time for merge.
> > Use cases merge
> > =======================
> > ### julianf wrote: How can we most easily implement an extension of "svn
> > ### merge" that achieves a copyfrom-history-sensitive diff (between WC
> > ### items) rather than an unaware diff?
>
> Like, before editor-v2? o_O
>
> General note: the wc-ng BASE tree is never modified by merge, nor by tree
> conflicts during merge. The conflict, as with uptch [1], exists entirely as
> local mods/schedules, and a 'revert' should clear that completely.
>
> Also, a tree-conflict during merge *should not* alter any other tree, be it
> the WORKING tree or the actual WC file system; it should only flag a
> conflict that notes which delta wanted to come in -- the pristine store will
> be helpful to make resolving this to THEIRS a non-repository action.
I'm watching the evolution of the pristine store spec with interest. :)
> Resolving to MINE then is always achieved by just removing the conflict
> marker and leaving the local state unchanged by the merge. (note, this so
> far stands for tree-conflicts only)
>
> [1] uptch = update/switch ;)
> invented that last night... not one of the greatest genius inventions, I
> know. More like a comic relief ;)
>
> >
> > Local add, incoming add
> > -------------------------
> > THEIRS: Replace WORKING with BASE. Merge START to END into WORKING.
> What do you mean by 'BASE'? I presume the final content and props (within
> the given merge range) of the incoming add.
Yes, that was what I meant.
> Schedule as locally added (the previous local add is discarded, now we want
> their local add).
So we must revert the previous local add? I'm assuming we can find that
target with our current API:s.
> > MINE: Keep current WORKING file/dir.
> see 'Resolving to MINE...' above.
>
> > RENAME-MINE:
> > Move WORKING file/dir to <user-suggest>. Merge START to END into
> > WORKING.
> Yes.
> Using my words, I'd say "schedule the local add at path <user-suggested>,
> remove local add schedule at this path and then carry out the merge's add
> normally". Same thing.
> >
> > Local del, incoming del
> > -------------------------
> > THEIRS: Nothing to do.
> > MINE: Nothing to do.
> > ELSEWHERE:
> > MERGE START to END from THEIRS <user-suggest> into WORKING
> > <user-suggest>.
> > ### This will be handled automatically when we can handle moves.
>
> Again, we can have "local del is/isn't part of move", "incoming del is/isn't
> part of a move" plus the more special case "both are part of a move to the
> same path" (which should not result in a tree conflict once detected).
> ("both are not part of a move" should not result in a tree conflict once
> detected, either.)
>
> >
> > Local del, incoming edit
> > -------------------------
> > THEIRS: Copy THEIRS to ACTUAL. Add to WORKING.
>
> No add! Discard the local add, i.e. discard the WORKING tree node,
> 'exposing' the current BASE tree node. Then carry out the merge's edit as usual.
>
> > MINE: Nothing to do.
> Yes.
>
> > ELSEWHERE1:
> > Merge WORKING add-half onto THEIRS file/dir. Copy THEIRS to
> > WORKING delete-half.
>
> Grief! THEIRS wants to edit 'this' path. Locally, 'this' path has gone
> elsewhere. I'd call this case MERGE-HERE or something.
>
> Keep local text and prop modifications, but bring these local mods back to
> 'this' path. Then, merge THEIR edit onto the local mods, stepping into the
> realm of text/prop conflicts.
>
> > ### This is in case of a move with mods. The user would have to
> > ### supply the path to the add-half. Editor-v2 will resolve this
> > ### automatically.
>
> ...will supply the path automatically, but the user still has to say how to
> resolve it.
>
> > ELSEWHERE2:
> > Merge THEIRS file/dir onto WORKING add-half.
> > ### The add-half would have to be supplied by the user.
>
> MERGE-ELSEWHERE:
> 'This' node has been moved to a different path. Apply THEIR edit to the
> other part, entering the realm of text/prop conflicts.
>
> hmm, prext conflicts? lol
>
> >
> > Locale edit, incoming del
> > --------------------------
> > THEIRS: Remove WORKING file/dir.
> Keep the BASE tree node, as always, and schedule as locally deleted.
>
> > MINE: Nothing to do.
> Yes.
>
> > RENAME1: Merge START to END from THEIRS add-half onto WORKING.
> MERGE-HERE:
> THEIR delete is part of a move, possibly with prext mods. argh! prop and
> text mods. Don't carry out THEIR move, but apply those prext mods to 'this'
> path, entering realm of prext conflicts.
>
> > RENAME2: Merge START to END from WORKING file/dir onto THEIRS add-half.
> MERGE-ELSEWHERE:
> THEIR delete is part of a move, possibly with prext mods. Carry out THEIR
> move, but apply local prext mods to the move-target path, entering realm of
> prext conflicts.
> Schedule 'this' path locally deleted, and schedule the move-target path
> locally added, which contains THEIR edits merged onto the local edits.
>
> Gee, what if THEIR move-target path is also locally added!?
> ALARM! ALARM! MORE DESIGN!
Received on 2010-02-18 11:56:12 CET