On Tue, Sep 01, 2009 at 11:25:15AM +0100, Julian Foad wrote:
> On Fri, 2009-08-28 at 15:41 +0100, Stefan Sperling wrote:
> > Any more opinions on this?
>
> I am interested. My main observation is: It seems wrong that a whole lot
> of new code should be needed in libsvn_wc and libsvn_repos. These should
> already be producing diffs in a standard "Subversion-internal" form (the
> delta_editor API, I presume) for both "diff" and "merge" purposes. The
> translation of this internal form to/from "svnpatch" format should
> happen in libsvn_client or thereabouts. If the WC and repos diff
> layering is not already structured so this is possible it should become
> so. I'm hand-waving on details, of course: there are some kinds of info
> that need to be propagated for svnpatch that were not needed for
> human-readable diff or for merge, and vice versa, but overall the
> principle holds that a single Subversion-internal API should suffice.
>
> Unfortunately, "diff" and "merge" currently use rather different code
> paths. We really ought to be able to unify them to a large extent. Neels
> looked in to this and found it's not easy. (And the current "diff" code
> has some serious omissions, which was the reason for wanting to re-work
> it in the first place.) So really the exercise may have to start in
> re-working the current "diff" and making it share code and patterns with
> the "merge" code path before we can cleanly add "svnpatch" to it.
>
> The suggestion to use GIT's format instead of the current svnpatch
> format sounds +1, but my comment above applies in that case too.
My guess is that with the git format, addressing your layering concerns
should be trivial (though we won't rewrite "diff" for this, that's a
different building site, a little down the road towards the supermarket
at the corner, and once you've passed that its in the first left, I think.
If you see Neels dangling upside-down high up in the scaffold, waving,
you've found it.).
The unidiff parsing logic is already entirely in libsvn_diff (or rather,
will be, once we have fixed #3459, the first step of which was the stream
line transformation diff by dannas).
The patch application logic is entirely in libsvn_client and e.g.
already adds files if they did not exist before patch application
(though this has a bug right now, #3473, but in theory it works).
It calls from libsvn_client into libsvn_wc to do this (svn_wc_addX()
whatever current X is).
I guess that all other copy/move/whatever information encoded by the git
format can be implemented by having libsvn_client call into libsvn_wc, too.
Neat layering, no messing with editors. As far as I understand the editors
were only added to interpret the svn protocol, modeled after libsvn_ra_svn.
Stefan
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2389732
Received on 2009-09-01 13:27:02 CEST