Skip to content

build: enable big toc for release builds in AIX#7508

Closed
gireeshpunathil wants to merge 1 commit into
nodejs:masterfrom
gireeshpunathil:addbigtoc
Closed

build: enable big toc for release builds in AIX#7508
gireeshpunathil wants to merge 1 commit into
nodejs:masterfrom
gireeshpunathil:addbigtoc

Conversation

@gireeshpunathil

@gireeshpunathil gireeshpunathil commented Jul 1, 2016

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

NOTE: There is an outstanding issue in AIX build (#7500) - build fail due to 787eddf . The tests are conducted with this change removed.

Affected core subsystem(s)
Description of change

Issue #7500

AIX linker has a table of contents with default size 64K
The recent code inclusions in V8 brings in lot of new
symbols which necessitates to increase this default.

Please note that the debug build already has this flag

@mscdex mscdex added the build Issues and PRs related to build files or the CI. label Jul 1, 2016
@bnoordhuis

Copy link
Copy Markdown
Member

Does that mean the -Wl,-bbigtoc,-bE:<(PRODUCT_DIR)/node.exp in node.gyp can be removed now or is that still necessary?

@gireeshpunathil

Copy link
Copy Markdown
Member Author

@bnoordhuis - I will check and get back on this.

AIX linker has a table of contents with default size 64K
The recent code inclusions in V8 brings in lot of new
symbols which necessitates to increase this default.

Please note that the debug build already has this flag
@gireeshpunathil

Copy link
Copy Markdown
Member Author

@bnoordhuis - you are right, the -bbigtoc flag in node.gyp is meant for the same purpose, but covers only the building of node. Now that the same is introduced in common.gypi, we don't need it in node.gyp any more. Please note however, that the -bE:<(PRODUCT_DIR)/node.exp needs to be retained.

And hence I have revised my changes. Please review and let me know.

@bnoordhuis

Copy link
Copy Markdown
Member

@mhdawson

mhdawson commented Jul 5, 2016

Copy link
Copy Markdown
Member

LGTM

@mhdawson mhdawson self-assigned this Jul 5, 2016
@mhdawson

mhdawson commented Jul 5, 2016

Copy link
Copy Markdown
Member

CI run green, landing

mhdawson pushed a commit that referenced this pull request Jul 5, 2016
AIX linker has a table of contents with default size 64K
The recent code inclusions in V8 brings in lot of new
symbols which necessitates to increase this default.

Please note that the debug build already has this flag

Fixes: #7500
PR-URL: #7508
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@mhdawson

mhdawson commented Jul 5, 2016

Copy link
Copy Markdown
Member

d80432d

@mhdawson

mhdawson commented Jul 5, 2016

Copy link
Copy Markdown
Member

AIX CI run to validate after landing: https://ci.nodejs.org/job/node-test-commit-aix/242/. We have identified that there is a new issue, but want to validate through CI that the toc issue is resolved.

@mhdawson

mhdawson commented Jul 5, 2016

Copy link
Copy Markdown
Member

Ok, toc issue resolve, and see new issue, will open separate issue for that one closing

@mhdawson mhdawson closed this Jul 5, 2016
Fishrock123 pushed a commit that referenced this pull request Jul 6, 2016
AIX linker has a table of contents with default size 64K
The recent code inclusions in V8 brings in lot of new
symbols which necessitates to increase this default.

Please note that the debug build already has this flag

Fixes: #7500
PR-URL: #7508
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@Fishrock123 Fishrock123 mentioned this pull request Jul 6, 2016
@MylesBorins

Copy link
Copy Markdown
Contributor

@mhdawson lts?

@MylesBorins

Copy link
Copy Markdown
Contributor

needs to land if #6734 lands

@mhdawson

Copy link
Copy Markdown
Member

Id say if needed due to other commits, yes, otherwise if it applies cleanly then it probably makes sense just to avoid problems in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Issues and PRs related to build files or the CI.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants