win, build: add arbitrary MSBuild flag option and binlog option by jkunkee · Pull Request #25994 · nodejs/node

jkunkee · 2019-02-07T20:44:28Z

This commit adds a 'verbose' flag to vcbuild.bat that turns on outputs
useful for build debugging, including MSBuild BinLogging and
DEBUG_HELPER logging.

It also adds a 'setlocal' call so internal environment variables no
longer leak out to the calling shell.

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
commit message follows commit guidelines

[Edit: Note new commit message (cf0681c)]

refack · 2019-02-07T20:58:02Z

Hello @jkunkee
and welcome. Thank you for your contribution 🥇
If you are not familiar with our review and landing process, it's covered in CONTRIBUTING.md

I like the idea behind the proposed change, but I think we can do it with a simpler, more generic way 🤷‍♂️.

P.S. If you have any questions you can also feel free to contact me directly.

refack · 2019-02-07T20:58:23Z

/CC @nodejs/build-files @nodejs/platform-windows

jkunkee · 2019-02-08T00:02:00Z

Thanks for the welcome and the review! I've reread the contributor and PR guides and will email you soon.

jkunkee · 2019-02-10T00:04:58Z

Note new commit message (cf0681c)

joaocgreis · 2019-02-13T02:51:26Z

CI: https://ci.nodejs.org/view/All/job/node-test-pull-request/20750/ ❌ (arm-fanned)
Resumed: https://ci.nodejs.org/view/All/job/node-test-pull-request/20765/ ✔️

@refack does your request for changes still hold?

This change adds a 'msbuild_arg' option to vcbuild.bat that can be used to pass arbitrary flags to MSBuild. It also adds a 'binlog' flag as a shortcut 'msbuild_arg' option to enable binary logging to `%config%\node.binlog`. This is especially convenient when debugging changes to the build system. In the process of developing this change, the idea of adding 'setlocal' to the beginning of the script was rejected since other scripts in this repo rely on the exported environment variables. This change adds a note describing this.

joaocgreis · 2019-03-04T11:32:50Z

@refack I believe your feedback has been addressed, I'll assume there's no problem with landing this.

CI: https://ci.nodejs.org/job/node-test-pull-request/21182/ ✔️

This change adds a 'msbuild_arg' option to vcbuild.bat that can be used to pass arbitrary flags to MSBuild. It also adds a 'binlog' flag as a shortcut 'msbuild_arg' option to enable binary logging to `%config%\node.binlog`. This is especially convenient when debugging changes to the build system. In the process of developing this change, the idea of adding 'setlocal' to the beginning of the script was rejected since other scripts in this repo rely on the exported environment variables. This change adds a note describing this. PR-URL: #25994 Reviewed-By: João Reis <reis@janeasystems.com>

joaocgreis · 2019-03-04T12:31:50Z

Landed in d57c526
Thanks @jkunkee!

jkunkee · 2019-03-04T18:07:09Z

Thanks, @joaocgreis!

PR-URL: nodejs#26431 Refs: nodejs#25994 Reviewed-By: João Reis <reis@janeasystems.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

This change adds a 'msbuild_arg' option to vcbuild.bat that can be used to pass arbitrary flags to MSBuild. It also adds a 'binlog' flag as a shortcut 'msbuild_arg' option to enable binary logging to `%config%\node.binlog`. This is especially convenient when debugging changes to the build system. In the process of developing this change, the idea of adding 'setlocal' to the beginning of the script was rejected since other scripts in this repo rely on the exported environment variables. This change adds a note describing this. PR-URL: nodejs#25994 Reviewed-By: João Reis <reis@janeasystems.com>

PR-URL: nodejs#26431 Refs: nodejs#25994 Reviewed-By: João Reis <reis@janeasystems.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

PR-URL: #26431 Refs: #25994 Reviewed-By: João Reis <reis@janeasystems.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

