Hi Julian & Stefan!
Some comments inline. A new classfication at the end and some initial
thoughts about API changes. Early I know but I want to code, doing all
this thinking is boring :-).
Is it possible to just do resolution for files as a first stage? I mean
both spec and implementation.
On Wed, Dec 02, 2009 at 12:57:31PM +0000, Julian Foad wrote:
> Stefan Sperling wrote:
> What I meant was that, in this file "resolution.txt", I listed other
> possibilities for resolutions that might be useful, and we can document
> those somewhere (on a web site) for users to follow manually (e.g. "copy
> X to Y, revert X, apply the patch you saved in step 1").
>
> We can also provide interactive menu options for making svn do some of
> those things, if we want to.
>
> I didn't mean we should provide interactive menu options that just print
> a list of instructions.
Should have figured. It seemed a bit odd.
> > What should definitely be there is a 'postpone' option, meaning "leave
> > the conflict markers in place and leave the WC in this state, I'll deal
> > with it manually". But I don't think the option needs to explain anything
> > or suggest ways to go about solving the conflict. What if we get it
> > wrong and give wrong advice? Users won't be happy. We'll have to rely
> > on them to engage their brain instead.
>
> Yup. (Though "What if we get it wrong" is a dubious argument.)
Noted. And it's so easy to write. I like that option.
> > We might also want to provide an option to view the revision log of
> > any file or its parent directory from within the menu, so people can
> > dig in the history without having to open another terminal etc.
>
> Heh, sure we could, and we could make the log history navigable and ...
> but that's more of a UI task that TortoiseSvn and RapidSvn and VisualSvn
> can do well, and svn can never do well.
If I understood Bert correctly, we're supposed to be able to deal with
the conflicts even after the up/sw/merge has run. [1] No need to make
things to interactive then.
> > > Categories of conflicts
> > > ========================
> > > 1. Locale add, incoming add
> > > 2. Locale del, incoming del
> > > 3. Locale del, incoming edit
> > > 4. Locale edit, incoming del
>
> It's also worth thinking about moves as different from delete-and-add,
> although we are not yet ready to detect them properly.
No svn_wc_conflict_reason_t or svn_wc_conflict_action_t for that yet. I
will have to wait.
> > > 1. Locale add, incoming add
> > > ============================
[...]
> > > - I was doing roughly the same thing but the item is a bit different.
> > > -> merge the two items -> manual 2-way merge (or 3-way if both are
> > > w/hist and it's the same copy-from source) -> MERGE.
> >
> > Possible options after the merge (based on OpenBSD's sysmerge which
> > merges configuration files after a system update):
> >
> > - edit the merged file again
> > - install the merged file in WORKING
svn_wc_conflict_choose_merged?
> > - view a diff between a WORKING file and the merged file
> > - view a diff between a BASE file and the merged file
> > - re-do the merge
> > - view the merged file
> > - discard the merged file and go back to previous menu
When looking in svn_cl__conflict_handler() I get a lot of guidance on
how to write theese. A bit unsure about the 're-do the merge' option but
that's for later.
> > > THEIRS
> > > --------
> > > Replace <file> in WORKING with the new BASE that comes in during the
> > > update.
> > > *Problems*: We may need to undo a local move or copy operation with
> > > destination <file>.
> >
> > Let's ignore existing copies of the file for now.
Ok, that simplifies things.
> > We just need to offer an option to either keep the current WORKING
> > file in WORKING or put the new BASE file in WORKING. Even if the local
> > add was the destination of a copy, there are only those two options.
> >
> > We don't need to worry about destroying anything. Maybe keep an unversioned
> > backup of whichever file was overwritten.
Keep a backup... I will need to think more about that.
>
> > > THEIRS
> > > -------
> > > Delete the WORKING <file>. If the WORKING <file> has been moved to
> > > <moved> then delete <moved> too.
> >
> > No, let's not follow moves/copies for now. We can add that later.
Ok, eases the task.
New classification
===================
Moves and copies are not handled. If a file is deleted but it has a copy
or is moved no action is taken. We wait until wc-ng will provide true
renames for us. I may have misunderstood how subversion will keep track
of renames. I'm assuming it will be able to resolv both a local mv and
an incoming mv. Have I understood things correctly?
I've saying below that a lot of options is better fixed once we have
wc-ng? Is this the right thinking? I guess many (most?) tree conflicts
is about moves, like when refactoring in Java. I'm suggesting that we'd
not handle moves until wc-ng.
Could we have some generic APPLY-MY-MODS-ELSEWHERE option? For RENAME,
ELSEWHERE, MOVE-MY-MODS we need user input to know where to do a merge.
Aren't they much the same? Merging is hard, something that says 'merge
this onto that' would probably help. I know that the resolver is
supposed to be handled by users with a good grasp on subversion but it
can still be tricky.
If that was possible then stsp:s suggested OpenBSD sysmerge menu (it's
in the beginning of this mail) could be displayed afterwards.
Local add, incoming add
-------------------------
THEIRS: Put new BASE file in WORKING.
MINE: Keep current WORKING file.
RENAME-MINE:
Move WORKING file to <user-suggest>. Replace WORKING file with
the BASE-TARGET file.
MERGE Merge BASE file1 onto WORKING file2.
Local del, incoming del
-------------------------
THEIRS: Delete the file from WORKING and ACTUAL.
MINE: Keep current WORKING file.
RENAME: If renamed to two different names. Merge BASE-TARGET <moved>
onto WORKING <moved>.
### We need the user to tell us the add-half of a rename until
### wc-ng is here. Until then <moved> must be a user suggestion.
Local del, incoming edit
-------------------------
THEIRS: Replace the deleted WORKING file with edited BASE file.
MINE: Keep current WORKING file.
ELSEWHERE:
### wc-ng will automatically find a move. No need for this
### option?
Locale edit, incoming del
--------------------------
THEIRS: Delete the file from WORKING and ACTUAL.
MINE: Keep current WORKING file.
MOVE-MY-MODS:
### wc-ng will automatically find a move. No need for this
### option?
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()
Or are we supposed to call the conflict resolver after we have done the
whole operation?
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.
-- Daniel
[1] http://svn.haxx.se/dev/archive-2009-10/0816.shtml
