readium / readium-sdk

A C++ ePub renderer SDK
BSD 3-Clause "New" or "Revised" License
385 stars 164 forks source link

Migrate from GCC to CLANG #240

Open clebeaupin opened 8 years ago

danielweck commented 7 years ago

Another Clang fix: https://github.com/readium/readium-sdk/blob/develop/ePub3/ThirdParty/sha1/sha1.h#L30

        static void SHA1::storeBigEndianUint32( unsigned char* byte, Uint32 num );
        static void SHA1::hexPrinter( unsigned char* c, int l );

(the SHA1:: qualifier must be removed)

danielweck commented 7 years ago

Info: I am using this code diff for the LSD feature branch, so I can test Clang instead of GCC in my Android build (with which I currently experience random segmentation faults). armv7 and x86 seem to be compiling fine...I will report later about linking and runtime. Watch this space :) See: https://github.com/readium/readium-sdk/commit/f65bfd83144d5ffc52ab2f773f092086fe4f4246

danielweck commented 7 years ago

Ah, I finished fixing compilation and linking bugs in my LCP/LSD branch (Android launcher app), and I now get runtime segmentation faults on std::regex (when I attempt to load a LCP file), and libzip (when I attempt to load an EPUB archive). Damn!! :( I believe we are using libc++ here, not libstdc++, right?

danielweck commented 7 years ago

Update: I implemented a boolean flag in local.properties similar to readium.ndk_debug (which is used to activate the Gradle "experimental" plugin that allows hybrid Java / C++ LLDB debugging) to easily control switching from GCC to CLANG: readium.ndk_clang (true or false). Whilst I was at it, I also implemented X86 and ARM switches (otherwise both build and it is quite time-consuming). Code in the feature/lsd branches: LCP client lib: https://github.com/readium/readium-lcp-client/commit/69a7c49c59dc1a2758626d1d57226b4df99cf146 Readium SDK: https://github.com/readium/readium-sdk/commit/997fe7a4ef82004ef866879c4dfaaa7657936d45 Android app: https://github.com/readium/SDKLauncher-Android/commit/a92e41acc07a3e2c36d3862419e6b466c6a9915d

danielweck commented 7 years ago

Interesting remarks about the LLVM libc++ runtime (c++_static and c++_shared / libc++_shared.so): https://developer.android.com/ndk/guides/cpp-support.html " The NDK's libc++ is not stable. Not all the tests pass, and the test suite is not comprehensive. Some known issues are: Using c++_shared on ARM can crash when an exception is thrown. "

danielweck commented 7 years ago

Note that the static ICU4c libs are pre-compiled with GCC, so this might need to be ported too: https://github.com/readium/readium-sdk/tree/develop/ePub3/ThirdParty/icu4c/lib ...oh, and Android's libxml2 is patched to support ICU unicode conversions: https://github.com/readium/readium-sdk/blob/develop/ePub3/ThirdParty/libxml2-android/patches/0001-Add-ICU-support-for-libxml.patch

danielweck commented 7 years ago

Regarding the ICU4c lib, there are build instructions here: https://github.com/readium/readium-sdk/issues/245 (ARM 64)

danielweck commented 7 years ago

Actually, ICU is not even required. See last comment in this PR discussion thread: https://github.com/readium/readium-sdk/issues/245#issuecomment-257232201

danielweck commented 7 years ago

Note that I have successfully been using NDK 13 with GCC / gnustl (shared lib), but Clang fails to link with libc++ (also shared lib), and the Clang build with gnustl fails at runtime. Google will not fully deprecate GCC until the remaining libc++ instability / missing API issues are solved, so we can continue to use Gcc for now.

Also note that sometimes Android Studio's own update channel announces a new NDK download before it is available in the Android SDK Manager (or on the official NDK web page). This is actually why/how I have been using NDK 13b for the past few days. Regarding the change log, I was advised to check this wiki page: https://github.com/android-ndk/ndk/wiki ...in addition to: https://developer.android.com/ndk/downloads/index.html

rkwright commented 7 years ago

@danielweck @bluefirepatrick @clebeaupin @llemeurfr I STRONGLY feel that we should simply REMOVE all the code associated with these future/synch code that is in the SDK. 0.26 and the merger of the LCP code seems like a really confluent point in time to do it.