You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Näslund <da...@longitudo.com> on 2010/02/08 14:55:10 UTC

Re: [RFC] v2 Tree conflict resolver spec.

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. 

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.

### 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...

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.

Requirements
=====================
It should be easy for the user to understand why the conflict has
happened and how to resolv it. 

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.

The solution should work for both files and dirs.

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.

### There should be a good way to view what has caused a conflict.
### Perhaps some info from 'svn info'.

The tree conflict resolver interface should be consistent with the
existing resolver. It should provide
{postpone,theirs,mine,diff}-options.

### 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.

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.

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
it manually. Once we have editor-v2 this would work automatically. 

Local add, incoming add
-------------------------
THEIRS: Put new BASE file/dir in WORKING.
MINE:   Keep current WORKING file/dir.
RENAME-MINE:
        Move WORKING file/dir to <user-suggest>. Replace WORKING
        file/dir with the BASE-TARGET file/dir.
MERGE   Merge BASE file1 onto WORKING file2.
        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.


Local del, incoming del
-------------------------
THEIRS: Nothing to do.
MINE:   Nothing to do.
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.

Local del, incoming edit
-------------------------
THEIRS: Replace the deleted WORKING file/dir with edited BASE file/dir.
MINE:   Keep current WORKING file/dir.
ELSEWHERE:
        Merge BASE file/dir onto WORKING <user-suggest>.
        ### editor-v2 will automatically find a move. No need for this
        ### option then?
        ### stsp:  I hope to get the local move case working
        ### automatically before 1.7 release

Local edit, incoming del
--------------------------
THEIRS: Delete the file/dir from WORKING and ACTUAL.
MINE:   Keep current WORKING file/dir.
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.

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?  

Local add, incoming add
-------------------------
THEIRS: Replace WORKING with BASE. Merge START to END into WORKING.
MINE:   Keep current WORKING file/dir.
RENAME-MINE:
        Move WORKING file/dir to <user-suggest>. Merge START to END into
        WORKING.

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.

Local del, incoming edit
-------------------------
### Should we need some way to merge with copyfrom info?
THEIRS: Copy THEIRS to ACTUAL. Add to WORKING.
MINE:   Nothing to do.
ELSEWHERE1:
        Merge WORKING add-half onto THEIRS file/dir. Copy THEIRS to
        WORKING delete-half.
        ### 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.
ELSEWHERE2:
        Merge THEIRS file/dir onto WORKING add-half.
        ### The add-half would have to be supplied by the user.

Locale edit, incoming del
--------------------------
THEIRS: Remove WORKING file/dir.
MINE:   Nothing to do.
RENAME1: Merge START to END from THEIRS add-half onto WORKING.
RENAME2: Merge START to END from WORKING file/dir onto THEIRS add-half.

API changes
==================

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

Re: [RFC] v2 Tree conflict resolver spec.

Posted by Neels J Hofmeyr <ne...@elego.de>.
Daniel Näslund wrote:
> 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. 

Hey father man :)

I'd like to make sure that these terms are clear. The way you wrote
"checked-in" sounds to me like something quite different than intended
(should be "going to check in" or "going to be checked in").

The "going to check in" or short check-in state as Julian and I started
calling it is a local edit or schedule that is going to be checked into the
repos with the next commit.

"Check*ed*-in" sounds like it is already committed ("already checked in")...
confusing.

The checked-out state (note that this one has the 'ed') is all the
unmodified data we have in the WC, that could be gotten from a clean
checkout. "...it is checked out".

Hmm, I wish we had more clarity around terms. Having written this, I don't
think check-in is such a good choice anymore -- with the English language's
subtleties of future/past tense involved.

"BASE" has also been overloaded with -- in some situations -- starkly
different meanings.

What was that naming we suggested for the WC-NG storage trees once?

Quoting from
http://mail-archives.apache.org/mod_mbox/subversion-dev/201001.mbox/%3C6cca3db31001300112j6e038d3bt126f5264001596b6@mail.gmail.com%3E
[Greg replying to Julian replying to Neels]

[[[
On Fri, Jan 29, 2010 at 06:59, Julian Foad <ju...@btopenworld.com> wrote:
> Neels J Hofmeyr wrote:
>> I'd call them the Unchanged tree, the Schedule tree, and the Actual tree.
>> And just to be wild, I'd not make them all-caps ;)
>
> For the first, "Unchanged" could be OK, although since we talk about
> changes so often it might be ambiguous in more contexts than some other
> words would be. ("If the Actual version is unchanged ...", "If the
> Unchanged version has changed ...") "Original" or "Repository" or
> "Checked out" might be better. "Base" might be OK if we decide that's
> what the meaning of the user-level "Base" concept should be.

I have no particular interest in ensuring the names line up, but I
definitely don't want to use the same name when it means something
else to our users.

"Repository" is troublesome because you might be talking about the
server-side trees. "Original", "Checked out", or "Pristine" seem okay.

> For the second, "Schedule" is a bad idea because it has strong and
> specific meanings in WC-1. Greg suggested "Restructured" which sounds
> perfect.
>
> For the third, "Actual" is fine.

Too busy to respond in depth here, but just had a thought: how about
"Edits" to replace the "Actual" name?
]]]

(Let me just say that the current BASE-tree will not be called "Pristine".
We should absolutely *not* call it that. We already have the PRISTINE table,
and a pristine is *linked to* from a BASE, not *is* a BASE.)


> 
> 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!  :)

I shouldn't have started philosophizing about hard parts, heh. No hard
feelings ;)

> 
> [...]
>  
>>> 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.

yes.

> 
>>> 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.

Yes, seen that. Now I'm thinking it is better to work on editor-v2 instead
of introducing user interaction that asks the user to supply other paths
(public API work, soon obsolete, but needs to be maintained).

> 
>>> ### 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@rev
>>   Source right: (kind) URL@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.

A diff/delta info should be enough? Like, not full listings, but only which
paths vanished and appeared, type of thing (same with file content, which is
already like that for the current interactive text resolution).

> * 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.

I believe the copy_from info in the repos (aka svn_fs_history_prev()) is
able to convey where the file came from. No idea why we aren't using that
yet... Oh right, because the code that reports what happened in the repos
needs editor-v2 to be able to communicate this info to the code that takes
the actions and flags the conflicts. :)

> * 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:

you mean copied-here vs. moved-here (add vs. add)?

> 
> [[[
> 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.

Hm, yes. We are wading deep into huge complexity with tree-conflict
resolution. What use is a circular-move-resolver design when the user has no
good way of recognizing a circular move?

I think you're right that the best way to help the user understand would be
to go graphical. But that in itself is complex enough. Can you figure it out?

I'm not sure if we should actually have a user interaction that will open an
image viewer on a jpeg produced by the svn client binary -- but maybe when
*thinking* about graphical representation we can find an easier way... What
kinds of figures would exist? Can they be broken down into simple highly
repetitive shapes? Also just thinking aloud :)

>  
>>> 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

I wasn't aware of this problem (until recently), and with tree-conflicts, we
are already heading in the desirable direction: Store all necessary info on
the conflict in the WC and don't lose that on 'postpone'.

> 
>>> 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. :)

Well, yes, but IMHO you were mixing them up. For example, you used BASE
referencing the source-right of a merge... currently, the "BASE tree" is
that SQLite table that remembers what the WC has checked out. The
commandline "@base" keyword reflects the pristine version of what is going
to be checked-in, which confusingly often means the same. But invariably,
'BASE' is always relative to the current WC. At least that is invariable :)

> 
>>> 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?

