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

r21 cross over in progress #4809

Closed
wants to merge 13 commits into from
Closed

Conversation

its-pointless
Copy link
Contributor

don't merge

@finagolfin
Copy link
Member

What's wrong with the current spawn.h for fish?

@its-pointless
Copy link
Contributor Author

What's wrong with the current spawn.h for fish?

i haven't looked beyond "it won't compile" for now.

@finagolfin
Copy link
Member

OK, it is now the official header from the NDK, so it should work. I've been using it without a problem, I'll look into it.

@finagolfin
Copy link
Member

Alright, just tried this pull and fish compiled fine after taking out d7c97d2, once I made sure libandroid-spawn was built first. This should fix it instead:

diff --git a/packages/fish/build.sh b/packages/fish/build.sh
index b58430de0..e151e7f48 100644
--- a/packages/fish/build.sh
+++ b/packages/fish/build.sh
@@ -7,7 +7,7 @@ TERMUX_PKG_SRCURL=https://github.com/fish-shell/fish-shell/releases/download/$TE
 TERMUX_PKG_SHA256=14728ccc6b8e053d01526ebbd0822ca4eb0235e6487e832ec1d0d22f1395430e
 # fish calls 'tput' from ncurses-utils, at least when cancelling (Ctrl+C) a command line.
 # man is needed since fish calls apropos during command completion.
-TERMUX_PKG_DEPENDS="libc++, ncurses, libandroid-support, ncurses-utils, man, bc, pcre2"
+TERMUX_PKG_DEPENDS="libc++, ncurses, libandroid-support, libandroid-spawn, ncurses-utils, man, bc, pcre2"
 TERMUX_PKG_BUILD_IN_SRC=true
 TERMUX_PKG_EXTRA_CONFIGURE_ARGS="
 ac_cv_file__proc_self_stat=yes

Also, I had to apply this NDK patch along with this pull, guessing you just forgot to include it from your local repo:

diff --git a/ndk-patches/paths.h.patch b/ndk-patches/paths.h.patch
index e7834203c..f98da5488 100644
--- a/ndk-patches/paths.h.patch
+++ b/ndk-patches/paths.h.patch
@@ -14,7 +14,7 @@
  #define _PATH_CONSOLE "/dev/console"
  
  /** Default shell search path. */
--#define _PATH_DEFPATH "/sbin:/system/sbin:/product/bin:/apex/com.android.runtime/bin:/system/bin:/system/xbin:/odm/bin:/vendor/bin:/vendor/xbin"
+-#define _PATH_DEFPATH "/product/bin:/apex/com.android.runtime/bin:/apex/com.android.art/bin:/system/bin:/system/xbin:/odm/bin:/vendor/bin:/vendor/xbin"
 +#define        _PATH_DEFPATH   "@TERMUX_PREFIX@/bin:@TERMUX_PREFIX@/bin/applets"
  
  /** Path to the directory containing device files. */

@its-pointless
Copy link
Contributor Author

ah right forgot paths patch

@finagolfin
Copy link
Member

Needs a rebase, anything holding this up?

@Grimler91
Copy link
Member

@buttaface we are waiting for r21b to avoid any problems/unexpected behaviour due to android/ndk#1166

@finagolfin
Copy link
Member

Just a head's up that the upcoming clang 10 breaks looking for the C++ include path for headers in Termux, as they assume the NDK layout instead. If someone has commit access to LLVM, maybe they can get it fixed now.

@finagolfin
Copy link
Member

Is anyone working on a clang 10 pull? I've been using it as part of Swift master and can probably submit a Termux pull for that this week, if nobody else is working on it.

Also, I've been trying this pull out with NDK 21 lately and noticed that binaries cross-compiled that way don't work when linked against libc++_shared.so in Termux, because of a missing __emutls_get_address symbol. If I download and make sure the libc++_shared.so from NDK 21 is used, it works again. I don't know if this is one of the reasons you're waiting on 21b or if that needs to be fixed separately, just letting you know.

@its-pointless
Copy link
Contributor Author

Every ndk update requires rebuilding all c++ code. The symbols in libc++_shared.so like __emutls_get_address should be hidden which means for us there is a high likely hood of broken stuff between ndk21 and ndk21b. For most android devs this is a non issue. For us it would mean rebuoldimg packages a second time when ndk21b is released.

@its-pointless
Copy link
Contributor Author

if you are going to do llvm 10 ( i havent looked at it yet) make sure you get lldb working as well.

@finagolfin
Copy link
Member

finagolfin commented Apr 4, 2020

Alright, I will get LLVM 10 building and submit a pull. Is there a problem with the current lldb 9.0.1 package for Termux or are you saying it somehow breaks in 10?

Update: Oh, I guess you mean that lldb is built separately? Sure, will try that too.

@its-pointless
Copy link
Contributor Author

Yeah in llvm 9 it was easier to build seperately both in terms of avoiding having to patch cmake files and packaging it so as to avoid having another sub package for libllvm.

@finagolfin
Copy link
Member

Looks like NDK 21b just dropped.

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.

None yet

3 participants