[ CCing dev@. Background: svnsync blows up on svn:ignore with non-LF
line endings when syncing 1.5 src to 1.6 dest. Issues #1796 and #3313. ]
B Smith-Mannschott wrote on Sun, 29 Mar 2009 at 15:38 +0200:
> On Sat, Mar 28, 2009 at 14:58, Daniel Shahaf <d.s_at_daniel.shahaf.name> wrote:
> > B Smith-Mannschott wrote on Sat, 28 Mar 2009 at 12:43 +0100:
>
> >> I've made a hacked-up variant of svnsync which will correct impure
> >> eol-style in property values where the real 1.6.0 complains and dies.
> >> (see patch at end of mail).
> >>
...
> Thanks for the tip. This is what I came up with:
>
> ----------------------------------
>
Bottom line, the patch looks good. However, assuming you want it
applied, I have a few changes to suggest:
> --- a/subversion/svnsync/main.c
> +++ b/subversion/svnsync/main.c
> @@ -754,6 +754,7 @@ typedef struct {
> svn_boolean_t mergeinfo_stripped; /* Did we strip svn:mergeinfo? */
> svn_boolean_t svnmerge_migrated; /* Did we convert svnmerge.py data? */
> svn_boolean_t svnmerge_blocked; /* Was there any blocked svnmerge data? */
> + svn_boolean_t fix_prop_val_eol_style; /* in svn:* props */
> } edit_baton_t;
>
>
> @@ -1084,6 +1085,22 @@ change_dir_prop(void *dir_baton,
> name = SVN_PROP_MERGEINFO;
> eb->svnmerge_migrated = TRUE;
It would be good to have the same logic applied in change_file_prop()
too (although currently it should be a no-op 99% of the time).
I'm not sure whether to apply the logic to revprops?
(Those can be fixed *without* svnsync... but, OTOH, it's probably more
convenient/ consistent to have svnsync fix both kinds of properties.
And so far, the issue was reported more often with revprops than with
versioned props.)
> }
> + /* Here we force consistent eol-style for svn:* properties that
> + don't have it. not, that by being on the else branch of the
> + previous if, this only kicks in if we haven't already
*nod*
(and typo: s/not/note/)
> + rewritten the property as part of a merge-info conversion. */
> + else if (eb->fix_prop_val_eol_style && svn_prop_is_svn_prop(name) && value)
> + {
s/svn_prop_is_svn_prop/svn_prop_needs_translation/
> + /* Only actually do the conversion if there's actually at
> + least one \r present. Is this premature optimzation? */
> + if (strchr(value->data, '\r'))
> + {
> + apr_array_header_t *lines =
> + svn_cstring_split(value->data, "\r\n", FALSE, pool);
> + const char *joined_lines = svn_cstring_join(lines, "\n", pool);
> + real_value = svn_string_create(joined_lines, pool);
> + }
Okay, this is *much* clearer than in your original patch :-)
(How would it handle "foo\n\nbar", though? For svn:ignore it doesn't
matter, but if we want to apply this logic to svn:log too, we should try
to avoid erasing blank lines, if possible.)
> + }
>
> /* Remember if we see any svnmerge-blocked properties. */
> if (eb->migrate_svnmerge && (strcmp(name, "svnmerge-blocked") == 0))
> @@ -1198,6 +1215,12 @@ get_sync_editor(const svn_delta_editor_t *wrapped_editor,
> eb->migrate_svnmerge = TRUE;
> eb->strip_mergeinfo = TRUE;
> }
> + if (getenv("SVNSYNC_FIX_PROP_VAL_EOL_STYLE"))
> + {
> + /* force consistent eol-style on svn:* properties by stripping
> + \r, leaving only \n as line terminator. */
> + eb->fix_prop_val_eol_style = TRUE;
> + }
>
> *editor = tree_editor;
> *edit_baton = eb;
>
> -----------------------
>
> It works for me, and I think I'll patch my local svnsync this way
> until we've migrated the central servers to 1.6 and cleaned up our
> repositories accordingly.
>
> The svn internals are still pretty opaque to me, so this is monkey
> (see, monkey do) code. Am I doing anything here that seems obviously
> wrong?
>
I think the comments I didn't make above should answer this question. :-)
> The bash script below demonstrates the behavior in combination with
> the repo.dump file attached to the first message in this thread.
>
> --------------------------
>
> #!/bin/bash
>
> # set as appropriate
> SVNSYNC=../installed/bin/svnsync
> SVNADMIN=../installed/bin/svnadmin
> DUMPFILE=repo.dump
>
> test ! -d repo || rm -rf repo
> $SVNADMIN create repo
> $SVNADMIN load repo < $DUMPFILE
>
> test ! -d clone || rm -rf clone
> $SVNADMIN create clone
> ln -s /bin/true clone/hooks/pre-revprop-change
> $SVNSYNC init file://$PWD/clone file://$PWD/repo
>
> unset SVNSYNC_FIX_PROP_VAL_EOL_STYLE
> echo "* svnsync without SVNSYNC_FIX_PROP_VAL_EOL_STYLE"
> if $SVNSYNC sync file://$PWD/clone
> then echo "** svnsync fix should have failed here."
> else echo "** svnsync failed as expected"
> fi
>
> export SVNSYNC_FIX_PROP_VAL_EOL_STYLE=1
> echo "* svnsync with SVNSYNC_FIX_PROP_VAL_EOL_STYLE"
> if $SVNSYNC sync file://$PWD/clone
> then echo "** svnsync succeeded as expected"
> else echo "** svnsync should have succeeded, but failed instead"
> fi
>
> --------------------------
>
> // Ben
>
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1484531
Received on 2009-03-30 20:25:28 CEST