win,build: improvements to vcbuild.bat #8412

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
3 participants
@joaocgreis
Member

joaocgreis commented Sep 5, 2016

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

Windows, Build.

Description of change

This PR includes 3 changes to vcbuild.bat:

  • 7642724 : Fail and exit immediately when an invalid option is passed to vcbuild.bat. Before there was only a warning, and since the output is quite verbose it was very easy to miss it and vcbuild would run without the options that the user probably wanted.
  • f26497c : When msbuild and other Visual Studio tools are not needed, do not search for Visual Studio. This allows for running the tests in a machine with an unsupported version of VS, or no VS at all. This unblocks #8067 , since for v6 onwards we want to build node only with VS2015 but still want to run the tests in machines with VS2013 (node-gyp will detect it independently).
  • f2f2aa4 : When addons fail to build with node-gyp, fail immediately. Before, the build would only fail because the tests would fail later. This makes the output easier to read, since the run ends immediately after the failed node-gyp invocation.

I think the commits should not be squashed because they address different concerns, but I can squash if this is not consensual.

cc @nodejs/platform-windows

@bnoordhuis

This comment has been minimized.

Show comment
Hide comment
@bnoordhuis

bnoordhuis Sep 6, 2016

Member

LGTM

Member

bnoordhuis commented Sep 6, 2016

LGTM

joaocgreis added a commit that referenced this pull request Sep 8, 2016

win,build: fail on invalid option in vcbuild
PR-URL: #8412
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

joaocgreis added a commit that referenced this pull request Sep 8, 2016

win,build: skip finding VS when not needed
PR-URL: #8412
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

joaocgreis added a commit that referenced this pull request Sep 8, 2016

win,build: exit when addons fail to build
PR-URL: #8412
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@joaocgreis

This comment has been minimized.

Show comment
Hide comment

@joaocgreis joaocgreis closed this Sep 8, 2016

Fishrock123 added a commit that referenced this pull request Sep 14, 2016

win,build: fail on invalid option in vcbuild
PR-URL: #8412
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

Fishrock123 added a commit that referenced this pull request Sep 14, 2016

win,build: skip finding VS when not needed
PR-URL: #8412
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

Fishrock123 added a commit that referenced this pull request Sep 14, 2016

win,build: exit when addons fail to build
PR-URL: #8412
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@MylesBorins

This comment has been minimized.

Show comment
Hide comment
@MylesBorins

MylesBorins Sep 30, 2016

Member

@joaocgreis should this be backported?

Member

MylesBorins commented Sep 30, 2016

@joaocgreis should this be backported?

@joaocgreis

This comment has been minimized.

Show comment
Hide comment
@joaocgreis

joaocgreis Oct 3, 2016

Member

@thealphanerd yes, I think all of this would also be helpful in v4. Thanks!

Member

joaocgreis commented Oct 3, 2016

@thealphanerd yes, I think all of this would also be helpful in v4. Thanks!

MylesBorins added a commit that referenced this pull request Oct 11, 2016

win,build: fail on invalid option in vcbuild
PR-URL: #8412
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

MylesBorins added a commit that referenced this pull request Oct 11, 2016

win,build: skip finding VS when not needed
PR-URL: #8412
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

MylesBorins added a commit that referenced this pull request Oct 11, 2016

win,build: exit when addons fail to build
PR-URL: #8412
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

rvagg added a commit that referenced this pull request Oct 18, 2016

win,build: fail on invalid option in vcbuild
PR-URL: #8412
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

rvagg added a commit that referenced this pull request Oct 18, 2016

win,build: skip finding VS when not needed
PR-URL: #8412
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

rvagg added a commit that referenced this pull request Oct 18, 2016

win,build: exit when addons fail to build
PR-URL: #8412
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

MylesBorins added a commit that referenced this pull request Oct 26, 2016

win,build: fail on invalid option in vcbuild
PR-URL: #8412
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

MylesBorins added a commit that referenced this pull request Oct 26, 2016

win,build: skip finding VS when not needed
PR-URL: #8412
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

MylesBorins added a commit that referenced this pull request Oct 26, 2016

win,build: exit when addons fail to build
PR-URL: #8412
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

@MylesBorins MylesBorins referenced this pull request Oct 26, 2016

Closed

V4.6.2 proposal #9298

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