Skip to content

build,tools: update make-v8.sh for ppc64le - #24293

Merged
refack merged 1 commit into
nodejs:masterfrom
refack:update-tools-make-v8-part2
Nov 16, 2018
Merged

build,tools: update make-v8.sh for ppc64le#24293
refack merged 1 commit into
nodejs:masterfrom
refack:update-tools-make-v8-part2

Conversation

@refack

Copy link
Copy Markdown
Contributor

Follow up to #23839
Fixes: nodejs/build#1536

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-botnodejs-github-bot added tools Issues and PRs related to the tools directory. v8 engine Issues and PRs related to the V8 dependency. labels Nov 10, 2018
@refack
refack requested review from gdams and mhdawsonNovember 10, 2018 22:16
@refackrefack self-assigned this Nov 10, 2018
@refackrefack added the build Issues and PRs related to build files or the CI. label Nov 10, 2018
@refack

refack commented Nov 10, 2018

Copy link
Copy Markdown
ContributorAuthor

Comment threadtools/make-v8.sh
ln -s /usr/bin/$CC "$BUILD_TOOLS/gcc"
g++ --version
export PKG_CONFIG_PATH=$BUILD_TOOLS/pkg-config-files
gn gen out.gn/$BUILD_ARCH_TYPE --args='is_component_build=false is_debug=false use_goma=false goma_dir="None" use_custom_libcxx=false v8_target_cpu="ppc64" target_cpu="ppc64"'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It took me a while to figure out how V8 knows the endianness, since it's not specified on the command line.

I thought it checked compiler defines but no, it's defined in build/config/host_byteorder.gni and it basically assumes LE unless the host arch is one of a select few. ppc64 is not one of the select except when os=AIX.

(And yes, host arch, not target arch. It's not cross-compile ready.)

@mhdawsonmhdawson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@refack

Copy link
Copy Markdown
ContributorAuthor

Lite-CI (since it's only used by test-v8 anyway): https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/1615/

@refack
refackforce-pushed the update-tools-make-v8-part2 branch from c8550fa to b5ecf2eCompareNovember 14, 2018 18:31
@refack
refackforce-pushed the update-tools-make-v8-part2 branch from b5ecf2e to c8550faCompareNovember 16, 2018 19:06
PR-URL: nodejs#24293Fixes: nodejs/build#1536
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
@refack
refackforce-pushed the update-tools-make-v8-part2 branch from c8550fa to 3d4b281CompareNovember 16, 2018 19:31
@refack
refack removed the request for review from gdamsNovember 16, 2018 20:24
@refack
refack merged commit 3d4b281 into nodejs:masterNov 16, 2018
@refack
refack deleted the update-tools-make-v8-part2 branch November 16, 2018 20:24
@refackrefack removed their assignment Nov 16, 2018
targos pushed a commit that referenced this pull request Nov 18, 2018
PR-URL: #24293Fixes: nodejs/build#1536
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
rvagg pushed a commit that referenced this pull request Nov 28, 2018
PR-URL: #24293Fixes: nodejs/build#1536
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
@BridgeARBridgeAR mentioned this pull request Dec 5, 2018
4 tasks
codebytere pushed a commit that referenced this pull request Jan 12, 2019
PR-URL: #24293Fixes: nodejs/build#1536
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
@codebyterecodebytere mentioned this pull request Jan 15, 2019
codebytere pushed a commit that referenced this pull request Jan 29, 2019
PR-URL: #24293Fixes: nodejs/build#1536
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

buildIssues and PRs related to build files or the CI.toolsIssues and PRs related to the tools directory.v8 engineIssues and PRs related to the V8 dependency.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CI-Change: tweaks to make-v8 test script

5 participants

@refack@nodejs-github-bot@fhinkel@bnoordhuis@mhdawson