build: fix cross-compile issue for Android

[robertchiras]

Checklist

• tests and code linting passes
• a test and/or benchmark is included
• documentation is changed or added
• the commit message follows commit guidelines

Affected core subsystem(s)

build

Description of change

Some tools from icu are building host binaries, which are failing
because the current logic is using the target_arch.
So, I added _host exports to local build tools in order to use them,
when building host binaries.
Also, in the commin.gypi file, we need to take care of the current
toolset used when generating makefiles. This needs to be done, so that
we will use the host_arch when generating makefiles for host and
target_arch when generating makefiles for target.
Also, removed the unnecessary define '_GLIBCXX_USE_C99_MATH'.

Signed-off-by: Robert Chiras robert.chiras@intel.com

[eljefedelrodeodeljefe]

Ref: #6733
Ref: nodejs/build#359

R=@bnoordhuis

[bnoordhuis]

I'm not that pleased with the duplication in common.gypi, to be honest. Surely there is a better way.

[robertchiras]

@bnoordhuis: me neither, but the gyp configuration file structure is still uncertain to me.
I tried using a variable, which I would then set it to the right architecture for target or host toolset. But, when I use that variable in 'conditions', I always have just the same one value, for both target and host generated make files: the one initially assigned to the variable.
Is anyone else more experimented in gyp configuration files that might suggest a better way?

[bnoordhuis]

Would something like this at the top of common.gypi work?

{
  'variables': {
    # ...
    'variables': {
      'conditions': [
        ['_toolset=="host", {
          'yourvar%': 1,
        }, {
          'yourvar%': 0,
        }]
      ]
    }
  },
  # ...
}

[robertchiras]

It doesn't work, because it complains about _toolset. Apparently, _toolset is available just in target_conditions. And, if I change 'conditions' to 'target_conditions', I can't access the variable. Probably, any variable assigned in 'target_conditions' is available just in this context.

[bnoordhuis]

Okay. Correct me if I'm wrong but the crux is you want to link with -llog iff targeting android? Would this work?

{
  # ...
  'conditions': [
    ['OS=="android"', {
      'target_conditions': [
        ['_toolset="target", {
          'libraries': ['-llog'],
        }]
      ]
    }]
  ],
  # ...
}

If that is not it, can you explain why you duplicate the host and target toolsets en masse in your current approach?

[robertchiras]

No, the idea is not just linking with -llog if targeting android.
Recently, I see that ICU has been added into node tree. ICU has some tools (like genccode, icutools, icupkg, and so on) that needed to be built for host. Of course, we don't need the Android Logger for these binaries built for host, so this is why I added the toolset condition for -llog.
The duplication of arch was added to use the correct arch when generating make files. For example, I am building node for Intel Edison on Ubuntu 64-bit. Since Intel Edison is 32-bit, I need all my target make files to have -m32, but my host make files needs -m64. So, in this case, the config.gypi will have host_arch=x64 and target_arch=ia32
With current common.gypi file, there is no use for different host_arch and target_arch variables, since just the target_arch will be used for both target and host make files, generating unusable host binaries.

[bnoordhuis]

Okay, I think I understand now. Would something like this work?

