[RFC] Refactoring some merge code - the repos-diff editor

[Julian Foad <ju...@wandisco.com>]

I'm looking at merge code improvements, with the ultimate aim of
functional improvements.  My immediate aim is finding ways to
restructure parts of the code to make it simpler to understand and
easier to work with.

Right now I'm looking at rationalizing the way a repos-repos diff is
generated.  In any merge the data flow starts with
libsvn_client/repos_diff.c generating a diff between two repository
trees and driving merge.c through the "diff callbacks" mechanism.  The
same diff editor is also used for "svn diff" in the repos-repos case,
but whereas that's the only source of changes for a merge there are
other sources for "svn diff".

Here's a few things I'm planning to try, ordered from easy to hard:

  * The merge passes a WC path into the repos-repos diff editor.  (Diff
doesn't.)  Eliminate this, as it is irrelevant to a repos-repos diff.
It is only used as a convenience prefix on paths sent to the callbacks,
plus in one small place to adjust a notification for part of the new
local-move functionality.  I'm currently working on this; see email
thread "svn commit: r1161219 - Auto-resolve 'local move vs. incoming
edit'" <http://svn.haxx.se/dev/archive-2011-08/0586.shtml>.

  * Eliminate "WC props" and "entry props" from the prop-change diff, if
possible.  (Diff doesn't want to know about them; I need to check
whether merge does.)

  * Eliminate notification from the repos-repos diff editor.  Again,
only used by merge, not diff.  Notification basically duplicates some
information that the diff callbacks are supplying.  Bert advises that
this would be hard to do but good.  One complication I've noticed is
that 'absent' dirs/files are currently notified but there's no 'absent'
indicator in the diff callbacks mechanism.

  * Think about all the "skip this item" and "note that this item hit a
tree conflict" feedback mechanisms and see if there's a way to make them
less specific to merging and hopefully simpler. (It seems wrong for the
repos diff code to have parameters that tell it about a "tree conflict"
because it should not know anything about such a concept.)

I'll see how many of these particular ideas pan out.  This is more of a
heads-up that I'll be looking for any such opportunities.

- Julian

Re: [RFC] Refactoring some merge code - the repos-diff editor

[Julian Foad <ju...@wandisco.com>]

I (Julian Foad) wrote:
> I (Julian Foad) wrote:
> > I'm looking at merge code improvements, with the ultimate aim of
> > functional improvements.  My immediate aim is finding ways to
> > restructure parts of the code to make it simpler to understand and
> > easier to work with.
> > 
> > Right now I'm looking at rationalizing the way a repos-repos diff is
> > generated.  In any merge the data flow starts with
> > libsvn_client/repos_diff.c generating a diff between two repository
> > trees and driving merge.c through the "diff callbacks" mechanism.  The
> > same diff editor is also used for "svn diff" in the repos-repos case,
> > but whereas that's the only source of changes for a merge there are
> > other sources for "svn diff".
> > 
> > Here's a few things I'm planning to try, ordered from easy to hard:
> > 
> >   * The merge passes a WC path into the repos-repos diff editor.  (Diff
> > doesn't.)  Eliminate this, as it is irrelevant to a repos-repos diff.
> > It is only used as a convenience prefix on paths sent to the callbacks,
> > plus in one small place to adjust a notification for part of the new
> > local-move functionality.  I'm currently working on this; see email
> > thread "svn commit: r1161219 - Auto-resolve 'local move vs. incoming
> > edit'" <http://svn.haxx.se/dev/archive-2011-08/0586.shtml>.

Done in r1163238 and r1163296.

- Julian


> >   * Eliminate "WC props" and "entry props" from the prop-change diff, if
> > possible.  (Diff doesn't want to know about them; I need to check
> > whether merge does.)
> 
> Well, that was naïve.  Merge does want to know about entry-prop changes.
> 
> >   * Eliminate notification from the repos-repos diff editor.  Again,
> > only used by merge, not diff.  Notification basically duplicates some
> > information that the diff callbacks are supplying.  Bert advises that
> > this would be hard to do but good.  One complication I've noticed is
> > that 'absent' dirs/files are currently notified but there's no 'absent'
> > indicator in the diff callbacks mechanism.
> > 
> >   * Think about all the "skip this item" and "note that this item hit a
> > tree conflict" feedback mechanisms and see if there's a way to make them
> > less specific to merging and hopefully simpler. (It seems wrong for the
> > repos diff code to have parameters that tell it about a "tree conflict"
> > because it should not know anything about such a concept.)
> > 
> > I'll see how many of these particular ideas pan out.  This is more of a
> > heads-up that I'll be looking for any such opportunities.
> > 
> > - Julian

Re: [RFC] Refactoring some merge code - the repos-diff editor

[Julian Foad <ju...@wandisco.com>]

On Tue, 2011-08-30 at 12:46 +0100, Julian Foad wrote:
> I'm looking at merge code improvements, with the ultimate aim of
> functional improvements.  My immediate aim is finding ways to
> restructure parts of the code to make it simpler to understand and
> easier to work with.
> 
> Right now I'm looking at rationalizing the way a repos-repos diff is
> generated.  In any merge the data flow starts with
> libsvn_client/repos_diff.c generating a diff between two repository
> trees and driving merge.c through the "diff callbacks" mechanism.  The
> same diff editor is also used for "svn diff" in the repos-repos case,
> but whereas that's the only source of changes for a merge there are
> other sources for "svn diff".
> 
> Here's a few things I'm planning to try, ordered from easy to hard:
> 
>   * The merge passes a WC path into the repos-repos diff editor.  (Diff
> doesn't.)  Eliminate this, as it is irrelevant to a repos-repos diff.
> It is only used as a convenience prefix on paths sent to the callbacks,
> plus in one small place to adjust a notification for part of the new
> local-move functionality.  I'm currently working on this; see email
> thread "svn commit: r1161219 - Auto-resolve 'local move vs. incoming
> edit'" <http://svn.haxx.se/dev/archive-2011-08/0586.shtml>.
> 
>   * Eliminate "WC props" and "entry props" from the prop-change diff, if
> possible.  (Diff doesn't want to know about them; I need to check
> whether merge does.)

Well, that was naïve.  Merge does want to know about entry-prop changes.

>   * Eliminate notification from the repos-repos diff editor.  Again,
> only used by merge, not diff.  Notification basically duplicates some
> information that the diff callbacks are supplying.  Bert advises that
> this would be hard to do but good.  One complication I've noticed is
> that 'absent' dirs/files are currently notified but there's no 'absent'
> indicator in the diff callbacks mechanism.
> 
>   * Think about all the "skip this item" and "note that this item hit a
> tree conflict" feedback mechanisms and see if there's a way to make them
> less specific to merging and hopefully simpler. (It seems wrong for the
> repos diff code to have parameters that tell it about a "tree conflict"
> because it should not know anything about such a concept.)
> 
> I'll see how many of these particular ideas pan out.  This is more of a
> heads-up that I'll be looking for any such opportunities.
> 
> - Julian
> 
>