Build single Python.Runtime.dll for all platforms #1365

lostmsu · 2021-01-22T08:14:14Z

What does this implement/fix? Explain your changes.

This allows building the same Python.Runtime.dll on all platforms and supported Python versions.

To invoke Python code from .NET, one needs to set Python.Runtime.Runtime.PythonDLL or manually preload DLL before invoking any Python.NET functions or other properties.

Implementation details

PInvoke signatures have been replaced with unmanaged functions pointers, that are loaded using ILibraryLoader on the first attempt to call any Python C API functions.

Marshaling attributes have been replaced with explicit marshaling calls.

PythonDLL gets its default value in the following order from:

PYTHONNET_PYDLL environment variable
Is set to null when launched from Python (so that DLL is already loaded in the process).
Set to a platform-dependent value, based on PYTHONNET_PYVER environment variable (e.g. '3.6' -> 'libpython3.6.so')

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

lostmsu · 2021-01-22T08:17:21Z

A few issues still in the works:

Presence of 'm' suffix in the python C API DLL name. Used to be determined by setup.py and embedded into Python.Runtime.dll. We need some reliable way to get it from CI environment.
On Mac build fails due to some internal .NET SDK/Roslyn issue.

lostmsu · 2021-01-22T08:21:41Z

Ideally, we should vend Python.Runtime.dll separately from the Python embedding module, and build it separately too.

filmor · 2021-01-22T10:30:14Z

Presence of 'm' suffix in the python C API DLL name. Used to be determined by setup.py and embedded into Python.Runtime.dll. We need some reliable way to get it from CI environment.

On Linux, pkg-config is the way to go. I also think that we should be able to pass the libpython directly, not just as version + abiflags.

Ideally, we should vend Python.Runtime.dll separately from the Python embedding module, and build it separately too.

That is the plan, with clr-loader we just put Python.Runtime.dll somewhere and load it via pure Python. I have this working against the P/Invoke implementation.

lostmsu · 2021-01-23T04:18:07Z

Mac build appears to be blocked on dotnet/roslyn#46772

lostmsu · 2021-01-23T07:25:49Z

One check is failing still, that's the same one everywhere else.

src/runtime/importhook.cs

filmor · 2021-01-23T11:12:17Z

src/runtime/native/StrPtr.cs

+namespace Python.Runtime.Native
+{
+    [StructLayout(LayoutKind.Sequential)]
+    struct StrPtr : IDisposable


Maybe NativeString would be more appropriate?

Naming things :-D

Not sure about NativeString. It has connotations to native string type supported on the platform, but this is not it.

It might make sense to have a StrPtr type per encoding for clarity. Then signatures could enforce it. But maybe that would be overengineering.

Do we use any encoding apart from UTF8? I thought all FromString functions nowadays used that.

Indeed, I find exactly one usage with Encoding.ASCII, everything else is UTF8. And the one case that uses ASCII is PyBuffer_SizeFromFormat, which doesn't specify ASCII encoding either, I highly doubt it will break if it's passed UTF8 instead as it will probably just compare bytes directly and bail out on anything that it doesn't understand.

The main thing I find "problematic" is that this object is not a pure pointer, it has ownership over the buffer it points to.

filmor · 2021-01-23T11:18:05Z

src/runtime/pydict.cs

@@ -22,6 +22,7 @@ public PyDict(IntPtr ptr) : base(ptr)
        {
        }

+        internal PyDict(BorrowedReference reference) : base(reference) { }


Maybe we should rename PyDict to something like PyDictFacade to make it clear that it doesn't take ownership.

This overload actually does incref internally (which makes sense, since to create a potentially long-lived PyDict object pointing to something we borrowed, we need to IncRef).

src/runtime/runtime_data.cs

src/runtime/runtime.cs

filmor · 2021-01-23T13:22:24Z

src/runtime/runtime.cs

-        internal static extern IntPtr PyObject_GetAttr(IntPtr pointer, IntPtr name);
+        internal static int PyObject_SetAttrString(IntPtr pointer, string name, IntPtr value)
+        {
+            using var namePtr = new StrPtr(name, Encoding.UTF8);


Since this pattern is used in a lot of places, we should have a function for this (GetUtf8String or something like that). That could open up potential optimisation paths with caching or preallocated buffers per thread.

At this point it would be a premature optimization. Also, I don't see how we could cache disposable struct instances easily.

src/runtime/runtime.cs

filmor · 2021-01-23T13:27:40Z

src/runtime/runtime.cs

@@ -2185,6 +2276,539 @@ internal static IntPtr GetBuiltins()
        {
            return PyImport_Import(PyIdentifier.builtins);
        }
+
+        private static class Delegates


Please make Runtime a partial class and move this into a separate file.

I can do this just before pushing. Otherwise it would require me to update the roslyn package, that does automatic conversion of [DllImport] into the pattern with Delegates.

filmor · 2021-01-23T13:30:15Z

src/runtime/runtime.cs

+                {
+                    // only in 3.9+
+                }
+                PyBuffer_IsContiguous = (delegate* unmanaged[Cdecl]<ref Py_buffer, char, int>)GetFunctionByName(nameof(PyBuffer_IsContiguous), GetUnmanagedDll(_PythonDll));


Did you generate this via a script? That would be a good addition (although I'd hope we wouldn't need too many changes of this class ;)).

The script converted DllImport declarations. It did not work on the Python source, headers, or docs. Essentially, signatures are identical to what we had except string parameters and a few minor exceptions where I switched to *Reference types.

workaround for dotnet/roslyn#49760

should fix Mac build

lostmsu force-pushed the features/VersionIndependent branch 4 times, most recently from 2cbce6a to f557fb5 Compare January 22, 2021 19:01

lostmsu force-pushed the features/VersionIndependent branch from e0c14cf to 478023d Compare January 23, 2021 05:29

lostmsu mentioned this pull request Jan 23, 2021

Importing clr module should prepare Python.Runtime #1366

Closed

lostmsu force-pushed the features/VersionIndependent branch from abaa42f to c0a751b Compare January 23, 2021 05:46

lostmsu requested a review from filmor January 23, 2021 07:25