diff --git a/arch.gypi b/arch.gypi
new file mode 100644
index 0000000..3bf67d9
--- /dev/null
+++ b/arch.gypi
@@ -0,0 +1,53 @@
+{
+  'conditions': [
+    [ 'target_arch=="ia32"', {
+      'cflags': [ '-m32' ],
+      'ldflags': [ '-m32' ],
+    }],
+    [ 'target_arch=="x32"', {
+      'cflags': [ '-mx32' ],
+      'ldflags': [ '-mx32' ],
+    }],
+    [ 'target_arch=="x64"', {
+      'cflags': [ '-m64' ],
+      'ldflags': [ '-m64' ],
+    }],
+    [ 'target_arch=="ppc" and OS!="aix"', {
+      'cflags': [ '-m32' ],
+      'ldflags': [ '-m32' ],
+    }],
+    [ 'target_arch=="ppc64" and OS!="aix"', {
+      'cflags': [ '-m64', '-mminimal-toc' ],
+      'ldflags': [ '-m64' ],
+    }],
+    [ 'target_arch=="s390"', {
+      'cflags': [ '-m31' ],
+      'ldflags': [ '-m31' ],
+    }],
+    [ 'target_arch=="s390x"', {
+      'cflags': [ '-m64' ],
+      'ldflags': [ '-m64' ],
+    }],
+    [ 'OS=="solaris"', {
+      'cflags': [ '-pthreads' ],
+      'ldflags': [ '-pthreads' ],
+      'cflags!': [ '-pthread' ],
+      'ldflags!': [ '-pthread' ],
+    }],
+    [ 'OS=="aix"', {
+      'conditions': [
+        [ 'target_arch=="ppc"', {
+          'ldflags': [ '-Wl,-bmaxdata:0x60000000/dsa' ],
+        }],
+        [ 'target_arch=="ppc64"', {
+          'cflags': [ '-maix64' ],
+          'ldflags': [ '-maix64' ],
+        }],
+      ],
+      'ldflags!': [ '-rdynamic' ],
+    }],
+    [ 'OS=="android" and _toolset=="target"', {
+      'libraries': [ '-llog' ],
+    }],
+  ],
+}
diff --git a/common.gypi b/common.gypi
index 8da603d..0099f78 100644
--- a/common.gypi
+++ b/common.gypi
@@ -242,60 +242,14 @@
           ['_type=="static_library"', {
             'standalone_static_library': 1, # disable thin archive which needs binutils >= 2.19
           }],
