-
Notifications
You must be signed in to change notification settings - Fork 204
Add Xamarin and WinStore/Phone support with PCL Profile 259 (.NET 4.5) #73
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
.NET 4.5 portable profile 259. Compatible with Xamarin for iOS and Android. PCL profile 259 description: portable-net45+netcore45+wpa81+wp8+MonoAndroid1+MonoTouch1
To avoid "already in use" errors when building from Visual Studio since they have the same name of the assembly file. Msbuild does not seem to have this problem.
Mostly related to reflection. Solved using compile conditions on PORTABLE45. WinStore introduced a new reflection API with TypeInfo and newer PCL profiles use this instead, but the existing Portable project is the older profile 136 that does not support this, so we need to distinguish between PORTABLE (profile 136) and PORTABLE45 (profile 259). The new reflection API is only available on .NET 4.5 and later, so as long as .NET 4.0 support is desired we will need to distinguish these two. An improvement would be to rename the PORTABLE condition to PORTABLE40. Breaking change in ExceptionFormatter.cs: Thread information is not available in profile 259 as far as I can see. I've set the string to "N/A" on the thread ID for now. Either we must inject a function to retrieve the thread ID using a platform specific API or find a way to access it from PCL profile 259.
It currently represents PCL profile 136 for .NET 4.0. Helps disinguish it from PORTABLE45 (profile 259).
For consistency among -net20, -net40 and -net45 variants.
Consistent paths for .net 2.0, 4.0 and 4.5. Fix paths nuspec file.
This is already covered by the portable target for .NET 4.0.
Also update checked in file to match contents after running a build. Note: I'm not entirely sure about the impact of these changes, specifically changing "portable" string to "portable40" and "portable45". I'd like some review feedback on it.
Generated by running build-package.bat
One issue I've found so far is that DebugOutLogger does not output anything to my Output window in Visual Studio when running the application in Debug configuration. During development of this pull request I built my own portable assembly in the app to test things out, and that worked. I built the assembly in Debug mode, and looking through the implementation it makes sense since DebugOutLogger simply calls Debug.WriteLine(), but that method call is removed by the compiler when compiling to Release, isn't it? If so, aren't the nugets always compiled in Release effectively rendering DebugOutLogger a NoOpLogger? A similar concern is discussed here: http://stackoverflow.com/a/9263339/134761 Hope you can provide some insight, maybe I've misunderstood something. Thanks! |
Sorry for the rant, but I've spent way too much time on this already and getting a little bit frustrated. So after I continued to hook this up to NLog in my WPF app, I installed the Common.Logging.NLog31 nuget, which brought in the non-portable Common.Logging library as a dependency. That gave me ConsoleOutLogger in the WPF app, nice, but I can't have both the portable and the non-portable logging nuget in my WPF app, and if I remove the portable one I get compile errors since one of my PCL libraries takes the ILog interface as a parameter, and it only knows about the portable one. I might be able to solve this with some binding redirects, but this is starting to smell bad. Let me know if I have misunderstood something, but here is my take on how it should have worked:
My proposed structure:
At least to me, and from a portable app standpoint, this makes a whole lot more sense. Adding support for other portable profiles in the future would also be a lot less pain if they didn't have to implement so much code, if any at all. Having just recently implemented this pull request, I know what part very well. |
My current workaround is creating my own PortableNLogLogger and PortableNLogLoggerAdapter, copied and modified from the Common.Logging.NLog nuget. This works with the portable common.logging nuget. |
Thanks for the comments/PR. From my read of this, you have come up against the blocker outlined here #71. Your proposed solution in re: "Common.Logging.Interfaces" sounds (nearly) identical to the outlined changes in Common.Logging.Core in that issue/thread, so I'm presuming that this set of changes in the PCL-EXPERIMENTATION branch (https://github.com/net-commons/common-logging/tree/pcl-experimentation) is likely to address your blockers as well (?) Can you comment on this --? |
Sorry, I've written my own log interface and adapter since then. It was just easier in the end. PortableLog. I have shamelessly copied quite a bit of the common-logging code, but added my own flavor to it. I hope you don't mind. You can close this pull request if you want. |
I don't mind at all; sorry you weren't able to wait for us to get the PCL story properly sorted on our end. Cheers, -Steve B. |
Thanks, cheers! |
Adds .NET 4.5 portable profile 259.
This profile is widely used and well supported by Xamarin for iOS and Android, as well as all the Windows targets for PC and mobile.
Target: portable-net45+netcore45+wpa81+wp8+MonoAndroid1+MonoTouch1
The biggest challenge was fixing all the reflection code to support the new reflection API around TypeInfo. This API is not supported in the current portable profile 136 for .NET 4.0, since it is only available in .NET 4.5 and later.
Portable nuget now has the following targets: .NET 3.5, .NET 4.0 (pcl) and .NET 4.5 (pcl)
Some cleanup:
I particularly want review on the following changes: