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.

Sign up for GitHub

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

Jump to bottom

build: add --v8-disable-object-print flag #45458

Merged
merged 1 commit into from Nov 17, 2022
+12 −2
Merged

build: add --v8-disable-object-print flag #45458

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 12 additions & 2 deletions configure.py
View file
Open in desktop
Expand Up @@ -776,7 +776,13 @@
action='store_true',
dest='v8_enable_object_print',
default=True,
help='compile V8 with auxiliar functions for native debuggers')
help='compile V8 with auxiliary functions for native debuggers')

parser.add_argument('--v8-disable-object-print',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could keep --v8-enable-object-print as a valid option in addition to adding --v8-disable-object-print in case anyone is using it, otherwise they'll break

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your suggestion, I keep the -v8-enable-object-print flag and add the --v8-disable-object-print flag.
If I specify both flags at the same time, We get an exception with a message like Only one of the --v8-enable-object-print or --v8-disable-object-print options an be specified at a time..

Copy link

@nettybun nettybun Jan 21, 2023 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this introduced a bug? I cannot use "disable" flag because "enable" flag is default True, so if I "disable" then they're both True...

Downloading the source tarball on Nodejs' website for LTS 18.13.0 yields:

python3 ./configure --prefix=/opt/base --shared-zlib --v8-disable-object-print --without-corepack --without-dtrace --without-etw --without-inspector --without-intl --without-node-code-cache --without-node-snapshot --without-npm --without-ssl
Node.js configure: Found Python 3.9.10...
Traceback (most recent call last):
  File "/Users/Hames/Repos/core/contrib/node-v18.13.0/./configure", line 28, in <module>
    import configure
  File "/Users/Hames/Repos/core/contrib/node-v18.13.0/configure.py", line 2062, in <module>
    configure_v8(output)
  File "/Users/Hames/Repos/core/contrib/node-v18.13.0/configure.py", line 1547, in configure_v8
    raise Exception(
Exception: Only one of the --v8-enable-object-print or --v8-disable-object-print options can be specified at a time.

Even though the enable flag is never specified.

This is because the options object has the argparse defaults resolved, which means the enable flag is True if not specified... :/

parser.add_argument('--v8-enable-object-print',
    action='store_true',
    dest='v8_enable_object_print',
    default=True, # HERE
    help='compile V8 with auxiliary functions for native debuggers')

parser.add_argument('--v8-disable-object-print',
    action='store_true',
    dest='v8_disable_object_print',
    default=False,
    help='disable the V8 auxiliary functions for native debuggers')

action='store_true',
dest='v8_disable_object_print',
default=False,
help='disable the V8 auxiliary functions for native debuggers')

parser.add_argument('--v8-enable-hugepage',
action='store_true',
Expand Down Expand Up @@ -1436,7 +1442,7 @@ def configure_v8(o):
o['variables']['v8_no_strict_aliasing'] = 1 # Work around compiler bugs.
o['variables']['v8_optimized_debug'] = 0 if options.v8_non_optimized_debug else 1
o['variables']['dcheck_always_on'] = 1 if options.v8_with_dchecks else 0
o['variables']['v8_enable_object_print'] = 1 if options.v8_enable_object_print else 0
o['variables']['v8_enable_object_print'] = 0 if options.v8_disable_object_print else 1
o['variables']['v8_random_seed'] = 0 # Use a random seed for hash tables.
o['variables']['v8_promise_internal_field_count'] = 1 # Add internal field to promises for async hooks.
o['variables']['v8_use_siphash'] = 0 if options.without_siphash else 1
Expand All @@ -1459,6 +1465,10 @@ def configure_v8(o):
o['variables']['v8_enable_hugepage'] = 1 if options.v8_enable_hugepage else 0
if options.v8_enable_short_builtin_calls or o['variables']['target_arch'] == 'x64':
o['variables']['v8_enable_short_builtin_calls'] = 1
if options.v8_enable_object_print and options.v8_disable_object_print:
raise Exception(
'Only one of the --v8-enable-object-print or --v8-disable-object-print options '
'can be specified at a time.')

def configure_openssl(o):
variables = o['variables']
Expand Down