Skip to content

Initial coreclr and build tooling #612

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

Closed
wants to merge 18 commits into from
Closed

Conversation

djoyce82
Copy link

Can load .NET Core into python

Pass clr PyObject back to Python

What does this implement/fix? Explain your changes.

Allows Python.Runtime to be embedded in python via the .NET Core CLR

...

Does this close any currently open issues?

Partially addresses #96
...

Any other comments?

...

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

Can load .NET Core into python

Pass clr PyObject back to Python
@Cronan
Copy link
Contributor

Cronan commented Feb 1, 2018

@denfromufa @djoyce82 @dmitriyse
Failing CI builds look like the tests failing due to the issue with Typebuilder discussed in #96
Can anyone think of an elegant way (or a way) to skip some tests, but only in .Net Core?


// Walk the directory for each extension separately so that we first get files with .ni.dll extension,
// then files with .dll extension, etc.
for (size_t extIndex = 0; extIndex < sizeof(tpaExtensions) / sizeof(tpaExtensions[0]); extIndex++)
Copy link
Contributor

Choose a reason for hiding this comment

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

Declare the variable outside the loop to avoid a compiler warning:

size_t extIndex;
for (extIndex = 0; extIndex < sizeof(tpaExtensions) / sizeof(tpaExtensions[0]); extIndex++)

const char *slash = "/";
int found = 0;

for (int ii = 0; ii < PyList_Size(syspath); ++ii)
Copy link
Contributor

Choose a reason for hiding this comment

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

Declare the variable outside the loop to avoid a compiler warning:

int ii;
for (ii = 0; ii < PyList_Size(syspath); ++ii)

@Cronan
Copy link
Contributor

Cronan commented Feb 2, 2018

@djoyce82 Are you able to get it all working correctly in Linux?

@djoyce82
Copy link
Author

djoyce82 commented Feb 2, 2018

@Cronan pythonnet works on Linux for my use case, which admittedly is a subset of the functionality provided by the library (e.g. no delegates/events), though it is a reasonable sized project with non-trivial dependencies.

I suspect that the test failures are the same as the issues tracked in #590, which may or may not be related to https://github.com/dotnet/corefx/issues/24229 (I encountered this issue when loading my project via pythonnet). It seems that a lot of this tooling is still in a state of flux and hopefully things will improve with .NET Core 2.1. Alternatively, some of the APIs used by pythonnet may have changed or may just be missing in .NET Core.

As it stands, this pull request only resolves the issue of hosting the .NET Core CLR within the python process, but doesn't fully address all of the compatibility issues between .NET Framework and .NET Core.

@djoyce82
Copy link
Author

djoyce82 commented Feb 2, 2018

@Cronan @denfromufa I am able to get a bit further (past the System.Reflection.Emit.TypeBuilder test failures) by having Python.Runtime.dll target netcoreapp2.0 instead of netstandard2.0:

Load clr import hook
================================================ test session starts ================================================
platform linux -- Python 3.6.1, pytest-3.0.7, py-1.4.33, pluggy-0.4.0
rootdir: /home/djoyce/work/pythonnet, inifile: setup.cfg
collected 376 items

src/tests/test_array.py .....................................F..
src/tests/test_callback.py ..
src/tests/test_class.py .......F..F..FF...
src/tests/test_clrmethod.py .....
src/tests/test_compat.py ......FFFF....F.
src/tests/test_constructors.py ....
src/tests/test_conversion.py ....................
src/tests/test_delegate.py .................
src/tests/test_docstring.py ...
src/tests/test_engine.py .ss
src/tests/test_enum.py ............
Unhandled Exception: System.InvalidCastException: Unable to cast object of type 'ObjObjArgFunc' to type 'Int_3_Delegate'.
   at Python.Runtime.NativeCall.Int_Call_3(IntPtr fp, IntPtr a1, IntPtr a2, IntPtr a3)
   at Python.Runtime.MetaType.tp_setattro(IntPtr tp, IntPtr name, IntPtr value)
Aborted (core dumped)

Though I don't really understand why this causes the dependencies to be resolved differently...
The subsequent core is inside libcoreclr.so:

#0  0x00007fae6c2c41d7 in raise () from /lib64/libc.so.6
#1  0x00007fae6c2c58c8 in abort () from /lib64/libc.so.6
#2  0x00007fae626eb6cc in PROCAbort () from /usr/share/dotnet/shared/Microsoft.NETCore.App/2.0.0/libcoreclr.so
#3  0x00007fae626ea5bb in PROCEndProcess(void*, unsigned int, int) ()
   from /usr/share/dotnet/shared/Microsoft.NETCore.App/2.0.0/libcoreclr.so
