gyp build process: configurations mishandling

[YarnSaw]

Version

v25.2.1

Platform

Linux atlas 6.8.0-47-generic #47~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Wed Oct  2 16:16:55 UTC 2 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

configure.py / gyp

What steps will reproduce the bug?

Run

./configure --debug
make -C out BUILDTYPE=Debug V=1

Whenever a file under deps/v8/ is compiling, it should include the -DV8_ENABLE_CHECKS flag, since it's building debug mode.

How often does it reproduce? Is there a required condition?

Always reproduces

What is the expected behavior? Why is that the expected behavior?

I would expect to see -DV8_ENABLE_CHECKS as a flag passed to the compiling file, so libnode.so is compiled with this option enabled, as they are intended to be in debug mode. Without it, when a libnode.so file is created along with it's corresponding header files, and those headers are used in an embedded system in debug (ie embedded system is using V8_ENABLE_CHECKS) it will error with undefined reference to v8::HandleScope::DoInitializeAsserts(v8::Isolate*)', since DoInitializeAsserts is defined in an #ifdef V8_ENABLE_CHECKSblock in thev8-local-handle.h` header.

What do you see instead?

The flag is missing, and the above error is thrown when using libnode.so+its header files in a later debug environment.

Additional information

Preface: I don't have much experience with gyp before this, so some terminology/understanding may be wrong.

Using git bisect, I narrowed down the commit where this changed to e192a32. There are 4 issues (1 major, 3 minor) as far as I can tell causing this flag to not be set in debug builds:

1. Major issue: reading gyp's documentation here, configurations sections can only be found within targets or target_defaults sections. This commit is moving v8_enable_v8_checks (the V8_ENABLE_CHECKS is set if this is true) to a configurations block that is in the top-level dictionary, where configurations has no meaning. Thus it's value is never used.
2. Minor issue, the call to set_configuration_variable(configs, 'v8_enable_v8_checks', release=1, debug=0) has release set to 1 and debug to 0, that should be reversed
3. Minor issue, set_configuration_variable is setting the v8_enable_v8_checks flag outside of the variables section, resulting in the object looking like { 'Debug': {'v8_enable_v8_checks': 1, 'variables': {}, ...}, instead of { 'Debug': { 'variables': {'v8_enable_v8_checks': 1 }, ...}
4. Minor issue, output['configurations'] = configurations is being set immediately before ouput is redefined, so it's not actually making its way into the final output.

2-4 are all minor issues I can easily solve, but the first one I'm not familiar with gyp enough for. I think the solution could be as simple as adding a

      ['build_type=="Debug"', {
        'defines': ['V8_ENABLE_CHECKS'],
      }],

to the conditions block of the right target in the right gyp file, but I haven't been able to get it in successfully yet.

Would appreciate if someone has the time to take a look and either fix this, or point me in the direction of more resources I can use to figure it out, and I can throw a PR in. Thanks!

[YarnSaw]

Wait, just realized I was misreading what was happening to the output object. Point 1 and 4 aren't actual issues, my brain glazed over the fact that

output = {
  'variables': variables,
  'target_defaults': output,
}

is setting the original output dictionary as the target_defaults, so that is being set right. Need to properly test if just fixing points 2 and 3 are sufficient.

Edit: doesn't look like it's just those 2 points. Compile flag still isn't set