android: update to use NDK 23c #10610
Conversation
benoit-pierre
force-pushed
the
pr/android_ndk_update
branch
from
bc68ac9 to
732e60b
Compare
|
Adding a note here, before this PR gets merged. We need to update otamanager, so it won't insist in updating to a version that will fail to install. |
|
You mean something like a file that says 2023.07 requires at least ABI 18? |
Not sure about the implementation. What I would like:
I'm afraid we cannot fix it backwards (ie: updating from v2023.01) so 2 won't work fully. |
|
@benoit-pierre: did you try to build for x86_64 with all your patches in?. I know it was borked with the old toolchain but I'm not sure if you tried to build to a yet unsupported target. |
|
On my meson branch, yes, with a small patch to luajit-launcher: diff --git a/assets/android.lua b/assets/android.lua
index 7e310856..99de6bd7 100644
--- a/assets/android.lua
+++ b/assets/android.lua
@@ -2654,9 +2654,10 @@ local function run(android_app_state)
-- load the dlopen() implementation
android.dl = require("dl")
+ android.dl.system_libdir = ffi.arch == "x64" and "/system/lib64" or "/system/lib"
android.dl.library_path = android.nativeLibraryDir..":"..
android.dir..":"..android.dir.."/libs:"..
- "/lib:/system/lib:/lib/lib?.so:/system/lib/lib?.so"
+ string.gsub("@:@/lib?.so", "@", android.dl.system_libdir)
-- register the dependency lib loader
table.insert(package.loaders, 3, android.deplib_loader)
diff --git a/assets/dl.lua b/assets/dl.lua
index a5f5dd5d..30917c07 100644
--- a/assets/dl.lua
+++ b/assets/dl.lua
@@ -54,6 +54,9 @@ end
local dl = {
-- set this to search in certain directories
library_path = '/lib/?;/usr/lib/?;/usr/local/lib/?',
+ -- set this to the directory of system libraries
+ -- (to be ignored when loading dependencies).
+ system_libdir = nil,
}
local function sys_dlopen(library, global, padding)
@@ -120,7 +123,7 @@ function dl.dlopen(library, load_func, depth)
-- libvulkan, and libz
-- c.f., https://android.googlesource.com/platform/bionic/+/master/android-changes-for-ndk-developers.md#private-api-enforced-for-api-level-24
-- Our current code should *never* hit any private system libs, so, this is basically overkill ;).
- if depth > 0 and (pspec == "/system/lib" or library == "libdl.so") then
+ if depth > 0 and (pspec == dl.system_libdir or library == "libdl.so") then
-- depth > 0 to allow explicitly loading a system lib
-- (because this might have genuine use cases, as some early API levels do not put DT_NEEDED libraries into the global namespace)
-- pspec to reject system libs |
Don't recreate `assets.7z` from scratch every time, and also ensure it's reproducible to avoid busting gradle's cache.
benoit-pierre
force-pushed
the
pr/android_ndk_update
branch
from
732e60b to
535cfed
Compare
|
@benoit-pierre With regard to the failing tests, did you rebase this on top of the latest master? |
|
Yes. |
|
This error is real, though I don't immediately see a clear culprit in the base PR. |
|
CI: vs locally: |
|
Nah, locally you just have an old one, presumably from before these changes.
Delete the old one and locally you get: Perhaps |
|
I don't see why… The link command looks alright: /usr/bin/ccache g++ -I/home/ko/project/base/thirdparty/freetype2/build/x86_64-linux-gnu/freetype2-prefix/src/freetype2-build/include/freetype2 \
-I/home/ko/project/base/thirdparty/harfbuzz/build/x86_64-linux-gnu/harfbuzz-prefix/src/harfbuzz-build/include/harfbuzz \
-I/home/ko/project/base/thirdparty/fribidi/build/x86_64-linux-gnu/fribidi-prefix/src/fribidi-build/include/fribidi \
-I/home/ko/project/base/thirdparty/libunibreak/build/x86_64-linux-gnu/libunibreak-prefix/src/libunibreak-build/include \
-L/home/ko/project/base/build/x86_64-linux-gnu/libs -O2 -ffast-math -pipe -fomit-frame-pointer -march=native -fPIC -I/home/ko/project/base/thirdparty/luajit/build/x86_64-linux-gnu/luajit-prefix/src/luajit/src -shared -Wl,-E -Wl,-rpath,'$ORIGIN' -fvisibility=hidden \
-l:libfreetype.so.6 \
-l:libfribidi.so.0 \
-l:libharfbuzz.so.0 \
-l:libunibreak.so.5 \
\
-Wall -o build/x86_64-linux-gnu/libs/libkoreader-xtext.so xtext.cpp |
|
I've cleaned and rebuilt from scratch. |
|
For me it works if I put them back behind as they were. |
|
|
Can't reproduce locally: /usr/bin/ccache g++ -I/home/bpierre/dev/koreader.bak01/base/thirdparty/freetype2/build/x86_64-pc-linux-gnu-debug/freetype2-prefix/src/freetype2-build/include/freetype2 \
-I/home/bpierre/dev/koreader.bak01/base/thirdparty/harfbuzz/build/x86_64-pc-linux-gnu-debug/harfbuzz-prefix/src/harfbuzz-build/include/harfbuzz \
-I/home/bpierre/dev/koreader.bak01/base/thirdparty/fribidi/build/x86_64-pc-linux-gnu-debug/fribidi-prefix/src/fribidi-build/include/fribidi \
-I/home/bpierre/dev/koreader.bak01/base/thirdparty/libunibreak/build/x86_64-pc-linux-gnu-debug/libunibreak-prefix/src/libunibreak-build/include \
-L/home/bpierre/dev/koreader.bak01/base/build/x86_64-pc-linux-gnu-debug/libs -Og -g -pipe -fno-omit-frame-pointer -march=native -fPIC -I/home/bpierre/dev/koreader.bak01/base/thirdparty/luajit/build/x86_64-pc-linux-gnu-debug/luajit-prefix/src/luajit/src -shared -Wl,-E -Wl,-rpath,'$ORIGIN' -fvisibility=hidden \
-l:libfreetype.so.6 \
-l:libfribidi.so.0 \
-l:libharfbuzz.so.0 \
-l:libunibreak.so.5 \
\
-Wall -o build/x86_64-pc-linux-gnu-debug/libs/libkoreader-xtext.so xtext.cppDynamic section at offset 0x8d80 contains 32 entries:
Tag Type Name/Value
0x0000000000000001 (NEEDED) Shared library: [libfreetype.so.6]
0x0000000000000001 (NEEDED) Shared library: [libfribidi.so.0]
0x0000000000000001 (NEEDED) Shared library: [libharfbuzz.so.0]
0x0000000000000001 (NEEDED) Shared library: [libunibreak.so.5]
0x0000000000000001 (NEEDED) Shared library: [libstdc++.so.6]
0x0000000000000001 (NEEDED) Shared library: [libm.so.6]
0x0000000000000001 (NEEDED) Shared library: [libgcc_s.so.1]
0x0000000000000001 (NEEDED) Shared library: [libc.so.6]
0x000000000000001d (RUNPATH) Library runpath: [$ORIGIN]
0x000000000000000c (INIT) 0x3000
0x000000000000000d (FINI) 0x70a8
0x0000000000000019 (INIT_ARRAY) 0x9c10
0x000000000000001b (INIT_ARRAYSZ) 8 (bytes)
0x000000000000001a (FINI_ARRAY) 0x9c18
0x000000000000001c (FINI_ARRAYSZ) 8 (bytes)
0x000000006ffffef5 (GNU_HASH) 0x300
0x0000000000000005 (STRTAB) 0xc40
0x0000000000000006 (SYMTAB) 0x328
0x000000000000000a (STRSZ) 1781 (bytes)
0x000000000000000b (SYMENT) 24 (bytes)
0x0000000000000003 (PLTGOT) 0x9fe8
0x0000000000000002 (PLTRELSZ) 2184 (bytes)
0x0000000000000014 (PLTREL) RELA
0x0000000000000017 (JMPREL) 0x1860
0x0000000000000007 (RELA) 0x1458
0x0000000000000008 (RELASZ) 1032 (bytes)
0x0000000000000009 (RELAENT) 24 (bytes)
0x000000006ffffffe (VERNEED) 0x13f8
0x000000006fffffff (VERNEEDNUM) 2
0x000000006ffffff0 (VERSYM) 0x1336
0x000000006ffffff9 (RELACOUNT) 39
0x0000000000000000 (NULL) 0x0 |
No, it works fine with GCC 11 too. But I'm on Arch Linux. |
The crengine teardown code need |
See discussion in <koreader/koreader#10610 (comment)>. It would seem Arch behaves differently.
|
Ah, that way. I'm grateful to you for taking the time! Anyway, if you update base in this PR with koreader/koreader-base@c1a5edb it should pass. |
benoit-pierre
force-pushed
the
pr/android_ndk_update
branch
from
0524fa7 to
88c6618
Compare
That probably explains my perspective. Interesting. |
|
Yeah, we've seen the linking order being either critical (or... just broken ;D) in the past (like, way, way, past), and I don't rightly recall all the details (but there's probably an old PR of me ranting about it somewhere ;p). (But as-needed shenanigans sounds sensible, I've seen very weird shit happening when mixing some binutils and GCC versions where as-needed is concerned...). |
|
@benoit-pierre Anyway, I'll go ahead and merge this in a few minutes once it passes? As I said it's fine locally, so it should be in CI here as well. |
|
Some hiccup at the end on the Android builds. I'm not going to go through the arduous ordeal of trying to fix my Android environment tonight, so at least from my end it'll be for tomorrow. Edit: |
|
The real error is before that: |
|
In your environment, is |
|
How many CIs does koreader have… |
|
That environment is identical to the one in base except base only checks that it compiles while front creates an Android package from the result (but we don't check that here because it all takes way too long). |
|
It's also (sufficiently) identical to your local environment normally, but apparently Arch is a wild card. 😅 |
|
But more to the point, |
|
Oh, yeah, missed that… and there are errors about the compiler not being found: |
|
I'll try to run |
I don't know, I think with proper caching it's doable, see the meson branch. |
|
I wouldn't want to do it on every single PR regardless, but I suppose it could potentially be interesting if it could be set up to run on a specific tag or something. But generically speaking I'd only want it to run once a night so as not to unnecessarily waste tons of energy, pretty much exactly as it's done right now. |
|
You don't think up-to-date release artifacts for each commit are useful? On the meson branch, installing is always done with symlinks, so the install can be cached and is small, and running the |
|
Same with running the tests, no need to (re)build anything. |
|
And running the testsuite is much faster. |
|
I have 1.2 MB, which has around 30-40 read books (50-60, if including manga volumes). |
|
Sorry, wrong ticket |
Not compared to the cost, no. Unless you're talking about manipulating the zip (apk) file directly without involving anything from the Android side of the equation, which might work but you'd probably have to fiddle around with the checksums.
This paints an extremely rosy picture of maven/gradle doing "nothing" for a minute. ;-) Make/ninja can do a multitude as much nothing in less than a second.
Nothing is rebuilt when running the tests, only when base is updated. |
|
OK, see koreader/koreader-base#1630 |
Depends on: koreader/android-luajit-launcher#419 and koreader/koreader-base#1620
Additionally, speedup Android development cycle: don't recreate
assets.7zfrom scratch every time, and also ensure it's reproducible to avoid busting gradle's cache.This change is