#4  0x00007fae62453748 in UnwindManagedExceptionPass1(PAL_SEHException&, _CONTEXT*) ()
   from /usr/share/dotnet/shared/Microsoft.NETCore.App/2.0.0/libcoreclr.so
#5  0x00007fae624537f9 in DispatchManagedException(PAL_SEHException&, bool) ()
   from /usr/share/dotnet/shared/Microsoft.NETCore.App/2.0.0/libcoreclr.so
#6  0x00007fae623ba697 in JITutil_ChkCastAny ()
   from /usr/share/dotnet/shared/Microsoft.NETCore.App/2.0.0/libcoreclr.so
...

@dmitriyse
Copy link
Contributor

Probably I can figure out why it behaves so. I spent too much time on netcoreapp2.0 vs netstandard2.0 differencies. I will try at this weekend.

@Cronan
Copy link
Contributor

Cronan commented Feb 7, 2018

@denfromufa @dmitriyse @djoyce82

Some progress

There are types in netcoreapp2.0 that are not available in netstandard2.0, like TypeBuilder.CreateType.

I'm able to replicate what @djoyce82 is seeing, where the delegate tests all work correctly now, by doing the following:

  • Added netcoreapp2.0 in the for all the *.15.csproj files
  • Modified the Conditional statements in all the *.15.csproj files so that netcoreapp2.0 has the same effect as netstandard2.0 (Python.Runtime.15.csproj had the most changes)
  • Changed setup.py so that the TargetFramework is set explicitly in the msbuild (see below)
elif DEVTOOLS == "dotnet":
            _xbuild = 'dotnet msbuild'
            _config = "{0}Mono".format(CONFIG)
            _solution_file = 'pythonnet.15.sln'
            _custom_define_constants = True,
            _target_framework = 'netcoreapp2.0'
if _target_framework:
            cmd.append('/p:TargetFramework="{}"'.format(_target_framework))

Once this was all done, I was able to build and run the tests, with all the delegate tests succeeding. The first hard failure was in test_enum.py, which I'm still trying to work out.

@dmitriyse
Copy link
Contributor

Changing NetStandard 2.0 to NetCoreApp 2.0 in Python.Runtime.15.csproj is a very wrong solution.

For example I have solution with graph of 100+ NetStandard 2.0 assemblies.
Some root assemblies references Python.Runtime.dll project/assemblyl/package (does not matter).
If we changing type of Python.Runtime.Dll to netcoreapp2.0, then all referencing project should also be changed from netstandard2.0 to netcoreapp2.0 wich is unacceptable for one project and futhermore for public nuget packages ecosystem.

@dmitriyse
Copy link
Contributor

So we needs to solve this problem somehow staying Python.Runtime.dll with netstandard2.0

@dmitriyse
Copy link
Contributor

I faced with this problem on my commercial project already. Other developers would not allow me to change project type in whole solution.

@dmitriyse
Copy link
Contributor

Also all python tests should be passed when I run them from "dotnet nPython.dll" through pytest.main() command.

@Cronan
Copy link
Contributor

Cronan commented Feb 8, 2018

@dmitriyse dotnet nPython.dll tests have major issues on Linux:
#590

Also, I understand what you mean about netstandard2.0, but the build currently seems very messy to me, especially if netstandard2.0 is the goal:

  • clrmodule.15.csproj targets net40
  • console.15.csproj and Python.EmbeddingTest.15.csproj both target net40;netcoreapp2.0
  • Python.Runtime.15.csproj and Python.Test.15.csproj both target net40;netstandard2.0

Also, as I pointed out, if you actually do a netstandard2.0 build across the board, TypeBuilder.CreateType won't actually compile, as it's not in the standard ...

@Cronan
Copy link
Contributor

Cronan commented Feb 8, 2018

@dmitriyse @denfromufa I think this proposed PR is a great piece of work, but I agree not ready for merge. I think the actual code on adding the coreclr C api shouldn't be lost.

@den-run-ai
Copy link
Contributor

den-run-ai commented Feb 8, 2018

