search

nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
106.22k stars 28.91k forks source link

vcbuild does not always generate field <TargetMachine> in project files. #13569

Open tamaroth opened 7 years ago

tamaroth commented 7 years ago

When generating project files for node, gyp does not include for librarian but does so for linker. As a result the linking fails as MSBuild will default unknown machine to x64 (on x64 platforms). The result is as follows:

vcbuild.bat debug static vs2017 x86

...
LINK : warning LNK4068: /MACHINE not specified; defaulting to X64 [c:\Users\tamaroth\Downloads\node\node.vcxproj]
Debug\obj\node\async-wrap.obj : fatal error LNK1112: module machine type 'X86' conflicts with target machine type 'x64' [c:\Users\tamaroth\Downloads\node\node.vcxproj]

This is manually easily fixed by modyfying .vcxproj file to include proper inside .

For instance, from the above command, the node.vcxproj file in Debug|Win32 configuration has this entry:

    <Lib>
      <OutputFile>$(OutDir)lib\$(ProjectName)$(TargetExt)</OutputFile>
    </Lib>

If I modify it to 

    <Lib>
      <OutputFile>$(OutDir)lib\$(ProjectName)$(TargetExt)</OutputFile>
      <TargetMachine>MachineX86</TargetMachine>
    </Lib>

Everything works great.

Inside

node\tools\gyp\pylib\gyp\msvs_emulation.py

in function GetLibFlags (line: 515) it says to add specific machine, but for some reason it's not added later on to the project file.

gibfahn commented 7 years ago

cc/ @nodejs/platform-windows

refack commented 7 years ago

@tamaroth just to make sure: it only happens with the static arg? i.e. vcbuild.bat debug static vs2017 x86?

refack commented 7 years ago

Repro. I think I'd rather add this in node.gyp then patch GYP Ref: https://gyp.gsrc.io/docs/LanguageSpecification.md#Detailed-Design

tamaroth commented 7 years ago

@refack Sorry for late response, yes, as far as I can tell it happens only when building as static library. There's also another error when building release library (problem with included resources), but I will make additional bug report for it.

Trott commented 6 years ago

@refack Suggestions for how to unstick this? Any team that might be cc'ed? Or maybe a label to add like help wanted or something?

refack commented 5 years ago

/CC @nodejs/gyp ¯_(ツ)_/¯

joyeecheung commented 4 months ago

Not sure if it's related but https://github.com/nodejs/node/blob/83eb4f285583036bedc1c908178d58b9b6e45d1f/common.gypi#L307-L315 adds TargetMachine for release builds, even though there doesn't appear to be one for the debug builds.