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,win: fix node.exe resource version #3053

Merged
merged 1 commit into from
Oct 1, 2015

Conversation

joaocgreis
Copy link
Member

As was described in #2963 , the resource file was not being compiled correctly and the version information in the details of node.exe was wrong.

Simply removing the quotes from node.rc solves this for releases and -pre builds, but does not work for nightlies and rcs, that require rc.exe to receive the NODE_TAG as a preprocessor definition.

When MSBuild invokes rc.exe, it passes NODE_TAG unstringified, but passes it correctly to cl.exe. Hence, this workaround was made to apply only to the resource file.

cc @nodejs/platform-windows

@joaocgreis joaocgreis added windows Issues and PRs related to the Windows platform. build Issues and PRs related to build files or the CI. labels Sep 24, 2015
@joaocgreis
Copy link
Member Author

CI: https://ci.nodejs.org/job/node-test-pull-request/373/

Also tested locally with both VS2013 and VS2015.

@orangemocha
Copy link
Contributor

LGTM

1 similar comment
@jbergstroem
Copy link
Member

LGTM

When MSBuild invokes rc.exe, it passes NODE_TAG unstringified, but
passes it correctly to cl.exe. Hence, this workaround was made to
apply only to the resource file.

Fixes: nodejs#2963
PR-URL: nodejs#3053
Reviewed-By: Alexis Campailla <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
@joaocgreis joaocgreis merged commit a6aab43 into nodejs:master Oct 1, 2015
joaocgreis added a commit that referenced this pull request Oct 2, 2015
When MSBuild invokes rc.exe, it passes NODE_TAG unstringified, but
passes it correctly to cl.exe. Hence, this workaround was made to
apply only to the resource file.

Fixes: #2963
PR-URL: #3053
Reviewed-By: Alexis Campailla <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
@rvagg rvagg mentioned this pull request Oct 3, 2015
joaocgreis added a commit that referenced this pull request Nov 24, 2015
When MSBuild invokes rc.exe, it passes NODE_TAG unstringified, but
passes it correctly to cl.exe. Hence, this workaround was made to
apply only to the resource file.

Fixes: #2963
PR-URL: #3053
Reviewed-By: Alexis Campailla <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
joaocgreis added a commit that referenced this pull request Dec 3, 2015
When MSBuild invokes rc.exe, it passes NODE_TAG unstringified, but
passes it correctly to cl.exe. Hence, this workaround was made to
apply only to the resource file.

Fixes: #2963
PR-URL: #3053
Reviewed-By: Alexis Campailla <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
rvagg pushed a commit to rvagg/io.js that referenced this pull request Dec 3, 2015
When MSBuild invokes rc.exe, it passes NODE_TAG unstringified, but
passes it correctly to cl.exe. Hence, this workaround was made to
apply only to the resource file.

Fixes: nodejs#2963
PR-URL: nodejs#3053
Reviewed-By: Alexis Campailla <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
joaocgreis added a commit that referenced this pull request Dec 3, 2015
When MSBuild invokes rc.exe, it passes NODE_TAG unstringified, but
passes it correctly to cl.exe. Hence, this workaround was made to
apply only to the resource file.

Fixes: #2963
PR-URL: #3053
Reviewed-By: Alexis Campailla <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
@rvagg rvagg mentioned this pull request Dec 3, 2015
gibfahn pushed a commit to ibmruntimes/node that referenced this pull request Apr 1, 2016
When MSBuild invokes rc.exe, it passes NODE_TAG unstringified, but
passes it correctly to cl.exe. Hence, this workaround was made to
apply only to the resource file.

Fixes: nodejs/node#2963
PR-URL: nodejs/node#3053
Reviewed-By: Alexis Campailla <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
jBarz pushed a commit to ibmruntimes/node that referenced this pull request Nov 4, 2016
When MSBuild invokes rc.exe, it passes NODE_TAG unstringified, but
passes it correctly to cl.exe. Hence, this workaround was made to
apply only to the resource file.

Fixes: nodejs/node#2963
PR-URL: nodejs/node#3053
Reviewed-By: Alexis Campailla <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
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.

3 participants