Skip to content

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

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

Merged
merged 19 commits into from
Sep 21, 2017

Conversation

dmitriyse
Copy link
Contributor

@dmitriyse dmitriyse commented Jul 29, 2017

What does this implement/fix? Explain your changes.

Added dotnet core 2.0 build target. All tests are passed under win, py35, x64.
...

Does this close any currently open issues?

Work towards #96
...

Any other comments?

After a half of a year CoreCLR 1.0 will go away, so we can only offer CoreCLR 2.0 support.

Porting to CoreCLR 2.0 is much simpler than to CoreCLR 1.0. So we can do all the work right in the master. Xplat build fortunately can coexist side-by-side with the current build.
(See #518)

...

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

@mention-bot
Copy link

@dmitriyse, thanks! @vmuriart, @tiran, @brianlloyd, @filmor and @tonyroberts, please review this.

@codecov
Copy link

codecov bot commented Jul 29, 2017

Codecov Report

Merging #519 into master will decrease coverage by 0.18%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #519      +/-   ##
==========================================
- Coverage   77.34%   77.16%   -0.19%     
==========================================
  Files          65       65              
  Lines        5611     5610       -1     
  Branches      889      888       -1     
==========================================
- Hits         4340     4329      -11     
- Misses        983      993      +10     
  Partials      288      288
Flag Coverage Δ
#setup_linux 73.64% <100%> (+0.19%) ⬆️
#setup_windows 76.31% <100%> (-0.19%) ⬇️
Impacted Files Coverage Δ
src/runtime/nativecall.cs 100% <ø> (+2.22%) ⬆️
src/runtime/runtime.cs 90.98% <ø> (ø) ⬆️
src/runtime/pyobject.cs 38.85% <100%> (-0.29%) ⬇️
src/runtime/pyscope.cs 57.71% <100%> (-1%) ⬇️
setup.py 89.81% <100%> (+0.07%) ⬆️
src/runtime/pythonexception.cs 64.28% <100%> (-14.29%) ⬇️
src/runtime/metatype.cs 71.55% <100%> (ø) ⬆️
src/runtime/delegatemanager.cs 88.18% <100%> (-0.32%) ⬇️
... and 2 more

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 4b381bf...441dacf. Read the comment docs.

@dmitriyse
Copy link
Contributor Author

Tested under windows, miniconda python 3.5, x64. All tests are passed.

@den-run-ai
Copy link
Contributor

@dmitriyse why do you have 2 similar pull requests?

@dmitriyse
Copy link
Contributor Author

This #518 PR contains only new build system which is able to build all current targets for windows and linux. And contains absolutely minimum changes in the code.

But this request contains NetStandard 2.0 build target + fixes in code to allow compile and pass tests.

@dmitriyse
Copy link
Contributor Author

Please see this comment #518 (comment), this PR should add new build target after #518 will be merged into the master.

@den-run-ai
Copy link
Contributor

den-run-ai commented Aug 8, 2017

@dmitriyse can you merge these 2 PR? I still don't understand why you separated them? And I still don't see any testing done for .NET Core target.

@dmitriyse
Copy link
Contributor Author

@dmitriyse can you merge these 2 PR? I still don't understand why you separated them? And I still don't see any testing done for .NET Core target.

@denfromufa
Please see this comment #518 (comment)

@dmitriyse
Copy link
Contributor Author

dmitriyse commented Aug 21, 2017

Debug session finished. I need to fix points received from this session.
All such imports:

        [DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)]
        internal static extern int PyDict_SetItemString(IntPtr pointer, string key, IntPtr value);

Should be modified to

        [DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl, CharSet = CharSet.Ansi)]
        internal static extern int PyDict_SetItemString(IntPtr pointer, string key, IntPtr value);

And other build script fixes.

@dmitriyse dmitriyse force-pushed the coreclr2 branch 2 times, most recently from f7516e7 to 7bc892e Compare August 22, 2017 14:01
@dmitriyse
Copy link
Contributor Author

I managed to get EmbeddingTests works under NetCoreApp 2.0 target on all configurations except x86
x86 Support we can add later, once somebody will have enough time.

@dmitriyse
Copy link
Contributor Author

Both targets (net40 and netcoreapp2.0) are build and tested in the same --xplat CI configuration. It's simpler to implement and saves some time on awaiting build result.

@dmitriyse
Copy link
Contributor Author

Also I found a problem under NetCoreApp 2.0 - import "numpy" failed.

@dmitriyse
Copy link
Contributor Author

This PR is ready for merging to the master.
Current work allows to use python from the NetCoreApp 2.0 x64.

There are two main missing features, that can be implemented later:

  1. x86 Support for .NetCoreApp 2.0
  2. Access to .NetCoreApp 2.0 libraries from the genuine python.

@dmitriyse dmitriyse force-pushed the coreclr2 branch 5 times, most recently from cc7f5f8 to 307f1dd Compare August 28, 2017 13:12
DEBUG;TRACE fix for the case VS + non empty PYTHONNET_DEFINE_CONSTANTS
Copy link
Member

@filmor filmor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as #518, I think we should go ahead with the current state for now.

@filmor filmor merged commit df1c224 into pythonnet:master Sep 21, 2017
testrunner123 pushed a commit to testrunner123/pythonnet that referenced this pull request Sep 22, 2017
* Full featured xplat build.

* NetCoreApp 2.0 target added, compile issues fixed, CI system improved.

* .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.

* EmbeddingTest fixes, and stubs.

* Fix for importing numpy and other python modules with native parts.

* Changelog improved.

* Build order improvement.

* NetCoreApp 2.0 fix. EmitCalli does not supports cdecl. Falling-back to a slow Marshal-based solution. + x86 fix.

* All finalizers are disabled until valid implementation. Helps to avoid non relevant CI build faults.

* Mono builds now can be build on Windows.
DEBUG;TRACE fix for the case VS + non empty PYTHONNET_DEFINE_CONSTANTS

* Python.Runtime.dll now targets NetStandard2.0 inplace of NetCoreApp 2.0

* Wrong NETSTANDARD/NETCOREAPP define constant change.

* Typo fix.
dmitriyse pushed a commit that referenced this pull request Nov 16, 2017
dmitriyse pushed a commit that referenced this pull request Nov 16, 2017
dmitriyse added a commit that referenced this pull request Nov 16, 2017
@dmitriyse dmitriyse deleted the coreclr2 branch November 17, 2017 07:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants