Login | Register
My pages Projects Community openCollabNet

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

subversion
Discussion topic

Back to topic list

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

Author Daniel Shahaf <d dot s at daniel dot shahaf dot name>
Full name Daniel Shahaf <d dot s at daniel dot shahaf dot name>
Date 2009-05-12 22:37:35 PDT
Message Paul Burba wrote on Tue, 12 May 2009 at 15:57 -0400:
> On Tue, May 12, 2009 at 2:32 PM, Daniel Shahaf <d dot s at daniel dot shahaf dot 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_merg​einfo_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 :-)
>

You've convinced me. :-)

For the record, I thought we could just pass through the value
unmodified (at least when we aren't doing any renumbering at all);
however, it appears the logic doesn't take that shortcut explicitly,
so that's not possible.

> > 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 5
> * editing path : A_COPY ... removing '\r' from svn:mergeinfo ... done.
> * editing path : A_COPY/D/H/psi ... done.
>
> ------- Committed revision 5 >>>
> ]]]
>

+1

> [[[
> 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().
> ]]]
>

Looks good (though I haven't dived too deeply into the load logic).

I see an strstr(svn_string_t.data) --- which normally makes me ask what
happens when the svn_string_t's value contains NULs --- but I suppose
the answer (in this case) is that svn:mergeinfo cannot legitimately
contain NULs.

> Paul
>

Thanks,

Daniel

« Previous message in topic | 5 of 11 | Next message in topic »

Messages

Show all messages in topic

Re: [PATCH] svnadmin load will not import dump with windows newline character in svn:mergeinfo markphip Mark Phippard 2009-05-12 09:35:53 PDT
     Re: [PATCH] svnadmin load will not import dump with windows newline character in svn:mergeinfo Paul Burba <ptburba at gmail dot com> Paul Burba <ptburba at gmail dot com> 2009-05-12 10:49:42 PDT
         Re: [PATCH] svnadmin load will not import dump with windows newline character in svn:mergeinfo Daniel Shahaf <d dot s at daniel dot shahaf dot name> Daniel Shahaf <d dot s at daniel dot shahaf dot name> 2009-05-12 11:33:34 PDT
             Re: [PATCH] svnadmin load will not import dump with windows newline character in svn:mergeinfo Paul Burba <ptburba at gmail dot com> Paul Burba <ptburba at gmail dot com> 2009-05-12 12:58:00 PDT
                 Re: [PATCH] svnadmin load will not import dump with windows newline character in svn:mergeinfo Daniel Shahaf <d dot s at daniel dot shahaf dot name> Daniel Shahaf <d dot s at daniel dot shahaf dot name> 2009-05-12 22:37:35 PDT
                     Re: [PATCH] svnadmin load will not import dump with windows newline character in svn:mergeinfo Paul Burba <ptburba at gmail dot com> Paul Burba <ptburba at gmail dot com> 2009-05-13 07:29:10 PDT
                         Re: [PATCH] svnadmin load will not import dump with windows newline character in svn:mergeinfo glasser David Glasser 2009-05-13 11:47:08 PDT
                             Re: [PATCH] svnadmin load will not import dump with windows newline character in svn:mergeinfo Paul Burba <ptburba at gmail dot com> Paul Burba <ptburba at gmail dot com> 2009-05-13 12:34:17 PDT
                                 Re: [PATCH] svnadmin load will not import dump with windows newline character in svn:mergeinfo glasser David Glasser 2009-05-14 17:33:50 PDT
                         Re: [PATCH] svnadmin load will not import dump with windows newline character in svn:mergeinfo Daniel Shahaf <d dot s at daniel dot shahaf dot name> Daniel Shahaf <d dot s at daniel dot shahaf dot name> 2009-05-13 16:40:25 PDT
                             Re: [PATCH] svnadmin load will not import dump with windows newline character in svn:mergeinfo Paul Burba <ptburba at gmail dot com> Paul Burba <ptburba at gmail dot com> 2009-05-18 08:02:15 PDT
Messages per page: