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

build: restore intl check for inspector #262

Merged
merged 2 commits into from
May 25, 2017
Merged

Conversation

kfarnung
Copy link
Contributor

@kfarnung kfarnung commented May 25, 2017

  • Upstream has a check that turns off inspector if intl is also turned
    off. Restoring that check so that without-intl builds will succeed.
  • Fixed upstream build break related to disabling inspector, will
    submit upstream PR.
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

build, src

@kfarnung kfarnung self-assigned this May 25, 2017
@kfarnung
Copy link
Contributor Author

Here's the potential upstream PR: kfarnung/node@0345651

@kfarnung
Copy link
Contributor Author

@kfarnung
Copy link
Contributor Author

Replaced my fix with the fix from an in-progress PR. It's approved by @bnoordhuis, so we'll use that to get us past the issue.

nodejs/node#13167

kfarnung added a commit to kfarnung/node-chakracore that referenced this pull request May 25, 2017
Upstream has a check that turns off inspector if intl is also turned
off. Restoring that check so that without-intl builds will succeed.

PR-URL: nodejs#262
Reviewed-By: Kunal Pathak <kunal.pathak@microsoft.com>
danbev and others added 2 commits May 25, 2017 15:20
Currently the InspectorStarted function is guarded by the else clause of
the NODE_USE_V8_PLATFORM macro. If node is configured --without-ssl then
NODE_USE_V8_PLATFORM will be 1 but the nested HAVE_INSPECTOR macro
will not be 0 which will lead to that there will be no InspectorStarted
function defined.

If building --without-inspector or --without-ssl the following
compilation error will occur:
../src/node.cc:4470:57: error: no member named 'InspectorStarted' in
'node::(anonymous struct at ../src/node.cc:241:8)'
  if (debug_options.inspector_enabled() &&
!v8_platform.InspectorStarted(&env))
                                            ~~~~~~~~~~~ ^
../src/node.cc:4470:57: error: no member named 'InspectorStarted' in
'node::(anonymous struct at ../src/node.cc:241:8)'
  if (debug_options.inspector_enabled() &&
!v8_platform.InspectorStarted(&env))
                                            ~~~~~~~~~~~ ^
1 error generated.

This commit adds a separate if preprocessor directive to catch the case
when either --without-ssl/--without-inspector and --without-v8-platform
combinations are used to configure node.

PR-URL: nodejs/node#13167
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Kyle Farnung <kfarnung@microsoft.com>
Upstream has a check that turns off inspector if intl is also turned
off. Restoring that check so that without-intl builds will succeed.

PR-URL: nodejs#262
Reviewed-By: Kunal Pathak <kunal.pathak@microsoft.com>
@kfarnung kfarnung merged commit 4da26c0 into nodejs:xplat May 25, 2017
@kfarnung kfarnung deleted the without-intl branch May 25, 2017 22:32
kfarnung added a commit to kfarnung/node-chakracore that referenced this pull request May 31, 2017
Upstream has a check that turns off inspector if intl is also turned
off. Restoring that check so that without-intl builds will succeed.

PR-URL: nodejs#262
Reviewed-By: Kunal Pathak <kunal.pathak@microsoft.com>
kfarnung added a commit to kfarnung/node-chakracore that referenced this pull request May 31, 2017
Upstream has a check that turns off inspector if intl is also turned
off. Restoring that check so that without-intl builds will succeed.

PR-URL: nodejs#262
Reviewed-By: Kunal Pathak <kunal.pathak@microsoft.com>
kfarnung added a commit to kfarnung/node-chakracore that referenced this pull request May 31, 2017
Upstream has a check that turns off inspector if intl is also turned
off. Restoring that check so that without-intl builds will succeed.

PR-URL: nodejs#262
Reviewed-By: Kunal Pathak <kunal.pathak@microsoft.com>
kfarnung added a commit to kfarnung/node-chakracore that referenced this pull request Jun 5, 2017
Upstream has a check that turns off inspector if intl is also turned
off. Restoring that check so that without-intl builds will succeed.

PR-URL: nodejs#262
Reviewed-By: Kunal Pathak <kunal.pathak@microsoft.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants