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

Build wheels with compiled libraries #1241

Open
naveen521kk opened this issue Oct 24, 2020 · 13 comments
Open

Build wheels with compiled libraries #1241

naveen521kk opened this issue Oct 24, 2020 · 13 comments
Labels
feature New feature that should be supported

Comments

@naveen521kk
Copy link
Contributor

naveen521kk commented Oct 24, 2020

One good thing about wheels is that it allow us to embed DLL files. So simply adding all DLL files and making the wheel platform specific would significantly help users by not installing system dependencies. I recently made a PR in cairocffi repo Kozea/cairocffi#164 providing a way to do so. I got a nice repo which builds cairo as a single DLL file so I could do that. But here I couldn't find the one for Pango ( I don't think anything else is needed). So, finding a way to build that would be good or rather it can also be taken from msys2 in case of windows and brew in case of MacOS. I don't about linux.

What I did there was not that correct I think but I don't know. Possibly there should be a better solution and I started searching for similar libraries.
I found freetype-py which had a script to build freetype same thing can be done here also I think.
Next, I found  this issue pypa/wheel#128 which is more related and from that thread I found pypandoc where their implementation was very good IMHO, See setup.py.
Next, I found this thread at stackoverflow https://stackoverflow.com/questions/45150304/how-to-force-a-python-wheel-to-be-platform-specific-when-building-it.
Next, I find another interesting thing which is used with cffi is milksnake which simply suited this one but I haven't tried using it.

Sorry, for a braindump I thought I would organize it a little better but I didn't find time.

Finally, the conclusion is there must be platform specific wheel so that weasyprint can simply be installed using pip without any system dependencies.

@liZe liZe added the feature New feature that should be supported label Oct 24, 2020
@naveen521kk
Copy link
Contributor Author

naveen521kk commented Dec 7, 2020

This 4f82a4a breaks the possibility of doing this. For doing this a setup.py is must and without which it can't be done.

@naveen521kk
Copy link
Contributor Author

@liZe I plan to help here. I think needed dependencies are Pango and Gdk-Pixbuf right?
For Pango, one of my repo, pango-build, can be used(actually made for another project), there the DLL files are name as CORE_MANIM_*.dll to avoid DLL hell. I need to look into Gdk-Pixbuf though, it also uses meson so it should be easy to compile it using meson. Thoughts?

@liZe
Copy link
Member

liZe commented Mar 1, 2021

@liZe I plan to help here.

Thank you!

I think needed dependencies are Pango and Gdk-Pixbuf right?

The libraries directly used by WeasyPrint in master are Pango and some of its dependencies:

  • PangoFT2 (independent DLL generated by Pango, for FreeType support),
  • HarfBuzz,
  • FontConfig,
  • GObject.

the DLL files are name as CORE_MANIM_*.dll to avoid DLL hell

I’m really scared of adding a way to specify custom DLL names in WeasyPrint, because I’m afraid that it will add another configuration layer and cause bugs with a more complex way to debug. If it’s only done in the wheel (with specific code in the building script), then why not.

I need to look into Gdk-Pixbuf though, it also uses meson so it should be easy to compile it using meson. Thoughts?

Gdk-Pixbuf is not needed anymore 😉.

@naveen521kk
Copy link
Contributor Author

naveen521kk commented Mar 2, 2021

So I think there is no compiling work, everything you want is ready in the other repo. It should be just about getting those and package it properly. I think it should need a setup.py to do so. I will work this week :)

If it’s only done in the wheel (with specific code in the building script), then why not.

Yeah I meant only for the wheel. Also, should it be fine to add tests on that?

@liZe
Copy link
Member

liZe commented Mar 2, 2021

So I think there is no compiling work, everything you want is ready in the other repo. It should be just about getting those and package it properly. I think it should need a setup.py to do so. I will work this week :)

We’ve worked hard to remove setup.py, so it would be a good idea to find a solution without it 😉.

@naveen521kk
Copy link
Contributor Author

Quick question: I have done it locally and wrote a script to do it. Now, when I run

weasyprint --help

I get a warning

Fontconfig error: Cannot load default config file: No such file: (null)
h:\weasyprint\download\wheelhouse\testenv\lib\site-packages\weasyprint\text\fonts
.py:66: UserWarning: FontConfig cannot load default config file. Expect ugly out
put.
  warnings.warn(

which I kinda expected, and setting an environment variable before loading it should fix it, but it needs to include the fontconfig files also. Should I do that, or it isn't required? @liZe

@Tontyna
Copy link
Contributor

Tontyna commented Mar 2, 2021

Fontconfig needs its configuration files, otherwise no @font-face, fallback to native system fonts. As the warning says: "Expect ugly output", see #592

That makes things a bit more complicated because AFAIK these files are searched in ..\etc\fonts (relative to the DLL folder)

Concerning the renaming of the DLLs:
How do you persuade the renamed main DLLs (gobject, pango, harfbuzz, fontconfig, pangoft2) to load the renamed auxiliary DLLs? I don't think anybody will look for e.g. CORE_MANIM_intl.DLL.

Did you test your wheel on a system where none of the required GTK files is anywhere in your PATH?

@naveen521kk
Copy link
Contributor Author

naveen521kk commented Mar 3, 2021

That makes things a bit more complicated because AFAIK these files are searched in ..\etc\fonts (relative to the DLL folder)

Setting an env var FONTCONFIG_PATH should fix this, possibly done inside of weasyprint in the init script, but that requires the fontconfig configuration to be included, in the wheel.

How do you persuade the renamed main DLLs (gobject, pango, harfbuzz, fontconfig, pangoft2) to load the renamed auxiliary DLLs? I don't think anybody will look for e.g. CORE_MANIM_intl.DLL.

Everything was compiled from the source and properly linked to necessary libraries so that it does it automatically. Essentially it was patching meson(the build system to do so). If interested have a look at the build scripts.

Did you test your wheel on a system where none of the required GTK files is anywhere in your PATH?

Works for me here, and it should be working. You could try that by downloading from https://github.com/Kozea/WeasyPrint/suites/2162062908/artifacts/44361621

@naveen521kk
Copy link
Contributor Author

That makes things a bit more complicated because AFAIK these files are searched in ..\etc\fonts (relative to the DLL folder)

They aren't relative sadly :(. It is hardcoded to the build folder actually, somewhere. Even Inskscape load it by setting an env var see this.

@naveen521kk
Copy link
Contributor Author

I am including them in the wheel. Let's see what happens.

@Tontyna
Copy link
Contributor

Tontyna commented Mar 3, 2021

I don't have a Windows system at hand to hard-core-test the wheel, e.g. by not using WeasyPrint standalone but within a project containing other modules requiring GTK libraries. Or inside an environment that does its own tricks to load GTK...

@naveen521kk
Copy link
Contributor Author

Locally eveything looks good, I also added a test on GHA, as much as install the wheel and running weasyprint https://weasyprint.org/ -o b.pdf. I would like to have smoketest or something which can be run to test those wheels.

@liZe liZe changed the title Building Wheels Building Wheels with compiled libraries Aug 17, 2021
@liZe liZe changed the title Building Wheels with compiled libraries Build wheels with compiled libraries Aug 17, 2021
@liZe
Copy link
Member

liZe commented Apr 25, 2022

Related to #1622.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature that should be supported
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants