net-commons / common-logging

A portable logging abstraction for .NET
http://net-commons.github.io/common-logging
Apache License 2.0
649 stars 203 forks source link

Using newer NLog versions? #144

Closed JonPaz closed 7 years ago

JonPaz commented 7 years ago

Hi,

I'm working on a system that uses NLog 4.4.3 (via a propriatory logging abstraction layer) and now pulls in other various (company common) components that use Common.Logging. Would I be correct in thinking that the current Common.Logging.NLog won't support v4.4.3 and I'll need to create an NLog4.4?

Thanks in advance.

sbohlen commented 7 years ago

There are technically two answers to your question -- the NuGet-based answer and the .NET runtime answer :)

re: NuGet, the latest version of the Common.Logging adapter for NLog looks to support NLog 4.2. This means that its not possible to have NuGet "match up" the dependencies of both Common.Logging.NLog for 4.2 and NLog 4.4.3.

However, if you're willing/able to resolve the dependencies manually (i.e., perhaps retrieve Common.Logging.NLog from NuGet but then up-version the referenced copy of the actual NLog binary to be your 4.4.3 version), you might be able to get this combination to work using a .NET binding redirect. This is entirely dependent upon NLog 4.4.3 being binary compatible (i.e., no changes to shape of public API) with NLog 4.2 (which I can't comment upon). Note that this approach will not induce NuGet to craft the binding redirect for you; instead you will need to craft the necessary entry(ies) in your app.config file by hand.

The ideal way to address this would be for you to craft a PR for Common.Logging to support NLog 4.4.3 so that others could benefit from using this adapter as well. If you've time to do this, please let us know -- otherwise, I can probably add the support myself sometime before the end of the coming weekend.

Thanks!

sbohlen commented 7 years ago