The copy_from is a pointer of one node to the path@rev where it came from.
It's basically an arrow backwards, to be saved in the repos. For example,
when doing 'svn copy ^/trunk@REV ^/new_branch', the repos points
new_branch's prev_history to trunk@REV.
Now, doing that in the WC 'svn copy trunk new_branch', the WC WORKING-tree
node for new_branch notes a copy_from of ^/trunk@base. When next committing,
the copy_from becomes the history saved in the repos, making new_branch
point to ^/trunk@what-was-base. (That's e.g. for 'svn log', going backwards,
to be able to continue showing the modification history regardless of path
changes.)
When having copy_from in the WC metadata, it is mostly to remember such a
backwards arrow to be committed to the repos.

We don't have copy_to information in the repos.

From looking at all the copy_from information in the repos, we can figure
out which operations the user actually carried out. But we can only go
backwards ... we can tell that an added node was copied from somewhere, then
we can tell that that other somewhere was removed in the same revision, and
derive a move. That's really horrible to have to do that everywhere, in
update, merge, diff etc..

Editor v2 acknowledges that we are actually interested in getting this info
easily. It provides an API that is able to communicate a move, replace, etc.
to the receiving end of the editor -- so this is much more "how our code
communicates" than "what the repos stores". For example, some places in the
code already know that something is a 'replace', but they are forced (by
editor-v1) to send a separate delete and then a separate add. The receiving
code then has to fumble them back together if it is interested in replaces
(cache all deletes, and then try to match adds with deletes). Editor-v2
acknowledges that replaces *are* interesting, over and above getting the
individual adds and deletes. "We" weren't anticipating that when writing
editor-v1.

Back to the WC:
A BASE-tree node / checked-out state has no "copy_from", because it does not
need to commit anything to the repos. The checked-out stuff's history is in
the repos. "copy_from" is stored in the WORKING-tree node /
going-to-check-in state, only when you do an 'svn copy/move' to here (to the
node's path). Note that the WORKING node does not know where it was copied
or moved to, either. Some other tree node will commit that.

For example, if a 'move' is coming in, then the history of that incoming
move is already saved in the repos, no need to write that to the WORKING
node or something.

BASE, WORKING etc. are the states before and after an operation. Editor-v2
communicates that *operation* (more clearly than editor-v1). The end driving
editor-v2 will still need to do some figuring-out to send the proper ops.
But at least the receiving end of editor-v2 doesn't have to painfully match
things up anymore (with far less leverage on available information than the
driving end).

> 
>>> 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?

Actually, sure, we can get this info when crawling, and even from a nice db
query once the wc.db is one-per-WC (and not one-per-dir).

>>
>>> 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.

When resolving to THEIRS, the user is *making* that decision. But, right, as
with 'svn delete --keep-local', we should probably have a way to leave the
locally edited version around... Yet another --accept option??

>  
>>> 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.

Right, before starting to resolve, the node is already in locally-added
state and marked conflicted. The tree-conflict *flagging* code must arrange
the BASE/WORKING-tree nodes to reflect the proper schedule for MINE.

> 
>>> 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.

I don't feel too strongly about that either ... yet.

>> 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.

Merge is almost always a real shocker... oxygen masks ready.

>  
>>> 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.

Alarm bells ringing: I have figured out that we can forget fully storing
THEIRS in a merge-TC and thus we can forget resolving to THEIRS without
contacting the repos again. We could only remember what parameters the merge
operation had and re-trigger the same merge. The pristines would be able to
store the fulltexts of some revisions, but the merge operation still has to
do all its figuring out which nodes existed in which revisions as normal.

The simplest proof is to point out that the second, third, fifteenth merge
ranges may also cause tree-conflicts each. Can't store THEIRS for
completely-offline resolution! :(

> 
> I'm watching the evolution of the pristine store spec with interest. :)

The pristine store is really sweet, I love it :)
It'll do a much better job than the explicit text-base files per node.

> 
>> 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.

(This is where I meant the terms get mixed up)

> 
>> 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.

Not sure what you mean?

>  
>>> 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!

And this remains open, see also the large <future> tag in the TC-cheatsheet
I posted, and the thread discussing that. Hey, reminds me of wanting to
commit that to notes/... (at least the top section)

http://mail-archives.apache.org/mod_mbox/subversion-dev/201002.mbox/%3C4B7A86CC.6040905@elego.de%3E

~Neels


Re: [RFC] v2 Tree conflict resolver spec.

Posted by Daniel Näslund <da...@longitudo.com>.
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@rev
>   Source right: (kind) URL@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!

Re: [RFC] v2 Tree conflict resolver spec.

Posted by Daniel Näslund <da...@longitudo.com>.
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@rev
>   Source right: (kind) URL@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!

Re: [RFC] v2 Tree conflict resolver spec.

Posted by Neels J Hofmeyr <ne...@elego.de>.
I sincerely apologize for the bugginess of my mail client. >:(

~Neels

Neels J Hofmeyr wrote:
>> Just a few comments from me on the first half of Neels' response to thi=
> s
>> draft.
>> =20
>> Neels J Hofmeyr wrote:
>>> Daniel N=C3=A4slund 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 th=
> e
>>> conflict resolution conform with adjacent WC states. (attention, high =
> degree
>>> of meta language. No need to understand me :P )
>> =20
>> I assumed Daniel meant, "The hard part is figuring out what state WE
>> WANT the WC to be in ...".
> 
> Yes, that's right.
> 
>> =20
>>>> 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=3Dtheirs should be identic=
> al to
>>> 'svn revert'.
>>>
>>> In general, 'svn update' should pull in the complete BASE tree state o=
> f the
>>> update's target revision regardless of conflicts, and any conflicts sh=
> ould
>>> be local schedules on top of that.
>> =20
>> Yup. And with merge, --accept=3Dmine should be identical to 'svn revert=
> '.
>> =20
>> Those two sentences state a fundamental property of the behaviour that
>> we want. We should write them in the document as a hard requirement.
>> =20
>>>> ### 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.
>> =20
>> Don't worry about physical size - it's reasonable to expect a user to
>> have enough free disk space to store another copy of part of their tree=
> ,
>> and it's easy to disable or modify that behaviour afterwards if somebod=
> y
>> really doesn't want it.
>> =20
>> The tricky bit there is just how to store and how to keep track of and
>> how to access that data.
>> =20
>> Let's not go there, at the moment, because it's secondary - it's no use=
> 
>> at all until the rest of this RFC is sorted out, whereas the rest of
>> this RFC IS useful without having "theirs" stored locally after a
>> conflict on merge.
>> =20
>> [...]
>> =20
>>>> Contents
>>>> =3D=3D=3D=3D=3D=3D=3D=3D=3D
>>>> Problem definition
>>>> Requirements
>>>> Terminology
>>>> Use cases update/switch
>>>> Use cases merge
>>>> API changes
>>>> User interface
>> =20
>> [...]
>> =20
>>>> Terminology
>>>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>>>> In this document, WORKING means the user's version, which possibly ha=
> s
>>>> text, property and/or tree modifications relative to the BASE; it doe=
> s
>>>> 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=
> =2E..
>>> Below, when I say 'BASE tree' or 'WORKING tree', I mean the wc-ng meta=
> data
>>> 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 abo=
> ut
>>> props to be missing below.)
>> =20
>> Terminology is crucial. We'll never understand each other without
>> agreeing on the terminology.
>> =20
>> Would it be OK with you if we just don't use the WC-NG DB terms here? I=
> 
>> don't believe they are well enough understood by most of us, and I don'=
> t
>> believe they are relevant at this level of specification. Specifically,=
> 
>> making a distinction between WORKING and ACTUAL is unneccessary in
>> defining the conflict behaviour, although of course it will be necessar=
> y
>> in *implementing* any parts of that behaviour that are right down in
>> WC-NG. From the user's point of view, which is a useful point of view
>> all the way down into the code paths that deal with tree conflicts,
>> there are only two trees of interest:
>> =20
>>   * what I checked out (or last updated to) - which is a mixed-revision=
> 
>> copy of parts of the repository;
>> =20
>>   * what I expect to check in - which is the versioned files and dirs o=
> n
>> my disk, plus "svn propget" to see props.
> 
> I think 'checked out' and 'to check in' are pretty good terms understandi=
> ng
> wise. 'Checked out' matches my concept of the BASE tree, and 'to check in=
> '
> matches my concept of the WORKING tree node. But they are a bit clunky
> grammatically unless we can tie them to a noun, IMHO. How to complete the=
> 
> sentences "Find the information in the checked out <noun>." ; "Register t=
> he
> incoming add in the [to-]check-in <noun>." node? tree? store?
> 
> While speaking of terminology, note to self. I wouldn't want to say
> "delete/add/replace/switch/edit the checked-out node", because we use tho=
> se
> terms for svn operations. Better is "clear/set/swap the checked-out node"=
> =2E
> 
> ~Neels
> 


Re: [RFC] v2 Tree conflict resolver spec.

Posted by Neels J Hofmeyr <ne...@elego.de>.
> Just a few comments from me on the first half of Neels' response to thi=
s
> draft.
>=20
> Neels J Hofmeyr wrote:
>> Daniel N=C3=A4slund 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 th=
e
>> conflict resolution conform with adjacent WC states. (attention, high =
degree
>> of meta language. No need to understand me :P )
>=20
> I assumed Daniel meant, "The hard part is figuring out what state WE
> WANT the WC to be in ...".

Yes, that's right.

>=20
>>> 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=3Dtheirs should be identic=
al to
>> 'svn revert'.
>>
>> In general, 'svn update' should pull in the complete BASE tree state o=
f the
>> update's target revision regardless of conflicts, and any conflicts sh=
ould
>> be local schedules on top of that.
>=20
> Yup. And with merge, --accept=3Dmine should be identical to 'svn revert=
'.
>=20
> Those two sentences state a fundamental property of the behaviour that
> we want. We should write them in the document as a hard requirement.
>=20
>>> ### 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.
>=20
> Don't worry about physical size - it's reasonable to expect a user to
> have enough free disk space to store another copy of part of their tree=
,
> and it's easy to disable or modify that behaviour afterwards if somebod=
y
> really doesn't want it.
>=20
> The tricky bit there is just how to store and how to keep track of and
> how to access that data.
>=20
> Let's not go there, at the moment, because it's secondary - it's no use=

> at all until the rest of this RFC is sorted out, whereas the rest of
> this RFC IS useful without having "theirs" stored locally after a
> conflict on merge.
>=20
> [...]
>=20
>>> Contents
>>> =3D=3D=3D=3D=3D=3D=3D=3D=3D
>>> Problem definition
>>> Requirements
>>> Terminology
>>> Use cases update/switch
>>> Use cases merge
>>> API changes
>>> User interface
>=20
> [...]
>=20
>>> Terminology
>>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>>> In this document, WORKING means the user's version, which possibly ha=
s
>>> text, property and/or tree modifications relative to the BASE; it doe=
s
>>> 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=
=2E..
>> Below, when I say 'BASE tree' or 'WORKING tree', I mean the wc-ng meta=
data
>> 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 abo=
ut
>> props to be missing below.)
>=20
> Terminology is crucial. We'll never understand each other without
> agreeing on the terminology.
>=20
> Would it be OK with you if we just don't use the WC-NG DB terms here? I=

