[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: John Skopis <jskopis_at_backstopsolutions.com>
Date: Mon, 11 May 2009 17:32:26 -0500

Paul, please excuse the top post.

More than likely they used the official 1.5.4 client as that was the latest and greatest available at the time (rev 40998 was the day after we upgraded the server to 1.5.4). I am leaning towards a propedit.


----- Original Message -----
From: Paul Burba <ptburba_at_gmail.com>
To: dev_at_subversion.tigris.org <dev_at_subversion.tigris.org>
Cc: Daniel Shahaf <d.s_at_daniel.shahaf.name>; John Skopis; B. Smith-Mannschott <bsmith.occs_at_gmail.com>; David Glasser <glasser_at_davidglasser.net>; senthil_at_collab.net <senthil_at_collab.net>
Sent: Mon May 11 16:01:14 2009
Subject: Re: [PATCH] svnadmin load will not import dump with windows newline character in svn:mergeinfo

On Mon, May 11, 2009 at 1:36 PM, David Glasser <glasser_at_davidglasser.net> wrote:
> On Mon, May 11, 2009 at 9:49 AM, Daniel Shahaf <d.s_at_daniel.shahaf.name> wrote:
>> John Skopis wrote on Mon, 11 May 2009 at 10:52 -0500:
>>> >-----Original Message-----
>>> >From: Daniel Shahaf [mailto:d.s_at_daniel.shahaf.name]
>>> >Sent: Monday, May 11, 2009 9:52 AM
>>> >To: B. Smith-Mannschott
>>> >Cc: David Glasser; John Skopis; dev_at_subversion.tigris.org
>>> >Subject: Re: [PATCH] svnadmin load will not import dump with windows
>>> >newline character in svn:mergeinfo
>>> >
>>> >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)?
>>> I just said that the patch below *does not* trigger segfault, implying
>>> that it "works", however correctly it works is more appropriate for
>>> this list to decide.
>> You said that it didn't segfault with the patch, I inferred that it did
>> segfault without.  I can call it a misunderstanding.
>>> It seems to me that no one is too happy about silently filtering out
>>> '\r' from svn:mergeinfo.
>>> I am not too happy that my svndump won't load.
>> 'svnadmin load' does allow loading properties with CRLFs, even though
>> you can't commit/svnsync them.  Per my other mail --- it seems that
>> svn:mergeinfo is special-cased somewhere (svn_mergeinfo_parse is called
>> and does its own validation).
> Haven't looked at this in a while, but I seem to recall that svnadmin
> load will renumber revs in mergeinfo if it's renumbering revs at all.
> I guess that's where this comes into play.
> --dave

Dave's recollection is correct. The fix for issue #3020
(http://subversion.tigris.org/issues/show_bug.cgi?id=3020) added
libsvn_repos/load.c:renumber_mergeinfo_revs() which is called from the
svn_repos_parse_fns2_t set_node_property function (Senthil - I'm ccing
you because you made this change).

Maybe the solution is to basically do what John's patch suggests, but
instead of simply tolerating \r\n in mergeinfo properties during
dump/load, actually canonicalize the line endings to \n. Is there any
reason not to do this?

>>> I was able to dump revs 1-40997, 40998, and 40998-head, hand edit
>>> 40998, import all the revs and dump the repo and reimport it (my issue
>>> has been resolved). I then created a patch, should some unfortunate
>>> user experience the same issue as me, and sent it upstream. The rest
>>> is up to you.

Hi John - Do you know how the mergeinfo in r40998 was created? Was it
the result of a merge or a propset/propedit or something else
entirely? Also, can you hazard a guess as to what version of
Subversion (or some other client tool) was used to make it?

If possible I'd like to figure out how the <CR> got in your repository
in the first place before making any fixes to dump/load.



Received on 2009-05-12 00:32:43 CEST

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