Portable implementation of Common.Logging

[Daniel-Svensson]

These changes fixes #24 and make common.logging into a portable library with support for net40+sl4+win8+wp7 while still (mainly) staying binary compatible with older releases.

The changes are based on #21 so many thanks to @ioancrisan which started the implementation and gave feedback on the model with separate assemblies.

Major changes

Common.Logging is split into two assemblies

Refer to #21 for some discussion about this

• Common.Logging.Core:
  Which is the portable part, this is most things in Common.Logging except the TraceLogger and ConsoleLogger
• Common.Logging:
  This is intended to be the platform dependent part of Common.Logging
  For .Net it includes the .Net System.Diagnostics Logging

New logger

Since the Console is not portable a DebugLogger has been added to Common.Logging.Core

_ AbstractSimpleLoggerFactoryAdapter _

The new Common.Logging.Configuration.NameValueCollection is used instead of the one in System.Collection.Specializations since the later is not portable. It should be enough to change your using statements if you are implementing your own LoggerFactoryAdapter

Answers to probable questions

What about .Net 2.0-3.5:

There are .net 2.0 versions of Common.Logging and Common.Logging.Core

What do you mean by mostly binary compilant?

Most users should just be able to swap out their common.logging.dll with the new common.logging.dll and common.logging.core.dll , there there are some cases in which this is not possible and a new compilation is required.

• I did not keep the constructors taking in NameValueCollection for the EntLib libraries (but this can be added just as it is done for nlog and log4net.
• There are TypeForwarders for most classes which you are expected to use in Common.Logging, but if you are an "advanced" user implementing your own LoggingFactoryAdapter deriving from AbstractLoggingFactoryAdapter then you will have to recompile your code.

[sbohlen]

Thanks for the pull-request; this looks like a pretty good direction (from what I can see thus far). I started to take a look at this with an eye towards merging it into the master branch, but there are a few issues I'd like to have you look into before I proceed:

1. The 'Portable' branch of your repo (ref-ed in the pull-request) seems to be missing the bin for Microsoft.Practices.Unity (referenced into the EntLib41 test and integration test projects); can you please commit this to the repo so that the desired checkout-and-build (without extra steps) experience is still supported?

2. Can you clarify for me intent of the new collection of .sln files in this pull-request? (e.g., help me understand the difference in use between Common.Logging.2010.sln, Common.Logging.2010-net20.sln, and Common.Logging.2010-portable.sln).

3. None of the three above sln files seems to compile for me (even when I remove the entirety of the EntLib41 tests with the missing bin ref mentioned in 1) above. All three of them produce errors in the compilation of the vb test project (as noted below).

_BEGIN COMPILATION ERRORS AND WARNINGS_

Warning 1 Namespace or type specified in the Imports 'Common.Logging' doesn't contain any public member or cannot be found. Make sure the namespace or the type is defined and contains at least one public member. Make sure the imported element name doesn't use any aliases. C:_oss\common-logging-portable-exploration\test\Common.Logging.Integration.Tests.Vb\Class1.vb 1 9 Common.Logging.Tests.Integration.Vb.2010-net40

Error 2 Type 'Common.Logging.ILog' is not defined. C:_oss\common-logging-portable-exploration\test\Common.Logging.Integration.Tests.Vb\Class1.vb 9 20 Common.Logging.Tests.Integration.Vb.2010-net40

_END COMPILATION ERRORS AND WARNINGS_

Can you please confirm/verify that these sln files in fact compile/build on your end before I begin to investigate whether the problem is (somehow) on my end? As part of your attempting to repro this on your end, I'd recomment that you do a fresh 'clone' of the PORTABLE branch of your repo into a brand new folder locally and attempt to open/build from that (to repro my steps that are leading to the above error and warning).

4. The Common.Logging.build file needs to be revised to properly respect these new .sln files. As now, its assuming (incorrectly, since invoking NANT to build the whole thing fails :-/ ) that the Common.Logging.2010.sln file targets .NET 2.0. If you can provide the clarification requested in 2) above I can attempt to update/modify the .build file accordingly once these other issues have been resolved if you're not perhaps sufficiently familiar with the NANT tooling to do this yourself.

Let me know if any of the above is in any way unclear and we can work together to try to make more sense of it. Otherwise, thanks again for the pull-request and we're looking forward to being able to incorporate these changes into the project (and release updated NUGET packages to reflect these updated platform targets).

[Daniel-Svensson]

1. The 'Portable' branch of your repo (ref-ed in the pull-request) seems to be missing the bin for Microsoft.Practices.Unity

Don't know how the reference appeared, the only changes I intended to to was to change reference from Common.Logging to Common.Logging.Core. I have removed to reference to Unity

2. Can you clarify for me intent of the new collection of .sln files in this pull-request?

Common.Logging.2010-portable.sln should be identical to Common.Logging.2010.sln and can be removed.(replaces the old "-net40.sln")
Common.Logging.2010-net20.sln is the equivalent of the old "Common.Logging.2010.sln" and can be used to target .net 2.0

The rationale for the name change is that I think that the portable (.net 40 compatible) build should be considered the "default" /recommended build while the -net20 sln is intended only for projects which must target older .net versions.

3. None of the three above sln files seems to compile for me

Strange if I open Common.Logging.2010.sln in visual studio everything compiles just fine (even on a new checkout), are you building the projects from within visual studio and if so can you build them one and one to find when you get the first build error? Start with Common.Logging.Core and the Common.Logging (-net40 and sl40).