> don't believe they are well enough understood by most of us, and I don'=
t
> believe they are relevant at this level of specification. Specifically,=

> making a distinction between WORKING and ACTUAL is unneccessary in
> defining the conflict behaviour, although of course it will be necessar=
y
> in *implementing* any parts of that behaviour that are right down in
> WC-NG. From the user's point of view, which is a useful point of view
> all the way down into the code paths that deal with tree conflicts,
> there are only two trees of interest:
>=20
>   * what I checked out (or last updated to) - which is a mixed-revision=

> copy of parts of the repository;
>=20
>   * what I expect to check in - which is the versioned files and dirs o=
n
> my disk, plus "svn propget" to see props.

I think 'checked out' and 'to check in' are pretty good terms understandi=
ng
wise. 'Checked out' matches my concept of the BASE tree, and 'to check in=
'
matches my concept of the WORKING tree node. But they are a bit clunky
grammatically unless we can tie them to a noun, IMHO. How to complete the=

sentences "Find the information in the checked out <noun>." ; "Register t=
he
incoming add in the [to-]check-in <noun>." node? tree? store?

While speaking of terminology, note to self. I wouldn't want to say
"delete/add/replace/switch/edit the checked-out node", because we use tho=
se
terms for svn operations. Better is "clear/set/swap the checked-out node"=
=2E

~Neels


Re: [RFC] v2 Tree conflict resolver spec.

Posted by Julian Foad <ju...@wandisco.com>.
Neels J Hofmeyr wrote:
> Julian Foad wrote:
> > 
> > I believe it should be a low level job [1] to detect when a (potential)
> > tree conflict is happening, and a higher-level job to decide how to
> > resolve the (potential) conflict. The low level should pass the details
> > to the higher layer, and the higher layer can say something like:
> > 
> > {
> >   if user-option is ACCEPT-THEIRS or ACCEPT-MINE or ...:
> >     return user-option
> > 
> >   switch (tree-conflict-type):
> > 
> >     delete+edit:
> >       choice = interactive-resolve(...)
> >       return choice
> > 
> >     add+add:
> >       if same-content(...):
> >         return ACCEPT-THEIRS  # THEIRS or MINE, doesn't matter
> >       choice = interactive-resolve(...)
> >       return choice
> > 
> >     ...
> > }
> 
> Hmm, would such higher layer also need to distinct between TC during
> update/switch and merge?

I would want it to be able to know and modify its behaviour based on
what kind of operation is happening. Every callback should take a
callback baton so that the higher level code can pass information such
as this to it.

- Julian


> > 
> > I say "(potential) conflict" because even in a case like delete+delete
> > or add+add when the two nodes conflicting are identical and most users
> > just want it resolved automatically, I think the low level should pass
> > the details to the higher layer, and the higher layer can say "if it's
> > delete+delete then return ACCEPT-THEIRS" or similar. Even if this
> > decision is hard-coded in the higher layer rather than customizable, I
> > believe that having it there will make a much better design than if the
> > lower layer made that decision and didn't bother to ask the higher
> > layer.
> 
> I like that :)
> 
> The low layer would be check_tree_conflict() in _wc/update_editor.c and <the
> code that determines parameters passed to tree_conflict() and
> tree_conflict_on_add()> in _client/merge.c, of which the result could be run
> by a common higher-layer function, after which the lower code carries out
> the actions indicated (like now, e.g. where the code decides to fall through
> the 'delete' code path in case of reason_deleted during update...)
> 
> ~Neels



Re: [RFC] v2 Tree conflict resolver spec.

Posted by Neels J Hofmeyr <ne...@elego.de>.
Julian Foad wrote:
> 
> I believe it should be a low level job [1] to detect when a (potential)
> tree conflict is happening, and a higher-level job to decide how to
> resolve the (potential) conflict. The low level should pass the details
> to the higher layer, and the higher layer can say something like:
> 
> {
>   if user-option is ACCEPT-THEIRS or ACCEPT-MINE or ...:
>     return user-option
> 
>   switch (tree-conflict-type):
> 
>     delete+edit:
>       choice = interactive-resolve(...)
>       return choice
> 
>     add+add:
>       if same-content(...):
>         return ACCEPT-THEIRS  # THEIRS or MINE, doesn't matter
>       choice = interactive-resolve(...)
>       return choice
> 
>     ...
> }

