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

Add package: gn #7290

Merged
2 commits merged into from
Sep 3, 2021
Merged

Add package: gn #7290

2 commits merged into from
Sep 3, 2021

Conversation

thunder-coding
Copy link
Member

@thunder-coding thunder-coding commented Aug 9, 2021

TODOs:

  • Add gn
  • Test if it works
    Sample project compile well on aarch64 Android 11
  • Create a termux_setup_gn script
    - [ ] Add a package that requires gn to build to test termux_setup_gn
    - [ ] Add package v8
    - [ ] nodejs link to shared v8

@thunder-coding thunder-coding marked this pull request as draft August 9, 2021 02:41
@thunder-coding
Copy link
Member Author

thunder-coding commented Aug 9, 2021

Its not picking up the correct linker

cxx = aarch64-linux-android-clang++
ar = aarch64-linux-android-ar
ld = aarch64-linux-android-clang++

Notice -clang++ at the end instead of -ld

Same problem for other archs

EDIT: The gn gen.py was correct, it choose the correct ld. I have disabled building of gn_unittests which is basically unit tests which are useless to build (especially on CI)

@termux termux deleted a comment from SPECIFIC10 Aug 9, 2021
@termux termux temporarily blocked SPECIFIC10 Aug 9, 2021
@termux termux deleted a comment from SPECIFIC10 Aug 9, 2021
@thunder-coding
Copy link
Member Author

thunder-coding commented Aug 10, 2021

The compiled gn works well and for testing termux_setup_gn script, I will be adding the v8 package and make nodejs link to that v8. The v8 version will be kept roughly near to nodejs' in order to reduce number of patches.

Also while building Node.js 16, v8 seems to compile for wrong arch, so having shared v8 makes sense. See suhan-paradkar#8

@xeffyr I need your approval before working on this

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Please make sure you have checked how termux_setup_golang or termux_setup_cmake work and adjust termux_setup_gn accordingly.

Specifically:

  • Need integrity check of downloaded tarball. No matter whether it has been downloaded and checked previously. You need to verify it always.
  • Do not re-install gn if it was already installed.

scripts/build/setup/termux_setup_gn.sh Outdated Show resolved Hide resolved
scripts/build/setup/termux_setup_gn.sh Outdated Show resolved Hide resolved
@ghost
Copy link

ghost commented Aug 12, 2021

Fails at

ERROR at //BUILD.gn:5285:3: Assertion failed.
  assert(!is_component_build)
  ^-----

Possibly you need to add is_component_build=false.

@suhan-paradkar
Copy link
Contributor

It requires libicu now??

@thunder-coding
Copy link
Member Author

It requires libicu now??

It requires icu for i18n support

@hnmn
Copy link

hnmn commented Aug 13, 2021

ig target_os="android" be correct
Also target_os = ['android'] need in .gclient

@hnmn
Copy link

hnmn commented Aug 13, 2021

@thunder-coding
Copy link
Member Author

thunder-coding commented Aug 13, 2021

I got it why v8 fails with shared libicu. libicu is missing pkg configs, I will push the fix soon.

And I got v8 building for x86_64. aarch64 build is going on.

If it passes, I will add patches for v8 which will be stealed from nodejs v8 patches

@suhan-paradkar
Copy link
Contributor

Well.. 32 bit CPUs are facing few strange problems with gn..

@thunder-coding
Copy link
Member Author

thunder-coding commented Aug 14, 2021

Well.. 32 bit CPUs are facing few strange problems with gn..

Probably that's due to some variables defined in scripts/build/termux_step_setup_toolchain.sh

Will try unsetting all of them after CI for aarch64 and x86_64 pass

@suhan-paradkar
Copy link
Contributor

Well... About aarch64 build... Looks like the CI ran out of space... Screenshot_2021-08-14-20-45-55-29.jpg

@suhan-paradkar
Copy link
Contributor

suhan-paradkar commented Aug 14, 2021

Same about x86_64 too...
You may need to free up space just like in rust...
See

if grep -qP '^rust$' ./built_packages.txt ; then

@thunder-coding
Copy link
Member Author

thunder-coding commented Aug 14, 2021

Now, that I have revised libicu to include pkg-config, I can confirm that V8 is looking for icu-i18n.pc at a really wrong place

https://github.com/chromium/chromium/blob/be8ee8535827682b07c0864c165eb9bca4005521/build/config/sysroot.gni#L20

Setting sysroot to /data/data/com.termux/files may help

The reason why it should be /data/data/com.termux/files instead of @TERMUX_PREFIX@ is https://github.com/chromium/chromium/blob/be8ee8535827682b07c0864c165eb9bca4005521/build/config/linux/pkg-config.py#L62

@thunder-coding thunder-coding mentioned this pull request Aug 15, 2021
@thunder-coding
Copy link
Member Author

thunder-coding commented Aug 17, 2021

https://github.com/chromium/chromium/blob/master/docs/clang.md

Seems like V8 uses a different clang binary than what we expect to compile stuff with.

I need some help with this since I don't have much knowledge about these compiler stuff

@ghost
Copy link

ghost commented Aug 17, 2021

@thunder-coding The problem is --sysroot=../../../../../../../data/data/com.termux/files. Termux prefix is not valid compiler system root. There should be something like --sysroot=$TERMUX_STANDALONE_TOOLCHAIN/sysroot instead. Any headers and library paths of dependencies should be included through CFLAGS and LDFLAGS respectively.

@thunder-coding
Copy link
Member Author

The input errors seem to be prominent even after setting sysroot to $TERMUX_STANDALONE_TOOLCHAIN/sysroot

@hnmn
Copy link

hnmn commented Aug 20, 2021

v8_enable_i18n_support=false try this

@thunder-coding
Copy link
Member Author

v8_enable_i18n_support=false try this

This works, but at the cost of i18n support which is not desired.

@hnmn
Copy link

hnmn commented Aug 21, 2021

Can we just follow the official build guide for cross compiling v8?
Ig gn is looking for ndk at $v8_root/third_party

@thunder-coding
Copy link
Member Author

thunder-coding commented Aug 26, 2021

Can we just follow the official build guide for cross compiling v8?
Ig gn is looking for ndk at $v8_root/third_party

I tried this approach and build fails with some GN import errors.

GN doesn't seem to respect $CC, $CXX and other environment variables. To cross compile for Android, target_os="android" needs ro be passed to GN args, but it fails as I said above. So I am gonna try providing a fake GCC instead.

@thunder-coding
Copy link
Member Author

V8 seems to have a lot of problems compiling for Termux. Should I skip adding it? Atleast we can have GN for those who would like to have it...

@thunder-coding thunder-coding marked this pull request as ready for review September 1, 2021 09:01
@thunder-coding
Copy link
Member Author

Okay, so I have removed all my commits for V8. I might try if I can get it building in another PR. And about the termux_setup_gn script, I am sure it will work, but for evey package that requires GN to build, we may need to patch respective BUILD.gn file of the same in order to make it choose correct build tools because GN doesn't by itself choose proper build tools from environment variables.

@thunder-coding thunder-coding requested a review from a user September 3, 2021 07:46
@ghost ghost merged commit 2d38944 into termux:master Sep 3, 2021
@sk0kanik sk0kanik mentioned this pull request Dec 11, 2021
Closed
This pull request was closed.
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.

3 participants