Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build, windows: use /bigobj for debug builds #16289

Closed
wants to merge 0 commits into from
Closed

build, windows: use /bigobj for debug builds #16289

wants to merge 0 commits into from

Conversation

seishun
Copy link
Contributor

@seishun seishun commented Oct 18, 2017

Fixes: #16288

cc @nodejs/platform-windows

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

build

@nodejs-github-bot nodejs-github-bot added the build Issues and PRs related to build files or the CI. label Oct 18, 2017
@seishun seishun requested a review from refack October 18, 2017 12:10
@seishun seishun added the windows Issues and PRs related to the Windows platform. label Oct 18, 2017
@bzoz
Copy link
Contributor

bzoz commented Oct 18, 2017

@seishun
Copy link
Contributor Author

seishun commented Oct 18, 2017

@refack can you (or anyone else) confirm that vcbuild debug throws C1128 with VS2015 on master and that this PR fixes it? I know it takes a lot of time, but I'd hate to add an unnecessary switch because of a problem with my local setup.

@targos
Copy link
Member

targos commented Oct 19, 2017

I'm running vcbuild debug with VS2015 on master right now

Edit: error confirmed:

C:\Users\Michael\Documents\git\nodejs\node\deps\v8\src\code-stub-assembler.cc : fatal error C1128: number of sections exceeded object file format limit: compile with /bigobj [C:\Users\Michael\Documents\git\nodejs\node\deps\v8\src\v8_base_1.vcxproj]

Edit: and confirmed this PR fixes it.

@seishun
Copy link
Contributor Author

seishun commented Oct 19, 2017

Landed in 8f5fedb.

@seishun seishun deleted the bigobj branch October 19, 2017 20:16
MylesBorins pushed a commit that referenced this pull request Oct 23, 2017
PR-URL: #16289
Fixes: #16288
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
addaleax pushed a commit to addaleax/ayo that referenced this pull request Oct 23, 2017
PR-URL: nodejs/node#16289
Fixes: nodejs/node#16288
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
addaleax pushed a commit to ayojs/ayo that referenced this pull request Oct 26, 2017
PR-URL: nodejs/node#16289
Fixes: nodejs/node#16288
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 16, 2017
PR-URL: #16289
Fixes: #16288
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@MylesBorins
Copy link
Contributor

landed on LTS v6.x

Please lmk if it should be backed out

@seishun
Copy link
Contributor Author

seishun commented Nov 16, 2017

It's unnecessary in v6.x, but won't hurt either.

@MylesBorins MylesBorins mentioned this pull request Nov 21, 2017
MylesBorins pushed a commit that referenced this pull request Nov 21, 2017
PR-URL: #16289
Fixes: #16288
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 28, 2017
PR-URL: #16289
Fixes: #16288
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
addaleax pushed a commit to ayojs/ayo that referenced this pull request Dec 7, 2017
PR-URL: nodejs/node#16289
Fixes: nodejs/node#16288
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
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. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fatal error C1128 when building a debug build with VS2015
8 participants