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

Issues building test addons with both VS2015 and VS2017 installed #13641

Closed
kfarnung opened this issue Jun 12, 2017 · 5 comments
Closed

Issues building test addons with both VS2015 and VS2017 installed #13641

kfarnung opened this issue Jun 12, 2017 · 5 comments
Labels
build Issues and PRs related to build files or the CI. windows Issues and PRs related to the Windows platform. wip Issues and PRs that are still a work in progress.

Comments

@kfarnung
Copy link
Contributor

kfarnung commented Jun 12, 2017 •
edited by refack
Loading

  • Version: master
  • Platform: windows
  • Subsystem: test

After the upgrade of npm to 5.0.3 the addon tests started failing to build. I tracked it down to the node-gyp change nodejs/node-gyp@bad903ac70 by @refack. What I think is happening is that node-gyp now detects and tries to build with VS2017, but the node default build uses VS2015.

gyp
 
info
 using [email protected]

gyp
 info
 
using
 [email protected] | win32 | x64

gyp
 
info
 
chdir test\addons\01_callbacks

gyp
 info 
spawn
 C:\Python27\python.EXE

gyp
 info
 
spawn args
 [ 'E:\\GitHub\\node\\deps\\npm\\node_modules\\node-gyp\\gyp\\gyp_main.py',

gyp
 info 
spawn args
   'binding.gyp',

gyp
 
info
 
spawn args
   '-f',

gyp info
 
spawn args
   'msvs',

gyp
 
info
 
spawn args
   '-G',

gyp
 
info
 
spawn args
   'msvs_version=2015',

gyp
 
info
 
spawn args
   '-I',

gyp
 
info
 
spawn args
   'E:\\GitHub\\node\\test\\addons\\01_callbacks\\build\\config.gypi',

gyp
 
info spawn args
   '-I',

gyp
 
info
 
spawn args
   'E:\\GitHub\\node\\deps\\npm\\node_modules\\node-gyp\\addon.gypi',

gyp
 
info
 
spawn args
   '-I',

gyp
 
info
 
spawn args
   'E:\\GitHub\\node\\common.gypi',

gyp
 
info
 
spawn args
   '-Dlibrary=shared_library',

gyp
 
info
 
spawn args
   '-Dvisibility=default',

gyp
 
info
 
spawn args
   '-Dnode_root_dir=E:\\GitHub\\node',

gyp info
 
spawn args
   '-Dnode_gyp_dir=E:\\GitHub\\node\\deps\\npm\\node_modules\\node-gyp',

gyp
 
info
 
spawn args
   '-Dnode_lib_file=E:\\GitHub\\node\\$(Configuration)\\node.lib',

gyp info
 
spawn args
   '-Dmodule_root_dir=E:\\GitHub\\node\\test\\addons\\01_callbacks',

gyp 
info
 
spawn args
   '-Dnode_engine=v8',

gyp
 
info
 
spawn args
   '--depth=.',

gyp
 
info
 
spawn args
   '--no-parallel',

gyp
 
info
 
spawn args
   '--generator-output',

gyp
 
info
 
spawn args
   'E:\\GitHub\\node\\test\\addons\\01_callbacks\\build',

gyp
 
info
 
spawn args
   '-Goutput_dir=.' ]

gyp
 info spawn
 msbuild

gyp
 
info
 
spawn args
 [ 'build/binding.sln',

gyp
 
info
 
spawn args
   '/clp:Verbosity=minimal',

gyp
 
info
 
spawn args
   '/nologo',

gyp
 
info
 
spawn args
   '/p:Configuration=Release;Platform=x64' ]

Building the projects in this solution one at a time. To enable parallel build, please add the "/m" switch.
C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\V140\Microsoft.Cpp.Platform.targets(57,5): error MSB8020: The build tools for v141 (Platform Toolset = 'v141') cannot be found. To build using the v141 build tools, please install v141 build tools.  Alternatively, you may upgrade to the current Visual Studio tools by selecting the Project menu or right-click the solution, and then selecting "Retarget solution". [E:\GitHub\node\test\addons\01_callbacks\build\addon.vcxproj]
gyp
 ERR!
 
build error
 

gyp
 ERR!
 
stack
 Error: `msbuild` failed with exit code: 1

gyp
 
ERR!
 
stack
     at ChildProcess.onExit (E:\GitHub\node\deps\npm\node_modules\node-gyp\lib\build.js:258:23)

gyp
 
ERR!
 
stack
     at emitTwo (events.js:125:13)

gyp ERR!
 
stack
     at ChildProcess.emit (events.js:213:7)

gyp
 
ERR!
 
stack
     at Process.ChildProcess._handle.onexit (internal/child_process.js:197:12)

gyp
 ERR!
 
System
 Windows_NT 10.0.16217

gyp
 ERR!
 
command
 "E:\\GitHub\\node\\Release\\node.exe" "E:\\GitHub\\node\\deps\\npm\\node_modules\\node-gyp\\bin\\node-gyp" "rebuild" 
"--directory=test\\addons\\01_callbacks" "--nodedir=E:\\GitHub\\node"

gyp
 ERR! 
cwd
 E:\GitHub\node\test\addons\01_callbacks

