You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@subversion.apache.org by B Smith-Mannschott <bs...@gmail.com> on 2009/03/27 20:43:53 UTC
svnsync 1.6.0 fails to sync repositories with ^M in an svn:ignore
svnsync 1.6.0 fails to sync repositories with ^M in an svn:ignore
I've discovered a regression in 1.6.0 relating the following error message:
.../1.6.0/subversion/libsvn_repos/fs-wrap.c:189: (apr_err=125005)
svnsync: Cannot accept non-LF line endings in 'svn:ignore' property
1. It is possible unknowingly commit a value for svn:ignore which
mixes eol-style, using a version of Subclipse on Windows.
2. The presence of this property value effectively breaks svnsnync
1.6.0 because it insists on eol-style purity.
# Reproduction
I have constructed a minimal dump file to reproduce this
issue. Revision 1 defines an svn:ignore on / containing a CR (^M).
- This dump file can be successfully loaded into a fresh repository
using either svnadmin 1.5.2 or svnadmin 1.6.0.
- svnsync 1.5.2 will successfully sync revision 1 to a clone
repository.
- svnsync 1.6.0 fails to sync revision 1.
.../1.6.0/subversion/libsvn_repos/fs-wrap.c:189: (apr_err=125005)
svnsync: Cannot accept non-LF line endings in 'svn:ignore' property
It appears that svnadmin load 1.6.0 is not as strict as svnsync
1.6.0.
# Problem
I now have a 1.6 GB repository which I no longer able to back up using
svnsync. Correcting the problem would mean dumping, editing the huge
dump file and reloading the repository. I have no way of preventing
this from occuring again since the server doesn't protect itself
against misbehaving clients.
# Story
I have a colleague using Rational Application Developer (an Eclipse
dirivative) with some as yet undetermined version of Subclipse,
running on Windows XP. Today he added an entry to the svn:ignores
property one directory in our 1.6 GB repository.
The new value for svn:ignores apparently had mixed
eol-conventions. Recall that this user's platform uses CR/LF -- it's
Windows. You may protest, rightly, that Subclipse *should* be using
only LF to terminate lines of textual svn: properties. But fact is:
it gladly mixes the LFs that are already there with CRLF for new
additions. *What's worse: the subversion server accepts this commit
without protest*
(The server is still on 1.4.x, running on Solaris. I have no direct
access to that box.)
So, now I have a repository containing a bad svn:ignore.
I maintain an offsite clone of our central repositories using svnsync
to clones on my work machine. This works fine with svnsync 1.5.x
(1.5.2, specifically) even in the presence of this broken svn:ignore
property.
I upgraded my personal machine (the one holding the clone
repositories) to 1.6.0 today and discovered this problem
coincidentally. It appears that I will have to roll back the update
and stay on 1.5.x until 1.6.0 becomes more tolerant.
# Query
This isn't the first time I've run into something like this. (I recall
similar problems with RAD+Subclipse+Windows a year or two ago relating
to svn:externals.)
Why is subversion so anal-retentive about eol-style purity in its own
properties? SVN clearly knows these properties are textual, so I see
three alternatives:
1. Subversion could be more tolerant of eol-style when reading these
properties without losing information.
2. Alternatively, it could normalize the eol-style when these
properties are written.
3. Failing all that, it could at least refuse to accept values using
an impure eol-style. It already does this for files judged textual.
Has anyone else seen this issue?
Is a fix likely?
What can I do to help?
// ben
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=1065&dsMessageId=1448375
To unsubscribe from this discussion, e-mail: [users-unsubscribe@subversion.tigris.org].
RE: svnsync 1.6.0 fails to sync repositories with ^M in an
svn:ignore
Posted by Paul Koning <Pa...@Dell.com>.
> > If Subversion starts to enforce a restriction that it didn't
> > enforce in
> > an earlier version, it's the responsibility of Subversion -- not of
> > the
> > admin -- to provide backwards compatibility.
>
> I agree. With the dump/filter/load cycle I was suggesting a way that
> the OP could move forward in the present absence of that support from
> Subversion.
Ok, that makes a lot of sense.
> > Preferably it should
> > accept the older, less restricted, data. Failing that, it should
> > provide a conversion process. Telling admins they have to construct
> > hook scripts to deal with something Subversion did to them is not a
> > good
> > answer.
>
> Hook scripts have often been the answer to restricting things from
> going into the repository that are generally a bad idea. In this
> case, prevent properties from having incorrect line endings.
> Personally, I think it's a good idea to also prevent case-only
> filename collisions, prevent filenames that are problematic on
> Windows (there's a whole list), prevent backup files, maybe you want
> to prevent files greater than a certain size, etc. etc.
True, but those examples are different. Each of those is an example of
something Subversion explicitly supports and allows, but which for some
installations may be undesirable (and, for others, perfectly ok). That
sort of site policy enforcement clearly belongs in hooks.
On the other hands "incorrect line endings" in svn:ignore is a new
Subversion notion; users don't particularly care one way or the other
and neither did Subversion before 1.6. So it's an internal restriction
rather than a site policy choice, as in the hook examples you gave.
Yes, as with the dump/filter/load, putting such a hook script in place
is a reasonable *workaround* for the lack of Subversion handling of this
issue. But only a workaround.
paul
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=1065&dsMessageId=1459085
To unsubscribe from this discussion, e-mail: [users-unsubscribe@subversion.tigris.org].
Re: svnsync 1.6.0 fails to sync repositories with ^M in an
svn:ignore
Posted by Ryan Schmidt <su...@ryandesign.com>.
On Mar 28, 2009, at 09:48, Paul Koning wrote:
>> On Mar 27, 2009, at 15:43, B Smith-Mannschott wrote:
>>
>>> I now have a 1.6 GB repository which I no longer able to back up
>> using
>>> svnsync. Correcting the problem would mean dumping, editing the
> huge
>>> dump file and reloading the repository. I have no way of preventing
>>> this from occuring again since the server doesn't protect itself
>>> against misbehaving clients.
>>
>> I have not tested to verify this problem. But I agree Subversion
>> should prevent it. Until it does, you can prevent it by writing a
>> pre-
>> commit hook which tests the svn:ignore property (and possibly
>> svn:externals and others). If you want to correct the problem in your
>> existing revisions and can tolerate a dump and load cycle,
>> svndumptool should be able to help you.
>
> I don't think that's the right approach.
>
> If Subversion starts to enforce a restriction that it didn't
> enforce in
> an earlier version, it's the responsibility of Subversion -- not of
> the
> admin -- to provide backwards compatibility.
I agree. With the dump/filter/load cycle I was suggesting a way that
the OP could move forward in the present absence of that support from
Subversion.
> Preferably it should
> accept the older, less restricted, data. Failing that, it should
> provide a conversion process. Telling admins they have to construct
> hook scripts to deal with something Subversion did to them is not a
> good
> answer.
Hook scripts have often been the answer to restricting things from
going into the repository that are generally a bad idea. In this
case, prevent properties from having incorrect line endings.
Personally, I think it's a good idea to also prevent case-only
filename collisions, prevent filenames that are problematic on
Windows (there's a whole list), prevent backup files, maybe you want
to prevent files greater than a certain size, etc. etc.
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=1065&dsMessageId=1458716
To unsubscribe from this discussion, e-mail: [users-unsubscribe@subversion.tigris.org].
RE: svnsync 1.6.0 fails to sync repositories with ^M in an
svn:ignore
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Paul Koning wrote on Sat, 28 Mar 2009 at 10:48 -0400:
> > I have not tested to verify this problem. But I agree Subversion
> > should prevent it. Until it does, you can prevent it by writing a pre-
> > commit hook which tests the svn:ignore property (and possibly
> > svn:externals and others). If you want to correct the problem in your
> > existing revisions and can tolerate a dump and load cycle,
> > svndumptool should be able to help you.
>
> I don't think that's the right approach.
>
> If Subversion starts to enforce a restriction that it didn't enforce in
> an earlier version, it's the responsibility of Subversion -- not of the
> admin -- to provide backwards compatibility. Preferably it should
> accept the older, less restricted, data. Failing that, it should
> provide a conversion process. Telling admins they have to construct
> hook scripts to deal with something Subversion did to them is not a good
> answer.
>
Just to clarify: One only needs hook scripts if one wants to commit
to a 1.5 server and then repeat those commits (e.g., by using svnsync)
on a 1.6 server. In other cases --- 1.6-to-1.5 syncs, or upgraded
installations --- no hooks are needed.
Re conversion process: yes, will be useful (especially when versioned
props are involved). Once this (or another) thread reaches a conclusion
on the best strategy, we can document that somewhere.
> paul
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=1065&dsMessageId=1456932
>
> To unsubscribe from this discussion, e-mail: [users-unsubscribe@subversion.tigris.org].
>
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=1065&dsMessageId=1460148
To unsubscribe from this discussion, e-mail: [users-unsubscribe@subversion.tigris.org].
RE: svnsync 1.6.0 fails to sync repositories with ^M in an
svn:ignore
Posted by Paul Koning <Pa...@Dell.com>.
> On Mar 27, 2009, at 15:43, B Smith-Mannschott wrote:
>
> > I now have a 1.6 GB repository which I no longer able to back up
> using
> > svnsync. Correcting the problem would mean dumping, editing the
huge
> > dump file and reloading the repository. I have no way of preventing
> > this from occuring again since the server doesn't protect itself
> > against misbehaving clients.
>
> I have not tested to verify this problem. But I agree Subversion
> should prevent it. Until it does, you can prevent it by writing a pre-
> commit hook which tests the svn:ignore property (and possibly
> svn:externals and others). If you want to correct the problem in your
> existing revisions and can tolerate a dump and load cycle,
> svndumptool should be able to help you.
I don't think that's the right approach.
If Subversion starts to enforce a restriction that it didn't enforce in
an earlier version, it's the responsibility of Subversion -- not of the
admin -- to provide backwards compatibility. Preferably it should
accept the older, less restricted, data. Failing that, it should
provide a conversion process. Telling admins they have to construct
hook scripts to deal with something Subversion did to them is not a good
answer.
paul
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=1065&dsMessageId=1456932
To unsubscribe from this discussion, e-mail: [users-unsubscribe@subversion.tigris.org].
Re: svnsync 1.6.0 fails to sync repositories with ^M in an
svn:ignore
Posted by Ryan Schmidt <su...@ryandesign.com>.
On Mar 27, 2009, at 15:43, B Smith-Mannschott wrote:
> I now have a 1.6 GB repository which I no longer able to back up using
> svnsync. Correcting the problem would mean dumping, editing the huge
> dump file and reloading the repository. I have no way of preventing
> this from occuring again since the server doesn't protect itself
> against misbehaving clients.
I have not tested to verify this problem. But I agree Subversion
should prevent it. Until it does, you can prevent it by writing a pre-
commit hook which tests the svn:ignore property (and possibly
svn:externals and others). If you want to correct the problem in your
existing revisions and can tolerate a dump and load cycle,
svndumptool should be able to help you.
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=1065&dsMessageId=1452922
To unsubscribe from this discussion, e-mail: [users-unsubscribe@subversion.tigris.org].
Re: svnsync 1.6.0 fails to sync repositories with ^M in an
svn:ignore
Posted by B Smith-Mannschott <bs...@gmail.com>.
On Sat, Mar 28, 2009 at 13:58, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> B Smith-Mannschott wrote on Sat, 28 Mar 2009 at 12:43 +0100:
>> As I understand it the Subversion server 1.6.x will refuse these kinds
>> of commits, even without a hook script, yes?
>>
>
> Yes.
>
>> 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).
>>
>> This is obviously not a permanent solution.
>>
>> ---------------------
>> Don't try this at home kids: this is an *evil* hack. A validation
>> function which mutates an input marked as const is truly not a pretty
>> sight.
>>
>
> Less evil would be to change svnsync itself rather than the validation
> logic --- see how subversion/svnsync/main.c handles the
> SVNSYNC_UNSUPPORTED_STRIP_MERGEINFO flag. That way you also wouldn't have
> to modify consts.
I'll look into that. Maybe I can come up with something cleaner. My
hopes are not that high. I find the Subversion codebase to be almost
completely opaque despite the copious comments. I'm a Java programmer
by day, a C programmer only in my nightmares. ;-)
// ben
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=1065&dsMessageId=1455825
To unsubscribe from this discussion, e-mail: [users-unsubscribe@subversion.tigris.org].
Re: svnsync 1.6.0 fails to sync repositories with ^M in an
svn:ignore
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
[ 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....@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
Re: svnsync 1.6.0 fails to sync repositories with ^M in an
svn:ignore
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Revprops are the easy part (they can be fixed in-place, without even
svnsync). All the clients out there that suddenly get their line-endings
rejected are the bigger problem of the two.
Anyway, I think you'll get more useful responses on dev@. Feel free to
summarize the thread there. :-)
Thanks.
Daniel
(I already CCed them on an earlier mail.)
B Smith-Mannschott wrote on Mon, 30 Mar 2009 at 12:54 +0200:
> On Mon, Mar 30, 2009 at 10:53, B Smith-Mannschott <bs...@gmail.com> wrote:
> > On Mon, Mar 30, 2009 at 09:54, B Smith-Mannschott <bs...@gmail.com> wrote:
> >> It appears my fix is only partial. Revision properties also need to be
> >> sanitized:
> >>
> >> svnsync: Cannot accept non-LF line endings in 'svn:log' property
> >>
> >> Crud. Time to fire up emacs. Oh, never mind, my emacs is always running. ;-)
> >>
> >> // Ben
> >
> > Upon further investigation, this problem is worse than I anticipated.
> > I believe it will derail our plan to upgrade our servers to 1.6.
> > (We're still on 1.4.x and the lack of merge tracking is a real
> > hindrance.):
> >
> > The developer who made the commit including ^M in its log message is running:
> >
> > Rational Application Developer 7.5.1 (~ Eclipse 3.4.1)
> > Subclipse 1.4.7 (using JavaHL 1.5.5)
> >
> > I had him update his Subclipse to 1.6.x (which is based on Subversion
> > 1.6.0), but this *made* *no* *difference*. Log messages still come
> > like this:
> >
> > test^M
> > comment^M
> > for^M
> > svn^M
> > linebreak
> >
> > This means that were we to upgrade our servers to 1.6.0 *today*, 90%
> > of the developers on my team (I'm the only one using Linux) would be
> > unable to commit to the repository because the server would refuse
> > their log comments for lack of eol-purity. And what's worse, not even
> > updating to the most recent version our Subversion client will help.
> >
> > I don't imagine that this was the intended consequence of making
> > subversion stricter about its internal properties. Perhaps it would be
> > smarter to make it more liberal in what it accepts and have it
> > normalize these properties -- on the server side -- before committing
> > them to disk.
> >
> > I'm perfectly willing to hack up my personal copy of svnsync to make
> > it work around these problems, but I'm not able or willing to do this
> > to the svn we'll be running on our central servers.
>
> I have since been able to reproduce this using plain Eclipse 3.4.1 (no
> RAD required) under Windows with both Subclipse 1.4.8 *and* Subclipse
> 1.6.2.
>
> In the process, I have discovered a work-around, of a sort.
>
> (1) check out a project form a repository hosted on svnserve 1.6.0
> (2) make local changes
> (3) commit
> (4) enter a commit message of at least two lines length
> (5) commit
> => 'SVN Commit' has encountered a problem.
> org.tigris.subversion.javahl.ClientException: Wrong or unexpected property value
> svn: Commit failed (details follow):
> svn: Cannot accept non-LF line endings in 'svn:log' property.
> => commit failed.
> (6) commit (again)
> (7) select the previously entered commit message form <Choose a
> previously entered comment>
> => Commit succeeds.
> => It appears that entry into and retrieval from commit log history
> normalizes line-breaks implicitly.
>
> IMHO this is something that should be fixed once in core, rather than
> N times in all the various clients.
>
> > Suggestions for course of action?
>
> // Ben
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=1065&dsMessageId=1480677
>
> To unsubscribe from this discussion, e-mail: [users-unsubscribe@subversion.tigris.org].
>
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=1065&dsMessageId=1485628
To unsubscribe from this discussion, e-mail: [users-unsubscribe@subversion.tigris.org].
Re: svnsync 1.6.0 fails to sync repositories with ^M in an
svn:ignore
Posted by B Smith-Mannschott <bs...@gmail.com>.
On Mon, Mar 30, 2009 at 10:53, B Smith-Mannschott <bs...@gmail.com> wrote:
> On Mon, Mar 30, 2009 at 09:54, B Smith-Mannschott <bs...@gmail.com> wrote:
>> It appears my fix is only partial. Revision properties also need to be
>> sanitized:
>>
>> svnsync: Cannot accept non-LF line endings in 'svn:log' property
>>
>> Crud. Time to fire up emacs. Oh, never mind, my emacs is always running. ;-)
>>
>> // Ben
>
> Upon further investigation, this problem is worse than I anticipated.
> I believe it will derail our plan to upgrade our servers to 1.6.
> (We're still on 1.4.x and the lack of merge tracking is a real
> hindrance.):
>
> The developer who made the commit including ^M in its log message is running:
>
> Rational Application Developer 7.5.1 (~ Eclipse 3.4.1)
> Subclipse 1.4.7 (using JavaHL 1.5.5)
>
> I had him update his Subclipse to 1.6.x (which is based on Subversion
> 1.6.0), but this *made* *no* *difference*. Log messages still come
> like this:
>
> test^M
> comment^M
> for^M
> svn^M
> linebreak
>
> This means that were we to upgrade our servers to 1.6.0 *today*, 90%
> of the developers on my team (I'm the only one using Linux) would be
> unable to commit to the repository because the server would refuse
> their log comments for lack of eol-purity. And what's worse, not even
> updating to the most recent version our Subversion client will help.
>
> I don't imagine that this was the intended consequence of making
> subversion stricter about its internal properties. Perhaps it would be
> smarter to make it more liberal in what it accepts and have it
> normalize these properties -- on the server side -- before committing
> them to disk.
>
> I'm perfectly willing to hack up my personal copy of svnsync to make
> it work around these problems, but I'm not able or willing to do this
> to the svn we'll be running on our central servers.
I have since been able to reproduce this using plain Eclipse 3.4.1 (no
RAD required) under Windows with both Subclipse 1.4.8 *and* Subclipse
1.6.2.
In the process, I have discovered a work-around, of a sort.
(1) check out a project form a repository hosted on svnserve 1.6.0
(2) make local changes
(3) commit
(4) enter a commit message of at least two lines length
(5) commit
=> 'SVN Commit' has encountered a problem.
org.tigris.subversion.javahl.ClientException: Wrong or unexpected property value
svn: Commit failed (details follow):
svn: Cannot accept non-LF line endings in 'svn:log' property.
=> commit failed.
(6) commit (again)
(7) select the previously entered commit message form <Choose a
previously entered comment>
=> Commit succeeds.
=> It appears that entry into and retrieval from commit log history
normalizes line-breaks implicitly.
IMHO this is something that should be fixed once in core, rather than
N times in all the various clients.
> Suggestions for course of action?
// Ben
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=1065&dsMessageId=1480677
To unsubscribe from this discussion, e-mail: [users-unsubscribe@subversion.tigris.org].
Re: svnsync 1.6.0 fails to sync repositories with ^M in an
svn:ignore
Posted by B Smith-Mannschott <bs...@gmail.com>.
On Mon, Mar 30, 2009 at 09:54, B Smith-Mannschott <bs...@gmail.com> wrote:
> It appears my fix is only partial. Revision properties also need to be
> sanitized:
>
> svnsync: Cannot accept non-LF line endings in 'svn:log' property
>
> Crud. Time to fire up emacs. Oh, never mind, my emacs is always running. ;-)
>
> // Ben
Upon further investigation, this problem is worse than I anticipated.
I believe it will derail our plan to upgrade our servers to 1.6.
(We're still on 1.4.x and the lack of merge tracking is a real
hindrance.):
The developer who made the commit including ^M in its log message is running:
Rational Application Developer 7.5.1 (~ Eclipse 3.4.1)
Subclipse 1.4.7 (using JavaHL 1.5.5)
I had him update his Subclipse to 1.6.x (which is based on Subversion
1.6.0), but this *made* *no* *difference*. Log messages still come
like this:
test^M
comment^M
for^M
svn^M
linebreak
This means that were we to upgrade our servers to 1.6.0 *today*, 90%
of the developers on my team (I'm the only one using Linux) would be
unable to commit to the repository because the server would refuse
their log comments for lack of eol-purity. And what's worse, not even
updating to the most recent version our Subversion client will help.
I don't imagine that this was the intended consequence of making
subversion stricter about its internal properties. Perhaps it would be
smarter to make it more liberal in what it accepts and have it
normalize these properties -- on the server side -- before committing
them to disk.
I'm perfectly willing to hack up my personal copy of svnsync to make
it work around these problems, but I'm not able or willing to do this
to the svn we'll be running on our central servers.
Suggestions for course of action?
// Ben
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=1065&dsMessageId=1479616
To unsubscribe from this discussion, e-mail: [users-unsubscribe@subversion.tigris.org].
Re: svnsync 1.6.0 fails to sync repositories with ^M in an
svn:ignore
Posted by B Smith-Mannschott <bs...@gmail.com>.
It appears my fix is only partial. Revision properties also need to be
sanitized:
svnsync: Cannot accept non-LF line endings in 'svn:log' property
Crud. Time to fire up emacs. Oh, never mind, my emacs is always running. ;-)
// Ben
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=1065&dsMessageId=1479179
To unsubscribe from this discussion, e-mail: [users-unsubscribe@subversion.tigris.org].
Re: svnsync 1.6.0 fails to sync repositories with ^M in an
svn:ignore
Posted by B Smith-Mannschott <bs...@gmail.com>.
On Sat, Mar 28, 2009 at 14:58, Daniel Shahaf <d....@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).
>>
>> This is obviously not a permanent solution.
>>
>> ---------------------
>> Don't try this at home kids: this is an *evil* hack. A validation
>> function which mutates an input marked as const is truly not a pretty
>> sight.
>>
>
> Less evil would be to change svnsync itself rather than the validation
> logic --- see how subversion/svnsync/main.c handles the
> SVNSYNC_UNSUPPORTED_STRIP_MERGEINFO flag. That way you also wouldn't have
> to modify consts.
Thanks for the tip. This is what I came up with:
----------------------------------
--- 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;
}
+ /* 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
+ 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)
+ {
+ /* 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);
+ }
+ }
/* 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?
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=1065&dsMessageId=1470762
To unsubscribe from this discussion, e-mail: [users-unsubscribe@subversion.tigris.org].
Re: svnsync 1.6.0 fails to sync repositories with ^M in an
svn:ignore
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
B Smith-Mannschott wrote on Sat, 28 Mar 2009 at 12:43 +0100:
> As I understand it the Subversion server 1.6.x will refuse these kinds
> of commits, even without a hook script, yes?
>
Yes.
> 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).
>
> This is obviously not a permanent solution.
>
> ---------------------
> Don't try this at home kids: this is an *evil* hack. A validation
> function which mutates an input marked as const is truly not a pretty
> sight.
>
Less evil would be to change svnsync itself rather than the validation
logic --- see how subversion/svnsync/main.c handles the
SVNSYNC_UNSUPPORTED_STRIP_MERGEINFO flag. That way you also wouldn't have
to modify consts.
> --- a/subversion/libsvn_repos/fs-wrap.c
> +++ b/subversion/libsvn_repos/fs-wrap.c
> @@ -182,15 +182,15 @@ validate_prop(const char *name, const
> svn_string_t *value, apr_pool_t *pool)
> "UTF-8"), name);
> }
>
> - /* Disallow inconsistent line ending style, by simply looking for
> - * carriage return characters ('\r'). */
> - if (strchr(value->data, '\r') != NULL)
> - {
> - return svn_error_createf
> - (SVN_ERR_BAD_PROPERTY_VALUE, NULL,
> - _("Cannot accept non-LF line endings in '%s' property"),
> - name);
> - }
> + /* Evil hack: replace '\r' in property value with space to
> + * maintain eol-style purity. */
> + {
> + char * p = value->data;
> + while (p = strchr(p, '\r'))
> + {
> + *p = ' ';
> + }
> + }
> }
>
Nice idea. :-)
/* five minutes later... */
But doesn't it get you an infinite loop whenever the property sync'd
contains a '\r'?
> /* "svn:date" should be a valid date. */
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=1065&dsMessageId=1455021
>
> To unsubscribe from this discussion, e-mail: [users-unsubscribe@subversion.tigris.org].
>
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=1065&dsMessageId=1455702
To unsubscribe from this discussion, e-mail: [users-unsubscribe@subversion.tigris.org].
Re: svnsync 1.6.0 fails to sync repositories with ^M in an
svn:ignore
Posted by B Smith-Mannschott <bs...@gmail.com>.
Thanks for the feedback. What I've done for the moment is this:
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).
This is obviously not a permanent solution.
Our central Subversion server will be seeing an upgrade from 1.4.x to
1.6.x sometime before summer (one hopes). At this point I'd like to
have our repositories dumped and reloaded anyway to be able to make
best use of the improvements in 1.5.x and 1.6.x. This would be a
natural time to filter through the whole dump file correcting the (no
doubt numerous) eol-style impurities. (A lot of us here use
RAD+Subclipse on Windows).
I don't intend to write a hook script to prevent this kind of
corruption. Even if I catch new errors of this kind, I'll still need
to clean up the dump repository when we transition to 1.6.x because of
the errors that have already crept in. As I understand it the
Subversion server 1.6.x will refuse these kinds of commits, even
without a hook script, yes?
---------------------
Don't try this at home kids: this is an *evil* hack. A validation
function which mutates an input marked as const is truly not a pretty
sight.
--- a/subversion/libsvn_repos/fs-wrap.c
+++ b/subversion/libsvn_repos/fs-wrap.c
@@ -182,15 +182,15 @@ validate_prop(const char *name, const
svn_string_t *value, apr_pool_t *pool)
"UTF-8"), name);
}
- /* Disallow inconsistent line ending style, by simply looking for
- * carriage return characters ('\r'). */
- if (strchr(value->data, '\r') != NULL)
- {
- return svn_error_createf
- (SVN_ERR_BAD_PROPERTY_VALUE, NULL,
- _("Cannot accept non-LF line endings in '%s' property"),
- name);
- }
+ /* Evil hack: replace '\r' in property value with space to
+ * maintain eol-style purity. */
+ {
+ char * p = value->data;
+ while (p = strchr(p, '\r'))
+ {
+ *p = ' ';
+ }
+ }
}
/* "svn:date" should be a valid date. */
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=1065&dsMessageId=1455021
To unsubscribe from this discussion, e-mail: [users-unsubscribe@subversion.tigris.org].
Re: svnsync 1.6.0 fails to sync repositories with ^M in an
svn:ignore
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
B Smith-Mannschott wrote on Fri, 27 Mar 2009 at 21:43 +0100:
> svnsync 1.6.0 fails to sync repositories with ^M in an svn:ignore
>
> I've discovered a regression in 1.6.0 relating the following error message:
>
> .../1.6.0/subversion/libsvn_repos/fs-wrap.c:189: (apr_err=125005)
> svnsync: Cannot accept non-LF line endings in 'svn:ignore' property
>
(using a maintainer-mode build?)
> 1. It is possible unknowingly commit a value for svn:ignore which
> mixes eol-style, using a version of Subclipse on Windows.
>
Possibly, but only with pre-1.6 servers.
> 2. The presence of this property value effectively breaks svnsnync
> 1.6.0 because it insists on eol-style purity.
>
> # Reproduction
>
> I have constructed a minimal dump file to reproduce this
> issue. Revision 1 defines an svn:ignore on / containing a CR (^M).
>
I believe you, but you wouldn't be able to create such a property (on
a 1.6 server) without using 'svnadmin load'.
> - This dump file can be successfully loaded into a fresh repository
> using either svnadmin 1.5.2 or svnadmin 1.6.0.
>
> - svnsync 1.5.2 will successfully sync revision 1 to a clone
> repository.
>
> - svnsync 1.6.0 fails to sync revision 1.
>
> .../1.6.0/subversion/libsvn_repos/fs-wrap.c:189: (apr_err=125005)
> svnsync: Cannot accept non-LF line endings in 'svn:ignore' property
>
> It appears that svnadmin load 1.6.0 is not as strict as svnsync
> 1.6.0.
>
Correct, 'load' applies less validity checks.
(However, some checks even 'load' doesn't skip: e.g., r33148.)
> # Problem
>
> I now have a 1.6 GB repository which I no longer able to back up using
> svnsync. Correcting the problem would mean dumping, editing the huge
> dump file and reloading the repository.
Yeah, ouch. It usually happened with svn:log so far, which are
unversioned.
> I have no way of preventing this from occuring again since the server
> doesn't protect itself against misbehaving clients.
>
You do. You can use hooks, or you can upgrade the server to 1.6
(which is the version in which the "protection against misbehaving
clients" was implemented).
> # Story
>
> I have a colleague using Rational Application Developer (an Eclipse
> dirivative) with some as yet undetermined version of Subclipse,
> running on Windows XP. Today he added an entry to the svn:ignores
> property one directory in our 1.6 GB repository.
>
> The new value for svn:ignores apparently had mixed
> eol-conventions. Recall that this user's platform uses CR/LF -- it's
> Windows. You may protest, rightly, that Subclipse *should* be using
> only LF to terminate lines of textual svn: properties. But fact is:
> it gladly mixes the LFs that are already there with CRLF for new
> additions. *What's worse: the subversion server accepts this commit
> without protest*
>
> (The server is still on 1.4.x, running on Solaris. I have no direct
> access to that box.)
>
> So, now I have a repository containing a bad svn:ignore.
>
> I maintain an offsite clone of our central repositories using svnsync
> to clones on my work machine. This works fine with svnsync 1.5.x
> (1.5.2, specifically) even in the presence of this broken svn:ignore
> property.
>
> I upgraded my personal machine (the one holding the clone
> repositories) to 1.6.0 today and discovered this problem
> coincidentally. It appears that I will have to roll back the update
> and stay on 1.5.x until 1.6.0 becomes more tolerant.
>
Yeah, using a 1.5 server in the mirror (you *can* use svnsync 1.6, as
long as the mirror runs a 1.5 server) is a workaround.
> # Query
>
> This isn't the first time I've run into something like this. (I recall
> similar problems with RAD+Subclipse+Windows a year or two ago relating
> to svn:externals.)
>
> Why is subversion so anal-retentive about eol-style purity in its own
> properties? SVN clearly knows these properties are textual, so I see
> three alternatives:
>
> 1. Subversion could be more tolerant of eol-style when reading these
> properties without losing information.
>
> 2. Alternatively, it could normalize the eol-style when these
> properties are written.
>
> 3. Failing all that, it could at least refuse to accept values using
> an impure eol-style. It already does this for files judged textual.
>
The whole issue is that we're implementing #3 --- and that the way
svnsync works means it re-applies validity checks.
> Has anyone else seen this issue?
Yes, several, with svn:log.
> Is a fix likely?
> What can I do to help?
>
You can look into a way to normalize the EOL in those properties (e.g.: if
you see "foo\r\nbar\n", lie and say you saw "foo\nbar\n"). This could be
done in svnsync (it already implements some optional transforms as it does
the sync) or at the dumpfiles layer (the reader/writer of the dump will
optionally lie about the data it sees).
The error message gives the location of the code doing the validity
checking.
Daniel
> // ben
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=1065&dsMessageId=1448375
>
> To unsubscribe from this discussion, e-mail: [users-unsubscribe@subversion.tigris.org].
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=1065&dsMessageId=1452196
To unsubscribe from this discussion, e-mail: [users-unsubscribe@subversion.tigris.org].