Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor and further simplify the build script #64

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Issam-b
Copy link
Collaborator

@Issam-b Issam-b commented Sep 16, 2024

This patch contains many changes, ideally more atomic changes, but since it touches most of the script and how it works, it's easier to put it all like this with 4 commits. The changes include the following changes:

  • The script now supports zsh and bash to allow building from macOS and Linux with builtin zsh and bash version 3 or 4.
  • It now uses curl instead of wget, wget is not builtin on macOS.
  • Use the default NDK path from the Android SDK.
  • Allow to set custom build and output folders.
  • The various build folders for each version are now kept around under the 'build' folder.
  • At the same time add the build folder to .gitignore.
  • Create relative symlinks Fix symlinks in the build script. #61.
  • Remove the dependency to patchelf.
  • Add a Qt test to check if openssl is detected with these libs at runtime.
  • Update the prebuilt libraries.

@bog-dan-ro
Copy link
Collaborator

It doesn't build anymore

bogdan@dragon:~/work/kdab/android_openssl$ ./build_ssl.sh
Downloading OpenSSL 1.1.1u
Build 1.1.1u
Extracting OpenSSL 1.1.1u under /home/bogdan/work/kdab/android_openssl
~/work/kdab/android_openssl/openssl-1.1.1u ~/work/kdab/android_openssl
Configuring OpenSSL 1.1.1u with parameters:  shared android-x86 -U__ANDROID_API__ -D__ANDROID_API__=
Building...
In file included from In file included from <built-in>:356:
<built-in>:356:
<command line><command line>::In file included from 25:9: 25<built-in>warning:: 9::'__ANDROID_API__' macro redefined [-Wmacro-redefined]356 :

warning<command line>: :'__ANDROID_API__' macro redefined [-Wmacro-redefined]25
:9: warning: '__ANDROID_API__' macro redefined [-Wmacro-redefined]
#define __ANDROID_API__ #define __ANDROID_API__ 

        ^        ^

#define __ANDROID_API__ 
        ^<command line><command line>
::2323::99<command line>:::  23notenote:: : 9previous definition is hereprevious definition is here:

 note: previous definition is here
#define __ANDROID_API__ 30#define __ANDROID_API__ 30
        ^

        ^
#define __ANDROID_API__ 30
        ^
In file included from <built-in>:356:
<command line>:25:9: warning: '__ANDROID_API__' macro redefined [-Wmacro-redefined]
#define __ANDROID_API__ 
        ^
<command line>:23:9: note: previous definition is here
#define __ANDROID_API__ 30
        ^
In file included from apps/bf_prefix.c:10:
In file included from /home/bogdan/android/ndk/21.4.7075529/sysroot/usr/include/stdio.h:41:
In file included from /home/bogdan/android/ndk/21.4.7075529/sysroot/usr/include/sys/cdefs.h:357:
/home/bogdan/android/ndk/21.4.7075529/sysroot/usr/include/android/api-level.h:60:21: error: invalid token at start of a preprocessor expression
#if __ANDROID_API__ != __ANDROID_API_FUTURE__
                    ^
/home/bogdan/android/ndk/21.4.7075529/sysroot/usr/include/android/api-level.h:141:21: error: invalid token at start of a preprocessor expression
#if __ANDROID_API__ >= 24
                    ^
/home/bogdan/android/ndk/21.4.7075529/sysroot/usr/include/android/api-level.h:146:21: error: invalid token at start of a preprocessor expression
#if __ANDROID_API__ < 29
                    ^
In file included from apps/apps.c:18:
In file included from /home/bogdan/android/ndk/21.4.7075529/sysroot/usr/include/stdio.h:41:
In file included from /home/bogdan/android/ndk/21.4.7075529/sysroot/usr/include/sys/cdefs.h:357:
/home/bogdan/android/ndk/21.4.7075529/sysroot/usr/include/android/api-level.h:60:21: error: In file included from invalid token at start of a preprocessor expression
apps/opt.c:9:
In file included from apps/apps.h:13:
In file included from ./e_os.h:13:
In file included from /home/bogdan/android/ndk/21.4.7075529/sysroot/usr/include/limits.h:38:
In file included from /home/bogdan/android/ndk/21.4.7075529/sysroot/usr/include/sys/cdefs.h:357#if __ANDROID_API__ != __ANDROID_API_FUTURE__:

