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.
Thanks,
Paul
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2197303
Received on 2009-05-11 23:01:32 CEST