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

Chris Shoemaker c.shoemaker at cox.net
Sat Jan 28 18:31:15 EST 2006


On Sat, Jan 28, 2006 at 10:40:36PM +0000, Neil Williams wrote:
> On Saturday 28 January 2006 9:53 pm, Chris Shoemaker wrote:
> > >    Set the logging level for all known log_modules.
> > > Note: Unless a log_module has been registered using qof_log_set_level, it
> > > will be unaffected by this change.
> >
> > Think about this.  _When_ is this function supposed to be called?
> 
> To modify the log levels _after_ initial (hard-coded) setup. The debug option 
> simply moderates the level of debugging of the modules identified as being 
> logged. This is exactly as it was before - global never did initialise the 
> modules for logging, that was done by the assumption in the lists. Global 
> simply moderated the AMOUNT of logging by tweaking the loglevel of each 
> module in the assumed list. It still does that, without the assumption.
> 
> 1. Developers enter debug macros, ENTER(" "), LEAVE (" ");
> 2. Developers call qof_log_set_level wherever it is needed to initialise this 
> module for logging later.
> 3. Users set the amount to log by specifying --debug and/or specifying a debug 
> integer level. (Developers are free to add a default OUTSIDE lib/libqof)
> 4. If it's a dynamic module, put the  qof_log_set_level call in the init 
> function so that you don't need to include the #define from the header 
> anywhere else, you get logging if it's loaded and it's ignored if it is not.

*sigh* And _how_ exactly is the global loglevel supposed to effect the
loaded module?  Am I supposed to re-call qof_log_set_level_global
after every time that new code might have been loaded?

If you still think the API isn't broken then maybe the only way you'll
understand this is if you try to restore the logging functionality in
GnuCash.  Go for it.

> > > What that all means is that the API was not broken when it moved to
> > > libqof, just clarified.
> >
> > The docs are clear that the API is broken. 
> 
> Nope. If the docs are unclear, I'll fix them but the API was fine until you 
> broke it.
> 
> > Setting a global level 
> > that has no effect on logging modules you don't explicitly register
> > MAKES ZERO SENSE!  It's USELESS!
> 
> It's absolutely logical and sensible - plus it's flexible, scalable and 
> generic. We cannot hardcode modules into QOF, neither can we assume that such 
> lists of modules exist.
> 
> gnucash has to define those lists itself and the simplest way of doing that is 
> to keep a list of those modules in a function that initialises them for 
> logging. The --debug option can then accept an integer value that scales that 
> logging up through the levels.
> 
> You initialise this in the main init code, then you use global to moderate the 
> level at which those initialised modules are actually logged, should the user 
> require a different level of logs.
> 
> At no point did global actually initialise the list of modules - it simply 
> read the hard-coded assumption. This assumption had to die and it has. Global 
> still does what it did before - changes the log level for each registered 
> module in one call. The init was separate and is separate.
> 
> > > Note that even with gnc-trace.h, if a module was not listed in the
> > > hardcoded list, it was also ignored.
> >
> > Ah, but they _were_ listed and they all WORKED. 
> 
> If you replicate the intentions of that list in gnucash (only), then they will 
> continue to work. What QOF cannot do is assume that such a list exists.

Umm.  There is no list.  There is no such assumption.

> > You're breaking 
> > things and making GnuCash _harder_ to debug with changes that aren't
> > even a good idea.  The logging system _needs_ to work.
> 
> It does work, when you setup the list to be logged - just as you had to with 
> the gnc-trace headers. It's just that this way it's more flexible and easier 
> to add new modules. (Because you can call qof_log_set_level anywhere in the 
> codebase.)

It's easy for you to break the API and then say, "well, it _would_
work if you'd only do such-and-such", (which it _wouldn't_ by the
way).  I'm saying, "it worked before, you broke it, it works again."
If you have a better solution for making it work, let's see the code.
Until then, we need logging to _work_.

> > > This was an essential step in the use of QOF outside gnucash because the
> > > hard coded list was completely irrelevant for any non-gnucash process.
> > >
> > > I accept I did not post to the list about the removal of this assumption
> > > - instead I did clearly document the results in the Doxygen output by
> > > making it explicit in the qoflog.h file.
> > >
> > > Please, Chris, can we revert your change and ditch the assumption?
> >
> > No.  The assumption is 
> 
> false
> 
> > sane and GnuCash depends on it. 
> 
> Sorry, it does not. It cannot. Assumptions are BAD.

Assumptions are why you use an API.  You assume it works.  It's not
bad.  It's a convenient substitute for doing everything yourself.

> > There is _no_  
> > other way to globally control the logging level in the presence of
> > dynamically loaded code.
> 
> Read the code again. Dynamically loaded code (like a module) can initialise 
> it's own logging - you don't need global control except to control the AMOUNT 
> of logging, not the LIST of log modules.

Why do you think every introduction of a logging module comes with an init?

> In the init function for the dynamic code, put a call to qof_log_set_level - 
> easy. If the module is loaded, it's setup for logging. If it's not, nobody 
> cares.
> 
> > BTW, notice that the only way you could have kept GnuCash's logging
> > system working with this broken API would have been to collect all the
> > used logging modules into one place and register them before setting
> > the global level, which would have basically been trading the
> > gnc-trace.h for a bunch of function calls.  Not smart.
> 
> No, that was simply a shorthand method for the base level modules. You can 
> call qof_log_set_level anywhere you like. Feel free to move all those to more 
> relevant places if you need to do so. The shorthand was a temporary measure 
> that was useful during the transition.
> 
> You can even call qof_log_set_level from within a test function - you lose 
> that ability if the list is hardcoded OR assumed.

WHAT ARE YOU TALKING ABOUT?!?!  Eeeets BARROOOKE!  I'm FIXED it.  

> > If anything should be reverted it's the commit that originally broke
> > the logging.  If you want to try to drastically change the way
> > gnucash's logging system works I suggest that you make your proposal
> > to gnucash-devel, build some consensus, and respond to constructive
> > criticism.
> 
> I am responding - but the problem is that you're stuck on this false 
> assumption that will bite.
> 
> > Your breaking things by making major changes with no approval is bad
> > enough.  Complaining when people fix things is BAD FORM.
> 
> You haven't fixed it, you've broken it.

Show me ONE thing that my change broke - just one.
 
> It was fine before your patch - any lack of logging in gnucash was down to:
> a) my lack of time to revisit the shorthand list
> b) your misunderstanding of the API, without the assumption.
> 
> > > If there are issues you want clarified in the docs, I'll gladly do that,
> > > but this patch is just plain wrong.
> >
> > The docs do need to be updated - to reflect this change.
> 
> OK - then help me see how it should be expressed in the docs and REVERT your 
> broken patch, please.

I think you misunderstand.  Please try again.

-chris


More information about the gnucash-devel mailing list