Hi Daniel,
please don't be put off by the numerous comments and corrections (I lost
count on how often I had to revise my concepts with the help of Julian,
Stefan and Steve). It *is* pretty complex. And to err is productive.
* neels pulls the red marker
Daniel Näslund wrote:
> On Sun, Feb 07, 2010 at 12:23:12AM +0000, Julian Foad wrote:
>> Can you post an updated RFC that incorporates the responses to my and
>> Stefan's comments so far?
>
> Here it is. I've only merged some of the comments from you and Stefan.
> The merge part is still rather sketchy.
>
> [[[
> 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 )
>
> The main difference between update/switch and merge is that we don't
> have somewhere to store the incoming changes during a merge. It would be
> nice if we could save a THEIRS tree.
Also note that with update/switch, --accept=theirs should be identical to
'svn revert'.
In general, 'svn update' should pull in the complete BASE tree state of the
update's target revision regardless of conflicts, and any conflicts should
be local schedules on top of that.
>
> ### Saving a THEIRS file is one thing but a tree? It could be a large
> ### tree. On the other hand, a file can be large too...
With the yet-to-be-implemented pristine-store, the file largeness may not be
such a problem after all.
I guess it's more straightforward to link to the pristine-store from the
conflict description of each particular node, and there we have full info on
THEIRS for each conflict.
I rather see a need for storing MINE somewhere safe, i.e. the locally
modified version of a file before update/switch/merge started adding
conflict markers to it -- which isn't really applicable to tree-conflicts.
(Currently, we dump a copy of the ACTUAL file into the WC before introducing
conflict markers, and it's the only information that is unchecked and
susceptible to accidental destruction by the user. However, now I'm talking
about text-conflicts. Back to *this* RFC!)
>
> 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!! :)
>
> 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 solution should work for both files and dirs.
Yes.
>
> 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.)
>
> ### 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?
> 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?
> 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.)
>
> Use cases update/switch
> ========================
> Since we don't know how to follow moves we just leave the half of the
> operation not affected by the conflict untouched. The user can remove
ok.
> it manually. Once we have editor-v2 this would work automatically.
ah, here it is, editor-v2 :)
>
> 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 :)
>
>
> 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.
I'm slowly getting the hang of your terminology, please bear with me ;) ...
>
> 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.
> 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)
> 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.
> 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 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*.
>
> 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.
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.
Schedule as locally added (the previous local add is discarded, now we want
their local add).
> 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
> -------------------------
> ### Should we need some way to merge with copyfrom info?
heh, "should we need" ;)
> 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
> --------------------------
don't start with locales now ;)
> 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!
>
> API changes
> ==================
I'd love to go into them but I don't have time left.
My red marker is running out of ink anyway. ;)
I'd love it if you could get this through another round of review. That's
unless you're busy otherwise, of course *loud wink*.
* neels holds thumbs
~Neels
>
> We have
> --------
> 1) We already know which operations has caused the conflict. It's in
> svn_wc_operation_t. In case some cases will need to differ between sw/up
> and merge which I think will be the case.
>
> 2) We know which type of tree conflict svn_wc_conflict_reason_t,
> svn_wc_conflict_action_t
>
> 3) We have conflict choises already svn_wc_conflict_choise_t {
> svn_wc_conflict_choose_postpone
> svn_wc_conflict_choose_base, /* original version */
> svn_wc_conflict_choose_theirs_full, /* incoming version */
> svn_wc_conflict_choose_mine_full, /* own version */
> svn_wc_conflict_choose_theirs_conflict, /* incoming (for conflicted hunks) */
> svn_wc_conflict_choose_mine_conflict, /* own (for conflicted hunks) */
> svn_wc_conflict_choose_merged /* merged version */
> }
>
> 4) A code structure for invoking interactive commands, displaying diffs
> and choosing versions.
>
> We need
> --------
> 1) svn_cl__conflict_handler must use svn_wc_conflict_description2_t for
> some additional tree conflict info.
>
> 2) Handle the cases we can in svn_cl__conflict_handler, let the other fall
> through. (Or return svn_wc_conflict_choose_postpone). I'm thinking of
> letting all dir conflicts fall through as a first step.
>
> 3) Add some choises to svn_wc_conflict_choise_t.
> svn_wc_conflict_choose_{rename,elsewhere,my_moved_mods}?
>
> 4) Add calls to eb->conflict_resolver if check_tree_conflict() returns a
> conflict in:
> libsvn_wc/update_editor.c
> do_entry_deletion()
> add_directory()
> open_directory()
> add_file_with_history()
> open_file()
>
> 5) Create code to handle and execute the svn_wc_conflict_choise_t
> choises. in libsvn_wc/update_editor.c
>
> 6) Test to verify the interactive callback. Some detection tests needs
> to use the --non-interactive flag.
>
> 7) It would be nice to be able to store the THEIRS tree in the wc db
> when we get a tree conflict during a merge operation.
>
> User interface
> ======================
>
> I will focus on the update stuff for files. These are the options I'm
> suggesting. We need to supply paths at a couple of places. Perhaps some
> bash-completion stuff would be useful. We're on the limit here of what's
> a good CLI. There shouldn't be too much interactivity.
>
> Perhaps something more than diff-full for viewing changes. Something
> like what svn info ^/trunk/conflicting_path, svn info wc_path can
> provide?
>
> Local add, incoming add
> -------------------------
> Tree conflict discovered in 'path'
> local add, incoming add upon update
> Select: (p) postpone, (df) diff-full,
> (mc) mine-conflict, (tc) theirs-conflict,
> (mr) mine-rename, (m) merge,
> (s) show all options: s
>
> (e) edit - change merged file in an editor
> (df) diff-full - show all changes made to merged file
> (r) resolved - accept merged version of file
>
> (dc) display-conflict - show all conflicts (ignoring merged version)
> (mc) mine-conflict - accept my version for all conflicts (same)
> (tc) theirs-conflict - accept their version for all conflicts (same)
> (mr) mine-rename <to-path>
> - Move my file to <to-path> and put their
> file at my files old place
> (m) merge - merge their file onto my file. You may be
> asked to choose copyfrom
>
> (mf) mine-full - accept my version of entire file (even non-conflicts)
> (tf) theirs-full - accept their version of entire file (same)
>
> (p) postpone - mark the conflict to be resolved later
> (l) launch - launch external tool to resolve conflict
> (s) show all - show this list
>
> Local del, incoming del
> -------------------------
> Tree conflict discovered in 'path'
> local del, incoming del upon update
> Select: (p) postpone, (df) diff-full,
> (mc) mine-conflict, (tc) theirs-conflict,
> (r) rename, (m) merge,
> (s) show all options: s
>
> (e) edit - change merged file in an editor
> (df) diff-full - show all changes made to merged file
> (r) resolved - accept merged version of file
>
> (dc) display-conflict - show all conflicts (ignoring merged version)
> (mc) mine-conflict - accept my version for all conflicts (same)
> (tc) theirs-conflict - accept their version for all conflicts (same)
> (r) rename <from-path> <to-path>
> - Move my file to <to-path> and put their
> file at my files old place
> (m) merge - merge their file onto my file. You may be
> asked to choose copyfrom
>
> (mf) mine-full - accept my version of entire file (even non-conflicts)
> (tf) theirs-full - accept their version of entire file (same)
>
> (p) postpone - mark the conflict to be resolved later
> (s) show all - show this list
>
> Local del, incoming edit
> --------------------------
> Tree conflict discovered in 'path'
> local del, incoming edit upon update
> Select: (p) postpone, (df) diff-full,
> (mc) mine-conflict, (tc) theirs-conflict,
> (me) merge-elsewhere,
> (s) show all options: s
>
> (e) edit - change merged file in an editor
> (df) diff-full - show all changes made to merged file
> (r) resolved - accept merged version of file
>
> (dc) display-conflict - show all conflicts (ignoring merged version)
> (mc) mine-conflict - accept my version for all conflicts (same)
> (tc) theirs-conflict - accept their version for all conflicts (same)
> (mr) mine-rename <to-path>
> - Move my file to <to-path> and put their
> file at my files old place
> (m) merge - merge their file onto my file. You may be
> asked to choose copyfrom
>
> (mf) mine-full - accept my version of entire file (even non-conflicts)
> (tf) theirs-full - accept their version of entire file (same)
>
> (p) postpone - mark the conflict to be resolved later
> (s) show all - show this list
>
> Local edit, incoming del
> --------------------------
> Tree conflict discovered in 'path'
> local edit, incoming del upon update
> Select: (p) postpone, (df) diff-full,
> (mc) mine-conflict, (tc) theirs-conflict,
> (mm) mine-move-mods
> (s) show all options: s
>
> (e) edit - change merged file in an editor
> (df) diff-full - show all changes made to merged file
> (r) resolved - accept merged version of file
>
> (dc) display-conflict - show all conflicts (ignoring merged version)
> (mc) mine-conflict - accept my version for all conflicts (same)
> (tc) theirs-conflict - accept their version for all conflicts (same)
> (m) merge <their-file>- merge my file onto their file.
> asked to choose copyfrom
>
> (mf) mine-full - accept my version of entire file (even non-conflicts)
> (tf) theirs-full - accept their version of entire file (same)
>
> (p) postpone - mark the conflict to be resolved later
> (s) show all - show this list
> ]]]
>
> cheers,
> Daniel
Received on 2010-02-09 15:21:45 CET