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

mupdf: 1.24.8 -> 1.24.9, fixes #342894

Merged
merged 6 commits into from
Sep 20, 2024
Merged

Conversation

emilazy
Copy link
Member

@emilazy emilazy commented Sep 18, 2024

Description of changes

All credit goes to @lilyinstarlight. Built mupdf, python3Packages.mupdf, mupdf.tests.mupdf-all.

cc @drupol @GaetanLepage

cc @sarahec I think this should fix the issue you were running into, and you should be able to bump cleanly on top of this.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@emilazy emilazy changed the title mupdf: fixes mupdf: 1.24.8 -> 1.24.9, fixes Sep 18, 2024
@sarahec
Copy link
Contributor

sarahec commented Sep 18, 2024

As a double-check, also build python3Packages.pymupdf after the bump. I could get mupdf to build then encounter compilation issues when pymupdf tried to build the parent.

Copy link
Contributor

@sarahec sarahec left a comment

Choose a reason for hiding this comment

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

Failure while building python311Packages.pymupdf on aarch64-Darwin:

error: builder for '/nix/store/4l9j0zyl7h7nvkp8ml1kci064p7hm9ah-mupdf-1.24.9.drv' failed with exit code 1;
       last 10 log lines:
       > Traceback (most recent call last):
       >     scripts/mupdfwrap.py:6:<module>(): wrap.__main__.main()
       >     scripts/wrap/__main__.py:3062:main(): jlib.exception_info()
       >     ^except raise:
       >     scripts/wrap/__main__.py:3060:main(): main2()
       >     scripts/wrap/__main__.py:2445:main2(): build( build_dirs, swig_command, args, vs_upgrade, make_command)
       >     scripts/wrap/__main__.py:1637:build(): build_0(
       >     scripts/wrap/__main__.py:1413:build_0(): cpp.cpp_source(
       >     scripts/wrap/cpp.py:5024:cpp_source(): raise Exception(f'libclang does not appear to be installed') from e
       > Exception: libclang does not appear to be installed
       For full logs, run 'nix log /nix/store/4l9j0zyl7h7nvkp8ml1kci064p7hm9ah-mupdf-1.24.9.drv'.
error: 1 dependencies of derivation '/nix/store/k5wqx2jc6z43ixgy5qfz8m066sh33lcd-python3.11-pymupdf-1.24.8.drv' failed to build

@emilazy
Copy link
Member Author

emilazy commented Sep 18, 2024

This still needs some more changes (e.g. pymupdf bump, and actually it turns out that the FreeGLUT fork is required). The libclang error there looks like what I fixed in 6e0421e551ac153e31014b294f6f03939071135a; odd if it’s still happening.

@sarahec
Copy link
Contributor

sarahec commented Sep 19, 2024

The libclang error there looks like what I fixed in 6e0421e; odd if it’s still happening.

Yes, that made its way into default.nix for mupdf. Somehow, it still seems to be happening. It's a mystery.

Build log: https://gist.github.com/sarahec/97d280ae7a8e0cdb0f24535f1ef40726

@sarahec
Copy link
Contributor

sarahec commented Sep 19, 2024

This looks salient (from the gist):

(+0.0s): parse.py:14:<module>: Warning, could not import clang: No module named 'clang'
(+0.1s): state.py:18:<module>: Warning: failed to import clang.cindex: e=ModuleNotFoundError("No module named 'clang'")
(+0.1s): state.py:18:<module>: We need Clang Python to build MuPDF python.
(+0.1s): state.py:18:<module>: Install with `pip install libclang` (typically inside a Python venv),
(+0.1s): state.py:18:<module>: or (OpenBSD only) `pkg_add py3-llvm.`

@emilazy
Copy link
Member Author

emilazy commented Sep 19, 2024

It should work on everything now (including cross).

@sarahec
Copy link
Contributor

sarahec commented Sep 19, 2024

What seems to be happening is that mupdf builds just fine standalone, but build python311Packages.pymupdf and it builds a fresh copy of mupdf and signals that missing package error.

@lilyinstarlight
Copy link
Member

lilyinstarlight commented Sep 19, 2024

What seems to be happening is that mupdf builds just fine standalone, but build python311Packages.pymupdf and it builds a fresh copy of mupdf and signals that missing package error.

Can you share how you are testing and details on your host system? (e.g. what exact command you are running to checkout the commit from this PR, what command to build, what macOS version, is sandbox enabled, etc)

@lilyinstarlight lilyinstarlight linked an issue Sep 19, 2024 that may be closed by this pull request
@ofborg ofborg bot added the 8.has: package (new) This PR adds a new package label Sep 19, 2024
@sarahec
Copy link
Contributor

sarahec commented Sep 19, 2024

What seems to be happening is that mupdf builds just fine standalone, but build python311Packages.pymupdf and it builds a fresh copy of mupdf and signals that missing package error.

Can you share how you are testing and details on your host system? (e.g. what exact command you are running to checkout the commit from this PR, what command to build, what macOS version, is sandbox enabled, etc)

  1. Checked out pr/emilazy/342894:push-truntykvnrrx from within vscode and use git pull before each round of testing to get the latest updates
  2. nix-build -A python311Packages.pymudf is what I'm running.
  3. System info:
❯ nix-shell -p nix-info --run "nix-info -m"
 - system: `"aarch64-darwin"`
 - host os: `Darwin 23.6.0, macOS 14.6.1`
 - multi-user?: `yes`
 - sandbox: `no`
 - version: `nix-env (Nix) 2.18.5`
 - channels(root): `"nixpkgs"`
 - nixpkgs: `/nix/var/nix/profiles/per-user/root/channels/nixpkgs`
  1. I get identical results with sandboxing on and off (nix-build -A python311Packages.pymupdf --option sandbox false vs. with the option set to true).

@sarahec
Copy link
Contributor

sarahec commented Sep 19, 2024

It should work on everything now (including cross).

The cross-compilation error is gone, and I think the issue may be that the python interface build is missing python3Packages.libclang.

@lilyinstarlight
Copy link
Member

The cross-compilation error is gone, and I think the issue may be that the python interface build is missing python3Packages.libclang.

python3Packages.libclang is present (see here)

  1. nix-build -A python311Packages.pymudf is what I'm running.

Ah! I'd only tested python3Packages.pymupdf (which is python312Packages.pymupdf), not the Python 3.11 version. I'll see if I can poke at it tomorrow morning and figure it out (it's late here and I'm about to sleep), thank you :)

@lilyinstarlight
Copy link
Member

@ofborg build python311Packages.pymupdf

@lilyinstarlight
Copy link
Member

lilyinstarlight commented Sep 19, 2024

@sarahec Well I couldn't sleep. So I remember this issue now. Darwin will always fail for any Python package that uses an alternate version of the python3Packages set when it also utilizes the desktopToDarwinBundle setup hook (like mupdf.override { python3 = python311 }), because that hook erroneously propagates default python3 via python3Packages.icnsutil and pollutes the python311 build with python312 references (and therefore fails to find the python311Packages.libclang since python3 command is erroneously ${lib.getBin python312}/bin/python3 in the build)

Until we have a more proper fix for Python environment management when needing just bins from a python package, the easiest general fix for every package in this situation would be to do this diff (which has been in my nixpkgs git stash for a long time now... also see #266544 (review)):

diff --git a/pkgs/top-level/all-packages.nix b/pkgs/top-level/all-packages.nix
index 1254d45dda59..ca71039f4293 100644
--- a/pkgs/top-level/all-packages.nix
+++ b/pkgs/top-level/all-packages.nix
@@ -1341,7 +1341,7 @@ with pkgs;
 
   desktopToDarwinBundle = makeSetupHook {
     name = "desktop-to-darwin-bundle-hook";
-    propagatedBuildInputs = [ writeDarwinBundle librsvg imagemagick python3Packages.icnsutil ];
+    propagatedBuildInputs = [ writeDarwinBundle librsvg imagemagick (onlyBin python3Packages.icnsutil) ];
   } ../build-support/setup-hooks/desktop-to-darwin-bundle.sh;
 
   keepBuildTree = makeSetupHook {

@sarahec
Copy link
Contributor

sarahec commented Sep 19, 2024

@lilyinstarlight I hope you managed to get some sleep. Thank you for unpacking the issue for Darwin.

Is there something I should do here?

@sarahec
Copy link
Contributor

sarahec commented Sep 19, 2024

@lilyinstarlight I tested the dozen or so packages that use desktopToDarwinBundle and python, and only this one and possibly rcu fail. So perhaps there's a local patch to [py]?mupdf that makes sense?

@emilazy
Copy link
Member Author

emilazy commented Sep 20, 2024

I think it would be best just to open a PR with the desktopToDarwinBundle workaround rather than adding more complicated and less general workarounds here.

@emilazy
Copy link
Member Author

emilazy commented Sep 20, 2024

Going to merge this so that it gets in for the next staging cycle; @sarahec if you open a PR for the desktopToDarwinBundle fix feel free to ping me for review/merge.

@emilazy emilazy merged commit acb9363 into NixOS:staging Sep 20, 2024
27 of 31 checks passed
@emilazy emilazy deleted the push-truntykvnrrx branch September 20, 2024 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Build failure: python312Packages.pymupdf
3 participants