fix(snapshot): enable snapshot builds for native and host targets

[mbroadst]

No description provided.

[imyller]

It is true that want_separate_host_toolset variable from config.gypi created by configure should apply and override the default value configured in common.gypi, but somehow it does not. This might be a bug in Node.js build scripts and before merging this PR I'd like to investigate if this requires upstream patch too.. or maybe we are missing something.

[mbroadst]

@imyller totally fine with me! I'll keep this open in the meantime, I just wanted to make sure the work wasn't lost. I've got this applied to my local copy so I can keep moving on with my work while I wait.

[imyller]

config.gypi seems to be correctly obeyed by Node build, but initial host_arch is detected the same as target_arch causing configure to set want_separate_host_toolset as 0. From Node.js standpoint this is not a cross-compilation situation. We might be able to correct that by properly setting the build environment before calling configure.

| DEBUG: Executing python function sysroot_cleansstate
| DEBUG: Python function sysroot_cleansstate finished
| DEBUG: Executing shell function do_configure
| creating  ./icu_config.gypi
| { 'target_defaults': { 'cflags': [],
|                        'default_configuration': 'Release',
|                        'defines': [],
|                        'include_dirs': [],
|                        'libraries': []},
|   'variables': { 'arm_float_abi': 'hard',
|                  'arm_fpu': 'neon',
|                  'arm_thumb': 0,
|                  'arm_version': '7',
|                  'asan': 0,
|                  'gas_version': '2.25',
|                  'host_arch': 'arm',
|                  'icu_small': 'false',
|                  'node_byteorder': 'little',
|                  'node_enable_v8_vtunejit': 'false',
|                  'node_install_npm': 'true',
|                  'node_no_browser_globals': 'false',
|                  'node_prefix': '/usr',
|                  'node_release_urlbase': '',
|                  'node_shared_http_parser': 'false',
|                  'node_shared_libuv': 'false',
|                  'node_shared_openssl': 'false',
|                  'node_shared_zlib': 'false',
|                  'node_tag': '',
|                  'node_use_dtrace': 'false',
|                  'node_use_etw': 'false',
|                  'node_use_lttng': 'false',
|                  'node_use_openssl': 'true',
|                  'node_use_perfctr': 'false',
|                  'openssl_fips': '',
|                  'openssl_no_asm': 0,
|                  'target_arch': 'arm',
|                  'uv_parent_path': '/deps/uv/',
|                  'uv_use_dtrace': 'false',
|                  'v8_enable_gdbjit': 0,
|                  'v8_enable_i18n_support': 0,
|                  'v8_no_strict_aliasing': 1,
|                  'v8_optimized_debug': 0,
|                  'v8_random_seed': 0,
|                  'v8_use_snapshot': 'false',
|                  'want_separate_host_toolset': 0}}
| creating  ./config.gypi
| creating  ./config.mk

[mbroadst]

@imyller yeah this was the approach I used initially. I didn't want to get too deep into the internals of the node configure process, but we have a number of variables involved: yocto's arches are different than node's, if we use a tuple for our arch passed in will that break other things, etc. This is why I chose to just override and set the variable directly.

[imyller]

@mbroadst There is something very wrong with .gypi file handling. Your "sed trick" works, but defining the variables with proper tools (configure) produces different result.

It might be that V8 compilation somewhere somehow bypasses config.gypi and only parses common.gypi. Also Node.js compilation does not properly obey environment variables used to define CC/CXX/CPP compilers.

I'm feeling bit uneasy to fix the issue this way, but seems that we have no choice if we want to get snapshots working.

This is something that affects Android builders also, so I'd expect the Node.js upstream to fix this is in upcoming releases.

[mbroadst]

@imyller agreed. I did try pretty hard originally to do it the "right way" before resigning to the sed trickery 😄 Having said that I think probably the easiest way going forward atm is to stick with sed (unless you feel inclined to play around with host and target name trickery)