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

Remove --build-v8-with-gn configure option #27576

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
8 participants
@hashseed
Copy link
Member

commented May 6, 2019

This option is now outdated and not used any longer.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Remove --build-v8-with-gn configure option
This option is now outdated and not used any longer.
@nodejs-github-bot

This comment has been minimized.

@targos

targos approved these changes May 6, 2019

@Trott

Trott approved these changes May 6, 2019

@Trott Trott added the author ready label May 6, 2019

@nodejs-github-bot

This comment has been minimized.

@refack

This comment has been minimized.

Copy link
Member

commented May 6, 2019

Refs: Conflicts with #26725

/CC @nodejs/build-files

v8_path = os.path.join('deps', 'v8')
print('Fetching dependencies to build V8 with GN')
options.build_v8_with_gn = FetchDeps(v8_path)
o['variables']['build_v8_with_gn'] = b(options.build_v8_with_gn)

This comment has been minimized.

Copy link
@refack

refack May 6, 2019

Member

We might consider setting this to

o['variables']['build_v8_with_gn'] = 'false'

as a make shift deprecation, since this value is exposed in process.config

@cjihrig

cjihrig approved these changes May 6, 2019

@refack
Copy link
Member

left a comment

Please address comment. Also I'd like this to land with at least one approval from @nodejs/build-files

@refack refack removed the author ready label May 13, 2019

@@ -550,12 +546,6 @@
help='do not use V8 includes from the bundled deps folder. ' +
'(This mode is not officially supported for regular applications)')

parser.add_option('--build-v8-with-gn',

This comment has been minimized.

Copy link
@refack

refack May 13, 2019

Member

Please instead of delete move to ~

node/configure.py

Lines 470 to 491 in 5cb8b25

# Dummy option for backwards compatibility
parser.add_option('--with-snapshot',
action='store_true',
dest='unused_with_snapshot',
help=optparse.SUPPRESS_HELP)
parser.add_option('--without-snapshot',
action='store_true',
dest='without_snapshot',
help=optparse.SUPPRESS_HELP)
parser.add_option('--without-siphash',
action='store_true',
dest='without_siphash',
help=optparse.SUPPRESS_HELP)
parser.add_option('--code-cache-path',
action='store',
dest='code_cache_path',
help='optparse.SUPPRESS_HELP')
# End dummy list.

with help=optparse.SUPPRESS_HELP

This comment has been minimized.

Copy link
@refack

refack May 14, 2019

Member

^^^
To avoid making this semver-major.

@refack refack added the semver-major label May 13, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.