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

Fix for FreeBSD #1724

Closed
wants to merge 3 commits into from
Closed

Fix for FreeBSD #1724

wants to merge 3 commits into from

Conversation

dsh2dsh
Copy link

@dsh2dsh dsh2dsh commented May 23, 2022

Without this change I'm getting an exception trying to do

>>> import fitz

Without this change I'm getting an exception trying to do
```
>>> import fitz
```
@jamie-lemon
Copy link
Collaborator

Please try again with the latest release!

@dsh2dsh
Copy link
Author

dsh2dsh commented Jun 23, 2022

But what did change? It still doesn't add "freetype" and "harfbuzz" into libraries. I fixed merge conflicts.

@julian-smith-artifex-com
Copy link
Collaborator

Thanks for this suggested change. Unfortunately i don't have access to a freebsd machine so am unable to test it.

The PR doesn't seem to have a commit that patches the current master, so it's a little tricky to see exactly what's going on. Could you rebase your branch on top of the latest master?

I think it might be good to make your change only apply when building on freebsd. So maybe a patch like this would work?:

index c283885..e36cb6b 100644
--- a/setup.py
+++ b/setup.py
@@ -607,6 +607,12 @@ if ('-h' not in sys.argv and '--help' not in sys.argv
         include_dirs.append("/opt/homebrew/include/freetype2")
 
         library_dirs.append("/opt/homebrew/lib")
+        
+        if sys.platform.startswith( 'freebsd'):
+            libraries += [
+                    'freetype',
+                    'harfbuzz',
+                    ]
 
     else:
         # Windows.

@dsh2dsh
Copy link
Author

dsh2dsh commented Jun 23, 2022

@julian-smith-artifex-com your diff shows exactly what my change is doing.

@dsh2dsh
Copy link
Author

dsh2dsh commented Jun 23, 2022

@julian-smith-artifex-com would you like me to close my PR in favor of yours?

@julian-smith-artifex-com
Copy link
Collaborator

Ok, in that case, could you try applying my diff to current master (removing your changes and merges), and verify that it works on your freebsd machine?

If all is ok, i'll apply my patch to the tree.

[This will avoid the need for a signed contributor license agreement in this case.]

@julian-smith-artifex-com
Copy link
Collaborator

@julian-smith-artifex-com would you like me to close my PR in favor of yours?

Ah, sorry, our postings crossed. Yes, that sounds like the simplest way forward. If you could verify it works, that would be a bonus.

@julian-smith-artifex-com
Copy link
Collaborator

I'll mention you in the commit message.

BTW if you prefer, i'd be happy to merge a pull request with you signing a contributor license agreement.

@dsh2dsh
Copy link
Author

dsh2dsh commented Jun 23, 2022

@julian-smith-artifex-com thank you, I don't think we need to complicate it by the agreement. I'll test the build and let you know.

@dsh2dsh
Copy link
Author

dsh2dsh commented Jun 23, 2022

OK, I've tested fresh master and system installed mupdf 1.20.0. Without the change I'm getting

❯ python
Python 3.8.13 (default, May 10 2022, 01:15:55) 
[Clang 11.0.1 (git@github.com:llvm/llvm-project.git llvmorg-11.0.1-0-g43ff75f2c on freebsd13
Type "help", "copyright", "credits" or "license" for more information.
>>> import fitz
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/www/pyx-py-env/lib/python3.8/site-packages/PyMuPDF-1.20.0-py3.8-freebsd-13.1-RELEASE-amd64.egg/fitz/__init__.py", line 10, in <module>
    from fitz.fitz import *
  File "/usr/local/www/pyx-py-env/lib/python3.8/site-packages/PyMuPDF-1.20.0-py3.8-freebsd-13.1-RELEASE-amd64.egg/fitz/fitz.py", line 17, in <module>
    from . import _fitz
ImportError: /usr/local/www/pyx-py-env/lib/python3.8/site-packages/PyMuPDF-1.20.0-py3.8-freebsd-13.1-RELEASE-amd64.egg/fitz/_fitz.cpython-38.so: Undefined symbol "FT_Get_First_Char"

With this change

diff --git a/setup.py b/setup.py
index c283885..a745943 100644
--- a/setup.py
+++ b/setup.py
@@ -608,6 +608,12 @@ if ('-h' not in sys.argv and '--help' not in sys.argv
 
         library_dirs.append("/opt/homebrew/lib")
 
+        if sys.platform.startswith( 'freebsd'):
+            libraries += [
+                    'freetype',
+                    'harfbuzz',
+                    ]
+
     else:
         # Windows.
         assert mupdf_local

it works fine

❯ python
Python 3.8.13 (default, May 10 2022, 01:15:55) 
[Clang 11.0.1 (git@github.com:llvm/llvm-project.git llvmorg-11.0.1-0-g43ff75f2c on freebsd13
Type "help", "copyright", "credits" or "license" for more information.
>>> import fitz
>>>

@julian-smith-artifex-com
Copy link
Collaborator

Great, thanks for testing. I'll push the fix to master soon.

julian-smith-artifex-com added a commit that referenced this pull request Jun 24, 2022
We need to explicitly link with the system's freetype and harfbuzz libraries.

Thanks to Denis Shaposhnikov (@dsh2dsh) for investigating this.
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.

3 participants