From a quick review, it looks to me as if NLog 4.4.4 is the latest (release) version in NuGet (https://www.nuget.org/packages/NLog/4.4.4)

Are you seeking support for 4.4.3 or 4.4.4 --?

JonPaz commented 7 years ago

Thanks @sbohlen,

In the spirit of having a go, I'll have a go at a PR to add support for v4.4.3 into Common.Logging, hopefully over the next day or so. Now to get VS2015 to re-install alongside VS2017 ;-)

JonPaz commented 7 years ago

Hi @sbohlen,

Looking at the solution, can I clarify which .Net framework version any new additions should target, is it still v4.0 ?

Cheers,

Jon.

JonPaz commented 7 years ago

Hi @sbohlen,

I'm following the build instructions on the main Github page and am having issues with there being multiple project.json in the solution.

Build output is:

NAnt 0.91 (Build 0.91.3881.0; alpha2; 17/08/2010) Copyright (C) 2001-2010 Gerry Shaw http://nant.sourceforge.net

Buildfile: file:///C:/Users/Fred/Source/Repos/common-logging/Common.Logging.build Target framework: Microsoft .NET Framework 4.0 Target(s) specified: build package-zip package-nuget

find-latest-msbuild.exe:

 [echo] Building all solutions using msbuild.exe found at C:\Program Files (x86)\MSBuild\14.0\Bin\msbuild.exe
 [echo] If build is failing due to e.g., 'invalid numeric comparison' errors, ensure you have the latest MSBUILD tools installed from https://www.microsoft.com/en-us/download/details.aspx?id=48159

clean:

[delete] Deleting directory 'C:\Users\Fred\Source\Repos\common-logging\build'. [delete] Deleting directory 'C:\Users\Fred\Source\Repos\common-logging\src\Common.Logging\obj'. [delete] Deleting directory 'C:\Users\Fred\Source\Repos\common-logging\src\Common.Logging.ApplicationInsights0170\obj'. [delete] Deleting directory 'C:\Users\Fred\Source\Repos\common-logging\src\Common.Logging.Core\obj'. [delete] Deleting directory 'C:\Users\Fred\Source\Repos\common-logging\src\Common.Logging.EntLib60\obj'. [delete] Deleting directory 'C:\Users\Fred\Source\Repos\common-logging\src\Common.Logging.Log4Net1215\obj'. [delete] Deleting directory 'C:\Users\Fred\Source\Repos\common-logging\src\Common.Logging.NLog41\obj'. [delete] Deleting directory 'C:\Users\Fred\Source\Repos\common-logging\src\Common.Logging.NLog44\obj'. [delete] Deleting directory 'C:\Users\Fred\Source\Repos\common-logging\src\Common.Logging.Portable\obj'. [delete] Deleting directory 'C:\Users\Fred\Source\Repos\common-logging\src\Common.Logging.Serilog1514\obj'. [delete] Deleting directory 'C:\Users\Fred\Source\Repos\common-logging\test\Common.Logging.ApplicationInsights0170.Tests\obj'. [delete] Deleting directory 'C:\Users\Fred\Source\Repos\common-logging\test\Common.Logging.BinaryCompatibility.Tests\obj'. [delete] Deleting directory 'C:\Users\Fred\Source\Repos\common-logging\test\Common.Logging.EntLib60.Tests\obj'. [delete] Deleting directory 'C:\Users\Fred\Source\Repos\common-logging\test\Common.Logging.EntLib60.Tests.Integration\obj'. [delete] Deleting directory 'C:\Users\Fred\Source\Repos\common-logging\test\Common.Logging.Integration.Tests\obj'. [delete] Deleting directory 'C:\Users\Fred\Source\Repos\common-logging\test\Common.Logging.Integration.Tests.Vb\obj'. [delete] Deleting directory 'C:\Users\Fred\Source\Repos\common-logging\test\Common.Logging.Log4Net1215.Integration.Tests\obj'. [delete] Deleting directory 'C:\Users\Fred\Source\Repos\common-logging\test\Common.Logging.Log4Net1215.Tests\obj'. [delete] Deleting directory 'C:\Users\Fred\Source\Repos\common-logging\test\Common.Logging.NLog41.Integration.Tests\obj'. [delete] Deleting directory 'C:\Users\Fred\Source\Repos\common-logging\test\Common.Logging.NLog41.Tests\obj'. [delete] Deleting directory 'C:\Users\Fred\Source\Repos\common-logging\test\Common.Logging.NLog44.Integration.Tests\obj'. [delete] Deleting directory 'C:\Users\Fred\Source\Repos\common-logging\test\Common.Logging.NLog44.Tests\obj'. [delete] Deleting directory 'C:\Users\Fred\Source\Repos\common-logging\test\Common.Logging.Serilog1514.Integration.Tests\obj'. [delete] Deleting directory 'C:\Users\Fred\Source\Repos\common-logging\test\Common.Logging.Serilog1514.Tests\obj'. [delete] Deleting directory 'C:\Users\Fred\Source\Repos\common-logging\test\Common.Logging.Tests\obj'. [delete] Deleting directory 'C:\Users\Fred\Source\Repos\common-logging\test\Common.Logging.TestUtils\bin'. [delete] Deleting directory 'C:\Users\Fred\Source\Repos\common-logging\test\Common.Logging.TestUtils\obj'.

update-dependencies:

restore-nuget-packages:

 [exec] MSBuild auto-detection: using msbuild version '14.0' from 'C:\Program Files (x86)\MSBuild\14.0\bin'.
 [exec] MSBuild auto-detection: using msbuild version '14.0' from 'C:\Program Files (x86)\MSBuild\14.0\bin'.
 [exec] All packages listed in packages.config are already installed.
 [exec] MSBuild auto-detection: using msbuild version '14.0' from 'C:\Program Files (x86)\MSBuild\14.0\bin'.
 [exec] All packages listed in packages.config are already installed.
 [exec] MSBuild auto-detection: using msbuild version '14.0' from 'C:\Program Files (x86)\MSBuild\14.0\bin'.
 [exec] All packages listed in packages.config are already installed.
 [exec] Restoring packages for C:\Users\Fred\Source\Repos\common-logging\src\Common.Logging.DotNetCore\project.json...
 [exec] Restoring packages for C:\Users\Fred\Source\Repos\common-logging\src\Common.Logging.Core.DotNetCore\project.json...
 [exec] Committing restore...
 [exec] Committing restore...
 [exec] C:\Users\Fred\Source\Repos\common-logging\src\Common.Logging.DotNetCore\Common.Logging.DotNetCore.xproj
 [exec] Restore completed in 327ms.
 [exec] C:\Users\Fred\Source\Repos\common-logging\src\Common.Logging.Core.DotNetCore\Common.Logging.Core.DotNetCore.xproj
 [exec] Restore completed in 326ms.
 [exec] 
 [exec] NuGet Config files used:
 [exec]     C:\Users\Fred\AppData\Roaming\NuGet\NuGet.Config
 [exec]     C:\ProgramData\nuget\Config\Microsoft.VisualStudio.Offline.config
 [exec] 
 [exec] Feeds used:
 [exec]     https://api.nuget.org/v3/index.json
 [exec]     C:\Program Files (x86)\Microsoft SDKs\NuGetPackages\
 [exec] MSBUILD : error MSB1011: Specify which project or solution file to use because this folder contains more than one project or solution file.

BUILD FAILED

C:\Users\Fred\Source\Repos\common-logging\Common.Logging.build(685,3): External Program Failed: dotnet (return code was 1)

Total time: 6.5 seconds.

sbohlen commented 7 years ago

Since all later versions of .NET support consuming all earlier versions of .NET, the goal we've tried to pursue where possible has always been to support the earliest version of .NET any given logger supports.

e.g., since anyone using .NET 4.6 can (safely) consume both a Common.Logging adapter and a logger built for .NET 4.0, there's generally no actual benefit to coding for the later .NET versions. Since the IL eventually produced by the JIT compiler from either is 100% identical, there's no run-time difference in re: either perf or features, meaning no penalty for coding to the earlier version.

You should be able to add this to the Common.Logging.2010.sln file, following the pattern I used a few days prior to push the support for NLog 4.4.4.

Make sense --?

JonPaz commented 7 years ago

Yep, sure does!

sbohlen commented 7 years ago

Hmmm...I don't get that error when I build locally and neither does the build server. Can you do a fresh CLONE into a new local folder and try the build again to see whether you're able to repro the success(es) elsewhere? This could help validate whether the issues are with e.g., your system (somehow) or with our repo (somehow).

JonPaz commented 7 years ago

@sbohlen ,

I've (finally) created the PR, please let me know your thoughts.

Also, do you have any idea as to when v3.4 will go RTM?

Cheers,

Jon.

sbohlen commented 7 years ago

@JonPaz

Thanks so much for the PR -- assume you resolved whatever build issue was blocking you (?)

I will take a look @ the PR this coming weekend and advise any comments. re: RTM timeline, see my reply here https://github.com/net-commons/common-logging/issues/145#issuecomment-287322738.

JonPaz commented 7 years ago

Hi @sbohlen ,

I think that the buld problem might have been related to not having the correct .Net Core SDK installed. It only told me when I opened the solution in Visual Studio - Or could just have been finger trouble ;-)

May I suggest the following small changes to the build instructions?

sbohlen commented 7 years ago

@JonPaz

Thx for the suggestions; I'll look at making those parts more clear. I've merged the PR so for now I will close this issue. Feel free to reopen it if any problems arise.

And thanks again for the contribution; its much-appreciated!

JonPaz commented 7 years ago

Thanks @sbohlen !

Glad to be of help and thanks for all of your efforts in keeping commons-logging going ;-)

Let me know if I can help in the future,

Jon.

somewhatabstract commented 7 years ago

@sbohlen Do you know when there will be a release with the new packages for Nlog newer than 4.1?

sbohlen commented 7 years ago

This is on my this-weekend-todo list, so I'm planning to get this updated and out formally before 7/17.

Defee commented 5 years ago

@sbohlen Is there any news about the Common.Logging and NLog 4.5.x? Have you got plans to support it?