[PATCH] issue 3459 - svn patch does not tolerate empty lines

On Fri, Aug 07, 2009 at 01:23:51PM +0100, Stefan Sperling wrote:
> > > Maybe libsvn_client should be shielded from the unidiff specifics
> > > entirely, and just get original and modified hunk lines from libsvn_diff
> > > with the extra '+', '-' etc. already removed.
> >
> > Involves changing svn_hunk_t to use some other classification than
> > original_text and modified_text? Beeing lazy I would prefer not having
> > to do that. :-)
>
> I'd still prefer a solution where libsvn_client conceptually
> ends up in a situation like:
>
> original_text = "foo"
> modified_text = "bar"
> latest_text = "foo"
>
> which would be much more easy to deal with instead of the current:
>
> original_text = "-foo"
> modified_text = "+bar"
> latest_text = "foo"
 
I was lucky. There was no need to change anything in svn_hunk_t. Just
removing the leading '+', '-' and ' ' was enough.
>
> Say we'd want to add a parser for context diffs (diff -c) at some point.
> That would be easier if libsvn_client was already shielded from the
> specifics of the unidiff format.

I agree.

> > Now I am able to patch the simple example given in the issue description
> > but when trying to create a file using this patch I get a conflict
> > (There is one empty line at the end of the patch) :
> >
> > Index: foo2
> > ===================================================================
> > --- foo2 (revision 0)
> > +++ foo2 (revision 0)
> > @@ -0,0 +1 @@
> > +test
> >
> > Is this an issue with the fuzz? If svn patch would allow at least one
> > line of fuzz then this patch would work?
> >
> > Or do I need a way to find out when an empty line is there for
> > readability (as the blank line before Property changes on ...) or when
> > the leading space has been chopped?
>
> There are two cases:
>
> 1) trailing empty line is part of the hunk's context
> 2) trailing empty line is part of the noise surrounding patches
> in the unidiff
>
> I guess we can assume that the number of lines of leading context
> in a hunk is equal to the number of lines of trailing context in
> a hunk. That should provide a way to resolve the ambiguity.

I used another method. Original_lines in svn_hunk_t is the number of
lines with leading ' ' or '-'. I read the number at the beginning of
each hunk and decrease for each ' ' or '+'. If an empty line is found -
it is added as long as the number of original_lines is not zero.

In subversion/test/libsvn_diff/parse-diff-tests.c I rewrote the hunk
header. According to me it was wrong, am I wrong?

Should I write a new test in subversion/tests/cmdline/patch-tests.py
with empty lines?

[[[
Fix issue 3459 - svn patch does not tolerate empty lines.

* subversion/libsvn_diff/parse-diff.c
  (set_hunk_text): Removes leading character before setting hunk text.
  (svn_diff__parse_next_hunk): Allow empty lines in hunk.

* subversion/tests/libsvn_diff/parse-diff-test.c
  (test_parse_unidiff): Fix changed representation of hunks.

* subversion/libsvn_client/patch.c
  (match_hunk, copy_hunk_text): Fix changed representation of hunks.
]]]

Mvh
Daniel