Skip to content
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

Windows build: Support for python 3.8 and up #248

Closed
wants to merge 1 commit into from
Closed

Windows build: Support for python 3.8 and up #248

wants to merge 1 commit into from

Conversation

mhertz
Copy link

@mhertz mhertz commented Jan 9, 2022

With the new add_dll_directory() to correctly load the runtime dll

@mhertz
Copy link
Author

mhertz commented Jan 10, 2022

For context, this is patch used by gvsbuild to build without failing on py3.8 and up, and I just added an improvement regarding workarounding possibility of trailing semicolon in PATH env-var I borrowed from gvsbuild dev used elsewhere, as for me and him fixed usage from pyinstaller freezed exe.

Initially requested inclusion in gvsbuild, but was suggested good idea of getting included upstream instead. Thanks in advance.

With the new add_dll_directory() to correctly load the runtime dll
@ylatuya
Copy link

ylatuya commented Jan 21, 2022

I think it shouldn't be pycairo's responsibility to add directories to the search path as it has limited knowledge about how the consumer application has been packaged or built. Cairo's shared library can have different filenames, depending on how it was built and the toolchain used for example. Your patch would not work with GStreamer's build, where the shareed llibrary can be named cairo-2.dll or libcairo-2.dll when it's built with MinGW for example.

The application importing pycairo should have enough knowledge to set the correct search paths for DLL's, either because it bundles the dependencies in a known path or using a similar logic as the one you implemented knowing how the user was instructed to install the dependencies required by the application.

@lazka
Copy link
Member

lazka commented Jan 25, 2022

I agree with @ylatuya

@stuaxo
Copy link
Collaborator

stuaxo commented Jan 25, 2022

I think one case is slightly different -
There is a use case for a WHL with an internal cairo dll - for the simple case of apps that want to draw using cairo and output to PNG, SVG or PDF and have no GUI.

There could be an API to opt into using the internally supplied dll.

@lazka
Copy link
Member

lazka commented Jan 26, 2022

If the wheel ships a DLL it should by default add the path, yeah, but in our case the the DLL is in the same directory as the .pyd file which is looked up by default anyway, so no action needed.

https://docs.python.org/3/whatsnew/3.8.html#bpo-36085-whatsnew -> "Only the system paths, the directory containing the DLL or PYD file, and directories added with add_dll_directory() are searched for load-time dependencies"

@mhertz
Copy link
Author

mhertz commented Jan 26, 2022

Thanks for comments, and please deem if/when need closing yourself, as honestly don't like myself judging that.

Anyway, of-course these are valid points, and frankly I'm not very much in the know by all of this, but just thought that as atleast in gvsbuild doesn't build without failing(without patching added, I mean, hence initially added), because can not find needed dll, when on py3.8+, and then just slight extra pyinstaller related fix on-top.

Regardless this is now merged by gvsbuild, so not that big deal, but just fit better upstream as per the sentiment of gvsbuild dev as mentioned.

Thanks again, appreciate your work and thoughts :)

@lazka
Copy link
Member

lazka commented Jan 28, 2022

hm, good point, pygobject will import pycairo to find its header files during build, but it wont know where the DLL is either, so importing would fail :/

@lazka lazka changed the title Support for python 3.8 and up Windows build: Support for python 3.8 and up Mar 19, 2022
@lazka
Copy link
Member

lazka commented Jul 24, 2022

Fixed in pygobject 3.42.2 https://pygobject.readthedocs.io/en/latest/changelog.html#id1

It will no longer try/require to import pycairo during the build.

This pull request was closed.
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