Hmm, would such higher layer also need to distinct between TC during
update/switch and merge?

> 
> I say "(potential) conflict" because even in a case like delete+delete
> or add+add when the two nodes conflicting are identical and most users
> just want it resolved automatically, I think the low level should pass
> the details to the higher layer, and the higher layer can say "if it's
> delete+delete then return ACCEPT-THEIRS" or similar. Even if this
> decision is hard-coded in the higher layer rather than customizable, I
> believe that having it there will make a much better design than if the
> lower layer made that decision and didn't bother to ask the higher
> layer.

I like that :)

The low layer would be check_tree_conflict() in _wc/update_editor.c and <the
code that determines parameters passed to tree_conflict() and
tree_conflict_on_add()> in _client/merge.c, of which the result could be run
by a common higher-layer function, after which the lower code carries out
the actions indicated (like now, e.g. where the code decides to fall through
the 'delete' code path in case of reason_deleted during update...)

~Neels

> 
> So this is a bit like the idea of the existing "resolver callback",
> except that that was only trying to pass information about certain kinds
> of conflict that we thought really needed the user's input. Instead of
> that, I think we should have a callback that passes references to all
> the information that the higher layer might need in order to make a
> decision. And it should do it for every case where two changes are being
> combined in the same node.
> 
> I'm only talking about the layering of where the decisions are made. I'm
> not saying where the code should be that performs the action requested.
> (So when my example says "return user-option" it might instead need to
> say "perform user-option; return DONE". Or whatever.)
> 
> 
> [1] The code is in libsvn_wc for updates, libsvn_client for merges.
> 
> - Julian
> 
> 
>> What to do?
>> ============
>> Finish the resolver spec of course.
>> Make svn resolved able to handle states other than 'working'
>> |-> We need to store the tree conflict data on the same node instead of
>>   |  its parent.
>>   |-> Things would be cleaner if we stored the different versions of a
>>       node in the db instead of temp files.
>>
>> Daniel
> 
> 
> 


Re: [RFC] v2 Tree conflict resolver spec.

Posted by Julian Foad <ju...@wandisco.com>.
On Thu, 2010-02-18, Daniel Näslund wrote:
> Hi!
> 
> My initial thought when I started writing on a spec for a TC resolver
> was that it would involve a lot of quirks and edge cases handled solely
> in subversion/svn/conflict-callbacks.c. That the svn resolver would only
> serve as a proof-of-concept for other API users. But if I could
> implement the ---accept options in libsvn_wc/conflicts.c we would have a
> firm foundation for others to build upon.
> 
> I could implement each --accept option in libsvn_wc/conflicts.c and
> write test cases for each. Is this a possible way to go? Many of the
> --accept options would require much more than the existing ones for
> property and text conflicts.

Here is a good time to say something that has always been in my mind,
about the layering of the decision making points in the code, when a
conflict is initially discovered.

I believe it should be a low level job [1] to detect when a (potential)
tree conflict is happening, and a higher-level job to decide how to
resolve the (potential) conflict. The low level should pass the details
to the higher layer, and the higher layer can say something like:

{
  if user-option is ACCEPT-THEIRS or ACCEPT-MINE or ...:
    return user-option

  switch (tree-conflict-type):

    delete+edit:
      choice = interactive-resolve(...)
      return choice

    add+add:
      if same-content(...):
        return ACCEPT-THEIRS  # THEIRS or MINE, doesn't matter
      choice = interactive-resolve(...)
      return choice

    ...
}

I say "(potential) conflict" because even in a case like delete+delete
or add+add when the two nodes conflicting are identical and most users
just want it resolved automatically, I think the low level should pass
the details to the higher layer, and the higher layer can say "if it's
delete+delete then return ACCEPT-THEIRS" or similar. Even if this
decision is hard-coded in the higher layer rather than customizable, I
believe that having it there will make a much better design than if the
lower layer made that decision and didn't bother to ask the higher
layer.

So this is a bit like the idea of the existing "resolver callback",
except that that was only trying to pass information about certain kinds
of conflict that we thought really needed the user's input. Instead of
that, I think we should have a callback that passes references to all
the information that the higher layer might need in order to make a
decision. And it should do it for every case where two changes are being
combined in the same node.

I'm only talking about the layering of where the decisions are made. I'm
not saying where the code should be that performs the action requested.
(So when my example says "return user-option" it might instead need to
say "perform user-option; return DONE". Or whatever.)


[1] The code is in libsvn_wc for updates, libsvn_client for merges.

- Julian


> What to do?
> ============
> Finish the resolver spec of course.
> Make svn resolved able to handle states other than 'working'
> |-> We need to store the tree conflict data on the same node instead of
>   |  its parent.
>   |-> Things would be cleaner if we stored the different versions of a
>       node in the db instead of temp files.
> 
> Daniel



Re: [RFC] v2 Tree conflict resolver spec.

Posted by Daniel Näslund <da...@longitudo.com>.
Hi!

My initial thought when I started writing on a spec for a TC resolver
was that it would involve a lot of quirks and edge cases handled solely
in subversion/svn/conflict-callbacks.c. That the svn resolver would only
serve as a proof-of-concept for other API users. But if I could
implement the ---accept options in libsvn_wc/conflicts.c we would have a
firm foundation for others to build upon.

I could implement each --accept option in libsvn_wc/conflicts.c and
write test cases for each. Is this a possible way to go? Many of the
--accept options would require much more than the existing ones for
property and text conflicts.

What to do?
============
Finish the resolver spec of course.
Make svn resolved able to handle states other than 'working'
|-> We need to store the tree conflict data on the same node instead of
  |  its parent.
  |-> Things would be cleaner if we stored the different versions of a
      node in the db instead of temp files.

Daniel

