Skip to content

Conversation

joaocgreis
Copy link
Contributor

@joaocgreis joaocgreis commented May 12, 2016

This variable is used to select 32 or 64 bit builds when invoking MSBuild.

The Visual C++ Build Tools also use the platform variable name, resulting in a conflict when vcvarsall.bat is called. Thus, it is necessary to rename it. This is similar to what was done in node in nodejs/node#5627 .

This enables building with Visual C++ Build Tools without any change.

Ref: #839

@bnoordhuis
Copy link
Member

Maybe call it msbuild_platform for clarity. LGTM apart from that.

This variable is used to select 32 or 64 bit builds when invoking
MSBuild. The Visual C++ Build Tools also use the `platform` variable
name, resulting in a conflict when vcvarsall.bat is called. Thus, it
is necessary to rename it.

Ref: libuv#839
@joaocgreis joaocgreis force-pushed the joaocgreis-G5C-rename-platform branch from c0386ed to f9e5a9f Compare May 12, 2016 09:56
@joaocgreis
Copy link
Contributor Author

Updated to msbuild_platform.

@saghul
Copy link
Member

saghul commented May 12, 2016

LGTM

saghul pushed a commit that referenced this pull request May 12, 2016
This variable is used to select 32 or 64 bit builds when invoking
MSBuild. The Visual C++ Build Tools also use the `platform` variable
name, resulting in a conflict when vcvarsall.bat is called. Thus, it
is necessary to rename it.

Ref: #839
PR-URL: #868
Reviewed-By: Saúl Ibarra Corretgé <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
@saghul
Copy link
Member

saghul commented May 12, 2016

Landed in 11e93aa, thanks Joao!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants