Skip to content
This repository has been archived by the owner on Oct 15, 2020. It is now read-only.

build: fix ARM build #265

Merged
merged 2 commits into from
May 28, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions configure
Original file line number Diff line number Diff line change
Expand Up @@ -787,6 +787,9 @@ def configure_arm(o):

o['variables']['arm_fpu'] = options.arm_fpu or arm_fpu

if flavor == 'win':
o['msvs_windows_sdk_version'] = 'v10.0'
Copy link
Contributor

@refack refack May 26, 2017

Choose a reason for hiding this comment

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

Should be o['target_defaults']['msvs_windows_sdk_version']

Copy link
Contributor

Choose a reason for hiding this comment

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

configure_arm is called from configure_node which is called from configure_engine (https://github.com/nodejs/node-chakracore/blob/xplat/configure#L1338). At the point configure_engine is called, output['target_defaults'] hasn't been defined yet- it gets defined later at https://github.com/nodejs/node-chakracore/blob/xplat/configure#L1376. So it looks like the way to get it propagated from configure_arm is to set the property directly on o here? Or did I misunderstand something?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep you are right.

Copy link
Contributor

Choose a reason for hiding this comment

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

There's just a wierd hack at https://github.com/nodejs/node-chakracore/blob/xplat/configure#L1355 for the ['variables'] that always confuses me

Copy link
Contributor

Choose a reason for hiding this comment

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

Haha yeah, I got thrown off by that too- it seemed like a roundabout way to do things 😄



def configure_mips(o):
can_use_fpu_instructions = (options.mips_float_abi != 'soft')
Expand Down
4 changes: 0 additions & 4 deletions deps/chakrashim/chakracore.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
'library%': 'static_library', # build chakracore as static library or dll
'component%': 'static_library', # link crt statically or dynamically
'chakra_dir%': 'core',
'msvs_windows_target_platform_version_prop': '',
'icu_args%': '',
'icu_include_path%': '',
'linker_start_group%': '',
Expand All @@ -19,8 +18,6 @@
['target_arch=="x64"', { 'Platform': 'x64' }],
['target_arch=="arm"', {
'Platform': 'arm',
'msvs_windows_target_platform_version_prop':
'/p:WindowsTargetPlatformVersion=$(WindowsTargetPlatformVersion)',
}],
['OS!="win"', {
'icu_include_path': '../<(icu_path)/source/common'
Expand Down Expand Up @@ -113,7 +110,6 @@
'/p:Configuration=$(ConfigurationName)',
'/p:RuntimeLib=<(component)',
'/p:AdditionalPreprocessorDefinitions=COMPILE_DISABLE_Simdjs=1',
'<(msvs_windows_target_platform_version_prop)',
'/m',
'<@(_inputs)',
],
Expand Down
10 changes: 10 additions & 0 deletions test/async-hooks/async-hooks.status
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
prefix async-hooks

# To mark a test as flaky, list the test name in the appropriate section
# below, without ".js", followed by ": PASS,FLAKY". Example:
# sample-test : PASS,FLAKY

[true] # This section applies to all platforms

[$jsEngine==chakracore]
test-promise : PASS,FLAKY
7 changes: 7 additions & 0 deletions vcbuild.bat
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,13 @@ if "%i18n_arg%"=="intl-none" set configure_flags=%configure_flags% --with-intl=n
if "%i18n_arg%"=="without-intl" set configure_flags=%configure_flags% --without-intl
if "%engine%"=="chakracore" set configure_flags=%configure_flags% --without-bundled-v8&set chakra_jslint=deps\chakrashim\lib

if "%target_arch%"=="arm" (
if "%PROCESSOR_ARCHITECTURE%" NEQ "ARM" (
echo Skipping building ARM with Intl on a non-ARM device
set configure_flags=%configure_flags% --without-intl
Copy link
Contributor

@refack refack May 26, 2017

Choose a reason for hiding this comment

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

[FYI]
--without-intl will disable the inspector nodejs/node#12978
Not sure what's the equivalent for chakra, but all the #if HAVE_INSPECTOR code section will be off

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @refack, I filed #268 to explore whether we can support building ICU in the build machine's architecture and hopefully re-enable it. For now we'll just have to keep the lack of inspector as a known issue.

)
)

if defined config_flags set configure_flags=%configure_flags% %config_flags%

if not exist "%~dp0deps\icu" goto no-depsicu
Expand Down