Unfortunately if we go down the "no log configuration == no logger" path, existing users who are already using log4net and upgrade to 3.3 will have to add an 'nhibernate-logger' entry to their <app.config /> that wasn't previously required.
I don't think it is reasonable for NHibernate, without any explicit configuration, to find and use a reference to log4net that may have been added to the GAC for use by an entirely different application. Particularly when the user may also not have configured log4net in their application. I think users prefer the simple xcopy deployment model where the application and its dependencies are contained in one folder. I'm voting to reject the pull request, avoid the GAC, and maintain the status quo. As an aside, I'm curious why the 'nhibernate-logger' setting is in <app.config /> and not <hibernate-configuration />...? On 24 January 2012 23:48, Oskar Berggren <[email protected]> wrote: > So to summarize, is this what we agree on? > > nhibernate-logging missing or empty > Use Nologging. Do not attempt to load log4net, even if present in > the deploy directory. > > > nhibernate-logging set to full name and assembly of a class. > Attempt to use that with System.Type.GetType("") and > Activator.CreateInstance() which will find it in either GAC or deploy > directory. > > > nhibernate-logging equal to "log4net" > Attempt to load log4net using Assembly.Load("log4net"), which will > find it in either the GAC or deploy directory. > This is to simplify and reduce impact of breaking change for those > who just want to keep using log4net. > > > /Oskar > > > 2012/1/24 Stephen Bohlen <[email protected]>: > > IMO that's the option that's most in-line w the > principle-of-least-surprise, > > so I think that's more the way to proceed. "Surprise -- you've got > > logging!" isn't something that makes any sense to me :) > > > > > > Steve Bohlen > > [email protected] > > http://blog.unhandled-exceptions.com > > http://twitter.com/sbohlen > > > > > > On Tue, Jan 24, 2012 at 9:12 AM, Julian Maughan < > [email protected]> > > wrote: > >> > >> I believe the pull request will cause NH to behave as follows: If no > >> logger is configured and the user happens to have log4net in their GAC, > >> log4net will be used. As no config is provided, nothing will actually be > >> logged anywhere, but personally I think this invites trouble - and goes > >> against the principle of least surprise. > >> > >> It wouldn't take much to change this behaviour to be as Fabio describes, > >> no logger configured == no logging (i.e. use > the NoLoggingLoggerFactory). > >> > >> On 24 January 2012 21:39, Stephen Bohlen <[email protected]> wrote: > >>> > >>> I tend to agree. Then this is this the most explicit way to achieve > that > >>> --? > >>> > >>> > >>>> If we want, I guess we could make even log4net require explicit > >>>> activation using the "nhibernate-logger" appsetting. Then you would > >>>> never get any logging unless you provide a "nhibernate-logger" > >>>> setting. > >>> > >>> > >>> Steve Bohlen > >>> [email protected] > >>> http://blog.unhandled-exceptions.com > >>> http://twitter.com/sbohlen > >>> > >>> > >>> On Tue, Jan 24, 2012 at 8:33 AM, Fabio Maulo <[email protected]> > >>> wrote: > >>>> > >>>> IMO > >>>> If a guy need a logging system for NH he should configure it. > >>>> No configuration = no logging. > >>>> > >>>> > >>>> On Tue, Jan 24, 2012 at 10:19 AM, Stephen Bohlen <[email protected]> > >>>> wrote: > >>>>> > >>>>> If that's true then I think this is probably fine to proceed with. > The > >>>>> only negative I could see in this scenario is that someone would be > >>>>> 'wasting' time and memory resolving and loading log4net.dll when > they don't > >>>>> want/need it. This seems a micro-optimization and so long as anyone > who > >>>>> *really* cares can configure this problem away by explicitly > selecting the > >>>>> NoLoggingLogger, it seems fine to me. If we do make this change, > then I'd > >>>>> also recommend that we make it obvious what's going on (in release > notes > >>>>> and/or documentation) so that nobody is surprised by the (possibly) > new > >>>>> behavior of their app. > >>>>> > >>>>> > >>>>> Steve Bohlen > >>>>> [email protected] > >>>>> http://blog.unhandled-exceptions.com > >>>>> http://twitter.com/sbohlen > >>>>> > >>>>> > >>>>> On Tue, Jan 24, 2012 at 8:13 AM, Richard Brown (gmail) > >>>>> <[email protected]> wrote: > >>>>>> > >>>>>> That’s my understanding. > >>>>>> > >>>>>> Also (as mentioned in the JIRA comments), unless you actually had a > >>>>>> log4net config section defining appenders, your not going to get > any log > >>>>>> messages you weren’t expecting (I don’t think). > >>>>>> > >>>>>> > >>>>>> From: Stephen Bohlen > >>>>>> Sent: Tuesday, January 24, 2012 1:07 PM > >>>>>> To: [email protected] > >>>>>> Subject: Re: [nhibernate-development] Regarding NH-2821 - better way > >>>>>> of finding log4net > >>>>>> So under this proposed change if Log4Net was in the GAC but you > didn't > >>>>>> want any logging you would have to explicitly configure NH for the > >>>>>> NoLoggingLogger, is that right? > >>>>>> > >>>>>> Steve Bohlen > >>>>>> [email protected] > >>>>>> http://blog.unhandled-exceptions.com > >>>>>> http://twitter.com/sbohlen > >>>>>> > >>>>>> > >>>>>> On Tue, Jan 24, 2012 at 8:03 AM, Richard Brown (gmail) > >>>>>> <[email protected]> wrote: > >>>>>>> > >>>>>>> I think it still won’t be required, it’s just that it will now be > >>>>>>> located if it’s in the GAC? > >>>>>>> > >>>>>>> Sounds ok to me as long as it still reverts to the NoLoggingLogger > >>>>>>> when log4net isn’t in the deploy folder or the GAC. > >>>>>>> > >>>>>>> From: Fabio Maulo > >>>>>>> Sent: Tuesday, January 24, 2012 2:00 AM > >>>>>>> To: [email protected] > >>>>>>> Subject: Re: [nhibernate-development] Regarding NH-2821 - better > way > >>>>>>> of finding log4net > >>>>>>> If no logging system is configured and there isn't log4net in the > >>>>>>> deploy folder the NoLoggingLogger is used.. > >>>>>>> In practice log4net should always not to be required > >>>>>>> > >>>>>>> On Mon, Jan 23, 2012 at 7:03 PM, Oskar Berggren > >>>>>>> <[email protected]> wrote: > >>>>>>>> > >>>>>>>> A pull request was submitted last year that delegates finding > >>>>>>>> log4net to .Net. > >>>>>>>> > >>>>>>>> https://github.com/nhibernate/nhibernate-core/pull/15 > >>>>>>>> > >>>>>>>> It was not merged at the time because there was some compatibility > >>>>>>>> concerns since if log4net was installed in the GAC, NHibernate > would > >>>>>>>> now suddenly find this. Someone mentioned delaying it until the > next > >>>>>>>> major version. > >>>>>>>> > >>>>>>>> Since we are now aiming for 3.3, perhaps we should try to make a > >>>>>>>> decision on this. > >>>>>>>> > >>>>>>>> > >>>>>>>> Since we are only trying to load log4net if no other logging > >>>>>>>> framework > >>>>>>>> has been configured, I think the proposed change would be ok. > Also, > >>>>>>>> before the introduction of the logging abstraction, log4net would > >>>>>>>> always be loaded even when not configured. > >>>>>>>> > >>>>>>>> > >>>>>>>> /Oskar > >>>>>>> > >>>>>>> > >>>>>>> > >>>>>>> > >>>>>>> -- > >>>>>>> Fabio Maulo > >>>>>>> > >>>>>> > >>>>> > >>>>> > >>>> > >>>> > >>>> > >>>> -- > >>>> Fabio Maulo > >>>> > >>> > >> > > >