This change adds a 'msbuild_arg' option to vcbuild.bat that can be used to pass arbitrary flags to MSBuild. It also adds a 'binlog' flag as a shortcut 'msbuild_arg' option to enable binary logging to `%config%\node.binlog`. This is especially convenient when debugging changes to the build system. In the process of developing this change, the idea of adding 'setlocal' to the beginning of the script was rejected since other scripts in this repo rely on the exported environment variables. This change adds a note describing this. PR-URL: #25994 Reviewed-By: João Reis <reis@janeasystems.com>

nodejs-github-bot added build Issues and PRs related to build files or the CI. windows Issues and PRs related to the Windows platform. labels Feb 7, 2019

jkunkee force-pushed the win-build-script-tweaks branch from 94eea9a to 623ec64Compare February 7, 2019 20:46

jkunkee changed the title ~~win, build: add 'verbose' mode and improve environment management~~win, build: add 'verbose' mode and scope environment changesFeb 7, 2019

refack suggested changes Feb 7, 2019
View reviewed changes

Comment threadvcbuild.bat Outdated

jkunkee mentioned this pull request Feb 8, 2019
Support ARM64 Windows desktop #25998
Closed

jkunkee force-pushed the win-build-script-tweaks branch 2 times, most recently from 4bed141 to 490f726Compare February 9, 2019 18:31

refack reviewed Feb 9, 2019
View reviewed changes

Comment threadvcbuild.bat Outdated

jkunkee force-pushed the win-build-script-tweaks branch 2 times, most recently from 4edce07 to cf0681cCompare February 9, 2019 23:59

jkunkee changed the title ~~win, build: add 'verbose' mode and scope environment changes~~win, build: add arbitrary MSBuild flag option and binlog optionFeb 10, 2019

joaocgreis approved these changes Feb 13, 2019
View reviewed changes

jkunkee force-pushed the win-build-script-tweaks branch from cf0681c to 11b5a48Compare February 20, 2019 22:07

joaocgreis closed this Mar 4, 2019

refack mentioned this pull request Mar 4, 2019
build,win: simplify new msbuild_arg option #26431
Closed
3 tasks

jkunkee deleted the win-build-script-tweaks branch March 8, 2019 02:40

BridgeAR mentioned this pull request Mar 13, 2019
2019-03-14, Version 11.12.0 (Current) #26644
Merged

BethGriggs mentioned this pull request May 1, 2019
v10.16.0 proposal #27514
Merged

Navigation Menu

Uh oh!

Uh oh!

win, build: add arbitrary MSBuild flag option and binlog option - #25994

win, build: add arbitrary MSBuild flag option and binlog option#25994
jkunkee wants to merge 1 commit into
nodejs:masterfrom
jkunkee:win-build-script-tweaks

jkunkee commented Feb 7, 2019•
edited
Loading

Uh oh!

Uh oh!

refack commented Feb 7, 2019

Uh oh!

refack commented Feb 7, 2019

Uh oh!

jkunkee commented Feb 8, 2019

Uh oh!

Uh oh!

jkunkee commented Feb 10, 2019

Uh oh!

joaocgreis commented Feb 13, 2019•
edited
Loading

Uh oh!

joaocgreis commented Mar 4, 2019•
edited
Loading

Uh oh!

joaocgreis commented Mar 4, 2019

Uh oh!

jkunkee commented Mar 4, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Uh oh!

Conversation

jkunkee commented Feb 7, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

Uh oh!

refack commented Feb 7, 2019

Uh oh!

refack commented Feb 7, 2019

Uh oh!

jkunkee commented Feb 8, 2019

Uh oh!

Uh oh!

jkunkee commented Feb 10, 2019

Uh oh!

joaocgreis commented Feb 13, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joaocgreis commented Mar 4, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joaocgreis commented Mar 4, 2019

Uh oh!

jkunkee commented Mar 4, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jkunkee commented Feb 7, 2019•
edited
Loading

joaocgreis commented Feb 13, 2019•
edited
Loading

joaocgreis commented Mar 4, 2019•
edited
Loading