@dmitriyse @Cronan this PR should be merged as soon as it has been shown to embed .net core in pythonnet, at least on one platform - Linux or Windows. All other failures can be addressed in later PRs, especially with regards to .netcoreapp vs .netstandard 2.0. let me know what you think.

I plan to test this PR this week.

@den-run-ai
Copy link
Contributor

I meant all other failures in .net core, all tests should still be passing in CI.

@dmitriyse
Copy link
Contributor

Give me a chance to fight with netcoreapp 2.0 and netstandard 2.0 differenceies this evening. If I would fail, than it's ok merge this PR even in master. (But personally I prefer ".Net standard 2.0 brainstorm promising branch" for this changes as they are also too damaging).

@Cronan
Copy link
Contributor

Cronan commented Feb 9, 2018

@dmitriyse What I didn't appreciate before this week is that netstandard2.0 is a subset - netcoreapp2.0 actually supports more methods/overloads and even whole types than is in netstandard2.0

@codecov
Copy link

codecov bot commented Feb 10, 2018

Codecov Report

Merging #612 into master will decrease coverage by 0.06%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #612      +/-   ##
==========================================
- Coverage      77%   76.93%   -0.07%     
==========================================
  Files          64       64              
  Lines        5909     5940      +31     
  Branches      974      976       +2     
==========================================
+ Hits         4550     4570      +20     
- Misses       1032     1040       +8     
- Partials      327      330       +3
Flag Coverage Δ
#setup_linux 65.88% <100%> (+0.58%) ⬆️
#setup_windows 76.11% <50%> (-0.12%) ⬇️
Impacted Files Coverage Δ
setup.py 87.29% <100%> (+0.21%) ⬆️
src/runtime/finalizer.cs 65.38% <0%> (-5.13%) ⬇️
src/runtime/exceptions.cs 80.86% <0%> (+1.73%) ⬆️

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 f544adc...9bea24d. Read the comment docs.

@den-run-ai
Copy link
Contributor

@Cronan @dmitriyse here is .netstandard repository, doesn't look like TypeBuilder is going to be extended soon. so we have to switch to .netcoreapp.

https://github.com/dotnet/standard

@dmitriyse
Copy link
Contributor

dmitriyse commented Feb 10, 2018 via email

@dmitriyse
Copy link
Contributor

dmitriyse commented Feb 11, 2018

I have bad news about .NetCoreApp 2.0 and .NetStandard 2.0.
The idea of .NetStandard 2.0 that is API intersection between multiple .Net CLR implementation (.NetCore 2.0 , .Net Framework 4.6+ , hypotetic WinRT.WebAssembly CLR 1.0)
.NetStandard 2.0 will disallow of direct usage (in compile time) of some API, even them if will be available in a particular platform.
.NetCoreApp 2.0.X is a version of CLR runtime, we have a net462( and in theory we should have mono548 for example). When we specify this as a target for an assembly we hints the compiler what will available at runtime (and it's more than .NetStandard 2.0), but we also binds this assembly to a runtime.

In typical modern solution, reusable class libraries should targets .NetStandard2.0, but terminal points (executable projects, UT projects, Web App projects, etc.) should targets to multiple runtimes:
.NetCoreApp2.0 + net46 + somethingelse.

If we want to use in .NetStandard 2.0 library some special method from some special runtime (in our case .NetCoreApp 2.0.0+), than we have two options:

  1. Split Python.Runtime.dll into libraries:
    a) Python.Runtime.dll (NetStandard 2.0) and declare extension points
    b) Python.Runtime.Bootstrap.dll (NetCoreApp 2.0, Net4.0) that will polifill extension points (probably differently for different runtimes).
    this is an idealistic approach, with many backward compatibility problems.

  2. Search required API through reflection at runtime, make API delegates and call them.
    (or fail fast with exception NotSupported("You runtime does not provides required API set."))

Of cource we should pick the second approach.
This trick should not take much time, we need to code the following:

  1. Searching required methods and obtaining MethodInfo(s);
  2. Creating delegates through MethodInfo.CreateDelegate (call to this delegetes have near the same speed as compiled call to API)
  3. Switching missing API calls to saved in vars delegate calls (or to dynamically compiled code with calls to dynamically discoverd API - for super performance critical places) .

@Cronan
Copy link
Contributor

Cronan commented Feb 12, 2018 via email

@den-run-ai
Copy link
Contributor

@dmitriyse @djoyce82 @Cronan have you tried this coreclr embedding on windows, so that I don't waste time if this has already been done? As I understand this PR is only addressing Linux .NET Core, but my primary platform is Windows.

