Mostly builds it again, it seems that some things were not up to date.
Also there's some workarounds that don't seem to be required, at least not
on our docker sdk.
Details
Been building it in https://hub.docker.com/r/sgclark/kde-ci-android/
Diff Detail
- Repository
- R321 KStars
- Branch
- master
- Lint
No Linters Available - Unit
No Unit Test Coverage
I worked a half year ago to make the hackish Android build to something more simple and organized inside CMake on Linux desktop environment.
I don't know how the Docker env. differs from Ubuntu Linux, but I don't wanna spend time to figure it out. I invested plenty of time to make it work inside the current CMake build.
Please:
- Don't remove any workaround, but e.g. the changes to kstars/main.cpp are okay which are compatible with the existing hacks.
- Changes related to be compatible with Docker are welcome if they don't break the desktop build.
What I propose:
- Please submit the minimal changes which are needed to make Docker build work and we can evaluate on the desktop build.
I don't know if you have any means to detect the Docker build inside CMake and make some workarounds conditional based upon the Docker build env.
The docker image is based on ubuntu xenial.
Sure, it's fine that you don't want to spend time on it, that's why I sent the patch. Also I know how this stuff work. After all I'm the author of the toolchain you forked into kstars.
Please:
- Don't remove any workaround, but e.g. the changes to kstars/main.cpp are okay which are compatible with the existing hacks.
Okay, I'll guard them then we can remove them at a later iteration if you guys want.
- Changes related to be compatible with Docker are welcome if they don't break the desktop build.
Of course.
What I propose:
- Please submit the minimal changes which are needed to make Docker build work and we can evaluate on the desktop build.
I don't know if you have any means to detect the Docker build inside CMake and make some workarounds conditional based upon the Docker build env.
Please note that I didn't add any workarounds, I just removed the ones that weren't needed.
The big difference isn't docker or not docker but if the forked toolchain is being used or not.
Ok. Your changes look good, but I don't have time to test it now because I go for a one week-long trip. I will test once I am back and give feedback.
After applying your patch and trying to make a desktop cross-compilation:
- libraw does not build (failed to configure):
CMake Error: The following variables are used in this project, but they are set to NOTFOUND.
Please set them or make sure they are set and tested correctly in the CMake files:
MATH_LIBRARY
linked by target "raw" in directory /home/kecsap/share/laptop_tools/robot_telescope/kstars/build_android/android/3rdparty/libraw linked by target "raw_r" in directory /home/kecsap/share/laptop_tools/robot_telescope/kstars/build_android/android/3rdparty/libraw
- indi does not build (failed to link because of the removal of -DCMAKE_AR=${ANDROID_NDK_ROOT}/toolchains/llvm/prebuilt/linux-x86_64/bin/llvm-ar):
- cfitsio does not build (failed to configure):
CMake Error: The following variables are used in this project, but they are set to NOTFOUND.
Please set them or make sure they are set and tested correctly in the CMake files:
M_LIB
linked by target "cfitsio" in directory /home/kecsap/share/laptop_tools/robot_telescope/kstars/build_android/android/3rdparty/cfitsio
Thanks for the update!
- libraw: fails to build because the swab-workaround is still needed on desktop otherwise:
[ 14%] Building CXX object CMakeFiles/raw.dir/internal/dcraw_common.cpp.o
.../kstars/build_android/android/3rdparty/libraw/internal/dcraw_common.cpp:211:5: error: use of undeclared identifier 'swab'
swab ((char*)pixel, (char*)pixel, count*2); ^
.../kstars/build_android/android/3rdparty/libraw/internal/dcraw_common.cpp:13760:7: error: use of undeclared identifier 'swab'
swab ((char*)ppm2, (char*)ppm2, width*colors*2); ^
2 errors generated.
- cfitsio: There is no sane reason, but CMake can't determinate the static linker correctly (even if -DCMAKE_AR is passed in configuration stage), if the toolchain file is not included in the cfitsio/CMakeLists.txt,. Please don't remove the following workaround line from the cfitsio patch:
include(${CMAKE_TOOLCHAIN_FILE})
I look forward for the next round.
I don't see where the issue in libraw is, can you see if the patching actually happened on your build?
Currently, only the following SDK versions are able to build the KStars Android package:
- Qt 5.10
- Android NDK 15c
- Android SDK with old platform tools v23.
Feedback:
You must patch cfitsio.patch with -p1 otherwise patch command can't apply it.
I don't see where the issue in libraw is, can you see if the patching actually happened on your build?
Yes, the libraw patch must be applied otherwise "swab" symbol is undefined because the NDK system headers are messed between Android platform versions.
Current problems:
- You can't remove this part of the libraw patch against CMakeLists.txt:
+IF (ANDROID)
+ INCLUDE_DIRECTORIES(${CMAKE_SOURCE_DIR}/android-hack)
+ENDIF ()
- You can't remove this line of the libraw patch against CMakeLists.txt otherwise linking fails:
+include(${CMAKE_TOOLCHAIN_FILE})
Next problem:
- After everything built, the final linking fails because of the updated cfitsio version with linking error:
[100%] Linking CXX shared library libkstars.so
/cfitsio/cfileio.c:function fits_init_cfitsio: error: undefined reference to 'https_checkfile'
/cfitsio/cfileio.c:function fits_init_cfitsio: error: undefined reference to 'https_open'
/cfitsio/cfileio.c:function fits_init_cfitsio: error: undefined reference to 'https_file_open'
/cfitsio/cfileio.c:function fits_init_cfitsio: error: undefined reference to 'https_checkfile'
/cfitsio/cfileio.c:function fits_init_cfitsio: error: undefined reference to 'https_file_open'
Oops! Fixed. I wonder why it worked...
I don't see where the issue in libraw is, can you see if the patching actually happened on your build?
Yes, the libraw patch must be applied otherwise "swab" symbol is undefined because the NDK system headers are messed between Android platform versions.
Current problems:
- You can't remove this part of the libraw patch against CMakeLists.txt:
+IF (ANDROID) + INCLUDE_DIRECTORIES(${CMAKE_SOURCE_DIR}/android-hack) +ENDIF ()- You can't remove this line of the libraw patch against CMakeLists.txt otherwise linking fails:
+include(${CMAKE_TOOLCHAIN_FILE})
Fixed and fixed.
Next problem:
- After everything built, the final linking fails because of the updated cfitsio version with linking error:
[100%] Linking CXX shared library libkstars.so /cfitsio/cfileio.c:function fits_init_cfitsio: error: undefined reference to 'https_checkfile' /cfitsio/cfileio.c:function fits_init_cfitsio: error: undefined reference to 'https_open' /cfitsio/cfileio.c:function fits_init_cfitsio: error: undefined reference to 'https_file_open' /cfitsio/cfileio.c:function fits_init_cfitsio: error: undefined reference to 'https_checkfile' /cfitsio/cfileio.c:function fits_init_cfitsio: error: undefined reference to 'https_file_open'
Rolled back the version to minimise the differences.