CoreCLR msbuild (xplat) support. Initial compilable version. #518

dmitriyse · 2017-07-28T22:06:03Z

What does this implement/fix? Explain your changes.

This is initial step in a long road to add "xplat" build tools support into python.net.
...

Does this close any currently open issues?

Not closes but will helps to solve those:
#96

...

Any other comments?

Or even do everything in the master, because this xplat support should be implemented as side-by-side to the current build system.
Later in the branch coreclr2 we can track master and also allow to build NetStandart 2.0 target.
...

Checklist

Check all those that are applicable and complete.

[] Make sure to include one or more tests for your change
[] If an enhancement PR, please create docs and at best an example
[] Add yourself to AUTHORS
[] Updated the CHANGELOG

den-run-ai · 2017-07-28T22:11:23Z

@dmitriyse can you merge *.15.csproj and *.15.sln as options to Solution Configurations next to *Win* and *Mono* options in the regular pythonnet.sln and its projects?

dmitriyse · 2017-07-28T22:18:54Z

@dmitriyse can you merge *.15.csproj and *.15.sln as options to Solution Configurations next to Win and Mono options in the regular pythonnet.sln and its projects?
May be it is possible to use single .sln file (i will try). But it is definitely impossible to fit xplat and classic builds into one csproj file.
Take a look at the first lines of classic a *.csproj and a new *.csproj 2017.

2017:

<Project Sdk="Microsoft.NET.Sdk">

2000+:

<Project DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003" ToolsVersion="4.0">

Msbuild and VS uses totally different toolsets for those project types.

dmitriyse · 2017-07-28T22:23:27Z

Also I think we need use only xplat in the future.
It can compile totally all targets, any runtime, any platform, any CLR version.

So current build system should become legacy. But some part of time (migration period) both build system should co-exits.

So current build system and xplat build system should have the same set of solution configurations.
Thus I also think usage of single .sln file is a bad idea (even if is possible).

den-run-ai · 2017-07-28T22:31:08Z

@dmitriyse can you point to any Microsoft references for understanding about the migration of build systems?

Also have you read this document, which suggests that it is possible to support both runtimes from the same solution?

https://docs.microsoft.com/en-us/dotnet/core/porting/project-structure

dmitriyse · 2017-07-28T22:32:34Z

P.S. Sorry, but I will create PR only for parts that I use in my commercial projects. I have no free time to support xplat build system for all pythonnet targets. But my contributions contain well-tested in the production solutions, and even if some PR is not complete, it can be merged into the main repo to move work forward.

dmitriyse · 2017-07-28T22:34:39Z

https://docs.microsoft.com/en-us/dotnet/core/porting/project-structure

Thank you for pointing. I will review this article, probably there is some better solution than in this PR.

dmitriyse · 2017-07-28T22:39:03Z

MS suggest the near the same solution that contained in this PR.
But, if we will do exactly like MS suggest, than current pythonnet build system will completely broken.
So this PR is still relevant.

codecov · 2017-07-28T22:40:24Z

Codecov Report

Merging #518 into master will increase coverage by 0.87%.
The diff coverage is 94.04%.

@@            Coverage Diff             @@
##           master     #518      +/-   ##
==========================================
+ Coverage   76.47%   77.34%   +0.87%     
==========================================
  Files          64       65       +1     
  Lines        5535     5611      +76     
  Branches      889      889              
==========================================
+ Hits         4233     4340     +107     
+ Misses       1014      983      -31     
  Partials      288      288

Flag	Coverage Δ
#setup_linux	`73.45% <73.17%> (?)`
#setup_windows	`76.49% <77.38%> (+0.01%)`	⬆️

