You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "C. Michael Pilato" <cm...@collab.net> on 2007/12/28 18:13:24 UTC
validate_eol_prop_against_file() inefficiency?
While updating the svnbook, I noticed something odd. In Subversion 1.4,
when you try to set svn:eol-style on a binary file, you get the obvious error:
% svn pset svn:eol-style CRLF suse.png
svn: File 'suse.png' has binary mime type property
In trunk, however, you get something far more bizarre:
% svn pset svn:eol-style CRLF suse.png
subversion/libsvn_wc/props.c:2425: (apr_err=200009)
svn: File 'suse.png' has inconsistent newlines
subversion/libsvn_subr/subst.c:736: (apr_err=135000)
svn: Inconsistent line ending style
I see that libsvn_wc/props.c:validate_eol_prop_against_file() has been
ordered differently in trunk. What I can't figure out is why the
far-more-expensive newline consistency check was moved ahead of the
super-cheap is-binary check? Besides being really inefficient, the new
error is just not as clear.
Was this fallout from the URL propsetting support or something?
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: validate_eol_prop_against_file() inefficiency?
Posted by "C. Michael Pilato" <cm...@collab.net>.
David Glasser wrote:
>>>> I see that libsvn_wc/props.c:validate_eol_prop_against_file() has been
>>>> ordered differently in trunk. What I can't figure out is why the
>>>> far-more-expensive newline consistency check was moved ahead of the
>>>> super-cheap is-binary check? Besides being really inefficient, the new
>>>> error is just not as clear.
>>>>
>>>> Was this fallout from the URL propsetting support or something?
>>> Not sure, but wasn't there also a patch recently to add validation of
>>> the eol-style values? Perhaps that is when this happened.
>> Well, I'm going to make the obvious optimization (which will restore the
>> more-obvious error message in the case presented).
>
> I'm currently running tests against a patch to fix this.
Cool. Thanks.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: validate_eol_prop_against_file() inefficiency?
Posted by David Glasser <gl...@davidglasser.net>.
On Jan 2, 2008 1:55 PM, David Glasser <gl...@davidglasser.net> wrote:
>
> On Jan 2, 2008 1:53 PM, C. Michael Pilato <cm...@collab.net> wrote:
> > Mark Phippard wrote:
> > > On Dec 28, 2007 1:13 PM, C. Michael Pilato <cm...@collab.net> wrote:
> > >> While updating the svnbook, I noticed something odd. In Subversion 1.4,
> > >> when you try to set svn:eol-style on a binary file, you get the obvious error:
> > >>
> > >> % svn pset svn:eol-style CRLF suse.png
> > >> svn: File 'suse.png' has binary mime type property
> > >>
> > >> In trunk, however, you get something far more bizarre:
> > >>
> > >> % svn pset svn:eol-style CRLF suse.png
> > >> subversion/libsvn_wc/props.c:2425: (apr_err=200009)
> > >> svn: File 'suse.png' has inconsistent newlines
> > >> subversion/libsvn_subr/subst.c:736: (apr_err=135000)
> > >> svn: Inconsistent line ending style
> > >>
> > >> I see that libsvn_wc/props.c:validate_eol_prop_against_file() has been
> > >> ordered differently in trunk. What I can't figure out is why the
> > >> far-more-expensive newline consistency check was moved ahead of the
> > >> super-cheap is-binary check? Besides being really inefficient, the new
> > >> error is just not as clear.
> > >>
> > >> Was this fallout from the URL propsetting support or something?
> > >
> > > Not sure, but wasn't there also a patch recently to add validation of
> > > the eol-style values? Perhaps that is when this happened.
> >
> > Well, I'm going to make the obvious optimization (which will restore the
> > more-obvious error message in the case presented).
>
> I'm currently running tests against a patch to fix this.
Committed in r28721. Thanks for spotting this, Mike.
--dave
--
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: validate_eol_prop_against_file() inefficiency?
Posted by David Glasser <gl...@davidglasser.net>.
On Jan 2, 2008 1:53 PM, C. Michael Pilato <cm...@collab.net> wrote:
> Mark Phippard wrote:
> > On Dec 28, 2007 1:13 PM, C. Michael Pilato <cm...@collab.net> wrote:
> >> While updating the svnbook, I noticed something odd. In Subversion 1.4,
> >> when you try to set svn:eol-style on a binary file, you get the obvious error:
> >>
> >> % svn pset svn:eol-style CRLF suse.png
> >> svn: File 'suse.png' has binary mime type property
> >>
> >> In trunk, however, you get something far more bizarre:
> >>
> >> % svn pset svn:eol-style CRLF suse.png
> >> subversion/libsvn_wc/props.c:2425: (apr_err=200009)
> >> svn: File 'suse.png' has inconsistent newlines
> >> subversion/libsvn_subr/subst.c:736: (apr_err=135000)
> >> svn: Inconsistent line ending style
> >>
> >> I see that libsvn_wc/props.c:validate_eol_prop_against_file() has been
> >> ordered differently in trunk. What I can't figure out is why the
> >> far-more-expensive newline consistency check was moved ahead of the
> >> super-cheap is-binary check? Besides being really inefficient, the new
> >> error is just not as clear.
> >>
> >> Was this fallout from the URL propsetting support or something?
> >
> > Not sure, but wasn't there also a patch recently to add validation of
> > the eol-style values? Perhaps that is when this happened.
>
> Well, I'm going to make the obvious optimization (which will restore the
> more-obvious error message in the case presented).
I'm currently running tests against a patch to fix this.
--dave
--
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: validate_eol_prop_against_file() inefficiency?
Posted by "C. Michael Pilato" <cm...@collab.net>.
C. Michael Pilato wrote:
> Mark Phippard wrote:
>> On Dec 28, 2007 1:13 PM, C. Michael Pilato <cm...@collab.net> wrote:
>>> While updating the svnbook, I noticed something odd. In Subversion 1.4,
>>> when you try to set svn:eol-style on a binary file, you get the obvious error:
>>>
>>> % svn pset svn:eol-style CRLF suse.png
>>> svn: File 'suse.png' has binary mime type property
>>>
>>> In trunk, however, you get something far more bizarre:
>>>
>>> % svn pset svn:eol-style CRLF suse.png
>>> subversion/libsvn_wc/props.c:2425: (apr_err=200009)
>>> svn: File 'suse.png' has inconsistent newlines
>>> subversion/libsvn_subr/subst.c:736: (apr_err=135000)
>>> svn: Inconsistent line ending style
>>>
>>> I see that libsvn_wc/props.c:validate_eol_prop_against_file() has been
>>> ordered differently in trunk. What I can't figure out is why the
>>> far-more-expensive newline consistency check was moved ahead of the
>>> super-cheap is-binary check? Besides being really inefficient, the new
>>> error is just not as clear.
>>>
>>> Was this fallout from the URL propsetting support or something?
>> Not sure, but wasn't there also a patch recently to add validation of
>> the eol-style values? Perhaps that is when this happened.
>
> Well, I'm going to make the obvious optimization (which will restore the
> more-obvious error message in the case presented).
Oops, disregard this. I hadn't yet read David's mail, which points out why
this little switcheroo won't accomplish much.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: validate_eol_prop_against_file() inefficiency?
Posted by "C. Michael Pilato" <cm...@collab.net>.
Mark Phippard wrote:
> On Dec 28, 2007 1:13 PM, C. Michael Pilato <cm...@collab.net> wrote:
>> While updating the svnbook, I noticed something odd. In Subversion 1.4,
>> when you try to set svn:eol-style on a binary file, you get the obvious error:
>>
>> % svn pset svn:eol-style CRLF suse.png
>> svn: File 'suse.png' has binary mime type property
>>
>> In trunk, however, you get something far more bizarre:
>>
>> % svn pset svn:eol-style CRLF suse.png
>> subversion/libsvn_wc/props.c:2425: (apr_err=200009)
>> svn: File 'suse.png' has inconsistent newlines
>> subversion/libsvn_subr/subst.c:736: (apr_err=135000)
>> svn: Inconsistent line ending style
>>
>> I see that libsvn_wc/props.c:validate_eol_prop_against_file() has been
>> ordered differently in trunk. What I can't figure out is why the
>> far-more-expensive newline consistency check was moved ahead of the
>> super-cheap is-binary check? Besides being really inefficient, the new
>> error is just not as clear.
>>
>> Was this fallout from the URL propsetting support or something?
>
> Not sure, but wasn't there also a patch recently to add validation of
> the eol-style values? Perhaps that is when this happened.
Well, I'm going to make the obvious optimization (which will restore the
more-obvious error message in the case presented).
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: validate_eol_prop_against_file() inefficiency?
Posted by Mark Phippard <ma...@gmail.com>.
On Dec 28, 2007 1:13 PM, C. Michael Pilato <cm...@collab.net> wrote:
> While updating the svnbook, I noticed something odd. In Subversion 1.4,
> when you try to set svn:eol-style on a binary file, you get the obvious error:
>
> % svn pset svn:eol-style CRLF suse.png
> svn: File 'suse.png' has binary mime type property
>
> In trunk, however, you get something far more bizarre:
>
> % svn pset svn:eol-style CRLF suse.png
> subversion/libsvn_wc/props.c:2425: (apr_err=200009)
> svn: File 'suse.png' has inconsistent newlines
> subversion/libsvn_subr/subst.c:736: (apr_err=135000)
> svn: Inconsistent line ending style
>
> I see that libsvn_wc/props.c:validate_eol_prop_against_file() has been
> ordered differently in trunk. What I can't figure out is why the
> far-more-expensive newline consistency check was moved ahead of the
> super-cheap is-binary check? Besides being really inefficient, the new
> error is just not as clear.
>
> Was this fallout from the URL propsetting support or something?
Not sure, but wasn't there also a patch recently to add validation of
the eol-style values? Perhaps that is when this happened.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: validate_eol_prop_against_file() inefficiency?
Posted by Julian Foad <ju...@btopenworld.com>.
David Glasser wrote:
> On Dec 28, 2007 1:13 PM, C. Michael Pilato <cm...@collab.net> wrote:
>
>>While updating the svnbook, I noticed something odd. In Subversion 1.4,
>>when you try to set svn:eol-style on a binary file, you get the obvious error:
>>
>> % svn pset svn:eol-style CRLF suse.png
>> svn: File 'suse.png' has binary mime type property
>>
>>In trunk, however, you get something far more bizarre:
>>
>> % svn pset svn:eol-style CRLF suse.png
>> subversion/libsvn_wc/props.c:2425: (apr_err=200009)
>> svn: File 'suse.png' has inconsistent newlines
>> subversion/libsvn_subr/subst.c:736: (apr_err=135000)
>> svn: Inconsistent line ending style
>>
>>I see that libsvn_wc/props.c:validate_eol_prop_against_file() has been
>>ordered differently in trunk. What I can't figure out is why the
>>far-more-expensive newline consistency check was moved ahead of the
>>super-cheap is-binary check? Besides being really inefficient, the new
>>error is just not as clear.
>>
>>Was this fallout from the URL propsetting support or something?
>
>
> Yes, this was fallout from the URL propsetting support, and you're
> absolutely right, the new error is not as clear.
>
> The property validation code may need access to the MIME type and the
> contents of the file. I decided to make a single callback
> (implemented both at the client layer using RA code, for URLs, and in
> libsvn_wc for wc propsets) that gives both the MIME type and the
> contents if necessary.
>
> The error issue could be trivially fixed by just doing the checks in a
> different order, but that wouldn't fix the efficiency issue. I could
> make two separate callbacks for MIME type and contents, but that seems
> like overkill. Maybe I'll change the callback so that you can pass it
> NULL prop-hash or stream, and the callback gets called twice...
For the email record, I see that David fixed this as suggested in r28721.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: validate_eol_prop_against_file() inefficiency?
Posted by David Glasser <gl...@davidglasser.net>.
On Dec 28, 2007 1:13 PM, C. Michael Pilato <cm...@collab.net> wrote:
> While updating the svnbook, I noticed something odd. In Subversion 1.4,
> when you try to set svn:eol-style on a binary file, you get the obvious error:
>
> % svn pset svn:eol-style CRLF suse.png
> svn: File 'suse.png' has binary mime type property
>
> In trunk, however, you get something far more bizarre:
>
> % svn pset svn:eol-style CRLF suse.png
> subversion/libsvn_wc/props.c:2425: (apr_err=200009)
> svn: File 'suse.png' has inconsistent newlines
> subversion/libsvn_subr/subst.c:736: (apr_err=135000)
> svn: Inconsistent line ending style
>
> I see that libsvn_wc/props.c:validate_eol_prop_against_file() has been
> ordered differently in trunk. What I can't figure out is why the
> far-more-expensive newline consistency check was moved ahead of the
> super-cheap is-binary check? Besides being really inefficient, the new
> error is just not as clear.
>
> Was this fallout from the URL propsetting support or something?
Yes, this was fallout from the URL propsetting support, and you're
absolutely right, the new error is not as clear.
The property validation code may need access to the MIME type and the
contents of the file. I decided to make a single callback
(implemented both at the client layer using RA code, for URLs, and in
libsvn_wc for wc propsets) that gives both the MIME type and the
contents if necessary.
The error issue could be trivially fixed by just doing the checks in a
different order, but that wouldn't fix the efficiency issue. I could
make two separate callbacks for MIME type and contents, but that seems
like overkill. Maybe I'll change the callback so that you can pass it
NULL prop-hash or stream, and the callback gets called twice...
--dave
--
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org