Master build is broken: could not load logging services because assembly versions don't match

[jonorossi]

Castle.MicroKernel.SubSystems.Conversion.ConverterException : Could not convert string 'Castle.Services.Logging.Log4netIntegration.ExtendedLog4netFactory,Castle.Services.Logging.Log4netIntegration,Version=4.0.0.0, Culture=neutral,PublicKeyToken=407dd0808d44fbdc' to a type.
----> System.IO.FileLoadException : Could not load file or assembly 'Castle.Services.Logging.Log4netIntegration, Version=4.0.0.0, Culture=neutral, PublicKeyToken=407dd0808d44fbdc' or one of its dependencies. The located assembly's manifest definition does not match the assembly reference. (Exception from HRESULT: 0x80131040)

I've updating Windsor to use Castle Core 4.1.0, however because of a defect in the 4.x Castle Core build scripts (see castleproject/Core#288) the assembly version contains too much.

We need to get castleproject/Core#288 fixed and released, and bump the minimum version again otherwise consumers of Windsor will run into these errors on .NET Framework, we don't want users to have to deal with binding redirects.

[ghost]

I am working on a fix for this. Hard coded assembly strings in logging facility ... yowser!

288 is done in Core. I am also implementing it down here in the same PR. Should have fix for this soon.

[jonorossi]

Hard coded assembly strings in logging facility ... yowser!

Yer, predates me, the logging facility needs an overhaul so it doesn't know about log4net and NLog, it really doesn't need to anyway since Castle Core provides the abstractions anyway, just a silly convenience function.

[ghost]

The logging facility is definitely something that is useful but the problem I find is that many different assembly versions converge on a single piece code which does "magic string" based invocations of inheritors of Castle.Core.Logging.AbstractExtendedLoggerFactory.

The culprits are:

• Castle.Services.Logging.Log4netIntegration.ExtendedLog4netFactory
• Castle.Services.Logging.NLogIntegration.ExtendedNLogFactory
• Castle.Services.Logging.Log4netIntegration.Log4netFactory
• Castle.Services.Logging.NLogIntegration.NLogFactory

The problem we got is that subsequent releases of Castle.Core will almost always cause the Castle.Facilities.Logging to break. Looking at the history for LoggingFacility I can see that changing these "magic strings" is a thing for Castle Windsor releases:

I cannot tell you how I feel about this except to quote EJ:

Options

We have 2 options I can think of right now:

• Keep going with what we got and fix the problem.
• Reduce this to tidy documentation, deprecate it altogether and let the consumers decide how manage it themselves.

Keep going

We simply got to separate our concerns here, this means breaking changes(v5).

public enum LoggerImplementation
{
	Custom,
	Null,
	Console,
#if FEATURE_EVENTLOG
	Diagnostics,
#endif
#if CASTLE_SERVICES_LOGGING
	NLog,
	Log4net,
	ExtendedNLog,
	ExtendedLog4net,
#endif
	Trace
}

Becomes:

public enum LoggerImplementation
{
	Custom,
	Null,
	Console,
#if FEATURE_EVENTLOG
	Diagnostics,
#endif
	Trace
}

We retain Castle.Facilities.Logging but for old school bare bones System.Diagnostics implementations. We then spin up some new logging facilities, namely:

• Castle.Facilities.Logging.log4net
• Castle.Facilities.Logging.NLog
• Castle.Facilities.Logging.Serilog

These little guys carry explicit package references to their respective logging counter parts but it all comes with a maintenance cost. It also raises the bigger question of how many logging frameworks is Castle willing to support in the long term?

Deprecate

We write some tidy documentation for how one could "roll your own". This reduces the cost of maintenance/ownership but has an awful draw back of the documentation going stale and hanging around forever.

@jonorossi - I am a fan of logging but in all honesty I think the divergent logging implementation between Castle.Core and Castle.Windsor needs a complete architectural overhaul. I personally found these extra bit's and bob's painful to drag with me when I was doing fortress work. The idiosyncrasies between the various vendor libraries, versioning, cost of maintenance and their ability to port to netcore has raised some big question marks around their existence for me. If we had a clean slate here how would we do it now? Look forward to your thoughts on this.

[ghost]

If we don't deprecate, you might want consider v5 windsor @ #330. If we release this before Castle Core, then we wont have anymore facility logging drama! :)

[jonorossi]

My plan for a few years was to initially mark the log4net and NLog members of the LoggerImplementation enum obsolete, and the same with LoggingFacility.UseLog4Net and LoggingFacility.UseNLog methods, informing users to use LoggingFacility.LogUsing<>. This is one of the reasons I didn't add Serilog into the facility, it just doesn't need to be there.

It'll mean instead of:

container.AddFacility<LoggingFacility>(f => f.LogUsing(LoggerImplementation.Log4net));
... or
container.AddFacility<LoggingFacility>(f => f.UseLog4Net());

... it'll be as simple as:

container.AddFacility<LoggingFacility>(f => f.LogUsing<Log4netFactory>());

(where Log4netFactory is Castle.Services.Logging.Log4netIntegration.Log4netFactory
 living in Castle.Services.Logging.Log4netIntegration)

Basically cut the middle man out and you directly tell Windsor which type rather than the facility knowing how to get the type. No need for additional DLLs in Windsor.

I'm now just thinking we should just deprecate the LoggerImplementation enum completely since it is trivial to reference the logger type (e.g. NullLogger, ConsoleFactory) from Castle Core (or somewhere else) and since it isn't Windsor's problem to know about loggers other than that there is an ILoggerFactory abstraction and you provide one to it to use. It'll simplify released dependencies greatly, but we should still keep the unit tests because they work the facility well.

I've gone ahead and done it in #336 (not merged yet), and updated the Castle Core docs with the list of logger abstraction implementations it provides and removed the outdated list from Windsor. What do you think?

[jonorossi]

This is the new docs page in Castle Core:
https://github.com/castleproject/Core/blob/master/docs/logging.md

[ghost]

Basically cut the middle man out and you directly tell Windsor which type rather than the facility knowing how to get the type. No need for additional DLLs in Windsor.

Mega happy with this approach. Not having to spin up additional NuGet's did not feel right.

I'm now just thinking we should just deprecate the LoggerImplementation enum completely since it is trivial to reference the logger type (e.g. NullLogger, ConsoleFactory) from Castle Core (or somewhere else) and since it isn't Windsor's problem to know about loggers other than that there is an ILoggerFactory abstraction and you provide one to it to use.

I agree, it is as useful as a poo flavoured lollipop :) In #330 in case you went with option 1(not to deprecate) this would have been my next investigation, I had to copy it down into the vendor specific facilities. Yuk!

It'll simplify released dependencies greatly, but we should still keep the unit tests because they work the facility well.

Test should always be preserved. Add more if possible.

I've gone ahead and done it in #336 (not merged yet), and updated the Castle Core docs with the list of logger abstraction implementations it provides and removed the outdated list from Windsor. What do you think?

Good stuff mate, will review this. Give me some time to review it(prob tomorrow at this point), I am currently looking at some of the issues I worked on today in Windsor.

Thanks for your input.

[jonorossi]

Fixed by #336.