Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

lostmsu
Copy link
Member

@lostmsulostmsu commentedJan 22, 2021
edited
Loading

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 setPython.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 usingILibraryLoader 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:

  1. PYTHONNET_PYDLL environment variable
  2. Is set tonull when launched from Python (so that DLL is already loaded in the process).
  3. Set to a platform-dependent value, based onPYTHONNET_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 toAUTHORS
  • Updated theCHANGELOG

@lostmsu
Copy link
MemberAuthor

A few issues still in the works:

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

@lostmsu
Copy link
MemberAuthor

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

@filmor
Copy link
Member

filmor commentedJan 22, 2021
edited
Loading

Presence of 'm' suffix in the python C API DLL name. Used to be determined bysetup.py and embedded intoPython.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 vendPython.Runtime.dll separately from the Python embedding module, and build it separately too.

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

@lostmsulostmsuforce-pushed thefeatures/VersionIndependent branch 4 times, most recently from2cbce6a tof557fb5CompareJanuary 22, 2021 19:01
@lostmsu
Copy link
MemberAuthor

Mac build appears to be blocked ondotnet/roslyn#46772

@lostmsu
Copy link
MemberAuthor

lostmsu commentedJan 23, 2021
edited
Loading

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

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

Choose a reason for hiding this comment

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

MaybeNativeString would be more appropriate?

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Naming things :-D

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

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

Copy link
Member

@filmorfilmorJan 28, 2021
edited
Loading

Choose a reason for hiding this comment

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

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

Indeed, I find exactly one usage withEncoding.ASCII, everything else isUTF8. And the one case that uses ASCII isPyBuffer_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.

Copy link
Member

Choose a reason for hiding this comment

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

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

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

internal PyDict(BorrowedReference reference) : base(reference) { }
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should renamePyDict to something likePyDictFacade to make it clear that it doesn't take ownership.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

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

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);
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

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

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

private static class Delegates
Copy link
Member

Choose a reason for hiding this comment

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

Please makeRuntime apartial class and move this into a separate file.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

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

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

Choose a reason for hiding this comment

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

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 ;)).

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

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

@lostmsulostmsuforce-pushed thefeatures/VersionIndependent branch from675ec5f toa6cbe20CompareJanuary 28, 2021 19:20
@lostmsulostmsu merged commit9e5887c intopythonnet:masterJan 28, 2021
@lostmsulostmsu deleted the features/VersionIndependent branchJanuary 28, 2021 19:31
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@filmorfilmorfilmor approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants
@lostmsu@filmor

[8]ページ先頭

©2009-2025 Movatter.jp