On Tue, May 12, 2009 at 2:32 PM, Daniel Shahaf <d.s_at_daniel.shahaf.name> wrote:
> Paul Burba wrote on Tue, 12 May 2009 at 13:49 -0400:
>> * Unlike mergeinfo with '\r', other versioned and unversioned props
>> are loaded intact, that is to say the '\r\n' are still present in the
>> loaded repos. My patch "fixes" the loaded mergeinfo props so there
>> are no '\r'. I could make loaded mergeinfo be treated the same way,
>> but I see no point in doing this!
>
> Consistency? Less special-cases logic?
Hi Daniel,
It would be consistent yes, but consistently wrong no? We don't want
'\r\n' in our svn:* properties right?
And AFAICT it would take *more* special case logic to preserve the
'\r\n' in loaded mergeinfo than simply fixing it. Sure, we can tweak
svn_parse_mergeinfo() as John suggested at the start of this thread,
but svn_parse_mergeinfo() is used elsewhere to *prevent* '\r\n' in
mergeinfo via propedit and propset, so we'd have to rev
svn_parse_mergeinfo() to add a "fix_eol" argument or something
similar. Not only that, but we'd have to keep track of where in the
loaded mergeinfo the offending '\r\n' where so we can put them back in
the name of "consistency" (keep in mind that we have no guarantee that
the prop is all '\n' or all '\r\n' it might be mixed). This would
also mean we couldn't simply use svn_mergeinfo_to_string() in
load.c:renumber_mergeinfo_revs() but would have to create a
specialized function to put the "\r\n"'s back in the right spots.
This seems a bit crazy to me :-)
> (The "remove CRs from versioned props" work is already done centrally for
> all props as part of #3404.)
You mean this patch by B. Smith-Mannschott?
http://svn.haxx.se/dev/archive-2009-04/0991.shtml
That work doesn't directly help here, it is for svnsync only. We
could make use of normalize_svn_string_eol_style if it was public, but
that's about it, unless I am missing something.
> Agreed, I don't see a reason against this fix.
> Though perhaps you should notify (on stderr) that you modify the data
> being loaded.
How's this look (see r5)?
[[[
<<< Started new transaction, based on original revision 1
* adding path : A ... done.
* adding path : A/B ... done.
* adding path : A/B/E ... done.
* adding path : A/B/E/alpha ... done.
* adding path : A/B/E/beta ... done.
* adding path : A/B/F ... done.
* adding path : A/B/lambda ... done.
* adding path : A/C ... done.
* adding path : A/D ... done.
* adding path : A/D/G ... done.
* adding path : A/D/G/pi ... done.
* adding path : A/D/G/rho ... done.
* adding path : A/D/G/tau ... done.
* adding path : A/D/H ... done.
* adding path : A/D/H/chi ... done.
* adding path : A/D/H/omega ... done.
* adding path : A/D/H/psi ... done.
* adding path : A/D/gamma ... done.
* adding path : A/mu ... done.
* adding path : iota ... done.
------- Committed revision 1 >>>
<<< Started new transaction, based on original revision 2
* adding path : A_COPY ...COPIED... done.
------- Committed revision 2 >>>
<<< Started new transaction, based on original revision 3
* adding path : A_COPY_2 ...COPIED... done.
------- Committed revision 3 >>>
<<< Started new transaction, based on original revision 4
* editing path : A/D/H/psi ... done.
------- Committed revision 4 >>>
<<< Started new transaction, based on original revision 5
* editing path : A_COPY ... removing '\r' from svn:mergeinfo ... done.
* editing path : A_COPY/D/H/psi ... done.
------- Committed revision 5 >>>
]]]
[[[
Make svnadmin load tolerate mergeinfo with "\r\n".
* subversion/libsvn_repos/load.c
(svn_subst.h): New include.
(parse_property_block): Add parse_baton argument. If necessary normalize
mergeinfo line endings to '\n' and print notification that this has
been done.
(svn_repos_parse_dumpstream2): Update call to parse_property_block().
]]]
Paul
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2220062
Received on 2009-05-12 21:58:23 CEST