Skip to content

Commit

Permalink
build: fix conflict gyp configs
Browse files Browse the repository at this point in the history
Gyp generated build files can be built in either Release/Debug mode.

- make: single directory, two configurations by cli:
  `make -C out BUILDTYPE=Release` and `make -C out BUILDTYPE=Debug`.
- msbuild: single directory, two configurations by cli:
  `msbuild node.sln /p:Configuration=Release` and
  `msbuild node.sln /p:Configuration=Debug`.
- ninja: two directories in `out/`, build with
  `ninja -C out/Release` or `ninja -C out/Debug`.

Variables that changes with either Release or Debug configuration
should be defined in a configuration level, instead of the root level.
This fixes generating invalid build files.

Additionally, `v8_gypfiles/toolchain.gypi` duplicates defines in
`v8_gypfiles/features.gypi`. Remove the duplications in
`toolchains.gypi`

PR-URL: nodejs#53605
Fixes: nodejs#53446
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
legendecas authored and Jason Zhang committed Oct 2, 2024
1 parent 47be236 commit b54b3cc
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 12 deletions.
20 changes: 15 additions & 5 deletions configure.py
Original file line number Diff line number Diff line change
Expand Up @@ -1268,6 +1268,9 @@ def host_arch_win():

return matchup.get(arch, 'ia32')

def set_configuration_variable(configs, name, release=None, debug=None):
configs['Release'][name] = release
configs['Debug'][name] = debug

def configure_arm(o):
if options.arm_float_abi:
Expand Down Expand Up @@ -1584,7 +1587,9 @@ def configure_library(lib, output, pkgname=None):
output['libraries'] += pkg_libs.split()


def configure_v8(o):
def configure_v8(o, configs):
set_configuration_variable(configs, 'v8_enable_v8_checks', release=1, debug=0)

o['variables']['v8_enable_webassembly'] = 0 if options.v8_lite_mode else 1
o['variables']['v8_enable_javascript_promise_hooks'] = 1
o['variables']['v8_enable_lite_mode'] = 1 if options.v8_lite_mode else 0
Expand All @@ -1602,7 +1607,6 @@ def configure_v8(o):
o['variables']['v8_enable_31bit_smis_on_64bit_arch'] = 1 if options.enable_pointer_compression else 0
o['variables']['v8_enable_shared_ro_heap'] = 0 if options.enable_pointer_compression or options.disable_shared_ro_heap else 1
o['variables']['v8_enable_extensible_ro_snapshot'] = 0
o['variables']['v8_enable_v8_checks'] = 1 if options.debug else 0
o['variables']['v8_trace_maps'] = 1 if options.trace_maps else 0
o['variables']['node_use_v8_platform'] = b(not options.without_v8_platform)
o['variables']['node_use_bundled_v8'] = b(not options.without_bundled_v8)
Expand Down Expand Up @@ -2118,6 +2122,10 @@ def make_bin_override():
'defines': [],
'cflags': [],
}
configurations = {
'Release': { 'variables': {} },
'Debug': { 'variables': {} },
}

# Print a warning when the compiler is too old.
check_compiler(output)
Expand All @@ -2144,7 +2152,7 @@ def make_bin_override():
configure_library('nghttp3', output, pkgname='libnghttp3')
configure_library('ngtcp2', output, pkgname='libngtcp2')
configure_library('uvwasi', output, pkgname='libuvwasi')
configure_v8(output)
configure_v8(output, configurations)
configure_openssl(output)
configure_intl(output)
configure_static(output)
Expand All @@ -2167,7 +2175,6 @@ def make_bin_override():
# move everything else to target_defaults
variables = output['variables']
del output['variables']
variables['is_debug'] = B(options.debug)

# make_global_settings should be a root level element too
if 'make_global_settings' in output:
Expand All @@ -2176,6 +2183,9 @@ def make_bin_override():
else:
make_global_settings = False

# Add configurations to target defaults
output['configurations'] = configurations

output = {
'variables': variables,
'target_defaults': output,
Expand All @@ -2186,7 +2196,7 @@ def make_bin_override():
print_verbose(output)

write('config.gypi', do_not_edit +
pprint.pformat(output, indent=2, width=1024) + '\n')
pprint.pformat(output, indent=2, width=128) + '\n')

write('config.status', '#!/bin/sh\nset -x\nexec ./configure ' +
' '.join([shlex.quote(arg) for arg in original_argv]) + '\n')
Expand Down
1 change: 0 additions & 1 deletion tools/v8_gypfiles/features.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@
'v8_enable_private_mapping_fork_optimization': 0,
}],
],
'is_debug%': 0,

# Variables from BUILD.gn

Expand Down
6 changes: 0 additions & 6 deletions tools/v8_gypfiles/toolchain.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -695,13 +695,7 @@
'configurations': {
'Debug': {
'defines': [
'ENABLE_DISASSEMBLER',
'V8_ENABLE_CHECKS',
'OBJECT_PRINT',
'DEBUG',
'V8_TRACE_MAPS',
'V8_ENABLE_ALLOCATION_TIMEOUT',
'V8_ENABLE_FORCE_SLOW_PATH',
],
'conditions': [
['OS=="linux" and v8_enable_backtrace==1', {
Expand Down

0 comments on commit b54b3cc

Please sign in to comment.