fix: Making ninja output on windows to work #1121
Conversation
|
I'm a little confused which PR I should review, this one or #1120? Also, cc @kunalspathak. |
| @@ -0,0 +1,38 @@ | |||
| From 06cea25b3af096b7fe82c25cc06ecba808ba9765 Mon Sep 17 00:00:00 2001 | |||
There was a problem hiding this comment.
We don't need this file i believe?
There was a problem hiding this comment.
We're not doing that anymore?
There was a problem hiding this comment.
never mind, i thought this was added accidentally.
|
@bnoordhuis This one is more complete. But requires a change in |
| @@ -109,7 +109,7 @@ | |||
| '-luuid.lib', | |||
| '-lodbc32.lib', | |||
| '-lDelayImp.lib', | |||
| '-l"<(node_root_dir)/$(ConfigurationName)/<(node_lib_file)"' | |||
| '-l<(node_root_dir)/<(CONFIGURATION_NAME)/<(node_lib_file)' | |||
There was a problem hiding this comment.
won't this stop working for MSBuild ? Where is `(CONFIGURATION_NAME) defined ?
There was a problem hiding this comment.
<(XXXXX) is GYP variable resolution (i.e. GYP will resolve this)
There was a problem hiding this comment.
Sorry, may be i was not clear. What i mean is for MsBuild, this variable is declared here which makes it available here as well as at other places. I am wondering, with this change if you build using MsBuild, what value will this variable resolve to?
There was a problem hiding this comment.
The process requires that GYP(CONFIGURATION_NAME) === MsBuild(ConfigurationName)
So now this resolution is a serpentius way, GYP sets it for MsBuild, and here MsBuild resolves it. And obviously it breaks for ninja/win32.
|
closing due to age, hopefully this is taken up in refack/GYP and we can pull it in there? |
No description provided.