Rework for Pillow/pil recipes & update jpeg and png #1573
Conversation
opacam
changed the title
opacam
changed the title
…ce code and fix imports We force to only build the static library to avoid that Pillow/pil gets linked with the shared library, plus we cannot use those libraries in his shared form because we will have conflicts with the ones distributed by android os...so no need to build those libraries unless properly versioned as we do with openssl libs
Also add a patch that may allow us to build the libraries in his shared form...in case we need them
- Fix compatibility for new jpeg - add freetype/harfbuzz support - move libraries from LDFLAGS to LIBS - format source code
- Fix compatibility for new jpeg - add freetype/harfbuzz support - move libraries from LDFLAGS to LIBS - make it work with python2 and python2legacy
opacam
force-pushed
the
python-core-pil
branch
from
ae1a906 to
987fd51
Compare
|
I am just testing this, and I ran into some detail: maybe https://github.com/kivy/python-for-android/blob/master/doc/source/quickstart.rst#installing-dependencies should be expanded to add |
|
Edit: outdated/irrelevant |
|
Tested build & runtime now, works 🆗 so given without this it won't even build, this is really an essential upgrade 👍 |
The recent rework of the jpeg recipe has introduced a new build dependency for us, so we add it to the section `Installing Dependencies` to no forgot about it.
opacam
changed the title
| version = '1.6.15' | ||
| url = 'https://github.com/julienr/libpng-android/archive/{version}.zip' | ||
| version = '1.6.29' | ||
| url = 'https://github.com/julienr/libpng-android/archive/master.zip' |
There was a problem hiding this comment.
Oops, I guess you hardcoded the branch/tag by mistake right?
There was a problem hiding this comment.
oh wow didn't notice that one 😮 now I feel blind. yeah, would be nice to know if that is an accident or not
There was a problem hiding this comment.
Unfortunately, there is no error...well...I forgot to notify it (my apologies for that...I completely forgot about it...)...
Well...the thing is that the author of the github's repo never released/tagged this version, despite He performed the necessary changes in master branch and notify about it in the README file...I thing that we should try to move it from that repo to the mainline repo, plus considering that the last commit is more than a year old...mmm...I have bad feelings about that...
Anyway...maybe we should target a commit instead of a version, this way we wouldn't hardcode the url...I still think that we should try to move it to mainline but maybe would be better to do it in another pr (this way we could solve the current Pillow/pil issues, and move the png recipe to the mainline repository when we can).
@Jonast and @AndreMiras, what do you think?
There was a problem hiding this comment.
@opacam yea hardcoding/pinning a commit seems like the best idea for now 👍
There was a problem hiding this comment.
OK thanks for the explanation.
Yes I would either hardcode the commit or the master branch in the version string. But in any case I would keep that {version} temple so people who use the recipe can override with png==whatever-they-want
We set the last commit published for a version because the author of the github's repo never released/tagged it, despite He performed the necessary changes in master branch and notify about it in the README file...so this way we don't hardcode the url. Note: we should try to move the png repo to mainline because of the issue mentioned above and the fact that the last commit of the repo is more than one year old
| jni_ln = join(build_dir, 'jni') | ||
| if not exists(jni_ln): | ||
| shprint(sh.ln, '-s', build_dir, jni_ln) | ||
| shprint(sh.rm, '-f', 'CMakeCache.txt', 'CMakeFiles/') |
There was a problem hiding this comment.
Would rm -f work on a directory without the -r flag?
There was a problem hiding this comment.
Got the bug report on Discord 2 years later 🤯
https://discord.com/channels/423249981340778496/678695971164520523/876867985355186228
| '-DCMAKE_SYSTEM_PROCESSOR={cpu}'.format(cpu='arm'), | ||
| '-DCMAKE_POSITION_INDEPENDENT_CODE=1', | ||
| '-DCMAKE_ANDROID_ARCH_ABI={arch}'.format(arch=arch.arch), | ||
| '-DCMAKE_ANDROID_NDK=' + self.ctx.ndk_dir, |
There was a problem hiding this comment.
Minor, why don't you use the same format string syntax as before/after for consistency?
Several actions taken here:
Note: This should work, tested in python3. I must review the code and create the proper commits plus I may change something that we may not need. I push it in this state in case that anyone has troubles building Pillow/pil with the new core.