1.23.1 build for distribution packaging broken #2628
Comments
|
Apologies, we don't have a good way to test building with a system mupdf so this was missed. For me, this patch fixes the immediate problem: Could you try this, and let us know if anything else is required to make things work for you? BTW we are in the process of migrating to a new implementation of PyMuPDF called "rebased" which will use the MuPDF C++ and Python bindings, instead of the C bindings. So it will require a system MuPDF to have the C++ and Python bindings available. Building them is straightforward and reliable, but requires SWIG and Python package |
|
Thanks! That gets me across the first bridge, but then I hit: |
|
So I guess here I'd need to prefix with |
|
Hmm, after overriding/ setting I guess this means libmupdf is missing something... |
|
I think what's happening here is that by default we build the original "classic" implementation of PyMuPDF, which only uses the MuPDF C API, and also the new "rebased" implementation, which requires the MuPDF C++ and Python bindings. In the medium/long term, only the rebased implementation will be available, so you'll need to extend your MuPDF installation to include the MuPDF C++ and Python bindings. For now though, if you set |
|
One other thing, when building with a system MuPDF, it would presumably help you if This could be as simple as adding Do you have any suggestions or preferences here? |
Please do note, that mupdf upstream offers zero integration for either of the last two (I looked into updating the build system but failed to understand where and what of the files should actually be installed).
This is usually covered by exposing |
|
What happened to good old |
|
Frankly, after looking into the new setup.py (and having a really hard time understanding what is even supposed to go on in there), I am really starting to wonder why this project does not rely on meson for building this (e.g. https://mesonbuild.com/Cython.html)? |
|
I have arrived at this patch for 1.23.3: However, now tests fail because of missing symbols, e.g. and I have no clue which lib they belong to...: IMHO, this type of breakage should have and could have been prevented. The current state is quite the huge step back for anyone trying to package this downstream and it takes considerable amounts of time to even try to get this to work. |
Yes, there's currently no top-level makefile install target for the C++/Python bindings, but we'll be adding one soon, e.g.
Ok, thanks for this, i'll have a look at using these, and let you know when i have something you can test. |
Thank you! |
Unfortunately the old setup.py's use of setuptools made it incapable of doing what we require. And setuptools is deprecated these days. Obviously we would have preferred to use an existing build tool, but there does not seem to be anything capable enough - Python's support for building complex extensions seems very poor. pipcl.py tries hard to follow all the available standards in the various PEPs and https://packaging.python.org documentation, and works well on our three main platforms - Linux, Windows and MacOS.
setup.py is not non-standard - it follows the official standards for packaging, e.g. PEP-517. Running setup.py directly is deprecated these days, one is supposed to drive things with a frontend such as pip. Having said that, we do try to support basic command-line usage, using https://pip.pypa.io/en/stable/reference/build-system/setup-py/ "setup.py (legacy)" as a reference. So I think setuptools' |
|
Right now I'm not sure what should be different - there simply are too many changes already (setup.py/pipcl here, plus the new rebasing, plus the new version) and in python (distutils/setup tools, py3.12) in a relatively short timeframe. I'll try to work with what's there - the info that you're trying to follow PEP was new to me, for example. This may help to switch to the newer Fedora packaging macros. |
I'm not familiar with meson. I guess that there are many different build systems around, with various pros and cons, and we all have our preferred ways of doing things. One of the good things about Python's use of a setup.py script is that it's written in a powerful and generic language, so we can directly download MuPDF and run commands to build it, run SWIG to generate the PyMuPDF extensions, and package things up into Python wheels and/or copy into an installation directory. Most of that is not really a build system, and i'm not sure that something like meson or cmake would be better than the pure Python code in setup.py and pipcl.py. The intention is that setup.py and pipcl.py can be understood by reading the code and documentation. It looks like this might not be the case though. Can you describe what's lacking for you? I'd be happy to add something, e.g. to the top of setup.py, or in a separate document, that's more aligned with what you're primarily looking for. |
Understood. For now i'd suggest you ignore the new rebased implementation - set |
|
Progress due to remarks by both of you :) Re so: I seem to recall that there used to be issues with soname versioning in mupdf at some point, which is why we build statically. So, this is one more thing to change before we can ship the new mupdf buildings and use them for the rebased PyMuPDF. |
Similar to swig, meson offers a PEP517 build backend. Using it has the upside, that you are using a tool that is actually made for building C/C++ software. Scripting all of this in setup.py (and pipcl.py) leads to a huge script, that is brittle and (currently) lacks all the common use-cases (e.g. passing in of build flags).
All of that is part of a good build system! Meson's declarative syntax is easy to read, has standardized functionality, powerful integration for dependencies, is tested across many platforms and therefore by all means better than custom Python code written for a single project.
Sorry, but having to write thousands of lines of custom Python code to build a project (and forcing others to read it) sounds a lot like NIH to me. :( To comment on what @mjg has been writing earlier: It would be very helpful to first clear this up (and even add standardized integration, such as pkgconfig and/or cmake files to e.g. extract and mupdf) so that detection can happen generically across consumers and nothing has to be hardcoded by thousands of lines of custom Python code. |
|
So, just in case this helps someone: Here is the Fedora dist-git tree which builds and passes the test suite. It contains the ugly version of patching in the required libraries and requesting a debug build for non-local builds. I'm still a bit worried about not picking up default build flags from the environment - we usually require that in Fedora land. The other two patches (to the test suite) are not new. Test builds can be found in my copr repo. |
Thanks for this. I've pushed some experimental MuPDF and PyMuPDF changes to: These allow system build and install of MuPDF and PyMuPDF. As a test, one can install MuPDF and PyMuPDF into ~/fake-root with: Then test with: and check that PyMuPDF's setup.py supports new environment variables CFLAGS, CXXFLAGS, LDFLAGS, PYMUPDF_INCLUDES, PYMUPDF_MUPDF_INCLUDE, PYMUPDF_MUPDF_LIB. See docs near top of setup.py. I've only tested PYMUPDF_MUPDF_INCLUDE and PYMUPDF_MUPDF_LIB so far, and i haven't tried making MuPDF use system-installed things such as freetype. |
|
@dvzrv wrote:
This is getting a little off-topic for this issue, so i've replied in a new discussion: #2649 |
|
I've pushed changes to MuPDF and PyMuPDF that will hopefully improve system installs.
Here are example commands i've used to install and test using I haven't tested installing into the real system root, but the intention is that one can do: |
|
Today's release, PyMuPDF-1.23.4, includes the changes described above. Let me know how you get on with building and installing system packages. |
|
Thanks. I understand this is intended to make distro life smoother in the future and I do appreciate that. That being said: 1.23.4 does not build with the same spec file with which 1.23.3 built (and which I had recrafted for the changes in 1.23.3), nor does it build if I remove an additional patch to setup.py that 1.23.3 still needed. Maybe I just need to cull lib related changes from that patch (and set the new env variables) but keep the mupdf_build_dir_flags change (so that setup.py does not clear it without local mupdf sources), or something else. I won't be able to invest the necessary time in the upcoming weeks, so please don't be disappointed if feedback does not arrive soon. It's too late for this to make it into the Fedora 39 release anyways, and - consequently - for 1.23.x at this point because it changed again rather than stabilizing. What is the intended incarnation to install PyMuPDF with a system mupdf? In your example above, you still have a mupdf src tree alongside the PyMuPDF src tree (which prevents the clearing of mupdf_build_dir_flags in setup.py that I'm patching out) |
|
I totally understand about not being able to update PyMuPDF for Fedora 39, and thanks for the warning about no feedback for a while. I'll jump on this issue whenever you're able to look at it some more. [I guess your The intention with 1.23.4 is that you can build without needing to do any patching of
The example i gave is intended to do this.
Just for clarity, here's my suggested (and untested) commands for a system install, with a couple of extra annotations: I hope that all makes sense. |
|
Yes, but |
|
You're right that But i don't think we look for a I've fixed the setting of |
|
OK, once I understood that |
|
I was able to fix the build (also by changing the way we build mupdf) - see https://gitlab.archlinux.org/archlinux/packaging/packages/python-pymupdf/-/blob/8333ee1f0a9ad99d5cb1368118e6a7027b93d055/PKGBUILD. |
|
Thanks for this, i'm glad the build is working for you. Please post here or create a new Github issue if you have any problems or suggestions in future, and i'll do my best to help. The switch to the rebased implementation will probably require some further changes for system installs because the MuPDF installation will need to contain the MuPDF Python bindings. As mentioned it all works fine in a test case, but i'll get in touch anyway. |
|
Please see new discussion about draft documentation for Linux system installs: #2977 |
Describe the bug (mandatory)
Hi! I package this project for Arch Linux.
With >= 1.23.0 the build is broken for distribution packaging, as we build against a system-provided mupdf and
setup.pynow sets a required build directory variable toNone.To Reproduce (mandatory)
Build script for Arch Linux: https://gitlab.archlinux.org/archlinux/packaging/packages/python-pymupdf/-/blob/main/PKGBUILD
Expected behavior (optional)
pymupdf can be build as before.
Screenshots (optional)
n/a
Your configuration (mandatory)
Additional context (optional)
The original setup.py has been renamed to setup-old.py which makes it very hard to track the changes as a new file has been put in place (which makes me wonder why changes have not just been added gradually).
The text was updated successfully, but these errors were encountered: