-
Notifications
You must be signed in to change notification settings - Fork 73
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
feat(msvs): add support for CL env var #131
base: main
Are you sure you want to change the base?
Conversation
Also, I'm not entirely sure but "cl" might need to also be added to msvs_emulation.py in the environment variable list in |
07259ac
to
30794de
Compare
Please add doctests to this function. Something like: def _GetValueFormattedForMSBuild(tool_name, name, value):
"""
>>> _GetValueFormattedForMSBuild(
... tool_name="tool_name", name="name", value=[]
... )
''
>>> _GetValueFormattedForMSBuild(
... tool_name="tool_name", name="name", value=["DelayLoadDLLs"]
... )
'DelayLoadDLLs'
>>> _GetValueFormattedForMSBuild(
... tool_name="Lib", name="AdditionalOptions", value=["DelayLoadDLLs"]
... )
'DelayLoadDLLs %(AdditionalOptions)'
>>> _GetValueFormattedForMSBuild(
... tool_name="ClCompile", name="AdditionalOptions", value=["DelayLoadDLLs"]
... )
'DelayLoadDLLs %(AdditionalOptions)'
""" |
30794de
to
27338b2
Compare
@cclauss Added. |
It looks like the unrelated windows integration failure is because node is floating changes on top of gyp that do not exist in node-gyp's vendored copy of gyp or in gyp-next itself. Specifically, gyp isn't setting |
@@ -3440,6 +3440,24 @@ def _FinalizeMSBuildSettings(spec, configuration): | |||
|
|||
|
|||
def _GetValueFormattedForMSBuild(tool_name, name, value): | |||
""" | |||
>>> _GetValueFormattedForMSBuild( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
>>> _GetValueFormattedForMSBuild( | |
>>> PRODUCT_DIR_ABS = “PRODUCT_DIR_ABS” | |
>>> _GetValueFormattedForMSBuild( |
I opened nodejs/node#40735 |
27338b2
to
ee4a6cf
Compare
ee4a6cf
to
3c64923
Compare
This is probably not the best place for this line to be as I'm not a gyp expert, but it works for me. I'm completely open to suggestions if anyone has better ideas on how to achieve this.
My use case is I wanted to easily set some compiler flags (e.g. _WIN32_WINNT) without having to touch every gyp/gypi file in a project. On *nix you can simply set the
CFLAGS
/CXXFLAGS
/etc. environment variables and it'll just work.However on Windows it's a different story. Technically you can set a
CL
environment variable to pass flags to CL.exe, but since gyp uses msbuild, environment variables likeCL
don't get picked up/used automatically. Because of this, we need to explicitly pass the values to the msbuild project file.I tried looking to see if node-gyp/gyp had some way of achieving the same thing using the command line, but I could not find anything, so using environment variables seems like the next best bet.