You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by David Glasser <gl...@davidglasser.net> on 2008/01/02 18:35:35 UTC
Re: validate_eol_prop_against_file() inefficiency?
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
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