[Gnucash-changes] r12999 - gnucash/trunk/lib/libqof/qof - Re-enable logging for GnuCash modules that haven't explicitly set their

[Neil Williams]

On Sunday 29 January 2006 2:07 am, Chris Shoemaker wrote:
> On Sun, Jan 29, 2006 at 12:16:23AM +0000, Neil Williams wrote:
> > If the module runs it's own init it can read the existing config for that
> > value and set that. This is a process value, not a library value. It has
> > no business being a static in the qof library.
>
> Um, you _do_ realize that libraries are mapped into the process's
> memory space, right?

Yes, but that's no reason to have a default value in the library that enables 
logging where the module has NOT explicitly done so.

> > Modules that are not meant to be logged in user-land in
> > pilot-qof are now logged when the output is completely meaningless to a
> > user.
>
> Complete B.S.  Either:

No - modules that have not explicitly set their log level are NOT to be 
logged, anyway, anyhow. Set the explicit log level and THEN work on them.

> A) you don't realize that because the variable is initialized to zero,
> NOT A SINGLE message will get logged that wasn't logged before in ANY
> program ANYWHERE, _or_

Wrong. Your modification to qof_log_check means that even if a module is NOT 
listed in the hashtable, the check returns TRUE even if loglevel is zero. 
That's plain wrong. Only modules in the hashtable should be logged. There can 
be no else{} statement. Zero is not "do not log", zero is GNC_LOG_FATAL which 
still results in some logging.

Your modification means that qof_log_check returns true every time for 
GNC_LOG_FATAL and that simply isn't acceptable.

If         log_string = (gchar*)g_hash_table_lookup(log_table, log_module);
returns non-NULL, then we can test if the loglevel is sufficient. When it 
returns NULL, log_check MUST return FALSE.

I cannot accept this modification:
        /* if log_module not found, use default. */
it is simply wrong. There can be no default in the library, only in the 
application, if you want it.

> B) you _do_ realize this and you're just making a big stink because
> you don't want people modifying qof.

Not true. I didn't appreciate your last change that happened days after the 
release, despite me highlighting the imminent release here a few days before. 
I have nothing against modification of qof - those changes will be in the 
next release - I DO have a problem with patches that override the API for the 
sole benefit of one program.

> ?!?!  Fact: Logging for all testing functions now works with my patch.

test code can simply call qof_log_set_level for whichever modules it needs - 
the macros can be copied if you'd rather not include the remote header file.

> Fact: Before my patch, almost none of the logging in test functions
> worked.  We _are_ talking about the 'make check' tests, right?

You tell me, that's the first you've mentioned about WHICH modules you wanted 
to actually enable.

> B.S.  Unless you use negative loglevels, this is mathematically
> impossible.

Not true. I'd prefer if a log level of zero HAD been defined as a no-op but it 
isn't and until libqof2 I can't change that. So the way to prevent logging of 
certain "developer-only" or "testing" modules is simply to NOT have that 
module name in the hash table. You've broken that. If your global variable is 
allowed to remain, then every module is log enabled whether or not it is 
listed in the hashtable - I simply cannot accept that.

It's fine for gnucash to have the attitude that everything should be logged 
but other applications do NOT require such treatment and only ever want to 
log when explicitly requested to do so by the user.

To achieve this, the code must include the ENTER(" ") etc. macros and have a 
QofLogModule but NOT have a log level explicitly set, i.e. no entry in the 
hashtable. Your patch enables logging for precisely these modules when they 
should be silent.

It's fine that --debug enables logging for some modules and not for others - 
that is the API and - despite what you may think - has always been the API 
because the old enum was gnucash-specific and other applications HAD to use 
their own methods.

> Just drop the charade and get back to the _real_ reason 
> you're complaining: You forked QOF

I did not fork QOF, QOF has not been forked. It's a spin-out, not a fork.

> , and the original maintainers won't 
> stop developing (and fixing) it like you told 'em to.

Not true. Please see a world beyond the narrow confines of gnucash where 
modules are logged only if explicitly requested and where the "log 
everything" mentality is simply unacceptable.

This is especially true in embedded systems where the logs should only ever be 
created when absolutely necessary due to the tiny resource limits.

You may think diskspace is cheap and infinite but QOF has to be able to turn 
everything off for those environments where temporary files are bad news.

Even logging fatal errors is unacceptable unless specifically enabled.

> > it breaks the API which specifically states that ONLY
> > modules that initialise their own log levels will ever be logged.
>
> Yes, the docs described a useless global loglevel that wasn't being
> used globally.  We should fix that.

It is used globally if you follow the API instead of breaking it. Initialise 
the modules you want logged explicitly, THEN you can set the log level for 
each one using the global routine should the level need to be changed from 
the initial value.

It's the same metaphor as QofObject - you have to register the object before 
you can do any foreach work with it.

> 1) I _refuse_ to stop developing QOF just because you moved it into
> lib.  I may change my mind someday, but not today.

I don't have a problem with that, my problem is that you often fail to take 
account of the uses of QOF outside gnucash. I have to take those aspects into 
account and I will NOT accept patches that are gnucash-specific.

> 2) I am _not_ sympathetic to your self-imposed hardship of having the
> same code developed in svn and where-ever else you choose to develop
> your forked code-base.

It is not forked.

> 4) I will _not_ rehash this flamewar everytime I fix something in
> lib/libqof.

Good, then maybe we can make some progress.

> 5) Please do not interpret any future lack of my response to this
> subject as approval for or agreement with your desire that GnuCash
> developers stop developing in lib/libqof.

All I have ever wanted is for you to consider the role of QOF outside gnucash 
and to consider that other processes may have different requirements. Drop 
the blinkers and I'm happy. I don't want people to consider lib/libqof 
"off-limits", just that changes in lib/libqof must NOT be gnucash specific.

-- 

Neil Williams
=============
http://www.data-freedom.org/
http://www.nosoftwarepatents.com/
http://www.linux.codehelp.co.uk/

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://lists.gnucash.org/pipermail/gnucash-devel/attachments/20060129/76c3c2c6/attachment.bin