gyp
 ERR!
 
node -v
 v9.0.0-pre

gyp
 ERR!
 
node-gyp -v
 v3.6.2

gyp
 ERR! not ok

After reverting the Win10SDK change, the build completes successfully. I was able to work around the issue by forcing the build to use VS2017:

vcbuild.bat vs2017 test

out.txt
config - before revert.txt
config - after revert.txt

[refack wrapped output in <details>]
@mscdex mscdex added build Issues and PRs related to build files or the CI. windows Issues and PRs related to the Windows platform. labels Jun 12, 2017
@refack
Copy link
Contributor

refack commented Jun 13, 2017

That is an interesting 🤔 edge case. I would think that it should have popped up after nodejs/node-gyp@ae141e1 since nodejs/node-gyp@bad903ac70 was just a bug fix (that probably just made node-gyp actualy work on your system).
I have an idea for a simple fix.

@refack refack self-assigned this Jun 13, 2017
@refack
Copy link
Contributor

refack commented Jun 13, 2017

Actually this is probably a node-gyp bug... But it should be fixed on both sides

@kfarnung
Copy link
Contributor Author

Thanks for taking a look, @refack, I thought it might be an upstream issue, but figured it would be good to start here. I also suspect you're right about the original change being a no-op on my system and that the bug fix finally activated it, exposing the issue.

joaocgreis added a commit to JaneaSystems/node that referenced this issue Jun 25, 2017
@joaocgreis
build,win: use latest installed VS by default
0c5b1aa 
vcbuild.bat should detect what version of Visual Studio to use, it
should simply work without any parameter if any supported version is
installed. It should default to the latest version, to match the
behavior of `node-gyp`.

Fixes: nodejs#13641
@joaocgreis joaocgreis mentioned this issue Jun 25, 2017
Closed
2 tasks
@joaocgreis
Copy link
Member

This is caused by 2 issues (solving either fixes this, but we should solve both):

@refack
Copy link
Contributor

refack commented Jun 25, 2017

I tought GYP_MSVS_VERSION=2017 / 2015 in vcbuild.bat should be the signals for downstream. Didn't get to make node-gyp respect it yet.

@refack refack added the wip Issues and PRs that are still a work in progress. label Jun 27, 2017
@refack refack removed their assignment Jun 27, 2017
addaleax pushed a commit to addaleax/node that referenced this issue Jul 3, 2017
@joaocgreis @addaleax
build,win: use latest installed VS by default
9556b4d 
vcbuild.bat should detect what version of Visual Studio to use, it
should simply work without any parameter if any supported version is
installed. It should default to the latest version, to match the
behavior of `node-gyp`.

PR-URL: nodejs#13911
Fixes: nodejs#13641
Reviewed-By: Refael Ackermann <[email protected]>
addaleax pushed a commit that referenced this issue Jul 11, 2017
@joaocgreis @addaleax
build,win: use latest installed VS by default
b1c7fe9 
vcbuild.bat should detect what version of Visual Studio to use, it
should simply work without any parameter if any supported version is
installed. It should default to the latest version, to match the
behavior of `node-gyp`.

PR-URL: #13911
Fixes: #13641
Reviewed-By: Refael Ackermann <[email protected]>
addaleax pushed a commit that referenced this issue Jul 18, 2017
@joaocgreis @addaleax
build,win: use latest installed VS by default
cd9ef93 
vcbuild.bat should detect what version of Visual Studio to use, it
should simply work without any parameter if any supported version is
installed. It should default to the latest version, to match the
behavior of `node-gyp`.

PR-URL: #13911
Fixes: #13641
Reviewed-By: Refael Ackermann <[email protected]>
jaimecbernardo added a commit to JaneaSystems/node-gyp that referenced this issue Jul 27, 2017
@jaimecbernardo
build,win: use config.gypi MSBuild path over env
d5c77fe 
When vs2017 and vs2015 are both installed and vs2015's MSBuild.exe
is in the Path environment variable, node-gyp fails to build if it
configured the project to build with vs2017, because it tries to use
vs2015's MSBuild.exe, which is unable to find the v141 (vs2017)
Platform Toolset build tools.

Change build behaviour to prefer using the MSBuild.exe path if it is
defined in config.gypi instead of using the MSBuild.exe found in Path
by "which".

Ref: nodejs/node#13641
@jaimecbernardo jaimecbernardo mentioned this issue Jul 27, 2017
Closed
2 tasks
jaimecbernardo added a commit to JaneaSystems/node-gyp that referenced this issue Oct 19, 2018
@jaimecbernardo
build,win: use config.gypi MSBuild path over env
e5c80eb 
When vs2017 and vs2015 are both installed and vs2015's MSBuild.exe
is in the Path environment variable, node-gyp fails to build if it
configured the project to build with vs2017, because it tries to use
vs2015's MSBuild.exe, which is unable to find the v141 (vs2017)
Platform Toolset build tools.

Change build behaviour to prefer using the MSBuild.exe path if it is
defined in config.gypi instead of using the MSBuild.exe found in Path
by "which".

Ref: nodejs/node#13641
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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. wip Issues and PRs that are still a work in progress.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@mscdex @refack @joaocgreis @kfarnung and others