/home/bogdan/android/ndk/21.4.7075529/sysroot/usr/include/android/api-level.h                    ^:60
:21: error: invalid token at start of a preprocessor expression
#if __ANDROID_API__ != __ANDROID_API_FUTURE__

@bog-dan-ro
Copy link
Collaborator

@Issam-b please apply

diff --git a/build_ssl.sh b/build_ssl.sh
index b7359f7..63c28bb 100755
--- a/build_ssl.sh
+++ b/build_ssl.sh
@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/bin/bash -x
 
 ## Prerequisites
 ## On both Linux, macOS: 'patchelf' command is needed for OpenSSL 3+
@@ -68,7 +68,7 @@ configure_ssl() {
         fi
     done
 
-    case $version_out_dir in
+    case $ssl_version in
     1.1.1*)
         ANDROID_API=21
         ;;

@Issam-b
Copy link
Collaborator Author

Issam-b commented Sep 20, 2024

@bog-dan-ro did you try all the commits, that part for using $ss_version in the switch is already at 7646d9c#diff-fb2021ccb7382cface7486855f6512a35fd4e0640987eda5d51d1f65947652d0R117

@bog-dan-ro
Copy link
Collaborator

@bog-dan-ro did you try all the commits, that part for using $ss_version in the switch is already at 7646d9c#diff-fb2021ccb7382cface7486855f6512a35fd4e0640987eda5d51d1f65947652d0R117

commit 0d3ffa0c441f5d89a748ba54d99f1b57da8bf6fb (HEAD -> Issam-b-master)
Author: Assam Boudjelthia <[email protected]>
Date:   Mon Jun 12 12:38:21 2023 +0300

    Add some notes about the build script to README.md

commit 4e137faaf94922a204fb7e51d5414ddc1e177ecf
Author: Assam Boudjelthia <[email protected]>
Date:   Mon Jun 12 12:26:43 2023 +0300

    Fix patchelf operations for OpenSSL 3.x
    
    The previous refactoring commit 841f289 changed that command by
    prepending relative path to the libs files, and that seem to break
    what we try to achieve with patchelf. This reverts that part.

commit dc132244fc4f5355e5ed8a31f9890de9df9c6233
Author: Assam Boudjelthia <[email protected]>
Date:   Sun Jun 11 19:08:41 2023 +0300

    Fix build script case when building an OpenSSL 3.0.x version
    
    If building for an OpenSSL version that doesn't start with 3.1.x, like
    3.0.7, the script fails to set ANDROID_API value and the build fails.
    This makes the script more inclusive for other OpenSSL 3.x.x versions.

commit de372204401e672cfdc3650051d3517bdb452e98 (master)
....

@Issam-b
Copy link
Collaborator Author

Issam-b commented Sep 20, 2024

Those are some older commits, and doesn't look like the commits in this PR

* The script now supports zsh and bash to allow building from macOS and
  Linux with builtin zsh and bash version 3 or 4.
* It now uses curl instead of wget, wget is not builtin on macOS.
* Use the default NDk path from the Android SDK.
* Allow to set custom build and output folders.
* The various build folders for each version are now kept around under
  the 'build' folder.
* At the same time add the build folder to .gitignore.
* Create relative symlinks KDAB#61.
OpenSSL has the configuration option shlib_variant, so we can use that instead.

This works for version 1.x and 3.x, so it would make the build script more similar between the two versions.

Also, this avoid issues that can come from patchelf, as this patch comes after a bug found in patchelf 0.18 that created wrongly aligned libraries. See NixOS/patchelf#492.
It's a simple check at runtime to see if ssl is supported using the included libs.
@Issam-b
Copy link
Collaborator Author

Issam-b commented Sep 20, 2024

Last push add 'bash -x', and actually allow passing custom build, output and ndk params to the script.

@Issam-b
Copy link
Collaborator Author

Issam-b commented Oct 16, 2024

@bog-dan-ro a colleague tried this PR on Linux and it worked as expected. From the error you got it seems that you're not using the commits from this PR. Could you give this another try?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants