You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by cwe1ss <gi...@git.apache.org> on 2018/04/03 13:16:53 UTC

[GitHub] thrift pull request #1532: [WIP] THRIFT-4541: Use new project system in lib/...

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

----


---