@rlordcardano
Copy link

I am trying to test this PR for our use case (calling functionality implemented in .NET Standard 2.0 DLLs from Python 3.x). We currently manage to do this on Linux by using Mono and the master, but are trying to avoid the dependency on Mono going forward.

I am running:

  • Python 3.6.7
  • Ubuntu 18.04
  • .NET Core SDK 2.1.502

I built the wheel using "python3 setup.py bdist_wheel --xplat" as recommended, and then installed pythonnet (pip3 install ...). Only tweak I had to do before being able to build was to copy interop36.cs to interop36m.cs (anyone else experiencing this?).

So far so good, the problem I'm running into is that when I call "import clr", the error message is:

>>>import clr
Failed to convert CLR files path to absolute path
Traceback (most recent call shown last):
  File "<stdin>", line 1, in <module>
ImportError: Unable to find clr path

Any thoughts, also on interop36m.cs?

@den-run-ai
Copy link
Contributor

@rlordcardano I'm afraid you cannot just "hack" by renaming interop36.cs file. It has to be generated by installing clang/pycparser and then letting setup.py do its job of generating the correct interop structs to be used in pythonnet via geninterop.py.

Another issue is that this PR does not include some fixes from the latest master branch wrt to interop generation, so I just updated this branch.

@rlordcardano
Copy link

@denfromufa thanks, I do have both clang and pycparser installed. Appears I'm running into a similar issue as #609 before. Any suggestions?

File "setup.py", line 220, in build_extension subprocess.check_call([sys.executable, geninterop, interop_file]) File "/usr/lib/python3.6/subprocess.py", line 291, in check_call raise CalledProcessError(retcode, cmd) subprocess.CalledProcessError: Command '['/usr/bin/python3', 'tools/geninterop/geninterop.py', 'src/runtime/interop36m.cs']' returned non-zero exit status 1.

@den-run-ai
Copy link
Contributor

@rlordcardano this is not the full error traceback - there should be more detailed error. but anyway open a separate issue for it or even better - comment on related issue with the same underlying error if there is one.

@rlordcardano
Copy link

rlordcardano commented Jan 2, 2019

@rlordcardano this is not the full error traceback - there should be more detailed error. but anyway open a separate issue for it or even better - comment on related issue with the same underlying error if there is one.

Thanks - I did a pip install clang rather than apt-get install clang, my bad. Interop36m.cs is now generated as part of the setup, but back to the original error once again. I've logged it as #801 together with the dockerfile I used.

@spinwards
Copy link

FYI, this PR and djoyce82#1 from @jgoday allowed me to build against core 2.2 targeting windows server nano.

I don't see recent activity here ... is this PR dead? I can pick it up @djoyce82 is no longer moving it forward.

@filmor
Copy link
Member

filmor commented May 8, 2019

This is not dead, I'm just currently trying to simplify the build, and an additional highly platform dependent compiled dependency doesn't help with that. I have a working prototype of a Python loader using CFFI and will try to push it as soon as possible.

@filmor filmor added this to the 2.5.0 milestone May 8, 2019
@spinwards
Copy link

I tried the loader from your gist in #857 and it caused a hard crash on windows nano server w/ .net core 2.2 and miniconda.

@filmor
Copy link
Member

filmor commented May 8, 2019

It works, but PythonEngine.InitExt has different return types depending on whether it's Python 2 or 3 right now, so the cast has to be "void* (*func)()" instead of "void (*func)()". Let's have further discussion on this in the linked issue :)

@Cronan
Copy link
Contributor

Cronan commented Jun 10, 2019

@filmor @spinwards I managed to get this working in .NET Core 2.1 / Centos 7 / Python 2.7, with a few minor build tweaks. However, I can't for the life of me get it working in the same environment, but using Python 3.6 - I get a hard segfault immediately. Is there anyone still interested in working through this and getting it merged?

@filmor filmor mentioned this pull request Nov 8, 2019
@lostmsu
Copy link
Member

lostmsu commented Mar 3, 2020

@filmor @Cronan , is this PR still valid, or is this work going to be superseded by @filmor 's work on the loader? If it stays, I think we should still move it off 2.x milestones.

@filmor
Copy link
Member

filmor commented Mar 4, 2020

This one is superseded.

@filmor filmor closed this Mar 4, 2020
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.

8 participants