Have you installed Portable Library Tools 2 (http://visualstudiogallery.msdn.microsoft.com/b0e0b5e9-e138-410b-ad10-00cb3caf4981), I forgot to mention it but it is required in order to compile the Common.Logging.Core?

Have you enabled "Treat warnings as errors", currently I get 19 warnings when building the while solution, but everything compiles successfully.

4. The Common.Logging.build file needs to be revised to properly respect these new .sln files

I've tried to update the file myself, but I don't have NANT installed and have no prior experience with it so please take a look at it yourself and feel free to fix any mistakes.

[sbohlen]

Thanks for the clarification(s)! I'm traveling this coming week and so
likely won't have a chance to dig into this until I return, but its queued
up for next weekend for sure. I'll dig into this and let you know my
results after.

Thanks again for your contributions to the project!

Steve Bohlen
[email protected]
http://blog.unhandled-exceptions.com
http://twitter.com/sbohlen

On Sat, Jul 20, 2013 at 7:39 AM, Daniel-Svensson
[email protected]:

1. The 'Portable' branch of your repo (ref-ed in the pull-request) seems
   to be missing the bin for Microsoft.Practices.Unity

Don't know how the reference appeared, the only changes I intended to to
was to change reference from Common.Logging to Common.Logging.Core. I have
removed to reference to Unity

2. Can you clarify for me intent of the new collection of .sln files in
   this pull-request?

Common.Logging.2010-portable.sln should be identical to
Common.Logging.2010.sln and can be removed.(replaces the old "-net40.sln")
Common.Logging.2010-net20.sln is the equivalent of the old
"Common.Logging.2010.sln" and can be used to target .net 2.0

The rationale for the name change is that I think that the portable (.net
40 compatible) build should be considered the "default" /recommended build
while the -net20 sln is intended only for projects which must target older
.net versions.

3. None of the three above sln files seems to compile for me

Strange if I open Common.Logging.2010.sln in visual studio everything
compiles just fine (even on a new checkout), are you building the projects
from within visual studio and if so can you build them one and one to find
when you get the first build error? Start with Common.Logging.Core and the
Common.Logging (-net40 and sl40).

Have you installed Portable Library Tools 2 (
http://visualstudiogallery.msdn.microsoft.com/b0e0b5e9-e138-410b-ad10-00cb3caf4981),
I forgot to mention it but it is required in order to compile the
Common.Logging.Core?

Have you enabled "Treat warnings as errors", currently I get 19 warnings
when building the while solution, but everything compiles successfully.

4. The Common.Logging.build file needs to be revised to properly respect
   these new .sln files

I've tried to update the file myself, but I don't have NANT installed and
have no prior experience with it so please take a look at it yourself and
feel free to fix any mistakes.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/30#issuecomment-21292296
.

[Daniel-Svensson]

Have you had some time to look any more at this?

I took a second look through it and have some further comments, I would appreciate your feedback.

Improving backwards compatibility

The currently committed code right now requires you to update all implementations (including all offical loggers) which derive from AbstractSimpleLoggerFactoryAdapter and which use the constructor taking a System.Configuration.NameValueCollection (since the constructor is removed). This means that you would have to update these if you update common.logging itself.

If we instead move the portable AbstractSimpleLoggerFactoryAdapter to another namespace such as "Core" then we can implement a AbstractSimpleLoggerFactoryAdapter with the old constructor in the "Common.Logging" assembly meaning that we should be able to become 100% backwards compatible.
My suggestion would then be to mark this Common.Logging.Simple.AbstractSimpleLoggerFactoryAdapter as depreciated with a comment that the (portable) Common.Logging.Core.AbstractSimpleLoggerFactoryAdapter should be used instead.

This does however raise another question and this is what other classes we should do this with?

• My suggestion in that case is to "move" the "AbstractSimpleLogger**" classes as well as the "ArgUtils" and "LogSetting" so they are defined under a Common.Logging.Core namespace. This way they can be defined in their "old namespaces" in the platform dependend Common.Logging assembly to match the previous public API.

An alternative would be to do the same for all portable classes under Common.Logging.Simple, while it will make it much more straightforward to know what is portable or not I am not sure about if it is worth it .

Bring back AllowPartiallyTrustedCallers

The previous assemblies of common.logging have been marked with AllowPartiallyTrustedCallers, this was not possible using the portable library configuration I submitted previously.
I think it would be a good idea to maybe provide a second compilation target for Common.Logging.Core which produces a portable library targeting only SL5+NET40+WINRT+WP8 where we add back the AllowPartiallyTrustedCallers attribute to the assembly.

Changes I missed to comment on

• Classes in Common.Logging.Core are no longer Serializable.
• The AllowPartiallyTrustedCallers attribute was removed for Common.Logging.Core, but it should be straightforward to add a second portable build in order to bring it back.

Possible future changes

It would be possible to move most of the code (the code which parses XML and returns a LogSetting) in the ConfigurationSectionHandler to the portable assembly, and make ConfigurationSectionHandler either call that code or inherit from it. This is a non-breaking change so it could be part of a future minor version if other platforms than the .net desktop platform would require xml configuration support.

[lianzhao]

@Daniel-Svensson Great Job! I believe many people like me are looking for a portable logging framework. Will you pls make a nuget package and upload to nuget.org?

[ghuntley]

+1 on merging/packaging this up on NuGet!

[sbohlen]

This is actually now DONE (locally) and I just yesterday managed to get it fully-merged and building again in the MASTER branch.. My plans are to push this back to ORIGIN here and release the corresponding nuget packages later today. Sorry this has taken so long, but life got in my way :P

[ghuntley]

❤️