[jira] [Commented] (THRIFT-4541) Use new project system in "lib/csharp" and define supported platforms

["ASF GitHub Bot (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/THRIFT-4541?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16424001#comment-16424001 ] 

ASF GitHub Bot commented on THRIFT-4541:
----------------------------------------

GitHub user cwe1ss opened a pull request:

    https://github.com/apache/thrift/pull/1532

    [WIP] THRIFT-4541: Use new project system in lib/csharp

    See [THRIFT-4541](https://issues.apache.org/jira/browse/THRIFT-4541). 
    
    This is a WIP/PoC and **MUST NOT BE MERGED** (because supported versions must be defined first etc).
    
    This PR updates all project files to the new project system and targets ".NET Standard 2.0" and ".NET 4.5". (Except for the MSBuild-task which currently can't target .NET 4.5 due to its dependencies)
    
    Some notes about it:
    * The solution can now be built using the cross-platform .NET Core SDK (v2.0+). (using the cmd `dotnet build`)
    * I have not changed the MAKE files yet as I don't yet know how they work. I'll look into this once I know that we'll actually do this change.
    * NuGet packages for the main Thrift library and for the MSBuild library can be created with `dotnet pack -c Release`. They will be placed in the "lib\csharp\artifacts" folder.
    * `AssemblyInfo.cs` files are no longer required as most things are now defined either in `Directory.Build.props` or directly in the `*.csproj` files.
    * **Thrift** project:
      * As there's some classes that depend on "System.Web" I already had to add some #if statements. These classes are only available when the package is consumed on the full .NET framework.
      * I also had to change something in `TNamedPipeServerTransport` as .NET standard doesn't have a NamedPipeServerStream constructor that accepts a PipeSecurity. We'll have to look into how to test this.
    * **ThriftMSBuildTask** project:
      * I had to reference NuGet packages for MSBuild and these no longer contain the "Csc" class. I therefore had to change the implementation to manually call "csc". **This is not yet tested though!!** I wanted to show you the general approach first before I invest more time.
      * The project now creates a NuGet package that includes props files etc. I got this from [this how-to](https://www.natemcmaster.com/blog/2017/07/05/msbuild-task-in-nuget/).
    * The test projects contain `PreBuildEvents` that generate `*.cs` files for `thrift` files. This currently is executed too late which results in the first build to fail. I still have to look into this or we should discuss using a different approach (e.g. using the MSBuild task)
    
    I'm looking for general feedback on this approach. If the approach is ok, we should discuss in THRIFT-4541, which platforms we'd like to support. We can then either merge this into a branch or I can create a new PR with all changes.
    
    /cc @Jens-G @jeking3 

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/cwe1ss/thrift cweiss/Netstd

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/thrift/pull/1532.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1532
    
----
commit 550df5b23dc9ba28198b8ccb92db46fb41be64e3
Author: Christian Weiss <ch...@...>
Date:   2018-04-03T11:49:53Z

    Target netstandard2.0 and net45

----


> Use new project system in "lib/csharp" and define supported platforms
> ---------------------------------------------------------------------
>
>                 Key: THRIFT-4541
>                 URL: https://issues.apache.org/jira/browse/THRIFT-4541
>             Project: Thrift
>          Issue Type: Improvement
>          Components: C# - Library
>            Reporter: Christian Weiss
>            Priority: Major
>
> As discussed in THRIFT-4535, it would be great if we could update "lib/csharp" to use the new "csproj" project system. This will allow us to target multiple platforms and the new ".NET Standard" with a single project.
> It's possible to support pretty much every platform there is with this new project format (see e.g. [Newtonsoft.Json|https://github.com/JamesNK/Newtonsoft.Json/blob/master/Src/Newtonsoft.Json/Newtonsoft.Json.csproj]), however supporting older platforms requires more work as this requires more #if statements etc.
> This means that we have to decide which platforms we want to support!
> Targeting ".NET Standard 2.0" would be the easiest option as this version [covers a much larger API surface|https://blogs.msdn.microsoft.com/dotnet/2017/08/14/announcing-net-standard-2-0/]. However, this would also mean that people have to use rather recent versions of their platforms to use it, as .NET Standard 2.0 requires the following minimum versions:
>  * .NET 4.6.1 (adding support for .NET 4.5 is no problem though)
>  * .NET Core 2.0
>  * Mono 5.4
>  * Xamarin.iOS 10.14
>  * Xamarin.Mac 3.8
>  * Xamarin.Android 8.0
>  * UWP 10.0.16299
> I will create a PR that targets .NET Standard 2.0 and .NET 4.5 to show what the new project files would look like. If the approach is OK for you in general, then I can try to add support for whatever minimum versions you'd like to support.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)