On Wed, Feb 10, 2010 at 02:41:29PM +0100, Neels J Hofmeyr wrote:
> Hi TC folks,
> 
> I tried to put the tree-conflicts design information so far into tables.
> Find the result attached: a tc-cheatsheet up for discussion. If it gets
> initial approval I'll check it into notes/tree-conflicts/ so we can edit.
> 
> TREE CONFLICT CHEAT SHEET (DESIGN PHASE)
> 
> 
>  After tree      | 'theirs'          | 'mine' is      | 'svn status' shows
>  conflict during | is found in       | found in       | TC and is otherwise
> -----------------+-------------------+----------------+---------------
>                  |                   |                |
>  update/switch   | checked-out state | check-in state | adapted to be a
>                  |                   | (unchanged)    | mod of the new
>                  |                   |                | checked-out state
>                  |                   |                |
>  merge           | conflict info     | check-in state | unchanged
>                  |                   | (unchanged)    |
>                  |                   |                |
>  
>  
>  
>  
>  Upon recording             |     ... changed during          |
>  a tree conflict, is        | update/switch? | merge?         |
> ----------------------------+----------------+----------------+
>  checked-out state          | Yes            | No             |
>  check-in state             | No             | No             |
>  'svn status' other than TC | Yes            | No             |
> 
> 
> 
> 
>  When resolving a     |     ...which was caused by a      |
>  tree-conflict with   | switch/update  |  merge           |
>                       |     , the check-in state is       |
> ----------------------+-----------------------------------+
>                       |                |                  |
>  --accept=theirs      | cleared        | reset to theirs  |
>                       |                |                  |
>  --accept=mine        | unchanged      | unchanged        |
>                       |                |                  |
>  'svn revert'         | cleared        | cleared          |
>                       |                |                  |
> 
> 
> 
> <future depends-on="editor-v2" status="needs-review">
> 
> Assuming that we have full info on copy/move (i.e. an add here is really
> just an add and *not* possibly part of a copy/move), then:
> 
>  When resolving a      | 
>  tree-conflict of      | 
>  local vs. | incoming  | offer these --accept= resolution options
> ===========+===========+=====================================================
>            |           |                                                      
>  (all combinations of) | 
>  add       | add       | theirs, mine, move-theirs, move-mine                 
>  copy-here | copy-here | 
>  move-here | move-here | 
>            |           | 
> -----------+-----------+-----------------------------------------------------
>            |           | 
>  delete    | delete    | (is not a tree-conflict)
>            |           | 
> -----------+-----------+-----------------------------------------------------
>            |           | 
>       (line-wise)      | 
>  delete    | move-away | (is not a tree-conflict)
>  move-away | delete    | 
>            |           | 
> -----------+-----------+-----------------------------------------------------
>            |           | 
>       (line-wise)      | 
>  delete    | edit      | theirs, mine 
>  edit      | delete    | 
>            |           | 
> -----------+-----------+-----------------------------------------------------
>            |           | 
>  move-away | edit      | mine, both, merge-here, merge-there,
>            |           | merge-there-keep-here, merge-there-merge-here
>            |           | [1] [2]
>            |           |           grain of salt ---> . 
> -----------+-----------+-----------------------------------------------------
>            |           | 
>  edit      | move-away | theirs, both, merge-here, merge-there,
>            |           | merge-there-keep-here, merge-there-merge-here
>            |           | [1] [2]
>            |           |           grain of salt ---> . 
> -----------+-----------+-----------------------------------------------------
>            |           | 
>  move-away | move-away | theirs, mine, both, merge-at-theirs, merge-at-mine,
>            |           | merge-at-theirs-keep-mine, merge-at-mine-keep-theirs,
>            |           | merge-at-both
>            |           | (not a tree-conflict when move targets match)
>            |           | [1] [2]
>            |           |           grain of salt ---> . 
> -----------+-----------+-----------------------------------------------------
> 
>  
> Where the --accept= options mean:
> 
>                   mine  Enforce my change, make sure theirs is lost/undone.
>                 theirs  Agree to theirs, make sure mine is lost.
>            move-theirs  Keep both additions, but move theirs to another path.
>              move-mine  Keep both additions, but move mine to another path.
>                   both  Keep both new move-targets (one becomes a simple "A +").
>            merge-there  Apply the edit to the moved-away path.
>             merge-here  Discard the move-away, but still merge text/prop mods.
>  merge-there-keep-here  Apply text/prop mods to the moved-away path, but
>                         keep an unmerged copy at this path.
> merge-there-merge-here  Apply text/prop mods to the moved-away path, and
>                         keep another, also merged, copy at this path.
>        merge-at-theirs  Apply the edits on my moved-away path to their moved-
>                         away path, discard my move.
>          merge-at-mine  Apply the edits on their moved-away path to my moved-
>                         away path, discard their move.
> merge-at-theirs-keep-mine Apply the edits on my moved-away path to their moved-
>                         away path, but keep my move-away with only *its* edits.
> merge-at-mine-keep-theirs Apply the edits on their moved-away path to my moved-
>                         away path, but keep their move-away with only *its*
>                         edits.
>          merge-at-both  Apply all edits to all moved-away paths and keep them.
> 
> [1] I am not sure whether resolution should only care about *this* path.
>     Some of the resolution options suggest that we would undo the add-part
>     of a move-away, at the *other* path, or apply edits to the *other* path.
>     While it would be nicer if we could get away with only resolving *this*
>     path, users might get annoyed with having to do two steps to get what
>     they wanted (i.e. manually modify the other path to the desired result)
>     when the resolver had all the information there and could have just done
>     the job automatically.
> 
> [2] The merge-* options indicate a mixture of text/prop mods with tree
>     conflicts. They say whether to keep the respective sides' text/prop mods
>     when resolving the tree-conflict. These options would entail that we
>     have to run an apply-textdelta-ish code that can introduce new text/prop
>     conflicts.
>     Should we really go there? What are the alternatives? We won't get away
>     without defining what happens to text/prop mods at two different paths
>     that are related via moves.
>     ### Analogy to copies?
> 
> </future>
> 



Re: [RFC] v2 Tree conflict resolver spec.

Posted by Neels J Hofmeyr <ne...@elego.de>.
Julian Foad wrote:
> On Wed, 2010-02-10, Neels J Hofmeyr wrote:
>> Hi TC folks,
>>
>> I tried to put the tree-conflicts design information so far into tables.
> 
> Thanks, Neels. This is a really useful way to present the info.

thanks :)

>> Find the result attached: a tc-cheatsheet up for discussion. If it gets
>> initial approval I'll check it into notes/tree-conflicts/ so we can edit.
> 
> [...]
> 
>> plain text document attachment (tc-cheatsheet.txt)
>> TREE CONFLICT CHEAT SHEET (DESIGN PHASE)
>>
>>
>>  After tree      | 'theirs'          | 'mine' is      | 'svn status' shows
>>  conflict during | is found in       | found in       | TC and is otherwise
>> -----------------+-------------------+----------------+---------------
>>                  |                   |                |
>>  update/switch   | checked-out state | check-in state | adapted to be a
>>                  |                   | (unchanged)    | mod of the new
>>                  |                   |                | checked-out state
>>                  |                   |                |
>>  merge           | conflict info     | check-in state | unchanged
>>                  |                   | (unchanged)    |
>>                  |                   |                |
>>  
>>
>>
>>
>>  Upon recording             |     ... changed during          |
>>  a tree conflict, is        | update/switch? | merge?         |
>> ----------------------------+----------------+----------------+
>>  checked-out state          | Yes            | No             |
>>  check-in state             | No             | No             |
>>  'svn status' other than TC | Yes            | No             |
>>
>>
>>
>>
>>  When resolving a     |     ...which was caused by a      |
>>  tree-conflict with   | switch/update  |  merge           |
>>                       |     , the check-in state is       |
>> ----------------------+-----------------------------------+
>>                       |                |                  |
>>  --accept=theirs      | cleared        | reset to theirs  |
>>                       |                |                  |
>>  --accept=mine        | unchanged      | unchanged        |
>>                       |                |                  |
>>  'svn revert'         | cleared        | cleared          |
>>                       |                |                  |
> 
> All the above: +1.

Nice. This is the next step. I've started verifying this behaviour, and
especially with add vs. add, we are quite off. I'm hitting situations (also
in 'svn diff' and 'svn cat') where there is a revert-base but no text-base
(which the current code fails at), or where the previous text-base's
checksum is confused with a locally replaced node's checksum; they would
take quite some spaghetti to fix. I've come to the conclusion that it makes
much more sense to use wc-ng than wc-1 concerning the text-base. I've
decided to rather help push the pristine store a little, so that we don't
need to fix behaviour concerning text-base/revert-base file confusion, and
to then come back to TC add-vs-add, diff_wc_url and from that
incoming-delete-should-not-delete-modified-tree.
The tail is getting longer :)

From Julian's remarks below (thanks, Julian!), I note many shortcomings in
what I wrote below. Maybe we can find a solution for the circular move (with
and without edits) that also includes the simpler cases for free?

I am getting a feeling that it would make sense to only offer resolving
*this* path (at least to begin with), so that a circular move needs explicit
resolution at each point of the circle, and that each resolution is
independent of resolving the other paths involved. That would mean to also
flag tree-conflicts at the *other* paths (e.g. moved-away-to or
moved-here-from paths), so that the user remembers to also decide for the
other ends whether anything should change...

