On Wed, Aug 05, 2009 at 10:46:44PM +0200, Daniel Näslund wrote:
> Hi!
> [Sorry for not putting the shell script as an attachment. Could not
> create a new thread with mutt.]
>
> I'm getting a conflict when using svn patch with the shell script
> below. According to the issue all lines after the empty line should be
> ignored. Did I do something wrong?
I wrote the issue description from human memory so it might not
be entirely accurate. It may well be that not all lines are ignored.
I remember that one way to reproduce the problem was applying your
patch committed in r38568 to a working copy which pre-dates r38568.
Your patch was mangled by tigris so that it contained empty lines
of context. When applying the mangled patch with 'svn patch', application
would succeed but some hunks were (silently) applied incompletely or
not at all.
> My explanation to the behavior is that svn patch in
> subversion/libsvn_client/patch.c (match_hunk) only compares hunk lines
> that begin with '-' or ' '. I must add a ' ' if the hunk line does not
> begin with one.
Yes, the parser right now assumes that every line which is part
of a hunk starts with '+', '-', or ' '.
This works for correctly formatted diffs as output by 'svn diff'
and 'diff', but not for mangled diffs.
> My first ideas briefly (not implemented):
> * subversion/libsvn_diff/parse_diff.c
> (svn_diff__parse_next_hunk): Allow empty lines to be part of hunk.
> Should use some library function.
>
> * subversion/libsvn_client/patch.c
> (apply_one_hunk): Allow for applying diffs with fuzz. Issue 3460.
Are you planning on addressing both issues in a single patch?
I'd prefer separate patches for each.
> * subversion/libsvn_client/patch.c
> (match_hunk): Right now it demands that hunk lines begin with ' ' or
> '-' and that the hunk line is one character longer than the target line.
> That must be fixed.
>
> Am I on the right track?
Yes.
>
> My idea for allowing empty lines:
>
> Index: subversion/libsvn_diff/parse-diff.c
> ===================================================================
> --- subversion/libsvn_diff/parse-diff.c (revision 38568)
> +++ subversion/libsvn_diff/parse-diff.c (arbetskopia)
> @@ -358,7 +358,10 @@
> }
>
> c = line->data[0];
> - if (c == ' ' || c == '-' || c == '+')
> + if (c == ' ' || c == '-' || c == '+'
> + || (line->len == 1 && c == '\n')
> + || (line->len == 1 && c == '\r')
> + || (line->len == 2 && c == '\r' && line->data[1] == '\n'))
> hunk_seen = TRUE;
> else
I tried something like that briefly, too. When you run it you will
find that the patch_tests.py tests fail, because as you said the
assumptions about the hunk line syntax actually extend into
libsvn_client/patch.c.
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.
Stefan
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2380804
Received on 2009-08-06 14:11:07 CEST