Re: svn commit: r30640 - branches/svnserve-logging/subversion/svnserve

[Eric Gillespie <ep...@google.com>]

epg@tigris.org writes:

> Author: epg
> Date: Wed Apr 16 12:58:09 2008
> New Revision: 30640
> 
> Log:
> Add new kind of log line, where the action starts with "ERR", and log
> such lines when the fs calls our new warning callback.  Later, we'll
> use this format to log other things, such as authn/authz failures.
> 
> * subversion/svnserve/serve.c
>   (fs_warning_baton_t): Add new baton type.
>   (MAX_STRING_LEN): Copy from httpd-2.2.4/include/httpd.h
>   (c2x_table, c2x, escape_errorlog_item): Copy from httpd-2.2.4/server/util.c
>   (log_fs_warning): Add svn_fs_warning_callback_t function to log the
>     svn_error_t object(s), partly based on log_error_core from
>     httpd-2.2.4/server/log.c
>   (serve): Setup warning callback with svn_fs_set_warning_func

This is bound to be controversial.  I suppose the best thing
would be for these facilities to move to apr(-util), but I'm not
sure how interested httpd/apr folks would be in that, not to
mention I have neither the time nor the energy to drive that.

Perhaps the next best thing would be to move these facilities to
a new file in svnserve/ with a copy of the Apache copyright
statement as well as our own?

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r30640 - branches/svnserve-logging/subversion/svnserve

[ep...@google.com]

Karl Fogel <kf...@red-bean.com> writes:

> discussion on dev@apr).  While apr-izing it would be the ideal solution,
> I think I'd rather live with the ever-so-slight-messiness for now.
> You've carefully left attributions intact, so it's all traceable.  

OK.

> To simplify things a bit, you could get rid of the MAX_STRING_LEN
> #define, since we only use it in one of our callers.  We could hardcode
> it or call it any name we want.  It's not like we got the idea of
> defining constants from httpd :-), and this way all of our copying would
> be confined to httpd/trunk/server/util.c.

Well, I don't think you can claim copyright over just '#define
MAX_STRING_LEN 8192' ;->.  I can rename the macro to MAX_ERR_LEN
or something, but I still want to say where I got the magic number.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r30640 - branches/svnserve-logging/subversion/svnserve

[Karl Fogel <kf...@red-bean.com>]

epg@google.com writes:
>> Putting it in a separate file and putting the Apache copyright -- just
>> the Apache copyright, not our own -- at the top of that file would be
>
> I've changed it, so it seems to me we should add our own
> copyright statement as well.

Ah, I didn't realize you'd changed it.  Okay, then yeah, I guess both
copyrights apply.  

It's too bad, it's the first dual-copyright thing in our core code.
We're okay to distribute, of course, since we're complying with the
license(s).  It'll just make things ever-so-slightly messier come CLA
time.

However, I just looked into what would be involved in moving that stuff
over to APR, and it's somewhat hairy (as there are other things
Subversion doesn't use that maybe also should be migrated over, == long
discussion on dev@apr).  While apr-izing it would be the ideal solution,
I think I'd rather live with the ever-so-slight-messiness for now.
You've carefully left attributions intact, so it's all traceable.  

To simplify things a bit, you could get rid of the MAX_STRING_LEN
#define, since we only use it in one of our callers.  We could hardcode
it or call it any name we want.  It's not like we got the idea of
defining constants from httpd :-), and this way all of our copying would
be confined to httpd/trunk/server/util.c.

-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r30640 - branches/svnserve-logging/subversion/svnserve

[ep...@google.com]

Karl Fogel <kf...@red-bean.com> writes:

> Well, we definitely can't just copy code and claim copyright over it.

Sure sure, I wasn't suggesting that.

> Putting it in a separate file and putting the Apache copyright -- just
> the Apache copyright, not our own -- at the top of that file would be

I've changed it, so it seems to me we should add our own
copyright statement as well.

> okay, I think.  (Or just rewriting it: it's just escape_errorlog_item(),
> not too complicated.)

Eh.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r30640 - branches/svnserve-logging/subversion/svnserve

[Karl Fogel <kf...@red-bean.com>]

Eric Gillespie <ep...@google.com> writes:
>> Log:
>> Add new kind of log line, where the action starts with "ERR", and log
>> such lines when the fs calls our new warning callback.  Later, we'll
>> use this format to log other things, such as authn/authz failures.
>> 
>> * subversion/svnserve/serve.c
>>   (fs_warning_baton_t): Add new baton type.
>>   (MAX_STRING_LEN): Copy from httpd-2.2.4/include/httpd.h
>>   (c2x_table, c2x, escape_errorlog_item): Copy from httpd-2.2.4/server/util.c
>>   (log_fs_warning): Add svn_fs_warning_callback_t function to log the
>>     svn_error_t object(s), partly based on log_error_core from
>>     httpd-2.2.4/server/log.c
>>   (serve): Setup warning callback with svn_fs_set_warning_func
>
> This is bound to be controversial.  I suppose the best thing
> would be for these facilities to move to apr(-util), but I'm not
> sure how interested httpd/apr folks would be in that, not to
> mention I have neither the time nor the energy to drive that.
>
> Perhaps the next best thing would be to move these facilities to
> a new file in svnserve/ with a copy of the Apache copyright
> statement as well as our own?

Well, we definitely can't just copy code and claim copyright over it.
Putting it in a separate file and putting the Apache copyright -- just
the Apache copyright, not our own -- at the top of that file would be
okay, I think.  (Or just rewriting it: it's just escape_errorlog_item(),
not too complicated.)

-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org