[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] svnadmin load will not import dump with windows newline character in svn:mergeinfo

From: B. Smith-Mannschott <bsmith.occs_at_gmail.com>
Date: Mon, 11 May 2009 07:31:16 +0200

> On May 8, 2009 3:23 PM, "John Skopis" <jskopis_at_backstopsolutions.com> wrote:
>
> Hello,
>
> svnadmin load fails while importing a dump that contains a windows newline
> in svn:mergeinfo prop. I have not done extensive testing on this patch, but
> it should work (in that it doesn't segfault when I attempt to import a
> revision with \r\n in mergeinfo). Be advised I am not actually a developer.
>
> Thanks,
> John Skopis
> Systems Administration
>
>
> [[[
> * subversion/libsvn_subr/mergeinfo.c
> (parse_revlist): Ignore windows newlines in svn:mergeinfo
> ]]]
>
> Index: subversion/libsvn_subr/mergeinfo.c
> ===================================================================
> --- subversion/libsvn_subr/mergeinfo.c (revision 37647)
> +++ subversion/libsvn_subr/mergeinfo.c (working copy)
> @@ -402,7 +403,7 @@
> svn_revnum_t firstrev;
>
> SVN_ERR(svn_revnum_parse(&firstrev, curr, &curr));
> - if (*curr != '-' && *curr != '\n' && *curr != ',' && *curr != '*'
> + if (*curr != '-' && *curr != '\n' && *curr != ',' && *curr != '*' &&
> *curr != '\r'
> && curr != end)
> return svn_error_createf(SVN_ERR_MERGEINFO_PARSE_ERROR, NULL,
> _("Invalid character '%c' found in revision
> "
> @@ -430,8 +431,10 @@
> mrange->end = secondrev;
> }
>
> - if (*curr == '\n' || curr == end)
> + if (*curr == '\r' || *curr == '\n' || curr == end)
> {
> + if ( *curr == '\r' )
> + curr++;
> APR_ARRAY_PUSH(revlist, svn_merge_range_t *) = mrange;
> *input = curr;
> return SVN_NO_ERROR;
> @@ -445,10 +448,10 @@
> {
> mrange->inheritable = FALSE;
> curr++;
> - if (*curr == ',' || *curr == '\n' || curr == end)
> + if (*curr == ',' || *curr == '\n' || *curr == '\r' || curr == end
> )
> {
> APR_ARRAY_PUSH(revlist, svn_merge_range_t *) = mrange;
> - if (*curr == ',')
> + if (*curr == ',' || *curr == '\r' )
> {
> curr++;
> }
>

On Sun, May 10, 2009 at 22:32, David Glasser <glasser_at_davidglasser.net> wrote:
> Why is there a windows newline in your dump file? Svn dumps are binary
> files, not text.
>
> Now, maybe the svn:mergeinfo property in general should allow windows
> newlines (which is what your patch does), but why?

No, I don't believe that svn:mergeinfo should allow windows newlines.
In fact, the fixes for issues 1796 and 3313 conspired to make
Subversion 1.6 very particular about the newline cleanliness of its
internal (i.e. svn:*) properties.

http://subversion.tigris.org/issues/show_bug.cgi?id=1796
http://subversion.tigris.org/issues/show_bug.cgi?id=3313

This new-found strictness lead to issue 3404 (svnload fails on ^M),
for which there is an as yet unreviewed patch linked to from the
issue.

http://subversion.tigris.org/issues/show_bug.cgi?id=3404

In any event, it appears that Subversion deliberately uses only \n
internally. some other parts of the code may well depend on this. I
don't think it would be advisable to relax the requirements, as above,
without first investigating this.

// Ben

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2184013
Received on 2009-05-11 07:31:33 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.