The game is on! :)

~Neels

* neels heads over to the pristine store lab

> 
> 
>> <future depends-on="editor-v2" status="needs-review">
>>
>> Assuming that we have full info on copy/move (i.e. an add here is really
>> just an add and *not* possibly part of a copy/move), then:
>>
>>  When resolving a      | 
>>  tree-conflict of      | 
>>  local vs. | incoming  | offer these --accept= resolution options
>> ===========+===========+=====================================================
>>            |           |                                                      
>>  (all combinations of) | 
>>  add       | add       | theirs, mine, move-theirs, move-mine                 
>>  copy-here | copy-here | 
>>  move-here | move-here | 
> 
> We also want to offer some kind of "auto-merge" option which keeps one
> version (just like "theirs" and like "mine") if both are identical, and
> offers some kind of merging if they differ. Or which re-prompts if they
> differ. Or the initial prompt should differ depending on whether the two
> added things differ. It gets complex with directories, and with
> differences in copy-from but identical content, etc.
> 
>>            |           | 
>> -----------+-----------+-----------------------------------------------------
>>            |           | 
>>  delete    | delete    | (is not a tree-conflict)
> 
> I agree that most users will want this silently allowed. That's a
> sensible behaviour for "svn".
> 
> Theoretically, however, that's only true in the same sense that
> add-on-add is not a tree conflict if the same node is added twice: it's
> an assumption that the user doesn't ever want to know about this kind of
> conflict. Some users in some merging tasks will want a strict mode,
> where they are not expecting any double-deletes and would like it
> flagged if it happens.
> 
> Somehow I want to introduce a "conflict rules" framework, not
> necessarily accessible through the "svn" UI, but within the
> implementation and APIs. The rules will be a read-only context passed in
> from the client that is like a table saying "delete-delete is to be a
> silent delete, add-add is to be flagged as a conflict, edit-rename is to
> be a silent edit of the renamed file, ...".
> 
> Then it will be easy for people to write a client that behaves more or
> less strictly without having to first rev all our APIs.
> 
> And a second issue with "delete + delete": in a merge, we have to
> consider whether the two things being deleted were identical. I haven't
> yet tried to expand the rules to take account of that.
> 
> I *think* the same considerations also apply to all the other cases that
> involve a delete during a merge.
> 
>>            |           | 
>> -----------+-----------+-----------------------------------------------------
>>            |           | 
>>       (line-wise)      | 
>>  delete    | move-away | (is not a tree-conflict)
>>  move-away | delete    | 
>>            |           | 
>> -----------+-----------+-----------------------------------------------------
>>            |           | 
>>       (line-wise)      | 
>>  delete    | edit      | theirs, mine 
>>  edit      | delete    | 
>>            |           | 
>> -----------+-----------+-----------------------------------------------------
>>            |           | 
>>  move-away | edit      | mine, both, merge-here, merge-there,
>>            |           | merge-there-keep-here, merge-there-merge-here
>>            |           | [1] [2]
>>            |           |           grain of salt ---> . 
> 
> Heh!
> 
> But "merge-there" is surely the almost universal wish, just like "delete
> + delete -> delete" and "add + add -> add" when the two adds are
> identical are almost universal wishes.
> 
> "merge-here" just means the same as "theirs", doesn't it? We need to
> keep options called "mine" and "theirs" in every menu, because they
> always make sense, even if we annotate them with extra description such
> as "theirs (undo the move and merge the incoming edit here)".
> 
> What is "merge-there-keep-here" for? I can't think of a realistic use
> case.
> 
> What is "merge-there-merge-here" for? In the case of a copy rather than
> a move, then yes I can see users wanting the incoming change applied to
> both copies, but for a move?
> 
> Ugh. Moves and copies get very complex. Partly because any move can have
> an edit as well. Partly because the destination location might also be a
> location from where something was moved away at the same time (perhaps
> also being in conflict, in which case we'd require that it be resolved
> first ... unless it's a cycle of moves like A>B, B>A or A>B, B>C, C>A in
> which case they might have to be resolved all at the same time).
> Seriously, the design has to cope with not falling over in these cases,
> though we certainly don't have to give special friendly options, just so
> long as there is some way for the user to resolve it.
> 
>> -----------+-----------+-----------------------------------------------------
>>            |           | 
>>  edit      | move-away | theirs, both, merge-here, merge-there,
>>            |           | merge-there-keep-here, merge-there-merge-here
>>            |           | [1] [2]
>>            |           |           grain of salt ---> . 
>> -----------+-----------+-----------------------------------------------------
>>            |           | 
>>  move-away | move-away | theirs, mine, both, merge-at-theirs, merge-at-mine,
>>            |           | merge-at-theirs-keep-mine, merge-at-mine-keep-theirs,
>>            |           | merge-at-both
>>            |           | (not a tree-conflict when move targets match)
>>            |           | [1] [2]
>>            |           |           grain of salt ---> . 
> 
> Ugh. This case gets even more complex.
>> -----------+-----------+-----------------------------------------------------
>>
> 
> We'll need to define copies as well :-/
> 
>>  
>> Where the --accept= options mean:
>>
>>                   mine  Enforce my change, make sure theirs is lost/undone.
>>                 theirs  Agree to theirs, make sure mine is lost.
>>            move-theirs  Keep both additions, but move theirs to another path.
>>              move-mine  Keep both additions, but move mine to another path.
>>                   both  Keep both new move-targets (one becomes a simple "A +").
>>            merge-there  Apply the edit to the moved-away path.
>>             merge-here  Discard the move-away, but still merge text/prop mods.
>>  merge-there-keep-here  Apply text/prop mods to the moved-away path, but
>>                         keep an unmerged copy at this path.
>> merge-there-merge-here  Apply text/prop mods to the moved-away path, and
>>                         keep another, also merged, copy at this path.
>>        merge-at-theirs  Apply the edits on my moved-away path to their moved-
>>                         away path, discard my move.
>>          merge-at-mine  Apply the edits on their moved-away path to my moved-
>>                         away path, discard their move.
>> merge-at-theirs-keep-mine Apply the edits on my moved-away path to their moved-
>>                         away path, but keep my move-away with only *its* edits.
>> merge-at-mine-keep-theirs Apply the edits on their moved-away path to my moved-
>>                         away path, but keep their move-away with only *its*
>>                         edits.
>>          merge-at-both  Apply all edits to all moved-away paths and keep them.
>>
>> [1] I am not sure whether resolution should only care about *this* path.
>>     Some of the resolution options suggest that we would undo the add-part
>>     of a move-away, at the *other* path, or apply edits to the *other* path.
>>     While it would be nicer if we could get away with only resolving *this*
>>     path, users might get annoyed with having to do two steps to get what
>>     they wanted (i.e. manually modify the other path to the desired result)
>>     when the resolver had all the information there and could have just done
>>     the job automatically.
>>
>> [2] The merge-* options indicate a mixture of text/prop mods with tree
>>     conflicts. They say whether to keep the respective sides' text/prop mods
>>     when resolving the tree-conflict. These options would entail that we
>>     have to run an apply-textdelta-ish code that can introduce new text/prop
>>     conflicts.
>>     Should we really go there? What are the alternatives? We won't get away
>>     without defining what happens to text/prop mods at two different paths
>>     that are related via moves.
>>     ### Analogy to copies?
>>
>> </future>
>>
> 
> - Julian
> 
> 


Re: [RFC] v2 Tree conflict resolver spec.

