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.

Sign up for GitHub

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

Jump to bottom

build,win: skip vcvarsall if already set #13806

Merged
merged 1 commit into from
Jul 9, 2017
Merged

build,win: skip vcvarsall if already set #13806

merged 1 commit into from
Jul 9, 2017

Conversation

refack
Copy link
Contributor

@refack refack commented Jun 20, 2017

Fixes: #13765

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 build Issues and PRs related to build files or the CI. windows Issues and PRs related to the Windows platform. labels Jun 20, 2017
@refack
Copy link
Contributor Author

refack commented Jun 20, 2017

/cc @nodejs/platform-windows @nodejs/build

richardlau
richardlau reviewed Jun 20, 2017
View reviewed changes
vcbuild.bat Outdated
@rem VS2015 vsvarsall is quick, so run anyway

@rem check if VS2015 is already setup
if "_%VisualStudioVersion%_" == "_14.0_" if "_%VCVARS_VER%_" == "_140_" goto :found_vs2015
Copy link
Member

Choose a reason for hiding this comment

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

The earlier similar line for 2017 doesn't prefix the goto target with a colon. Please can you make these consistent?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Defo

@refack refack force-pushed the vcbuild-idempotency-for-vs2015 branch from e189614 to 514654a Compare June 20, 2017 03:34
@refack
build,win: skip vcvarsall.bat if env is set
8206a01 
PR-URL: nodejs#13806
Fixes: nodejs#13765
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Kunal Pathak <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@refack refack force-pushed the vcbuild-idempotency-for-vs2015 branch from 514654a to 8206a01 Compare July 9, 2017 17:22
@refack refack merged commit 8206a01 into nodejs:master Jul 9, 2017
@refack refack deleted the vcbuild-idempotency-for-vs2015 branch July 9, 2017 17:31
addaleax pushed a commit that referenced this pull request Jul 11, 2017
@refack @addaleax
build,win: skip vcvarsall.bat if env is set
6ac4e94 
PR-URL: #13806
Fixes: #13765
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Kunal Pathak <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@addaleax addaleax mentioned this pull request Jul 11, 2017
Merged
addaleax pushed a commit that referenced this pull request Jul 18, 2017
@refack @addaleax
build,win: skip vcvarsall.bat if env is set
897405d 
PR-URL: #13806
Fixes: #13765
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Kunal Pathak <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Fishrock123 pushed a commit that referenced this pull request Jul 19, 2017
@refack @Fishrock123
build,win: skip vcvarsall.bat if env is set
9675734 
PR-URL: #13806
Fixes: #13765
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Kunal Pathak <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@MylesBorins
Copy link
Contributor

Should this be backported to v6.x-staging? If yes please follow the guide and raise a backport PR, if no let me know or add the dont-land-on label.

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

@kunalspathak kunalspathak kunalspathak approved these changes

@richardlau richardlau richardlau approved these changes

@tniessen tniessen tniessen approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
build Issues and PRs related to build files or the CI. windows Issues and PRs related to the Windows platform.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

vcbuild.bat fails with "input line too long"
7 participants
@refack @MylesBorins @jasnell @tniessen @richardlau @kunalspathak @nodejs-github-bot