search

cjbhaines / Log4Net.Async

Asynchronous Log4Net appenders and forwarder
http://www.nuget.org/packages/Log4Net.Async/
MIT License
121 stars 37 forks source link

Started migrating to net standard (supporting 1.3 - 1.6 and net40) #34

Closed wjdavis5 closed 4 years ago

wjdavis5 commented 7 years ago

I started the work to migrate this project to NET Standard 1.6.

There are some dependency issues with Rhino Mock. For now I have removed the Test project from the SLN. I have already updated the test project for net standard 1.6, however we'll need a new mocking library, or forgo the mocking entirely.

I noticed you'll need to update your msbuild version in appveyor too.

wjdavis5 commented 7 years ago

I'll be refactoring the tests using Moq as it is net standard compat.

wjdavis5 commented 7 years ago

Here are the tests that aren't passing:

image

I'm not really sure whats going on with the - ParallelForwarderFromConfigTest - tests. The XmlConfigurator implementation changed in log4net and my 30 seconds of Googling didnt yield good documentation. I'll take another swing at it tomorrow, if someone else doesnt beat me to it :)

wjdavis5 commented 7 years ago

Ok all tests are working now with the new NUnit3 Test Adapter in VS 2017, as well as from the command line with dotnet test.

I'm going to do some integration tests on my project that needs this package and I'll let you know.

wjdavis5 commented 7 years ago

I'm going to start working on the HttpContext issue. I wanted to see if you'd be able to get a nuget pre-release build started in the meantime so I can resolve this dependency in my application.

wjdavis5 commented 7 years ago

Sorry for the delay, but I think I have this wired up correctly now. Please take a look and let me know what you think.

wjdavis5 commented 7 years ago

Added back in support for NET40, so now the nuget that gets generated contains everything for NET40 and Net Standard 1.6

wjdavis5 commented 7 years ago

Nice work. I get what you are saying about no Windows identity in .net core but .net core's identity should carry. Probably already in the httpcontext.

Yeah I'll be able to figure this out. Just moved it for the time being to get the rest of the build stuff squared away.

If you update Appveyor to call dotnet build -c Release and change it to collect the generated nupkg file this could be ready for a pre-release soon

rcollette commented 7 years ago

Sorry for the delay in responding. I was on vacation and am now waiting for a rainy day to take me away from fixing my boat so I can verify this locally. As far as Appveyor changes, I don't believe @cjbhaines has given me access to that so he may need to participate.

wjdavis5 commented 7 years ago

Any updates?

cjbhaines commented 7 years ago

Hey guys, sorry I've been slammed at work. I don't have time to look at this at the moment but I can give you guys admin rights on the AppVeyor project if you want to continue with it. Drop me your email addresses and will arrange that ASAP.

rcollette commented 7 years ago

I finally had some time (on the soccer practice field) to try to run this locally. I am running on a Mac with DotNetCore 1.1 installed and the Mac version of Visual Studio 2017 with the Packet extension installed.

I opened the project and all dependencies pulled in, and I could build.

When I first went to run the tests it complained that it couldn't find the netstandard1.6 build of Log4Net.Async. I switched the build target for the Log4Net.async project in the IDE to to netstandard1.6 and the test project built fine. It added a targetFramwork (non-plural) element to the csproj file. I'm not sure if the targetFrameworks element of the csproj file is recognized by Mac Visual Studio 2017 (I have the latest version), or if it just needs that element as the current build. Without that element I think it was targeting netstandard 1.1, and it definitely doesn't build all of the targetFrameworks element specified frameworks as seems to be the documented behavior for normal visual studio.

When I try to run the tests I get the error "UserException: Unable to run tests. Test discover failed." It says "Stack Trace" below that but no stack trace is provided.

There about 22 warnings in the test project, most of them similar to: /usr/local/share/dotnet/sdk/1.0.3/Sdks/NuGet.Build.Tasks.Pack/build/NuGet.Build.Tasks.Pack.targets(5,5): Warning: Description: The assembly 'content/NUnit3.TestAdapter.dll' is not inside the 'lib' folder and hence it won't be added as reference when the package is installed into a project. (Log4net.Async.Tests)

I also see the warning: /usr/local/share/dotnet/sdk/1.0.3/Sdks/NuGet.Build.Tasks.Pack/build/NuGet.Build.Tasks.Pack.targets(5,5): Warning: Solution: Either modify the version spec of dependency "Microsoft.Extensions.Configuration [2.0.0-preview1-final, )" or update the version field in the nuspec. (Log4net.Async.Tests)

I tried updating the preview Test package to the 2.0.0 version but it requires .NET core 2.0. Should we just target .NET 2.0 at this point, and avoid using the alpha/preview package dependencies since it is released and there isn't a current base of dotnet core 1.x applications that depend on this library?

I ran the current master branch tests on my Mac and the unit tests that have timings were running almost twice as slow, causing those tests to fail, which is a little bit surprising. I've never been a fan of the timings in the tests because of the variances between machines and the build servers have tended to be much slower as well, which tends to result in failed tests. So had I been able to run the tests, in this branch, a couple would probably have failed.

I can try on my windows machine next, but if the intent is to produce a dotnetcore compatible library, it probably should all run on a Mac as I am trying to do now.

Sorry for the delay in reviewing. It's pretty hard to find time with late work nights and family coming first. Plus I had to refurbish the outdrive on my boat this summer which was an all consuming chore.

owlstack commented 7 years ago

Can I use this package for log4net support in .NET standard 1.4 project now? If so, how do I setup the configs for log4net since I get errors for using XmlConfigurator.Configure() and MethodBase.GetMethod() due to standard version without a main cs file. Thanks!

wjdavis5 commented 6 years ago

@rcollette - I'm not sure whats going on with your Mac having problems. I dont have one to test with, however I did pull the project onto my linux box and found a few issues. I've updated the PR with the changes required to get the tests passing on the linux machine

image

I also explicitly added in netstandard2.0, but its kind of redundant.

Looks like the appveyor script needs updated.

rcollette commented 6 years ago

I will be starting a dotnetcore 2.0 project at work this week. We are using Log4Net so hopefully I will get continuing with this in the not too distant future.

cjbhaines commented 6 years ago

Sorry everyone for the delay, we are well overdue sorting this out. Does anyone see any blockers to merging this? The AppVeyor build is failing but I'm happy to look at that post merge.

@rcollette any objections on this?

rcollette commented 6 years ago

While I am actually well into .NET core development on an active project. We have moved over to Serilog due to it's object logging capability (this is not an endorsement based on performance, capability or anything else other than it filled our need to log JSON structures easily)

My hesitation with this originally was, if you're going to move to .NET standard it would seem to be reasonable to expect it to run on .NET core (i.e. on a Mac.) I just have not had time to pull it and try it, and still don't.

I wouldn't hold any process up due to me at this point. Consider me the guy who did a decent bit of graffiti and now it's time for someone else's contribution to rise to the surface.

wjdavis5 commented 6 years ago

I've been running this code for a bit now: https://www.nuget.org/packages/Log4Net.Async.Standard/