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

Overriding default flags in common.gypi #1087

Closed
ahmelsayed opened this issue Jan 7, 2017 · 7 comments
Closed

Overriding default flags in common.gypi #1087

ahmelsayed opened this issue Jan 7, 2017 · 7 comments

Comments

@ahmelsayed
Copy link

ahmelsayed commented Jan 7, 2017

I have a case where I'd like to disable all multi-process builds on windows. Basically I'm wondering if there is a way to negate this msbuild flag /MP on the build machine machine wide.
I took a look at #26 but that seems to be for package developers, rather than as a build step after the fact, please correct me if I'm wrong.
so are there any flags, env vars, configuration files, command line argument, etc that I can set to override that either globally on the machine or when installing any npm packages?

@bnoordhuis
Copy link
Member

I think the answer is no. node-gyp doesn't set /MP itself, it inherits it from node's common.gypi. As a workaround:

  1. Download or check out the node source code to $nodedir
  2. Remove the /MP switch from $nodedir/common.gypi
  3. Build your add-on with node-gyp rebuild --nodedir=$nodedir (or pass the switch to npm install.)

@ahmelsayed
Copy link
Author

Got it. One last question, would that be the same if I wanted to change DebugInformationFormat as well?

@bnoordhuis
Copy link
Member

bnoordhuis commented Jan 10, 2017

You might be able to override that with a 'msvs_settings': { 'VCCLCompilerTool': { ... } } section in your binding.gyp.

If that works for you, you could file an issue with nodejs/node requesting that node switches over to 'MultiProcessorCompilation': 'true'. The /MP switch is set through 'AdditionalOptions': [ '/MP' ] now and that is additive, I think, not overridable.

@ahmelsayed
Copy link
Author

sounds good. Thank you for the help!

@skelliam
Copy link

skelliam commented Sep 2, 2018

As a follow-up to this:

I am finding that adding MultiProcessorCompilation in the msvs_settings section of binding.gyp has no impact on the /MP that is getting written into AdditionalOptions. This value comes from the default values in common.gypi and I think the only way to get rid of it is from there.

Interestingly, I find in the file nodejs\node_modules\npm\node_modules\node-gyp\gyp\pylib\gyp\msvs_emulation.py that there is some effort to filter out /MP:

    # ninja handles parallelism by itself, don't have the compiler do it too.
    cflags = filter(lambda x: not x.startswith('/MP'), cflags)
    return cflags

I think this needs to be removed from common.gypi as it makes the MultiProcessorCompilation setting in msvs_settings useless.

skelliam added a commit to skelliam/node that referenced this issue Sep 2, 2018
Forcing this option makes the option ```MultiProcessorCompilation``` in ```msvs_settings``` useless.  It also means that one cannot use ```#import``` or they will be faced with this error when trying to build:

```
error C2813: #import is not supported with /MP (compiling source file...)
```

Please see additional discussion of this here:  nodejs/node-gyp#1087
and here: nodejs/node-gyp#26
@refack
Copy link
Contributor

refack commented Sep 2, 2018

FYI: GYP allows for removing items from list with the ! suffix, so the following msvs_settings block removes the /MP:

{
  'targets': [
    {
      'target_name': 'binding',
      'defines': [ 'V8_DEPRECATION_WARNINGS=1' ],
      'sources': [ 'binding.cc' ],
      'msvs_settings': {
        'VCCLCompilerTool': {
          'AdditionalOptions!': [
            '/MP'
          ]
        }
      }
    }
  ]
}

or using the = suffix:

...
'msvs_settings': { 'VCCLCompilerTool': { 
  'AdditionalOptions=': []
} }
...

@DanielRamosAcosta
Copy link

DanielRamosAcosta commented Feb 2, 2020

@refack I'm not being able to remove the /MP flag from my builds, here is my binding.gyp:

{
    "targets": [{
        "target_name": "testaddon",
        "cflags!": [ "-fno-exceptions" ],
        "cflags_cc!": [ "-fno-exceptions" ],
        "sources": [
            "cppsrc/main.cpp",
            "cppsrc/Samples/functionexample.cpp",
            "cppsrc/Samples/actualclass.cpp",
            "cppsrc/Samples/classexample.cpp"
        ],
        'msvs_settings': {
          'VCCLCompilerTool': {
            'AdditionalOptions!': [
              '/MP'
            ]
          }
        },
        'include_dirs': [
            "<!@(node -p \"require('node-addon-api').include\")"
        ],
        'libraries': [],
        'dependencies': [
            "<!(node -p \"require('node-addon-api').gyp\")"
        ],
        'defines': [ 'NAPI_DISABLE_CPP_EXCEPTIONS' ]
    }]
}

Build output:

npm run build                                                                                                                                    
> [email protected] build E:\Documentos\Proyectos Personales\Mios\aura-sdk
> node-gyp rebuild


E:\Documentos\Proyectos Personales\Mios\aura-sdk>if not defined npm_config_node_gyp (node "C:\Users\danie\AppData\Roaming\npm\node_modules\npm\node_modules\npm-lifecycle\node-gyp-bin\\..\..\node_modules\node-gyp\bin\node-gyp.js" rebuild )  else (node "C:\Users\danie\AppData\Roaming\npm\node_modules\npm\node_modules\node-gyp\bin\node-gyp.js" rebuild )
Los proyectos de esta solución se van a compilar de uno en uno. Para habilitar la compilación en paralelo, agregue el modificador "/m".
  nothing.vcxproj -> E:\Documentos\Proyectos Personales\Mios\aura-sdk\build\Release\\nothing.lib
  main.cpp
  functionexample.cpp
  actualclass.cpp
  classexample.cpp
  win_delay_load_hook.cc
e:\documentos\proyectos personales\mios\aura-sdk\cppsrc\samples\actualclass.h(1): error C2813: #import is not supported with /MP (compiling source file ..\cppsrc\Samples\actualclass.cpp) [E:\Documentos\Proyectos Personales
\Mios\aura-sdk\build\testaddon.vcxproj]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants