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

Rework of freetype/harfbuzz recipes #1825

Merged
merged 1 commit into from
May 28, 2019

Conversation

opacam
Copy link
Member

@opacam opacam commented May 26, 2019

To fix cyclic dependency between freetype and harfbuzz and to produce shared libraries

And because we use those libraries in multiple recipes and building those libraries as shared libraries can reduce a little the apk size in some circumstances (when we build multiple recipes that depends on those libraries)

This changes are friendly with #1822 which needs shared libraries for freetype and harfbuzz

Important note: if freetype and harfbuzz are in requirements, then the cyclic dependency of those libraries will be solved otherwise the library will be build without linking with the other library (as expected)

As a side note: tested with a Pillow test app and arch armv7a (#1826)

AndreMiras
AndreMiras previously approved these changes May 26, 2019
Copy link
Member

@AndreMiras AndreMiras left a comment

Choose a reason for hiding this comment

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

Smart and clearly documented thanks!
I haven't tried the run time, but code and build log makes sense and looks fine.
The title says WIP, but looks like ready to merge to me. Let me know when you want to merge it.
For records, below are some relevant CI build log parts.

Initial state) recipes to be built contains harfbuzz and freetype

�[0K$ docker run -e CI p4a /bin/sh -c "$COMMAND"
[INFO]:    recipes modified: {'harfbuzz', 'freetype'}
[INFO]:    recipes to build: {'harfbuzz', 'freetype'}
[INFO]:    Found a single valid recipe set: ['freetype', 'hostpython3', 'libffi', 'openssl', 'sqlite3', 'harfbuzz', 'python3']
[INFO]:    Trying to find a bootstrap that matches the given recipes.
[INFO]:    Found 3 acceptable bootstraps: ['service_only', 'webview', 'sdl2']
[INFO]:    Using the first of these: service_only
[INFO]:    Found a single valid recipe set: ['freetype', 'hostpython3', 'libffi', 'openssl', 'sqlite3', 'harfbuzz', 'python3', 'genericndkbuild', 'six', 'pyjnius', 'android']
[INFO]:    recipes to build (no broken): {'harfbuzz', 'freetype'}
[INFO]:    requirements: harfbuzz,freetype,python3
[INFO]:    -> directory context testapps/
running apk

...

  1. build freetype without harfbuzz
�[1m[INFO]�[0m:    �[1m�[32mBuilding freetype for armeabi-v7a�[0m�[39m

�[1m[INFO]�[0m:    First Build of freetype (without harfbuzz, for now...)

�[1m[INFO]�[0m:    Build freetype for First time (without harfbuzz)

�[1m[INFO]�[0m:    Configure args are: --disable-shared

	---prefix=/home/user/.local/share/python-for-android/build/other_builds/freetype/armeabi-v7a__ndk_target_21/freetype/install

	---without-zlib

	---with-png=no

	---host=arm-linux-androideabi

�[1m[INFO]�[0m:    �[36m-> directory context /home/user/.local/share/python-for-android/build/other_builds/freetype/armeabi-v7a__ndk_target_21/freetype�[39m

...

  1. build harfbuzz with freetype
�[1m[INFO]�[0m:    �[1m�[32mBuilding harfbuzz for armeabi-v7a�[0m�[39m

�[1m[INFO]�[0m:    �[36m-> directory context /home/user/.local/share/python-for-android/build/other_builds/harfbuzz/armeabi-v7a__ndk_target_21/harfbuzz�[39m

stty: 'standard input': Inappropriate ioctl for device

�[1m[INFO]�[0m:    �[90m->�[0m running configure --without-icu --host=arm-linux-androidea...(and 153 more)�[0m

stty: 'standard input': Inappropriate ioctl for device

�[1m[INFO]�[0m:    �[90m->�[0m running make -j 2�[0m

stty: 'standard input': Inappropriate ioctl for device

�[1m[INFO]�[0m:    �[90m->�[0m running cp src/.libs/libharfbuzz.so /home/user/.local/shar...(and 98 more)�[0m

...

  1. build freetype with harfbuzz support
�[1m[INFO]�[0m:    Second Build of freetype: enabling harfbuzz support ...

�[1m[INFO]�[0m:    Build freetype for Second time (with harfbuzz)

�[1m[INFO]�[0m:    Configure args are: --prefix=/home/user/.local/share/python-for-android/build/other_builds/freetype/armeabi-v7a__ndk_target_21/freetype

	---enable-shared

	---without-zlib

	---with-png=no

	---host=arm-linux-androideabi

	---disable-static

�[1m[INFO]�[0m:    �[36m-> directory context /home/user/.local/share/python-for-android/build/other_builds/freetype/armeabi-v7a__ndk_target_21/freetype�[39m

stty: 'standard input': Inappropriate ioctl for device

�[1m[INFO]�[0m:    �[90m->�[0m running configure --prefix=/home/user/.local/share/python-...(and 166 more)�[0m

stty: 'standard input': Inappropriate ioctl for device

�[1m[INFO]�[0m:    �[90m->�[0m running make -j 2�[0m

stty: 'standard input': Inappropriate ioctl for device

�[1m[INFO]�[0m:    �[90m->�[0m running cp objs/.libs/libfreetype.so /home/user/.local/sha...(and 99 more)�[0m

inclement
inclement previously approved these changes May 26, 2019
Copy link
Member

@inclement inclement left a comment

Choose a reason for hiding this comment

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

Great work @opacam, very clean!
I'll test this with the matplotlib recipe tomorrow, but no objection to merging before then if it's clearly working.

pythonforandroid/recipes/freetype/__init__.py Show resolved Hide resolved
@opacam opacam dismissed stale reviews from inclement and AndreMiras via 99be9b2 May 27, 2019 08:41
@opacam opacam force-pushed the freetype-harfbuzz-cyclic branch from d2b88f7 to 99be9b2 Compare May 27, 2019 08:41
@opacam opacam changed the title [WIP] Rework of freetype/harfbuzz recipes Rework of freetype/harfbuzz recipes May 27, 2019
@opacam
Copy link
Member Author

opacam commented May 27, 2019

Yesterday, I labeled this WIP because I wanted to take another look and also I wanted to o share a little app so this could be tested at runtime....unfortunately I could not finish the job...now I think that it's ready, but I don't have any problem in merging it after that @inclement test this with matplotlib...maybe even better.

The push I made today was made to apply the changes mentioned by @AndreMiras, thanks 😄!!!

@opacam opacam mentioned this pull request May 27, 2019
@inclement
Copy link
Member

I'm having a build issue on the matplotlib branch with harfbuzz not producing a .so output, probably not a major thing but just noting to not merge quite yet while I check it out.

@inclement
Copy link
Member

I haven't been able to resolve the issue I'm having. I'm testing by adding freetype and harfbuzz to the requirements of the basic python3 testapp, but it consistently fails at the following part of the build:


make[4]: warning: -jN forced in submake: disabling jobserver mode.
  CXX      libharfbuzz_la-hb-buffer-serialize.lo
  CXX      libharfbuzz_la-hb-blob.lo
  CXX      libharfbuzz_la-hb-buffer.lo
  CXX      libharfbuzz_la-hb-face.lo
  CXX      libharfbuzz_la-hb-common.lo
  CXX      libharfbuzz_la-hb-font.lo
  CXX      libharfbuzz_la-hb-ot-tag.lo
  CXX      libharfbuzz_la-hb-set.lo
  CXX      libharfbuzz_la-hb-shape.lo
  CXX      libharfbuzz_la-hb-shape-plan.lo
  CXX      libharfbuzz_la-hb-shaper.lo
  CXX      libharfbuzz_la-hb-unicode.lo
  CXX      libharfbuzz_la-hb-warning.lo
  CXX      libharfbuzz_la-hb-ot-font.lo
  CXX      libharfbuzz_la-hb-ot-layout.lo
  CXX      libharfbuzz_la-hb-ot-map.lo
  CXX      libharfbuzz_la-hb-ot-shape.lo
  CXX      libharfbuzz_la-hb-ot-shape-complex-arabic.lo
  CXX      libharfbuzz_la-hb-ot-shape-complex-default.lo
  CXX      libharfbuzz_la-hb-ot-shape-complex-hangul.lo
  CXX      libharfbuzz_la-hb-ot-shape-complex-hebrew.lo
  CXX      libharfbuzz_la-hb-ot-shape-complex-indic.lo
  CXX      libharfbuzz_la-hb-ot-shape-complex-indic-table.lo
  CXX      libharfbuzz_la-hb-ot-shape-complex-myanmar.lo
  CXX      libharfbuzz_la-hb-ot-shape-complex-sea.lo
  CXX      libharfbuzz_la-hb-ot-shape-complex-thai.lo
  CXX      libharfbuzz_la-hb-ot-shape-complex-tibetan.lo
  CXX      libharfbuzz_la-hb-ot-shape-normalize.lo
  CXX      libharfbuzz_la-hb-ot-shape-fallback.lo
  CXX      libharfbuzz_la-hb-fallback-shape.lo
  CXX      libharfbuzz_la-hb-ft.lo
  CXX      libharfbuzz_la-hb-ucdn.lo
  CXX      main-main.o
  CXX      test-test.o
  CXX      test_buffer_serialize-test-buffer-serialize.o
  CXX      test_size_params-test-size-params.o
  CXX      test_would_substitute-test-would-substitute.o
  GEN      harfbuzz.pc
  GEN      libharfbuzz.la
/home/sandy/android/android-ndk-r17c/toolchains/arm-linux-androideabi-4.9/prebuilt/linux-x86_64/bin/../lib/gcc/arm-linux-androideabi/4.9.x/../../../../arm-linux-androideabi/bin/ld: error: cannot find -lharfbuzz
collect2: error: ld returned 1 exit status
make[4]: *** [Makefile:1094: libharfbuzz.la] Error 1
make[4]: Leaving directory '/home/sandy/.local/share/python-for-android/build/other_builds/harfbuzz/armeabi-v7a__ndk_target_21/harfbuzz/src'
make[3]: *** [Makefile:1723: all-recursive] Error 1
make[3]: Leaving directory '/home/sandy/.local/share/python-for-android/build/other_builds/harfbuzz/armeabi-v7a__ndk_target_21/harfbuzz/src'
make[2]: *** [Makefile:1015: all] Error 2
make[2]: Leaving directory '/home/sandy/.local/share/python-for-android/build/other_builds/harfbuzz/armeabi-v7a__ndk_target_21/harfbuzz/src'
make[1]: *** [Makefile:480: all-recursive] Error 1
make[1]: Leaving directory '/home/sandy/.local/share/python-for-android/build/other_builds/harfbuzz/armeabi-v7a__ndk_target_21/harfbuzz'
make: *** [Makefile:411: all] Error 2

Building with --disable-shared does work, but then of course the second freetype build fails.

@opacam Did you have any issue like this, any idea what's wrong?

@opacam
Copy link
Member Author

opacam commented May 27, 2019

mmmm...this is strange...probably I faced this 3 years ago when I made the initial changes but not now...

I just did a build with the following requirements using testapp_python3_sqlite_openssl:

harfbuzz,requests,peewee,sdl2,pyjnius,kivy,python3,freetype

This builds fine on my system

Also tested with the basic testapp_python3:

freetype,harfbuzz,sdl2,numpy,pyjnius,kivy,python3

Also builds fine for me...so...:thinking:

  • did you remove all the files from the previous builds you made? (I always remove all the files from the build and dists folders when testing things...specially when playing with libraries)

AndreMiras
AndreMiras previously approved these changes May 27, 2019
Copy link
Member

@AndreMiras AndreMiras left a comment

Choose a reason for hiding this comment

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

looking good

@AndreMiras
Copy link
Member

error: cannot find -lharfbuzz

@inclement is the harfbuzz shared lib generated somewhere at least? I mean could it be somewhere, but the linker doesn't find it because some -L flags are wrong? I wouldn't be surprised if for instance Arch and Ubuntu end up building the output lib to different directories.
What if you `find .buildozer/ -name "libharfbuzz*.so*" ? (I used buildozer, but adapt if you use p4a directly obviously)

@inclement
Copy link
Member

No, this happens even with a clean build, and harfbuzz does not generate any .so file. I've been trying to work out if it's supposed to do that before running this part of the build, or if this part of the build shouldn't be trying to link it in the first place. I've found it difficult to even work out where the -lharfbuzz is coming from.

@inclement
Copy link
Member

Would anyone be able to provide the full build_arch debug logs for freetype and harfbuzz in one of the testapps? I'd like to compare with the output I'm getting.

@AndreMiras
Copy link
Member

Oh right actually the recipe that doesn't build for you is harfbuzz itself right? So it doesn't really make sense that it's trying to put a -lharfbuzz when compiling itself 🤔
As for the debug log isn't the one from Travis enough? Otherwise I can try to build one tomorrow

@inclement
Copy link
Member

inclement commented May 27, 2019

@AndreMiras Actually I forgot about travis, but it doesn't print the debug level output which is what I'm most interested in. I've been meaning to add an option to output that to a file (i.e. add a file log output to the default logger). Unless we have such a thing already.

@opacam
Copy link
Member Author

opacam commented May 27, 2019

@inclement, Let me do one build and I will send you the log

@inclement
Copy link
Member

The build log I'm getting is at https://gist.github.com/inclement/484db3a42ec4225b175a3d3e68212707

@inclement
Copy link
Member

Also posted my config.log at https://gist.github.com/inclement/7f87e55802e76f0c30d9e43d60b726df, could be interesting to compare.

@opacam
Copy link
Member Author

opacam commented May 27, 2019

@inclement
Copy link
Member

Thanks @opacam. I have to sleep now, but I'll do some detective work tomorrow. I'm sure it will be some small issue!

@opacam
Copy link
Member Author

opacam commented May 27, 2019

@opacam
Copy link
Member Author

opacam commented May 27, 2019

No problem @inclement, tomorrow I will be connected almost all day, so let me know if you need anything

@opacam
Copy link
Member Author

opacam commented May 28, 2019

@inclement, Good morning,

I've find the problem...

I've been checking the logs we post yesterday, they are almost the same but I noticed that your build system has the development libraries for libharfbuzz, mine not, so I installed the harfbuzz-dev package and 💥...I faced the same error that you got yesterday...

So the system's harfbuzz development files are conflicting with our harfbuzz files...and this shouldn't happen...I will try to find a solution for that...if I remember well we face something similar with libffi recently.

I found this comparing our config.log from harfbuzz:

Your freetype's flags/libs from your harfbuzz's configure.log (line 1122):

FREETYPE_CFLAGS='-I/usr/include/freetype2 -I/usr/include/libpng16 -I/usr/include/harfbuzz -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include '
FREETYPE_LIBS='-lfreetype '

Here are mine before installing harfbuzz-dev:

FREETYPE_CFLAGS='-I/usr/include/freetype2 -I/usr/include/libpng16'
FREETYPE_LIBS='-lfreetype'

So, if you want to do some tests with matplotlib recipe, just removing the harfbuzz-dev package should allow you to successfully build harfbuzz's recipe (harfbuzz-devel in arch os ...I think)

@opacam

This comment has been minimized.

@opacam opacam changed the title Rework of freetype/harfbuzz recipes [WIP] Rework of freetype/harfbuzz recipes May 28, 2019
@opacam opacam force-pushed the freetype-harfbuzz-cyclic branch from 99be9b2 to 288ce6a Compare May 28, 2019 12:36
To fix cyclic dependency between freetype and harfbuzz and to produce shared libraries, because we use those libraries in multiple recipes and building those libraries as shared libraries can reduce a little the apk size in some circumstances (when we build multiple recipes that depends on those libraries)
@opacam opacam force-pushed the freetype-harfbuzz-cyclic branch from 288ce6a to 5691837 Compare May 28, 2019 13:35
@opacam opacam changed the title [WIP] Rework of freetype/harfbuzz recipes Rework of freetype/harfbuzz recipes May 28, 2019
@opacam
Copy link
Member Author

opacam commented May 28, 2019

Ok, modified the recipes to fix the conflict between system's harfbuzz development files and our harfbuzz recipe (I also simplified the logic to be more readable and maintainable compared to the first version I posted).

Now we should be able to build the harfbuzz recipe even if we have installed the harfbuzz-dev package (debian/ubuntu) or the equivalent in other os.

I tested it in both situations with the Pillow test app (#1826) and seems to work fine for me, travis give us a green tick mark...so...@inclement, when you have time, could you test it again please?

Copy link
Member

@AndreMiras AndreMiras left a comment

Choose a reason for hiding this comment

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

awesome work 👏

@inclement
Copy link
Member

Very nice work @opacam, I thought it might be something like this but I got lost inside libtool!

I've tested again, including both with/without harfbuzz in the matplotlib PR, without any issues.

@inclement inclement merged commit 80fe311 into kivy:master May 28, 2019
@opacam opacam deleted the freetype-harfbuzz-cyclic branch May 29, 2019 11:08
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