Skip to content

Conversation

@fossamagna
Copy link
Contributor

--v8-enable-object-print flag is set by default true. so, no way of disable this flag.
remove --v8-enable-object-print flag.
add a --v8-disable-object-print flag instead that defaults to false.

Fixes: #45433

@nodejs-github-botnodejs-github-bot added build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. labels Nov 14, 2022
'support, thus much slower execution)')

parser.add_argument('--v8-enable-object-print',
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
ContributorAuthor

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

@tsugabloomtsugabloomJan 21, 2023

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, # HEREhelp='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')

configure.py Outdated
help='compile V8 with auxiliar functions for native debuggers')
dest='v8_disable_object_print',
default=False,
help='disable the V8 auxiliar functions for native debuggers')
Copy link
Member

Choose a reason for hiding this comment

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

Since we're editing this line, might as well fix this typo:

Suggested change
help='disable the V8 auxiliar functions for native debuggers')
help='disable the V8 auxiliary functions for native debuggers')

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Thanks, I will fix typo.

--v8-enable-object-print flag is set by default true. so, no way of disable this flag. add a --v8-disable-object-print flag instead that defaults to false. Fixes: nodejs#45433
@fossamagnafossamagnaforce-pushed the add-v8-disable-object-print-flag branch from c041989 to f50e2b4CompareNovember 15, 2022 00:24
@richardlaurichardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 15, 2022
@github-actionsgithub-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 15, 2022
@nodejs-github-bot
Copy link
Collaborator

@richardlaurichardlau added the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 17, 2022
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 17, 2022
@nodejs-github-botnodejs-github-bot merged commit 6638f09 into nodejs:mainNov 17, 2022
@nodejs-github-bot
Copy link
Collaborator

Landed in 6638f09

ruyadorno pushed a commit that referenced this pull request Nov 21, 2022
--v8-enable-object-print flag is set by default true. so, no way of disable this flag. add a --v8-disable-object-print flag instead that defaults to false. Fixes: #45433 PR-URL: #45458 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Richard Lau <[email protected]>
@ruyadornoruyadorno mentioned this pull request Nov 24, 2022
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
--v8-enable-object-print flag is set by default true. so, no way of disable this flag. add a --v8-disable-object-print flag instead that defaults to false. Fixes: #45433 PR-URL: #45458 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Richard Lau <[email protected]>
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
--v8-enable-object-print flag is set by default true. so, no way of disable this flag. add a --v8-disable-object-print flag instead that defaults to false. Fixes: #45433 PR-URL: #45458 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Richard Lau <[email protected]>
danielleadams pushed a commit that referenced this pull request Jan 3, 2023
--v8-enable-object-print flag is set by default true. so, no way of disable this flag. add a --v8-disable-object-print flag instead that defaults to false. Fixes: #45433 PR-URL: #45458 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Richard Lau <[email protected]>
danielleadams pushed a commit that referenced this pull request Jan 4, 2023
--v8-enable-object-print flag is set by default true. so, no way of disable this flag. add a --v8-disable-object-print flag instead that defaults to false. Fixes: #45433 PR-URL: #45458 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Richard Lau <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

buildIssues and PRs related to build files or the CI.needs-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

build: no way to disable V8 "object print" during ./configure

5 participants

@fossamagna@nodejs-github-bot@joyeecheung@richardlau@tsugabloom