Fix MuPDF vendoring #892
Merged
Fix MuPDF vendoring #892
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Regression (I hope, at least) since the last MuPDF bump (!!). While we're there, make it build against our own harfbuzz version, too.
We always enforce CROSS_COMPILE
houqp
approved these changes
Apr 19, 2019
|
The idea was testing things a bit first, to see how it performed ;).
Also, I can only vouch for it with GCC 6+, but ideally, 7+ (i.e.,
koxtoolchain platforms ;)).
…On Fri, Apr 19, 2019, 06:12 Qingping Hou ***@***.***> wrote:
***@***.**** approved this pull request.
Good catch. Why not enable LTO by default?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#892 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAA3KZVEEUHZ7O65I3TNMM3PRFBDJANCNFSM4HHBKVXQ>
.
|
|
(also, it takes roughly twice as long to build ;)).
…On Fri, Apr 19, 2019, 07:05 NiLuJe ***@***.***> wrote:
The idea was testing things a bit first, to see how it performed ;).
Also, I can only vouch for it with GCC 6+, but ideally, 7+ (i.e.,
koxtoolchain platforms ;)).
On Fri, Apr 19, 2019, 06:12 Qingping Hou ***@***.***> wrote:
> ***@***.**** approved this pull request.
>
> Good catch. Why not enable LTO by default?
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <#892 (review)>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AAA3KZVEEUHZ7O65I3TNMM3PRFBDJANCNFSM4HHBKVXQ>
> .
>
|
|
Do we get a noticeable performance boost with LTO? |
|
Does Clang have LTO as well? |
Frenzie
reviewed
Apr 19, 2019
| @@ -12,7 +12,9 @@ assert_var_defined(OUTPUT_DIR) | |||
|
|
|||
| ep_get_source_dir(SOURCE_DIR) | |||
|
|
|||
| set(BUILD_CMD sh -c "${KO_MAKE_RECURSIVE} -j${PARALLEL_JOBS} CC=\"${CC}\" LDFLAGS=\"${LDFLAGS}\" OUTPUT_DIR=${OUTPUT_DIR}") | |||
| set(PATCH_CMD sh -c "patch -N -p1 < ${CMAKE_CURRENT_SOURCE_DIR}/lua-serialize-lto.patch || true") | |||
There was a problem hiding this comment.
Btw, not directly related to this PR but I've been wondering why we let the patches fail semi-quietly? That is, you won't notice without studying the logs.
There was a problem hiding this comment.
Wasn't that to handle rebuilds? I have no idea if that still makes sense with the CMake system, though :D.
There was a problem hiding this comment.
Afaik CMake handles when to patch or not to patch with an uncharacteristic lack of stupidity.
There was a problem hiding this comment.
Then yeah, it doesn't make sense anymore, and that's just a bad habit carried over from the old makefile ;)
There was a problem hiding this comment.
In that case I'll submit a PR to get rid of the load of 'em. It's tripped me up more than once.
There was a problem hiding this comment.
Oh, you can see I already got rid of the || true there, so you'd have noticed if that were somehow trying to apply itself multiple times. ;-)
Frenzie
added a commit
to Frenzie/koreader
that referenced
this pull request
Apr 19, 2019
Frenzie
added a commit
to koreader/koreader
that referenced
this pull request
Apr 19, 2019
Frenzie
added a commit
to Frenzie/koreader-base
that referenced
this pull request
Apr 19, 2019
Frenzie
added a commit
to Frenzie/android-luajit-launcher
that referenced
this pull request
Apr 22, 2019
A bit of an emergency fix to just get it working ASAP. A patch was added to base LuaJIT in koreader/koreader-base#892, which caused a mismatch and build failures over here. Unfortunately that was hidden by some excessive Android verbosity inadvertently introduced in koreader/koreader-base#888, and on my local instance where I did a sanity check LuaJIT was already compiled…
Frenzie
added a commit
to koreader/android-luajit-launcher
that referenced
this pull request
Apr 22, 2019
A bit of an emergency fix to just get it working ASAP. A patch was added to base LuaJIT in koreader/koreader-base#892, which caused a mismatch and build failures over here. Unfortunately that was hidden by some excessive Android verbosity inadvertently introduced in koreader/koreader-base#888, and on my local instance where I did a sanity check LuaJIT was already compiled…
Frenzie
added a commit
to Frenzie/koreader
that referenced
this pull request
Apr 22, 2019
* [fix] Add LuaJIT patch to sync with koreader-base koreader/android-luajit-launcher#141 A bit of an emergency fix to just get it working ASAP. A patch was added to base LuaJIT in koreader/koreader-base#892, which caused a mismatch and build failures over here. Unfortunately that was hidden by some excessive Android verbosity inadvertently introduced in koreader/koreader-base#888, and on my local instance where I did a sanity check LuaJIT was already compiled…
Frenzie
added a commit
to koreader/koreader
that referenced
this pull request
Apr 22, 2019
* [fix] Add LuaJIT patch to sync with koreader-base koreader/android-luajit-launcher#141 A bit of an emergency fix to just get it working ASAP. A patch was added to base LuaJIT in koreader/koreader-base#892, which caused a mismatch and build failures over here. Unfortunately that was hidden by some excessive Android verbosity inadvertently introduced in koreader/koreader-base#888, and on my local instance where I did a sanity check LuaJIT was already compiled…
mwoz123
pushed a commit
to mwoz123/koreader
that referenced
this pull request
Mar 29, 2020
mwoz123
pushed a commit
to mwoz123/koreader
that referenced
this pull request
Mar 29, 2020
* [fix] Add LuaJIT patch to sync with koreader-base koreader/android-luajit-launcher#141 A bit of an emergency fix to just get it working ASAP. A patch was added to base LuaJIT in koreader/koreader-base#892, which caused a mismatch and build failures over here. Unfortunately that was hidden by some excessive Android verbosity inadvertently introduced in koreader/koreader-base#888, and on my local instance where I did a sanity check LuaJIT was already compiled…
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We were failing to properly instruct it to use our own libjpeg-turbo, meaning it was building & linking against its own bundled copy (of jpeg 9c, so, not the same API).
How that never blew up before is beyond me ;).
Now that we build harfbuzz, make it use our own harfbuzz, too :).
Also, a few assorted fixes to make LTO builds possible.