Posted by Julian Foad <ju...@wandisco.com>.
On Wed, 2010-02-10, Neels J Hofmeyr wrote:
> Hi TC folks,
> 
> I tried to put the tree-conflicts design information so far into tables.

Thanks, Neels. This is a really useful way to present the info.


> Find the result attached: a tc-cheatsheet up for discussion. If it gets
> initial approval I'll check it into notes/tree-conflicts/ so we can edit.

[...]

> plain text document attachment (tc-cheatsheet.txt)
> TREE CONFLICT CHEAT SHEET (DESIGN PHASE)
> 
> 
>  After tree      | 'theirs'          | 'mine' is      | 'svn status' shows
>  conflict during | is found in       | found in       | TC and is otherwise
> -----------------+-------------------+----------------+---------------
>                  |                   |                |
>  update/switch   | checked-out state | check-in state | adapted to be a
>                  |                   | (unchanged)    | mod of the new
>                  |                   |                | checked-out state
>                  |                   |                |
>  merge           | conflict info     | check-in state | unchanged
>                  |                   | (unchanged)    |
>                  |                   |                |
>  
> 
> 
> 
>  Upon recording             |     ... changed during          |
>  a tree conflict, is        | update/switch? | merge?         |
> ----------------------------+----------------+----------------+
>  checked-out state          | Yes            | No             |
>  check-in state             | No             | No             |
>  'svn status' other than TC | Yes            | No             |
> 
> 
> 
> 
>  When resolving a     |     ...which was caused by a      |
>  tree-conflict with   | switch/update  |  merge           |
>                       |     , the check-in state is       |
> ----------------------+-----------------------------------+
>                       |                |                  |
>  --accept=theirs      | cleared        | reset to theirs  |
>                       |                |                  |
>  --accept=mine        | unchanged      | unchanged        |
>                       |                |                  |
>  'svn revert'         | cleared        | cleared          |
>                       |                |                  |

All the above: +1.


> <future depends-on="editor-v2" status="needs-review">
> 
> Assuming that we have full info on copy/move (i.e. an add here is really
> just an add and *not* possibly part of a copy/move), then:
> 
>  When resolving a      | 
>  tree-conflict of      | 
>  local vs. | incoming  | offer these --accept= resolution options
> ===========+===========+=====================================================
>            |           |                                                      
>  (all combinations of) | 
>  add       | add       | theirs, mine, move-theirs, move-mine                 
>  copy-here | copy-here | 
>  move-here | move-here | 

We also want to offer some kind of "auto-merge" option which keeps one
version (just like "theirs" and like "mine") if both are identical, and
offers some kind of merging if they differ. Or which re-prompts if they
differ. Or the initial prompt should differ depending on whether the two
added things differ. It gets complex with directories, and with
differences in copy-from but identical content, etc.

>            |           | 
> -----------+-----------+-----------------------------------------------------
>            |           | 
>  delete    | delete    | (is not a tree-conflict)

I agree that most users will want this silently allowed. That's a
sensible behaviour for "svn".

Theoretically, however, that's only true in the same sense that
add-on-add is not a tree conflict if the same node is added twice: it's
an assumption that the user doesn't ever want to know about this kind of
conflict. Some users in some merging tasks will want a strict mode,
where they are not expecting any double-deletes and would like it
flagged if it happens.

Somehow I want to introduce a "conflict rules" framework, not
necessarily accessible through the "svn" UI, but within the
implementation and APIs. The rules will be a read-only context passed in
from the client that is like a table saying "delete-delete is to be a
silent delete, add-add is to be flagged as a conflict, edit-rename is to
be a silent edit of the renamed file, ...".

Then it will be easy for people to write a client that behaves more or
less strictly without having to first rev all our APIs.

And a second issue with "delete + delete": in a merge, we have to
consider whether the two things being deleted were identical. I haven't
yet tried to expand the rules to take account of that.

I *think* the same considerations also apply to all the other cases that
involve a delete during a merge.

>            |           | 
> -----------+-----------+-----------------------------------------------------
>            |           | 
>       (line-wise)      | 
>  delete    | move-away | (is not a tree-conflict)
>  move-away | delete    | 
>            |           | 
> -----------+-----------+-----------------------------------------------------
>            |           | 
>       (line-wise)      | 
>  delete    | edit      | theirs, mine 
>  edit      | delete    | 
>            |           | 
> -----------+-----------+-----------------------------------------------------
>            |           | 
>  move-away | edit      | mine, both, merge-here, merge-there,
>            |           | merge-there-keep-here, merge-there-merge-here
>            |           | [1] [2]
>            |           |           grain of salt ---> . 

Heh!

But "merge-there" is surely the almost universal wish, just like "delete
+ delete -> delete" and "add + add -> add" when the two adds are
identical are almost universal wishes.

"merge-here" just means the same as "theirs", doesn't it? We need to
keep options called "mine" and "theirs" in every menu, because they
always make sense, even if we annotate them with extra description such
as "theirs (undo the move and merge the incoming edit here)".

What is "merge-there-keep-here" for? I can't think of a realistic use
case.

What is "merge-there-merge-here" for? In the case of a copy rather than
a move, then yes I can see users wanting the incoming change applied to
both copies, but for a move?

Ugh. Moves and copies get very complex. Partly because any move can have
an edit as well. Partly because the destination location might also be a
location from where something was moved away at the same time (perhaps
also being in conflict, in which case we'd require that it be resolved
first ... unless it's a cycle of moves like A>B, B>A or A>B, B>C, C>A in
which case they might have to be resolved all at the same time).
Seriously, the design has to cope with not falling over in these cases,
though we certainly don't have to give special friendly options, just so
long as there is some way for the user to resolve it.

> -----------+-----------+-----------------------------------------------------
>            |           | 
>  edit      | move-away | theirs, both, merge-here, merge-there,
>            |           | merge-there-keep-here, merge-there-merge-here
>            |           | [1] [2]
>            |           |           grain of salt ---> . 
> -----------+-----------+-----------------------------------------------------
>            |           | 
>  move-away | move-away | theirs, mine, both, merge-at-theirs, merge-at-mine,
>            |           | merge-at-theirs-keep-mine, merge-at-mine-keep-theirs,
>            |           | merge-at-both
>            |           | (not a tree-conflict when move targets match)
>            |           | [1] [2]
>            |           |           grain of salt ---> . 

Ugh. This case gets even more complex.
> -----------+-----------+-----------------------------------------------------
> 

We'll need to define copies as well :-/

>  
> Where the --accept= options mean:
> 
>                   mine  Enforce my change, make sure theirs is lost/undone.
>                 theirs  Agree to theirs, make sure mine is lost.
>            move-theirs  Keep both additions, but move theirs to another path.
>              move-mine  Keep both additions, but move mine to another path.
>                   both  Keep both new move-targets (one becomes a simple "A +").
>            merge-there  Apply the edit to the moved-away path.
>             merge-here  Discard the move-away, but still merge text/prop mods.
>  merge-there-keep-here  Apply text/prop mods to the moved-away path, but
>                         keep an unmerged copy at this path.
> merge-there-merge-here  Apply text/prop mods to the moved-away path, and
>                         keep another, also merged, copy at this path.
>        merge-at-theirs  Apply the edits on my moved-away path to their moved-
>                         away path, discard my move.
>          merge-at-mine  Apply the edits on their moved-away path to my moved-
>                         away path, discard their move.
> merge-at-theirs-keep-mine Apply the edits on my moved-away path to their moved-
>                         away path, but keep my move-away with only *its* edits.
> merge-at-mine-keep-theirs Apply the edits on their moved-away path to my moved-
>                         away path, but keep their move-away with only *its*
>                         edits.
>          merge-at-both  Apply all edits to all moved-away paths and keep them.
> 
> [1] I am not sure whether resolution should only care about *this* path.
>     Some of the resolution options suggest that we would undo the add-part
>     of a move-away, at the *other* path, or apply edits to the *other* path.
>     While it would be nicer if we could get away with only resolving *this*
>     path, users might get annoyed with having to do two steps to get what
>     they wanted (i.e. manually modify the other path to the desired result)
>     when the resolver had all the information there and could have just done
>     the job automatically.
> 
> [2] The merge-* options indicate a mixture of text/prop mods with tree
>     conflicts. They say whether to keep the respective sides' text/prop mods
>     when resolving the tree-conflict. These options would entail that we
>     have to run an apply-textdelta-ish code that can introduce new text/prop
>     conflicts.
>     Should we really go there? What are the alternatives? We won't get away
>     without defining what happens to text/prop mods at two different paths
>     that are related via moves.
>     ### Analogy to copies?
> 
> </future>
> 

