What Ben said.
Also, I think the patch here is orthogonal to issue #3404 et al, since
John said something about a segfault. John, could you say where the
segfault is? Can you reproduce it without using svnadmin (by direct API
usage)?
B. Smith-Mannschott wrote on Mon, 11 May 2009 at 07:31 +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
>
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2191834
Received on 2009-05-11 16:52:33 CEST