Impacted Files	Coverage Δ
src/runtime/runtime.cs	`90.98% <50%> (+0.39%)`	⬆️
setup.py	`89.73% <95.12%> (+18.52%)`	⬆️
src/runtime/delegatemanager.cs	`88.49% <0%> (-0.11%)`	⬇️
src/tests/_compat.py	`100% <0%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4bac694...afc90d5. Read the comment docs.

dmitriyse · 2017-07-28T22:45:15Z

@dmitriyse can you point to any Microsoft references for understanding about the migration of build systems?

Unfortunately, no. I wasn't faced with any one.
I am just sitting on the bleeding edge tools and know what they are really can (or cannot) now.

den-run-ai · 2017-07-28T22:59:09Z

@dmitriyse note that you skipped the first option suggested in that article :)

https://docs.microsoft.com/en-us/dotnet/core/porting/project-structure#replace-existing-projects-with-a-multi-targeted-net-core-project

dmitriyse · 2017-07-29T04:11:12Z

@dmitriyse note that you skipped the first option suggested in that article :)
https://docs.microsoft.com/en-us/dotnet/core/porting/project-structure#replace-existing-projects-with-a-multi-targeted-net-core-project

This is the revolution way. xbuild and old msbuild (prior to 15.x) will fail to build anything.
This would be a huge breaking change for python.net users, and will stress contributors to solve big number of problems at once.
IMO this way is a worst one.

vmuriart · 2017-07-30T18:45:55Z

I agree w @denfromufa that I'd rather combine the solution files into a single solution. If this means that contributors will need to use VS2017 to build but otherwise it outputs the same, I'm ok with this. Most windows users likely install from pypi anyways.

If combining them creates issues for mono, then I'd hold off altogether. My main concern for it is based on the clean-up I did earlier in the year in which we had 3 or 4 different solution files because of multiple upgrades of VS and then those files lingered around for a long time.
Second problem is that these changes are not being tested at all as part of the CI. If a breaking change is introduced somewhere in the code, we will be oblivious to it for a while.

dmitriyse · 2017-07-30T20:38:53Z

If combining them creates issues for mono, then I'd hold off altogether.

What do you mean ?

If combining them creates issues for mono, then I'd hold off altogether. My main concern for it is based on the clean-up I did earlier in the year in which we had 3 or 4 different solution files because of multiple upgrades of VS and then those files lingered around for a long time.

We can contain one set of csproj files and one sln file only if we totally move to VS 2017/Xplat build system. (And MonoDevelop users should migrate to JetBrains Project Rider. )
This work is too long, thus even in this revolutionary case python.net project should have
two set of csproj files and two sln files, for new and old build system, but in different git branches.

So the choice is between

Avoid NetCoreApp 2.0 target and use workarounds(see later)
use two sets of csproj files in the same branch
some period of time maintain two branches with old and new build systems.

I prefer option 2) - use two sets of csproj files, because option 3) is a different type of hell with merging with even more required efforts to maintain two sln files (and csproj files).

dmitriyse · 2017-07-30T20:44:23Z

NetCoreApp 2.0 offer ability to reference binaries that targets framework 4.6.1 and earlier. I did not try this ability. But in this case existing pythonnet binaries can be referenced to Linux CoreCLR 2.0 projects as is.

But even if this magic is really exists, we needs to duplicate all CI builds for CoreCLR 2.0 hosts.

dmitriyse · 2017-07-30T20:57:47Z

Off topic. I have a bunch of stability&performance fixes for pythonnet/CoreCLR from the high-load production project (Currently they are contained here https://github.com/dmitriyse/pythonnet/tree/pythonnet-plus).

Lacks of official support of CoreCLR stops me from creating pull requests.

den-run-ai · 2017-07-31T06:00:35Z

@dmitriyse even if we decide together to split into 2 projects/solutions, still this pull request does not include testing for new set of .NET Core DLLs. Can you please add this?

After some more thinking, I might actually incline towards two solutions. The reason is that the current setup.py assumes .NET Framework for Windows, and Mono for other platforms.

But we need a new setup.py that uses .NET Core on all platforms.
Of course we could also add a flag to setup.py to be able to use .NET Core as a non-default option like described here:

https://stackoverflow.com/questions/677577/distutils-how-to-pass-a-user-defined-parameter-to-setup-py

dmitriyse · 2017-07-31T07:58:58Z

Yes, I will try to inject some option in the setup.py to allow build the same output but with xplat build tools and new *.15.csproj + *.15.sln files.
Finally we needs to have 3 times more CI build configurations:

net40 + Current Tools (msbuild < 15.0, mono)
net40 + xplat build tools
NetStandard20 + xplat build tools

Only with this set of CI builds we can ensure that codebase have no regression.

Current PR relates only for new build system. NetStandard 2.0 target should be added by this PR #519

vmuriart · 2017-07-31T14:39:27Z

@dmitriyse what I meant earlier is due to the fact that Linux users will be building the project alot more often than windows users, so if there's any regressions introduced there because of having a single solution on then I'd rather keep the current solution file.

I'm ok with having the dual solutions as long as we add the CI for them and open an issue to eventually merge them back into one but leave a transition period in which we'll have both solution files in existence.

On the CI, I'm ok tripling the build time for now so that we test out all configurations. After the PR is merged in, I'll rewrite the CI config to test all PY version on one configuration, and then limit the other configuration to subset of them.

dmitriyse · 2017-07-31T17:21:42Z

Eventually we can totally move from mono/msbuild < 15.0 to xplat/msbuild >15,0 and keep all users onboard. Eventually even MonoDevelop/XamarinStudio will obtain xplat/NetCoreApp 2.0 support (The work is in progress http://lastexitcode.com/blog/2016/06/05/AspNetCoreRC2SupportInXamarinStudio).

Thank you for acceptance. I will try to find a time soon to improve setup.py as a part of this PR.

dmitriyse · 2017-08-08T22:05:21Z

Now it's not the same pull requests :).
As I already say, this PR covers only injection of the new build system (xplat).
So we can build just the same output (net40 assemblies) under linux and under windows but with CoreCLR tooling.

New tooling requires completely different .sln and csproj files.

Netcoreapp 2.0 target covered by anther PR.

We needs to ensure that with the same sources and the same build targets new build system works good.
So "step by step".

This is prerequisite step before Netcoreapp 2.0. Because Netcoreapp2.0 can be build only with CoreCLR tooling.

dmitriyse · 2017-08-08T22:19:31Z

On the CI, I'm ok tripling the build time for now so that we test out all configurations. After the PR is merged in, I'll rewrite the CI config to test all PY version on one configuration, and then limit the other configuration to subset of them.

@vmuriart, you can create new CI configs with setup.py build_ext --inplace --xplat build command and CoreCLR 2.0 Preview 2.0+ installed.
(see https://www.microsoft.com/net/core/preview#linuxubuntu
and https://www.microsoft.com/net/core/preview#windowscmd)

Currently --xplat option implementation is just in very beginning, but it passes package restore and build.
(setup.py, *15.csproj , pythonnet.15.sln should be improved, to create the same output as mono and msbuild)

den-run-ai · 2017-08-08T22:23:18Z

@dmitriyse i don't see --xplat being passed to setup.py in appveyor.yml and travis.yml.

den-run-ai · 2017-08-08T22:25:04Z

looks like hitting this issue with python 3.6:

pypa/setuptools#1101

dmitriyse · 2017-08-08T22:48:52Z

@dmitriyse i don't see --xplat being passed to setup.py in appveyor.yml and travis.yml.

I did not figure out that I can change build options just in the PR to perform tests, until it's merged to the master.
After some improvements it will be reasonable to perform CI tests.

P.S.
Just before merging to the master we needs to find a way to multiply build configurations (with --xplat and without --xplat).

DEBUG;TRACE fix for the case VS + non empty PYTHONNET_DEFINE_CONSTANTS

…t vars introduced.

…troduced. + Fix for building mono versions under windows.

den-run-ai · 2017-09-13T19:11:04Z

setup.py

+        """Return full path to one of the Microsoft build tools"""
+        try:
+            basePathes = subprocess.check_output(
+                ["vswhere", "-latest",


@dmitriyse can you change this to tools\vswhere\vswhere.exe like I did in my pull request #540?

I added tools\vswhere\vswhere.exe file into the repository instead of chocolatey installation.
See PR #546

den-run-ai · 2017-09-13T19:28:26Z

setup.py

+            _custom_define_constants = False
+        elif DEVTOOLS == "dotnet":
+            _xbuild = 'dotnet msbuild'
+            _config = "{0}Mono".format(CONFIG)


@dmitriyse why dotnet DEVTOOLS corresponds to Mono _config?

The first problem:
DebugMonoPY3, ReleaseMono, etc. build configuration is a historically names from times where nothing than mono had exists for the linux.
Now it is more correctly to name DebugLinuxPY3, ReleaseLinux, etc.

But we "cannot just rename" build configurations without affecting users.
We should introduce new build configurations (DebugLinuxPY3, ReleaseLinux), give a time to users to migrate to new names, and than remove old names DebugMonoPY3, ReleaseMono, etc.

The second problem:
We cannot use dotnet build command under windows due to DllExport project msbuild targets. We needs to use msbuild >=15 under windows

The result:

DEVTOOLS=dotnet ==> linux (--xplat) === ReleaseLinuxPY3 <== ReleaseMonoPY3

So currently we have DOVTOOLS=dotnet <===> ReleaseMonoPY3

If I am wrong in somewhere in my conclusions, lets improve something.

den-run-ai · 2017-09-13T20:01:28Z

@dmitriyse you mentioned that you are not using MonoDevelop, how are you testing these 2 solutions on Linux and OSX? Are you using Rider IDE on these platforms?

I tested and both solutions look good on Windows in VS 2017, just don't try to open both solutions at the same time :)

den-run-ai · 2017-09-13T20:22:33Z

@dmitriyse can you please avoid a lot of duplication in travis.yml in the matrix by using excludes?

https://docs.travis-ci.com/user/customizing-the-build/#Excluding-Jobs

filmor

This works for me, denfromufa is right about the matrix thing, but that can easily be added in a new PR. Let's go ahead and merge this first.

den-run-ai · 2017-09-21T14:12:23Z

@filmor @dmitriyse note that some comments are still not addressed:

#518 (comment)
#518 (comment)
#518 (comment)

dmitriyse · 2017-09-21T15:39:40Z

I will "address" those today. Please sorry for delay.

dmitriyse · 2017-09-21T22:14:27Z

Answering to the #518 (comment)
Unfortunately travis-ci currently does not supports conditional "addons" section.
See travis-ci/travis-ci#3618
So exclude solution is totally impossible now and in near future.

But fortunately we have a little yaml reference sugar, I will try to apply it.

dmitriyse · 2017-09-21T22:44:46Z

I was managed to solve copy-past .travis.yaml problem with help of yaml references Solution is included into #546

dmitriyse · 2017-09-21T23:30:03Z

@dmitriyse you mentioned that you are not using MonoDevelop, how are you testing these 2 solutions on Linux and OSX? Are you using Rider IDE on these platforms?
I tested and both solutions look good on Windows in VS 2017, just don't try to open both solutions at the same time :)

I tried Project rider 2017.1.2 for the pythonnet.15.sln and...
It's unbelievable:

I am successfully build, run, debug, and run/debug unit tests for .Net 4.0
I am successfully build and run CoreApp 2.0 target, but unable to debug it and unable to run/debug NUnit tests

Probably full support will be added soon.

But in general Project rider works great and looks like a very mature tool.
RIP MonoDevelop. (Nothing personal, this product played big role)

P.S. some fix for better usability inside IDEs are added to #546

dmitriyse · 2017-09-21T23:32:25Z

We can advertise full migration to --xplat in the 2.5 release. IDEs already can handle development in any OS.

…et#518) * Full featured xplat build. * .Net 45 TargetingPack System.XML.dll naming fix. (For xplat linux build). * Setup.py --xplat option refactored. Travis-ci build matrix extended. * AppVeyor matrix extended, xplat build added. * appveyor.yml yaml syntax fix. * NUnit dependency upgraded to 3.7. Changelog improved. * Build order improvement. * Mono builds now can be build on Windows. DEBUG;TRACE fix for the case VS + non empty PYTHONNET_DEFINE_CONSTANTS * PYTHONNET_PY3_VERSION, PYTHONNET_PY2_VERSION build related environment vars introduced. * Small compile fixes. * PYTHONNET_WIN_DEFINE_CONSTANTS and PYTHONNET_MONO_DEFINE_CONSTANTS introduced. + Fix for building mono versions under windows.

#518 #519 #546 merge fix

Full featured xplat build.

0abfde0

den-run-ai requested review from filmor, tonyroberts, den-run-ai and vmuriart July 28, 2017 22:07

dmitriyse mentioned this pull request Jul 29, 2017

Added CoreCLR 2.0 build target. Compile issues fixed. #519

Merged

4 tasks

dmitriyse force-pushed the xplat branch from c018696 to 2ac981b Compare August 28, 2017 10:08

Mono builds now can be build on Windows.

ed48618

DEBUG;TRACE fix for the case VS + non empty PYTHONNET_DEFINE_CONSTANTS

dmitriyse force-pushed the xplat branch from 0852392 to ed48618 Compare September 6, 2017 14:28

den-run-ai added the requires review label Sep 6, 2017

dse added 2 commits September 7, 2017 12:49

PYTHONNET_PY3_VERSION, PYTHONNET_PY2_VERSION build related environmen…

966f035

…t vars introduced.

Small compile fixes.

f4a6608

dmitriyse force-pushed the xplat branch from 347ecd7 to f4a6608 Compare September 7, 2017 10:25

PYTHONNET_WIN_DEFINE_CONSTANTS and PYTHONNET_MONO_DEFINE_CONSTANTS in…

a89685e

…troduced. + Fix for building mono versions under windows.

dmitriyse mentioned this pull request Sep 11, 2017

Update setup.py - use vswhere for msbuild 15+ #540

Closed

4 tasks

den-run-ai reviewed Sep 13, 2017

View reviewed changes

Merge branch 'master' into xplat

afc90d5

filmor approved these changes Sep 21, 2017

View reviewed changes

filmor merged commit 4b381bf into pythonnet:master Sep 21, 2017

dmitriyse mentioned this pull request Sep 21, 2017

Build system improvements pack - addressing comments to PR and some issues. #546

Merged

4 tasks

dmitriyse mentioned this pull request Sep 22, 2017

Python to CLR string marshaling LRU cache. #538

Closed

4 tasks

dmitriyse mentioned this pull request Sep 28, 2017

Interop methods with Py_ssize_t works differently in NetCoreApp 2.0 #531

Merged

4 tasks

dmitriyse pushed a commit that referenced this pull request Nov 16, 2017

Merge #518

2ba6bff

dmitriyse pushed a commit that referenced this pull request Nov 16, 2017

backout #518, wrong merge !

1c79426

dmitriyse added a commit that referenced this pull request Nov 16, 2017

Merge pull request #573 from pythonnet/master-merge-fix

f5a002f

#518 #519 #546 merge fix

dmitriyse deleted the xplat branch November 17, 2017 07:36

CoreCLR msbuild (xplat) support. Initial compilable version. #518

CoreCLR msbuild (xplat) support. Initial compilable version. #518

Conversation

dmitriyse commented Jul 28, 2017 • edited Loading

What does this implement/fix? Explain your changes.

Does this close any currently open issues?

Any other comments?

Checklist

den-run-ai commented Jul 28, 2017

dmitriyse commented Jul 28, 2017 • edited Loading

dmitriyse commented Jul 28, 2017

den-run-ai commented Jul 28, 2017

dmitriyse commented Jul 28, 2017

dmitriyse commented Jul 28, 2017

dmitriyse commented Jul 28, 2017

codecov bot commented Jul 28, 2017 • edited Loading

Codecov Report

dmitriyse commented Jul 28, 2017 • edited Loading

den-run-ai commented Jul 28, 2017

dmitriyse commented Jul 29, 2017

vmuriart commented Jul 30, 2017

dmitriyse commented Jul 30, 2017 • edited Loading

dmitriyse commented Jul 30, 2017

dmitriyse commented Jul 30, 2017

den-run-ai commented Jul 31, 2017

dmitriyse commented Jul 31, 2017 • edited Loading

vmuriart commented Jul 31, 2017

dmitriyse commented Jul 31, 2017 • edited Loading

dmitriyse commented Aug 8, 2017 • edited Loading

dmitriyse commented Aug 8, 2017 • edited Loading

den-run-ai commented Aug 8, 2017 • edited Loading

den-run-ai commented Aug 8, 2017

dmitriyse commented Aug 8, 2017 • edited Loading

den-run-ai Sep 13, 2017

Choose a reason for hiding this comment

dmitriyse Sep 21, 2017

Choose a reason for hiding this comment

den-run-ai Sep 13, 2017 • edited Loading

Choose a reason for hiding this comment

dmitriyse Sep 21, 2017 • edited Loading

Choose a reason for hiding this comment

den-run-ai commented Sep 13, 2017 • edited Loading

den-run-ai commented Sep 13, 2017

filmor left a comment

Choose a reason for hiding this comment

den-run-ai commented Sep 21, 2017

dmitriyse commented Sep 21, 2017

dmitriyse commented Sep 21, 2017

dmitriyse commented Sep 21, 2017

dmitriyse commented Sep 21, 2017

dmitriyse commented Sep 21, 2017

dmitriyse commented Jul 28, 2017 •

edited

Loading

dmitriyse commented Jul 28, 2017 •

edited

Loading

codecov bot commented Jul 28, 2017 •

edited

Loading

dmitriyse commented Jul 28, 2017 •

edited

Loading

dmitriyse commented Jul 30, 2017 •

edited

Loading

dmitriyse commented Jul 31, 2017 •

edited

Loading

dmitriyse commented Jul 31, 2017 •

edited

Loading

dmitriyse commented Aug 8, 2017 •

edited

Loading

dmitriyse commented Aug 8, 2017 •

edited

Loading

den-run-ai commented Aug 8, 2017 •

edited

Loading

dmitriyse commented Aug 8, 2017 •

edited

Loading

den-run-ai Sep 13, 2017 •

edited

Loading

dmitriyse Sep 21, 2017 •

edited

Loading

den-run-ai commented Sep 13, 2017 •

edited

Loading