-        ],
-        'conditions': [
-          [ 'target_arch=="ia32"', {
-            'cflags': [ '-m32' ],
-            'ldflags': [ '-m32' ],
-          }],
-          [ 'target_arch=="x32"', {
-            'cflags': [ '-mx32' ],
-            'ldflags': [ '-mx32' ],
-          }],
-          [ 'target_arch=="x64"', {
-            'cflags': [ '-m64' ],
-            'ldflags': [ '-m64' ],
-          }],
-          [ 'target_arch=="ppc" and OS!="aix"', {
-            'cflags': [ '-m32' ],
-            'ldflags': [ '-m32' ],
-          }],
-          [ 'target_arch=="ppc64" and OS!="aix"', {
-       'cflags': [ '-m64', '-mminimal-toc' ],
-       'ldflags': [ '-m64' ],
-      }],
-          [ 'target_arch=="s390"', {
-            'cflags': [ '-m31' ],
-            'ldflags': [ '-m31' ],
+          ['_toolset=="host"', {
+            'includes': [ 'arch.gypi' ],
           }],
-          [ 'target_arch=="s390x"', {
-            'cflags': [ '-m64' ],
-            'ldflags': [ '-m64' ],
-          }],
-          [ 'OS=="solaris"', {
-            'cflags': [ '-pthreads' ],
-            'ldflags': [ '-pthreads' ],
-            'cflags!': [ '-pthread' ],
-            'ldflags!': [ '-pthread' ],
-          }],
-          [ 'OS=="aix"', {
-            'conditions': [
-              [ 'target_arch=="ppc"', {
-                'ldflags': [ '-Wl,-bmaxdata:0x60000000/dsa' ],
-              }],
-              [ 'target_arch=="ppc64"', {
-                'cflags': [ '-maix64' ],
-                'ldflags': [ '-maix64' ],
-              }],
-            ],
-            'ldflags!': [ '-rdynamic' ],
+          ['_toolset=="target"', {
+            'includes': [ 'arch.gypi' ],
           }],
         ],
       }],
-      [ 'OS=="android"', {
-        'defines': ['_GLIBCXX_USE_C99_MATH'],
-        'libraries': [ '-llog' ],
-      }],
       ['OS=="mac"', {
         'defines': ['_DARWIN_USE_64_BIT_INODE=1'],
         'xcode_settings': {

(diff)

[lundibundi]

Hello, I'm not that familiar with gyp build system but I needed to build node for android arm{,64} and applied your patches with slight changes, because they are a bit outdated. With this I compiled node for mentioned architectures.
Also added support for arm64, as it was missing. Hope it helps.

diff --git a/android-configure b/android-configure
index 1dc238e..ab1f38c 100755
--- a/android-configure
+++ b/android-configure
@@ -18,10 +18,15 @@ fi
 CC_VER="4.9"
 case $ARCH in
     arm)
-        DEST_CPU="$ARCH"
+        DEST_CPU="${ARCH}"
         SUFFIX="$ARCH-linux-androideabi"
         TOOLCHAIN_NAME="$SUFFIX"
         ;;
+    arm64)
+        DEST_CPU="${ARCH}"
+        SUFFIX="aarch64-linux-android"
+        TOOLCHAIN_NAME="$SUFFIX"
+        ;;
     x86)
         DEST_CPU="ia32"
         SUFFIX="i686-linux-android"
@@ -50,6 +55,10 @@ export AR=$TOOLCHAIN/bin/$SUFFIX-ar
 export CC=$TOOLCHAIN/bin/$SUFFIX-gcc
 export CXX=$TOOLCHAIN/bin/$SUFFIX-g++
 export LINK=$TOOLCHAIN/bin/$SUFFIX-g++
+export AR_host=ar
+export CC_host=gcc
+export CXX_host=g++
+export LINK_host=g++

 GYP_DEFINES="target_arch=$ARCH"
 GYP_DEFINES+=" v8_target_arch=$ARCH"
@@ -57,6 +66,11 @@ GYP_DEFINES+=" android_target_arch=$ARCH"
 GYP_DEFINES+=" host_os=linux OS=android"
 export GYP_DEFINES

+# You might want to add the following options if you target arm or arm64
+# --enable-static  to build node as static lib
+# --with-intl=none to disable icu because it's quite bothersome to add support
+#                  for it in android and it bloats outputs a lot
+
 if [ -f "configure" ]; then
     ./configure \
         --dest-cpu=$DEST_CPU \
diff --git a/arch.gypi b/arch.gypi
new file mode 100644
index 0000000..894ad9c
--- /dev/null
+++ b/arch.gypi
@@ -0,0 +1,54 @@
+{
+  'conditions': [
+    [ 'target_arch=="ia32"', {
+      'cflags': [ '-m32' ],
+      'ldflags': [ '-m32' ],
+    }],
+    [ 'target_arch=="x32"', {
+      'cflags': [ '-mx32' ],
+      'ldflags': [ '-mx32' ],
+    }],
+    [ 'target_arch=="x64"', {
+      'cflags': [ '-m64' ],
+      'ldflags': [ '-m64' ],
+    }],
+    [ 'target_arch=="ppc" and OS!="aix"', {
+      'cflags': [ '-m32' ],
+      'ldflags': [ '-m32' ],
+    }],
+    [ 'target_arch=="ppc64" and OS!="aix"', {
+      'cflags': [ '-m64', '-mminimal-toc' ],
+      'ldflags': [ '-m64' ],
+    }],
+    [ 'target_arch=="s390"', {
+      'cflags': [ '-m31' ],
+      'ldflags': [ '-m31' ],
+    }],
+    [ 'target_arch=="s390x"', {
+      'cflags': [ '-m64' ],
+      'ldflags': [ '-m64' ],
+    }],
+    [ 'OS=="solaris"', {
+      'cflags': [ '-pthreads' ],
+      'ldflags': [ '-pthreads' ],
+      'cflags!': [ '-pthread' ],
+      'ldflags!': [ '-pthread' ],
+    }],
+    [ 'OS=="aix"', {
+      'conditions': [
+        [ 'target_arch=="ppc"', {
+          'ldflags': [ '-Wl,-bmaxdata:0x60000000/dsa' ],
+        }],
+        [ 'target_arch=="ppc64"', {
+          'cflags': [ '-maix64' ],
+          'ldflags': [ '-maix64' ],
+        }],
+      ],
+      'ldflags': [ '-Wl,-bbigtoc' ],
+      'ldflags!': [ '-rdynamic' ],
+    }],
+   [ 'node_shared=="true"', {
+     'cflags': [ '-fPIC' ],
+   }],
+  ],
+}
diff --git a/common.gypi b/common.gypi
index 646db0d..cf8b124 100644
--- a/common.gypi
+++ b/common.gypi
@@ -247,64 +247,18 @@
           ['_type=="static_library"', {
             'standalone_static_library': 1, # disable thin archive which needs binutils >= 2.19
           }],
-        ],
-        'conditions': [
-          [ 'target_arch=="ia32"', {
-            'cflags': [ '-m32' ],
-            'ldflags': [ '-m32' ],
-          }],
-          [ 'target_arch=="x32"', {
-            'cflags': [ '-mx32' ],
-            'ldflags': [ '-mx32' ],
-          }],
-          [ 'target_arch=="x64"', {
-            'cflags': [ '-m64' ],
-            'ldflags': [ '-m64' ],
-          }],
-          [ 'target_arch=="ppc" and OS!="aix"', {
-            'cflags': [ '-m32' ],
-            'ldflags': [ '-m32' ],
-          }],
-          [ 'target_arch=="ppc64" and OS!="aix"', {
-       'cflags': [ '-m64', '-mminimal-toc' ],
-       'ldflags': [ '-m64' ],
-      }],
-          [ 'target_arch=="s390"', {
-            'cflags': [ '-m31' ],
-            'ldflags': [ '-m31' ],
-          }],
-          [ 'target_arch=="s390x"', {
-            'cflags': [ '-m64' ],
-            'ldflags': [ '-m64' ],
-          }],
-          [ 'OS=="solaris"', {
-            'cflags': [ '-pthreads' ],
-            'ldflags': [ '-pthreads' ],
-            'cflags!': [ '-pthread' ],
-            'ldflags!': [ '-pthread' ],
-          }],
-          [ 'OS=="aix"', {
-            'conditions': [
-              [ 'target_arch=="ppc"', {
-                'ldflags': [ '-Wl,-bmaxdata:0x60000000/dsa' ],
-              }],
-              [ 'target_arch=="ppc64"', {
-                'cflags': [ '-maix64' ],
-                'ldflags': [ '-maix64' ],
-              }],
-            ],
-            'ldflags': [ '-Wl,-bbigtoc' ],
-            'ldflags!': [ '-rdynamic' ],
-          }],
-          [ 'node_shared=="true"', {
-            'cflags': [ '-fPIC' ],
+           ['_toolset=="host"', {
+            'includes': [ 'arch.gypi' ],
+           }],
+           ['_toolset=="target"', {
+            'includes': [ 'arch.gypi' ],
           }],
+           ['OS=="android" and _toolset=="target"', {
+            'defines': ['_GLIBCXX_USE_C99_MATH'],
+            'libraries': [ '-llog' ],
+           }],
         ],
-      }],
-      [ 'OS=="android"', {
-        'defines': ['_GLIBCXX_USE_C99_MATH'],
-        'libraries': [ '-llog' ],
-      }],
+     }],
       ['OS=="mac"', {
         'defines': ['_DARWIN_USE_64_BIT_INODE=1'],
         'xcode_settings': {

diff

[jasnell]

Is this still necessary?

[robertchiras]

No, you can drop this.

[jasnell]

Thank you!