- Julian


Re: [RFC] v2 Tree conflict resolver spec.

Posted by Neels J Hofmeyr <ne...@elego.de>.
Hi TC folks,

I tried to put the tree-conflicts design information so far into tables.
Find the result attached: a tc-cheatsheet up for discussion. If it gets
initial approval I'll check it into notes/tree-conflicts/ so we can edit.


(replying to Julian...)

> On Wed, 2010-02-10, Neels J Hofmeyr wrote:
>> Julian Foad wrote:
>>> Yup. And with merge, --accept=mine should be identical to 'svn revert'.
>> Hey, wait a minute. That's not true when there were local modifications
>> before the merge. I see it like this:
> 
> Oh yes. Sorry for the confusion. I forgot about local mods at the moment
> I was writing that.

Phew :)
I thought I had lost it for a moment. Thanks.

[...]
[storing 'theirs' in TC-info during merge]
>> That's simple enough for a two-URL merge. But for a number of partitioned
>> revision ranges (e.g. with a --reintegrate), that may be a problem. What if
>> only the third or fiftieth revision range that is merged causes a tree
>> conflict? Does merge combine them before passing the resulting changes to
>> the merge_editor? Either case, recording or indicating those changes in
>> tree-conflict information is a tricky question on its own...
> 
> I have some thoughts on this. Later.

Heh, keep the suspense up, right until the happy end (I hope) ;)

~Neels

Re: [RFC] v2 Tree conflict resolver spec.

Posted by Julian Foad <ju...@btopenworld.com>.
On Wed, 2010-02-10, Neels J Hofmeyr wrote:
> Julian Foad wrote:
> > Yup. And with merge, --accept=mine should be identical to 'svn revert'.
> 
> Hey, wait a minute. That's not true when there were local modifications
> before the merge. I see it like this:

Oh yes. Sorry for the confusion. I forgot about local mods at the moment
I was writing that.

> Merge tree-conflicts, when they are encountered, should change neither the
> checked-out nor the check-in states other than flagging a conflict and
> storing some info with it.
> 
> --accept=mine should then simply remove the conflict marker and done.
> 
> --accept=theirs should be identical to 'revert' plus pulling in 'their'
> changes into the check-in state. In other words, --accept=theirs should
> reset the check-in state to the incoming action. The info stored with the
> conflict must somehow convey the incoming action. Oh my, *that's* the hard
> part ;)
> 
> Or am I twisting things now?

Sounds totally right.

> I am thinking incrementally with merge. Local mods are allowed to exist
> before the merge, and I want to be able to go back to exactly those with
> --accept=mine.

Yup. Definitely.

> That's simple enough for a two-URL merge. But for a number of partitioned
> revision ranges (e.g. with a --reintegrate), that may be a problem. What if
> only the third or fiftieth revision range that is merged causes a tree
> conflict? Does merge combine them before passing the resulting changes to
> the merge_editor? Either case, recording or indicating those changes in
> tree-conflict information is a tricky question on its own...

I have some thoughts on this. Later.

> Have I left orbit?
> 
> Still, I think it's quite insane to equal --accept=mine with a *revert*, of
> all things :) Revert *loses* mine, which is dangerous. Right?

Yup, sorry for writing that.

- Julian


Re: [RFC] v2 Tree conflict resolver spec.

Posted by Neels J Hofmeyr <ne...@elego.de>.
Julian Foad wrote:
> Yup. And with merge, --accept=mine should be identical to 'svn revert'.

Hey, wait a minute. That's not true when there were local modifications
before the merge. I see it like this:

Merge tree-conflicts, when they are encountered, should change neither the
checked-out nor the check-in states other than flagging a conflict and
storing some info with it.

--accept=mine should then simply remove the conflict marker and done.

--accept=theirs should be identical to 'revert' plus pulling in 'their'
changes into the check-in state. In other words, --accept=theirs should
reset the check-in state to the incoming action. The info stored with the
conflict must somehow convey the incoming action. Oh my, *that's* the hard
part ;)

Or am I twisting things now?

I am thinking incrementally with merge. Local mods are allowed to exist
before the merge, and I want to be able to go back to exactly those with
--accept=mine.

That's simple enough for a two-URL merge. But for a number of partitioned
revision ranges (e.g. with a --reintegrate), that may be a problem. What if
only the third or fiftieth revision range that is merged causes a tree
conflict? Does merge combine them before passing the resulting changes to
the merge_editor? Either case, recording or indicating those changes in
tree-conflict information is a tricky question on its own...

Have I left orbit?

Still, I think it's quite insane to equal --accept=mine with a *revert*, of
all things :) Revert *loses* mine, which is dangerous. Right?

~Neels


Re: [RFC] v2 Tree conflict resolver spec.

Posted by Julian Foad <ju...@wandisco.com>.
Just a few comments from me on the first half of Neels' response to this
draft.

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 )

I assumed Daniel meant, "The hard part is figuring out what state WE
WANT the WC to be in ...".

> > 
> > 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.

Yup. And with merge, --accept=mine should be identical to 'svn revert'.

Those two sentences state a fundamental property of the behaviour that
we want. We should write them in the document as a hard requirement.

> > 
> > ### 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.

Don't worry about physical size - it's reasonable to expect a user to
have enough free disk space to store another copy of part of their tree,
and it's easy to disable or modify that behaviour afterwards if somebody
really doesn't want it.

The tricky bit there is just how to store and how to keep track of and
how to access that data.

Let's not go there, at the moment, because it's secondary - it's no use
at all until the rest of this RFC is sorted out, whereas the rest of
this RFC IS useful without having "theirs" stored locally after a
conflict on merge.

[...]

> > 
> > Contents
> > =========
> > Problem definition
> > Requirements
> > Terminology
> > Use cases update/switch
> > Use cases merge
> > API changes
> > User interface

[...]

> > 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.)

Terminology is crucial. We'll never understand each other without
agreeing on the terminology.

Would it be OK with you if we just don't use the WC-NG DB terms here? I
don't believe they are well enough understood by most of us, and I don't
believe they are relevant at this level of specification. Specifically,
making a distinction between WORKING and ACTUAL is unneccessary in
defining the conflict behaviour, although of course it will be necessary
in *implementing* any parts of that behaviour that are right down in
WC-NG. From the user's point of view, which is a useful point of view
all the way down into the code paths that deal with tree conflicts,
there are only two trees of interest:

  * what I checked out (or last updated to) - which is a mixed-revision
copy of parts of the repository;

  * what I expect to check in - which is the versioned files and dirs on
my disk, plus "svn propget" to see props.

- Julian


Re: [RFC] v2 Tree conflict resolver spec.

Posted by Neels J Hofmeyr <ne...@elego.de>.
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@rev
  Source right: (kind) URL@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