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

scripts: add termux_setup_gnu_as_23c for NDK r25 #11615

Merged
merged 18 commits into from
Aug 24, 2022

Conversation

truboxl
Copy link
Contributor

@truboxl truboxl commented Aug 15, 2022

Fixes #11614
Fixes #11587

@truboxl truboxl force-pushed the libpixman-ndk-r25 branch 2 times, most recently from 1f0004c to 039fd7c Compare August 16, 2022 00:31
@truboxl truboxl marked this pull request as draft August 16, 2022 00:37
@truboxl truboxl force-pushed the libpixman-ndk-r25 branch from 039fd7c to 2eec541 Compare August 16, 2022 00:48
@truboxl
Copy link
Contributor Author

truboxl commented Aug 16, 2022

Not building Arm with custom assembly might have performance penalty... --disable-arm-simd --disable-arm-neon

Next step either choose:

  1. Go ahead build without custom assembly
  2. Patch all the assembly files to be LLVM compatible
  3. Bring back GAS from NDK r23c or another source (can be from Ubuntu package or static build)

@truboxl truboxl force-pushed the libpixman-ndk-r25 branch 15 times, most recently from 223a802 to 3d1d436 Compare August 17, 2022 12:17
@truboxl truboxl marked this pull request as ready for review August 17, 2022 12:22
@truboxl
Copy link
Contributor Author

truboxl commented Aug 17, 2022

I added back GAS from NDK r23c... Seems to work fine with NDK r25...

Copy link
Member

@Grimler91 Grimler91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for looking into this!

Editing/enhancing the ndk really should only be dons as a last resort I think, would be better to fix the assembly in the affected packages, but obviously that isn't trivial.

So, this seems like a good approach for me where we would otherwise have to disable assembly and get massive penalties

# 1. bin/*-linux-android*-as
# 2. *-linux-android*/bin/as (symlink to #1)
# 3. lib/gcc/*-linux-android*/4.9.x/crtbegin.o (dummy file)
cp -f "$NDK"/toolchains/llvm/prebuilt/linux-x86_64/bin/{arm-linux-androideabi,{aarch64,i686,x86_64}-linux-android}-as \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better to add this stuff to a separate folder (maybe something like ~/.termux-build/_cache/android-r23c-gas-api-24-v0) to make it similar to what we do for the ndk toolchain.

@truboxl truboxl marked this pull request as draft August 18, 2022 06:39
@truboxl truboxl force-pushed the libpixman-ndk-r25 branch from 3d1d436 to 082366f Compare August 18, 2022 14:11
truboxl and others added 7 commits August 23, 2022 23:03
Useful if we need to download a separate ndk version for some packages.
termux_step_ functions should be the ones run from build-package.sh,
while non step functions are run from within other functions, or in
build recipes.
And update PATH so as is found.

Co-authored-by: Henrik Grimler <grimler@termux.dev>
@truboxl truboxl force-pushed the libpixman-ndk-r25 branch from a3d7704 to 0d1b991 Compare August 23, 2022 15:06
@truboxl truboxl requested a review from kcubeterm as a code owner August 23, 2022 22:48
@truboxl
Copy link
Contributor Author

truboxl commented Aug 24, 2022

Apparently openssl has a typo ASLAGS instead of ASFLAGS which supposedly should be CFLAGS and fno-integrated-as never worked 😅

Remove it since its no longer needed

I still need to refine the comments

@truboxl truboxl changed the title chore(main): bump libpixman, libgcrypt to rebuild with NDK r25 chore(main): bump hors, libffi, libgcrypt, libpixman, openssl-1.1, openssl to rebuild with NDK r25 Aug 24, 2022
@truboxl truboxl changed the title chore(main): bump hors, libffi, libgcrypt, libpixman, openssl-1.1, openssl to rebuild with NDK r25 scripts: add termux_setup_gnu_as_23c for NDK r25 Aug 24, 2022
@truboxl
Copy link
Contributor Author

truboxl commented Aug 24, 2022

Added a small repro case to determine clang does detect the GCC toolchain (in this case GAS only) so it should clear up the confusion whether the path is needed in the first place.

Will squash all this into one commit if everybody agrees.

Copy link
Member

@Grimler91 Grimler91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work, looks good to me!

if [ ! -d "$NDK" ]; then, that I added, can be removed as well, as setup-android-sdk already checks the same thing (see comment)

local GAS_TOOLCHAIN_REVISION="-v0"
local GAS_TOOLCHAIN_DIR="$TERMUX_COMMON_CACHEDIR/android-r23c-gas-api-${TERMUX_PKG_API_LEVEL}${GAS_TOOLCHAIN_REVISION}"
local NDK="$(dirname "$NDK")"/android-ndk-r23c
if [ ! -d "$NDK" ]; then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this if (that I added) is unnecessary, setup-android-sdk.sh already checks if folder exists

@truboxl
Copy link
Contributor Author

truboxl commented Aug 24, 2022

Gah... messed up the commit message... welp

@truboxl truboxl deleted the libpixman-ndk-r25 branch August 24, 2022 15:21
termux-pacman-bot referenced this pull request in termux-pacman/termux-packages Aug 24, 2022
NDK r25 has removed GNU Assembler (GAS). Removal of GAS introduced a number of build issues.
The most prominent is:
/usr/bin/as: unrecognized option '-EL'

Some options to solve this:
1. Disable building custom assembly and suffer performance penalty
2. Hand rewrite the custom assembly to be LLVM compatible
3. Wait for upstream to write LLVM compatible assembly (openssl, openssl-1.1)
4. Bring back GAS from NDK r23c

In this commit, GAS is brought back as a separate toolchain instead of following NDK r23c file hierarchy.
We pass "--gcc-toolchain=GAS_TOOLCHAIN_DIR" to NDK r25 clang to detect.
Packages only have to add "termux_step_gnu_as_23c" to build.sh to enable GAS.
In the future, we expect packages should follow option 3 more than option 4 as that is a last resort.

This commit also bumps revision for packages that rely (or previously rely) on "-fno-integrated-as":
hors, libffi, libgcrypt, libpixman, openssl, openssl-1.1

Co-authored-by: Henrik Grimler <grimler@termux.dev>
Co-authored-by: Chongyun Lee <45286352+licy183@users.noreply.github.com>
ifurther pushed a commit to ifurther/termux-packages that referenced this pull request Aug 30, 2022
NDK r25 has removed GNU Assembler (GAS). Removal of GAS introduced a number of build issues.
The most prominent is:
/usr/bin/as: unrecognized option '-EL'

Some options to solve this:
1. Disable building custom assembly and suffer performance penalty
2. Hand rewrite the custom assembly to be LLVM compatible
3. Wait for upstream to write LLVM compatible assembly (openssl, openssl-1.1)
4. Bring back GAS from NDK r23c

In this commit, GAS is brought back as a separate toolchain instead of following NDK r23c file hierarchy.
We pass "--gcc-toolchain=GAS_TOOLCHAIN_DIR" to NDK r25 clang to detect.
Packages only have to add "termux_step_gnu_as_23c" to build.sh to enable GAS.
In the future, we expect packages should follow option 3 more than option 4 as that is a last resort.

This commit also bumps revision for packages that rely (or previously rely) on "-fno-integrated-as":
hors, libffi, libgcrypt, libpixman, openssl, openssl-1.1

Co-authored-by: Henrik Grimler <grimler@termux.dev>
Co-authored-by: Chongyun Lee <45286352+licy183@users.noreply.github.com>
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.

[Bug]: libpixman build failed for [arm] ndk25 [Bug]: libgcrypt build failed ndk25
3 participants