You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Stefan Sperling <st...@elego.de> on 2009/08/28 14:41:13 UTC
Re: reducing code bloat by removing svnpatch? (except unidiff)
On Wed, Aug 26, 2009 at 05:42:30PM +0200, Bert Huijben wrote:
> > Here is what diffstat has to say about this diff:
> > include/svn_client.h | 67 -
> > include/svn_wc.h | 39
> > libsvn_client/client.h | 11
> > libsvn_client/deprecated.c | 50 -
> > libsvn_client/diff.c | 107 --
> > libsvn_client/merge.c | 3
> > libsvn_client/patch.c | 1744 ----------------------------------
> > ---------
> > libsvn_client/repos_diff.c | 479 -----------
> > libsvn_wc/deprecated.c | 3
> > libsvn_wc/diff.c | 1064 --------------------------
> > svn/diff-cmd.c | 10
> > svn/main.c | 28
> > svn/patch-cmd.c | 4
> > tests/cmdline/patch_tests.py | 267 ------
> > 14 files changed, 50 insertions(+), 3826 deletions(-)
> >
> > Thoughts? Comments? Should we remove this?
> > If not, ideas about how to reduce the bloat?
>
> As noted earlier, we could use skels instead of a reimplementation of the
> svn protocol. Other suggestions include implementing an editor driver
> instead of direct calls into libsvn_wc.
>
> In its current state I think it is a big code bloat that will be hard to
> maintain and I would be +1 on disabling until it is better maintainable.
Any more opinions on this?
Are some people planning to review, or still busy reviewing,
the implementation to make their judgement, and just need some time?
Please post a short note if you are at all interested in this discussion,
otherwise I will assume that people don't care, and apply the patch in
a few days.
Stefan
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2388293
Re: reducing code bloat by removing svnpatch? (except unidiff)
Posted by Stefan Sperling <st...@elego.de>.
On Sat, Aug 29, 2009 at 07:01:08AM +0200, Daniel Näslund wrote:
> I've been doing some small contributions to the code for a couple of
> weeks. It would be fun to help out with something fresh and learn a bit
> about the design process when creating a new feature.
>
> With the proper guidance (Bert, stsp?) I would be interrested to take on
> this! If it's manageable for a person with my level of knowledge of the
> code base and programming skills, it's up to others to judge.
The unidiff application code already does adds and deletes (though
adds are broken right now), so to a really small extent it is already
doing things needed for git diff extensions.
It should not be too hard to extend the diff parser and the application
logic to properly use git diff extensions. Since you're already working
on bug fixes for that code anyway, I guess this would be a nice long-term
project you could help with. And you don't have to do all of it on your
own, we can work on it together. We can take it slow, milestone 1.8 or
later, there's absolutely no need to rush.
Let's take it step-by-step and get the known bugs fixed, first :)
I'd like to get those fixed by 1.7.0.
Stefan
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2388690
Re: reducing code bloat by removing svnpatch? (except unidiff)
Posted by Daniel Näslund <da...@longitudo.com>.
On Fri, Aug 28, 2009 at 07:46:30PM -0500, Ben Collins-Sussman wrote:
> Comment from the peanut gallery:
>
> Git has apparently done a fantastic job of 'extending' the standard
> diff/patch format with backward-compatible metadata that modern
> version control systems need. Mercurial has essentially standardized
> around this format as well. (Everyone in the hg world runs 'hg diff
> --git' semi-automatically now, and the --git option may soon become
> the default.) I hear rumors that bzr is now supporting it too
> (kfogel?).
>
> A long, long time ago we had a mailing list in which different SCM
> people came together to discuss a "universal format for representing
> changesets". The list ended up being a failure, but N years later it
> seems like at least the distributed version control world *has*
> finally standardized on a common format. If we want subversion to
> remain relevant/useful/interoperable for the future, we should
> probably strongly consider rewriting this whole feature to use the new
> defacto standard.
>
> This is not to trivialize all the work that went in already; but in
> hindsight, it may not be the best overall product strategy. And if
> we're busy questioning whether the existing code is even maintainable,
> all the more reason to consider a fresh start on this problem.
I've been doing some small contributions to the code for a couple of
weeks. It would be fun to help out with something fresh and learn a bit
about the design process when creating a new feature.
With the proper guidance (Bert, stsp?) I would be interrested to take on
this! If it's manageable for a person with my level of knowledge of the
code base and programming skills, it's up to others to judge.
/Daniel
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2388446
Re: reducing code bloat by removing svnpatch? (except unidiff)
Posted by Stefan Sperling <st...@elego.de>.
On Sun, Aug 30, 2009 at 09:09:14PM +0100, Stefan Sperling wrote:
> On Sat, Aug 29, 2009 at 09:54:25AM -0500, Augie Fackler wrote:
> > On Aug 28, 2009, at 8:03 PM, Mark Phippard wrote:
> >
> > > I was going to suggest the same thing for the same reasons. I tried
> > > to find documentation and details on this format but couldn't. Which
> > > is why I did not comment.
> >
> > Documentation for the format is pretty sparse. Here's an overview (now
> > that I've written this out, it might be more like "everything you need
> > to know to support this format other than specifics of how base85
> > works"):
>
> Wow, thanks!
>
> Using this format sounds like a good plan to me, too.
>
> So I guess we have a better idea of how this should be done,
> and at least one person is motivated to work on it.
>
> I guess I can apply the diff so?
See r39070.
include/private/svn_patch.h | 66 -
include/svn_client.h | 67 -
include/svn_wc.h | 39
libsvn_client/client.h | 11
libsvn_client/deprecated.c | 50 -
libsvn_client/diff.c | 107 --
libsvn_client/merge.c | 3
libsvn_client/patch.c | 1744 -------------------------------------------
libsvn_client/repos_diff.c | 481 -----------
libsvn_subr/patch.c | 538 -------------
libsvn_wc/deprecated.c | 3
libsvn_wc/diff.c | 1081 +-------------------------
libsvn_wc/patch.c | 441 ----------
svn/cl.h | 1
svn/diff-cmd.c | 10
svn/main.c | 32
svn/patch-cmd.c | 4
tests/cmdline/diff_tests.py | 404 ---------
tests/cmdline/patch_tests.py | 266 ------
19 files changed, 69 insertions(+), 5279 deletions(-)
Stefan
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2390251
Re: reducing code bloat by removing svnpatch? (except unidiff)
Posted by Stefan Sperling <st...@elego.de>.
On Sat, Aug 29, 2009 at 09:54:25AM -0500, Augie Fackler wrote:
> On Aug 28, 2009, at 8:03 PM, Mark Phippard wrote:
>
> > I was going to suggest the same thing for the same reasons. I tried
> > to find documentation and details on this format but couldn't. Which
> > is why I did not comment.
>
> Documentation for the format is pretty sparse. Here's an overview (now
> that I've written this out, it might be more like "everything you need
> to know to support this format other than specifics of how base85
> works"):
Wow, thanks!
Using this format sounds like a good plan to me, too.
So I guess we have a better idea of how this should be done,
and at least one person is motivated to work on it.
I guess I can apply the diff so?
Stefan
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2388685
Re: reducing code bloat by removing svnpatch? (except unidiff)
Posted by Julian Foad <ju...@btopenworld.com>.
On Sat, 2009-08-29, Augie Fackler wrote:
[re. Git's patch format]
> Documentation for the format is pretty sparse. Here's an overview (now
> that I've written this out, it might be more like "everything you need
> to know to support this format other than specifics of how base85
> works"):
[...]
Thanks, Augie, that's great. I've committed the body of your email as
<notes/svnpatch/svnpatch-git.txt>. Hope that's OK :-)
- Julian
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2390139
Re: reducing code bloat by removing svnpatch? (except unidiff)
Posted by Augie Fackler <li...@durin42.com>.
On Aug 28, 2009, at 8:03 PM, Mark Phippard wrote:
> I was going to suggest the same thing for the same reasons. I tried
> to find documentation and details on this format but couldn't. Which
> is why I did not comment.
Documentation for the format is pretty sparse. Here's an overview (now
that I've written this out, it might be more like "everything you need
to know to support this format other than specifics of how base85
works"):
Copies and renames are both handled like this:
diff --git a/{oldname} b/{newname}
copy from {oldname}
copy to {newname}
{normal unidiff data here if there was a difference, only for how new
is different from old}
(naturally, with copy instead of rename if that was the case}
New files:
diff --git a/alpha b/alpha
new file mode 100644
--- /dev/null
+++ b/alpha
@@ -0,0 +1,1 @@
+alpha
(the longer file mode is used for representing symlinks, they come in
with mode 120000, and the contents are listed as *just* the path of
the symlink, that is, svn's symlink format without the leading 'link ')
Removals:
diff --git a/README b/README
deleted file mode 100644
--- a/README
+++ /dev/null
@@ -1,77 +0,0 @@
{pile of unidiff data}
File mode changes:
diff --git a/README b/README
old mode 100644
new mode 100755
(I think it's normal for patch implementations to not care about the
old mode, and just apply the new one blindly - the old mode I believe
is more for human consumption or reverse-patching than anything else.
Also, you don't really ever see any modes other than these two - the
first is just "not executable", the second is executable. I've never
*seen* any other modes for regular files).
Binaries (this is for literals, which I believe is the new binary in
its entirety. I think there's a delta format which Mercurial does not
support, per a "TODO: deltas" in the code):
index {old hash}..{new hash}
GIT binary patch
literal {file length}
{base85-encoded zlib-compressed data}
The hashes are either the nullid (40 0's) if there was (or is) no
text, and are
sha1('blob %d\0%s' % (len(text), text, )) (using python syntax).
The base85 encoding seems to use a leading z to indicate this isn't
the last line of data, and a leading e to indicate this is the last
line of data. There's more to base85 than that, and I'd gladly take
some time to write down how it works for those interested in
reimplementing it. It appears at first glance to be the part that
requries the most detailed documentation.
Supporting binary deltas is something that can obviously come later,
since Mercurial doesn't appear to implement it at all right now. Note
that with binary literals it does not appear that patch -R is
possible. That's really a minor limitation, since you're never using
this on non-version-controlled files anyway.
It should be possible to add your own hunk types to this format (once
the base format is supported) that would allow you to transmit
property modifications inline as well (other than symlink and
executable, which are already handled sanely).
For those interested in some source code (GPLv2):
http://mercurial.selenic.com/hg/hg/file/ac02b43bc08a/mercurial/patch.py#l195
http://mercurial.selenic.com/hg/hg/file/ac02b43bc08a/mercurial/patch.py#l710
http://mercurial.selenic.com/hg/hg/file/ac02b43bc08a/mercurial/pure/base85.py
I'm a big fan of nonproliferation of custom diff formats, so it'd be
really great if this could work out. I just don't have the time or
enough motivation to write the implementation for svn. I hope the
contents of this email helps!
> ISTR glasser suggested this recently too.
>
> Good thing is that all the unidiff work done recently would be very
> relevant for this too.
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2388540
Re: reducing code bloat by removing svnpatch? (except unidiff)
Posted by Mark Phippard <ma...@gmail.com>.
On Aug 28, 2009, at 8:46 PM, Ben Collins-Sussman <sussman@red-
bean.com> wrote:
> Comment from the peanut gallery:
>
> Git has apparently done a fantastic job of 'extending' the standard
> diff/patch format with backward-compatible metadata that modern
> version control systems need. Mercurial has essentially standardized
> around this format as well. (Everyone in the hg world runs 'hg diff
> --git' semi-automatically now, and the --git option may soon become
> the default.) I hear rumors that bzr is now supporting it too
> (kfogel?).
>
> A long, long time ago we had a mailing list in which different SCM
> people came together to discuss a "universal format for representing
> changesets". The list ended up being a failure, but N years later it
> seems like at least the distributed version control world *has*
> finally standardized on a common format. If we want subversion to
> remain relevant/useful/interoperable for the future, we should
> probably strongly consider rewriting this whole feature to use the new
> defacto standard.
>
> This is not to trivialize all the work that went in already; but in
> hindsight, it may not be the best overall product strategy. And if
> we're busy questioning whether the existing code is even maintainable,
> all the more reason to consider a fresh start on this problem.
>
+1
I was going to suggest the same thing for the same reasons. I tried
to find documentation and details on this format but couldn't. Which
is why I did not comment.
ISTR glasser suggested this recently too.
Good thing is that all the unidiff work done recently would be very
relevant for this too.
Mark
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2388430
Re: reducing code bloat by removing svnpatch? (except unidiff)
Posted by Ben Collins-Sussman <su...@red-bean.com>.
Comment from the peanut gallery:
Git has apparently done a fantastic job of 'extending' the standard
diff/patch format with backward-compatible metadata that modern
version control systems need. Mercurial has essentially standardized
around this format as well. (Everyone in the hg world runs 'hg diff
--git' semi-automatically now, and the --git option may soon become
the default.) I hear rumors that bzr is now supporting it too
(kfogel?).
A long, long time ago we had a mailing list in which different SCM
people came together to discuss a "universal format for representing
changesets". The list ended up being a failure, but N years later it
seems like at least the distributed version control world *has*
finally standardized on a common format. If we want subversion to
remain relevant/useful/interoperable for the future, we should
probably strongly consider rewriting this whole feature to use the new
defacto standard.
This is not to trivialize all the work that went in already; but in
hindsight, it may not be the best overall product strategy. And if
we're busy questioning whether the existing code is even maintainable,
all the more reason to consider a fresh start on this problem.
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2388429
Re: reducing code bloat by removing svnpatch? (except unidiff)
Posted by "C. Michael Pilato" <cm...@collab.net>.
Stefan Sperling wrote:
> On Fri, Aug 28, 2009 at 11:36:44AM -0400, C. Michael Pilato wrote:
>> Stefan Sperling wrote:
>>> On Fri, Aug 28, 2009 at 11:07:30AM -0400, C. Michael Pilato wrote:
>>>> Is the plan to remove the support, cut a new branch, and re-add the support
>>>> on that branch with a note about the need for refactoring?
>>> I guess we can just
>>> svn copy ^/trunk@rev-before-it-was-removed ^/branches/svnpatch
>>> as soon as someone wants to work on it again.
>>>
>>> Or they can just work on it on trunk.
>>>
>>> All the code of the old implementation would still be in the history,
>>> for reference.
>> I think I'd rather have the code more "visible" on a branch or something
>> than just "in the history for reference", just so there's that visual
>> reminder -- and the email one Hyrum will send us every six months when he
>> gets into a branch-purging mood :-). Branch before removing is fine, and +1
>> from me on removing the bloated code for now.
>
> I think that anyone working on this would want to start with a clean
> state anyway, rather than with what there is now. But I may be mistaken.
>
> So if you think a branch as a reminder is useful we can do that.
> But usually we use issues for this purpose. Would an issue also do,
> with milestone 1.7-consider?
There's a negligible chance that it'll be me working on re-adding this code,
so *shrug* whatever floats your boat.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2388360
Re: reducing code bloat by removing svnpatch? (except unidiff)
Posted by Stefan Sperling <st...@elego.de>.
On Fri, Aug 28, 2009 at 11:36:44AM -0400, C. Michael Pilato wrote:
> Stefan Sperling wrote:
> > On Fri, Aug 28, 2009 at 11:07:30AM -0400, C. Michael Pilato wrote:
> >> Is the plan to remove the support, cut a new branch, and re-add the support
> >> on that branch with a note about the need for refactoring?
> >
> > I guess we can just
> > svn copy ^/trunk@rev-before-it-was-removed ^/branches/svnpatch
> > as soon as someone wants to work on it again.
> >
> > Or they can just work on it on trunk.
> >
> > All the code of the old implementation would still be in the history,
> > for reference.
>
> I think I'd rather have the code more "visible" on a branch or something
> than just "in the history for reference", just so there's that visual
> reminder -- and the email one Hyrum will send us every six months when he
> gets into a branch-purging mood :-). Branch before removing is fine, and +1
> from me on removing the bloated code for now.
I think that anyone working on this would want to start with a clean
state anyway, rather than with what there is now. But I may be mistaken.
So if you think a branch as a reminder is useful we can do that.
But usually we use issues for this purpose. Would an issue also do,
with milestone 1.7-consider?
Stefan
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2388338
Re: reducing code bloat by removing svnpatch? (except unidiff)
Posted by "C. Michael Pilato" <cm...@collab.net>.
Stefan Sperling wrote:
> On Fri, Aug 28, 2009 at 11:07:30AM -0400, C. Michael Pilato wrote:
>> Is the plan to remove the support, cut a new branch, and re-add the support
>> on that branch with a note about the need for refactoring?
>
> I guess we can just
> svn copy ^/trunk@rev-before-it-was-removed ^/branches/svnpatch
> as soon as someone wants to work on it again.
>
> Or they can just work on it on trunk.
>
> All the code of the old implementation would still be in the history,
> for reference.
I think I'd rather have the code more "visible" on a branch or something
than just "in the history for reference", just so there's that visual
reminder -- and the email one Hyrum will send us every six months when he
gets into a branch-purging mood :-). Branch before removing is fine, and +1
from me on removing the bloated code for now.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2388312
Re: reducing code bloat by removing svnpatch? (except unidiff)
Posted by Stefan Sperling <st...@elego.de>.
On Fri, Aug 28, 2009 at 11:07:30AM -0400, C. Michael Pilato wrote:
> Is the plan to remove the support, cut a new branch, and re-add the support
> on that branch with a note about the need for refactoring?
I guess we can just
svn copy ^/trunk@rev-before-it-was-removed ^/branches/svnpatch
as soon as someone wants to work on it again.
Or they can just work on it on trunk.
All the code of the old implementation would still be in the history,
for reference.
Stefan
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2388309
Re: reducing code bloat by removing svnpatch? (except unidiff)
Posted by "C. Michael Pilato" <cm...@collab.net>.
Stefan Sperling wrote:
> On Wed, Aug 26, 2009 at 05:42:30PM +0200, Bert Huijben wrote:
>>> Here is what diffstat has to say about this diff:
>>> include/svn_client.h | 67 -
>>> include/svn_wc.h | 39
>>> libsvn_client/client.h | 11
>>> libsvn_client/deprecated.c | 50 -
>>> libsvn_client/diff.c | 107 --
>>> libsvn_client/merge.c | 3
>>> libsvn_client/patch.c | 1744 ----------------------------------
>>> ---------
>>> libsvn_client/repos_diff.c | 479 -----------
>>> libsvn_wc/deprecated.c | 3
>>> libsvn_wc/diff.c | 1064 --------------------------
>>> svn/diff-cmd.c | 10
>>> svn/main.c | 28
>>> svn/patch-cmd.c | 4
>>> tests/cmdline/patch_tests.py | 267 ------
>>> 14 files changed, 50 insertions(+), 3826 deletions(-)
>>>
>>> Thoughts? Comments? Should we remove this?
>>> If not, ideas about how to reduce the bloat?
>> As noted earlier, we could use skels instead of a reimplementation of the
>> svn protocol. Other suggestions include implementing an editor driver
>> instead of direct calls into libsvn_wc.
>>
>> In its current state I think it is a big code bloat that will be hard to
>> maintain and I would be +1 on disabling until it is better maintainable.
>
> Any more opinions on this?
>
> Are some people planning to review, or still busy reviewing,
> the implementation to make their judgement, and just need some time?
>
> Please post a short note if you are at all interested in this discussion,
> otherwise I will assume that people don't care, and apply the patch in
> a few days.
Is the plan to remove the support, cut a new branch, and re-add the support
on that branch with a note about the need for refactoring?
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2388302
Re: reducing code bloat by removing svnpatch? (except unidiff)
Posted by Stefan Sperling <st...@elego.de>.
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
Re: reducing code bloat by removing svnpatch? (except unidiff)
Posted by Julian Foad <ju...@btopenworld.com>.
On Fri, 2009-08-28 at 15:41 +0100, Stefan Sperling wrote:
> On Wed, Aug 26, 2009 at 05:42:30PM +0200, Bert Huijben wrote:
> > > Here is what diffstat has to say about this diff:
> > > include/svn_client.h | 67 -
> > > include/svn_wc.h | 39
> > > libsvn_client/client.h | 11
> > > libsvn_client/deprecated.c | 50 -
> > > libsvn_client/diff.c | 107 --
> > > libsvn_client/merge.c | 3
> > > libsvn_client/patch.c | 1744 ----------------------------------
> > > ---------
> > > libsvn_client/repos_diff.c | 479 -----------
> > > libsvn_wc/deprecated.c | 3
> > > libsvn_wc/diff.c | 1064 --------------------------
> > > svn/diff-cmd.c | 10
> > > svn/main.c | 28
> > > svn/patch-cmd.c | 4
> > > tests/cmdline/patch_tests.py | 267 ------
> > > 14 files changed, 50 insertions(+), 3826 deletions(-)
> > >
> > > Thoughts? Comments? Should we remove this?
> > > If not, ideas about how to reduce the bloat?
> >
> > As noted earlier, we could use skels instead of a reimplementation of the
> > svn protocol. Other suggestions include implementing an editor driver
> > instead of direct calls into libsvn_wc.
> >
> > In its current state I think it is a big code bloat that will be hard to
> > maintain and I would be +1 on disabling until it is better maintainable.
>
> Any more opinions on this?
>
> Are some people planning to review, or still busy reviewing,
> the implementation to make their judgement, and just need some time?
>
> Please post a short note if you are at all interested in this discussion,
> otherwise I will assume that people don't care, and apply the patch in
> a few days.
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.
- Julian
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2389715