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

build: Use option groups in configure output #1533

Closed
wants to merge 1 commit into from
Closed

build: Use option groups in configure output #1533

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 45 additions & 33 deletions configure
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,16 @@ valid_arm_float_abi = ('soft', 'softfp', 'hard')
valid_mips_arch = ('loongson', 'r1', 'r2', 'r6', 'rx')
valid_mips_fpu = ('fp32', 'fp64', 'fpxx')
valid_mips_float_abi = ('soft', 'hard')
valid_intl_modes = ('none', 'small-icu', 'full-icu', 'system-icu')

# create option groups
shared_optgroup = optparse.OptionGroup(parser, "Shared libraries",
"Flags that allows you to control whether you want to build against "
"built-in dependencies or its shared representations. If necessary, "
"provide multiple libraries with comma.")
intl_optgroup = optparse.OptionGroup(parser, "Internationalization",
"Flags that lets you enable i18n features in io.js as well as which "
"library you want to build against.")

# Options should be in alphabetical order but keep --prefix at the top,
# that's arguably the one people will be looking for most.
Expand Down Expand Up @@ -78,90 +88,92 @@ parser.add_option("--openssl-no-asm",
dest="openssl_no_asm",
help="Do not build optimized assembly for OpenSSL")

parser.add_option('--shared-http-parser',
shared_optgroup.add_option('--shared-http-parser',
action='store_true',
dest='shared_http_parser',
help='link to a shared http_parser DLL instead of static linking')

parser.add_option('--shared-http-parser-includes',
shared_optgroup.add_option('--shared-http-parser-includes',
action='store',
dest='shared_http_parser_includes',
help='directory containing http_parser header files')

parser.add_option('--shared-http-parser-libname',
shared_optgroup.add_option('--shared-http-parser-libname',
action='store',
dest='shared_http_parser_libname',
default='http_parser',
help='alternative lib name to link to [default: %default]')

parser.add_option('--shared-http-parser-libpath',
shared_optgroup.add_option('--shared-http-parser-libpath',
action='store',
dest='shared_http_parser_libpath',
help='a directory to search for the shared http_parser DLL')

parser.add_option('--shared-libuv',
shared_optgroup.add_option('--shared-libuv',
action='store_true',
dest='shared_libuv',
help='link to a shared libuv DLL instead of static linking')

parser.add_option('--shared-libuv-includes',
shared_optgroup.add_option('--shared-libuv-includes',
action='store',
dest='shared_libuv_includes',
help='directory containing libuv header files')

parser.add_option('--shared-libuv-libname',
shared_optgroup.add_option('--shared-libuv-libname',
action='store',
dest='shared_libuv_libname',
default='uv',
help='alternative lib name to link to [default: %default]')

parser.add_option('--shared-libuv-libpath',
shared_optgroup.add_option('--shared-libuv-libpath',
action='store',
dest='shared_libuv_libpath',
help='a directory to search for the shared libuv DLL')

parser.add_option('--shared-openssl',
shared_optgroup.add_option('--shared-openssl',
action='store_true',
dest='shared_openssl',
help='link to a shared OpenSSl DLL instead of static linking')

parser.add_option('--shared-openssl-includes',
shared_optgroup.add_option('--shared-openssl-includes',
action='store',
dest='shared_openssl_includes',
help='directory containing OpenSSL header files')

parser.add_option('--shared-openssl-libname',
shared_optgroup.add_option('--shared-openssl-libname',
action='store',
dest='shared_openssl_libname',
default='crypto,ssl',
help='alternative lib name to link to [default: %default]')

parser.add_option('--shared-openssl-libpath',
shared_optgroup.add_option('--shared-openssl-libpath',
action='store',
dest='shared_openssl_libpath',
help='a directory to search for the shared OpenSSL DLLs')

parser.add_option('--shared-zlib',
shared_optgroup.add_option('--shared-zlib',
action='store_true',
dest='shared_zlib',
help='link to a shared zlib DLL instead of static linking')

parser.add_option('--shared-zlib-includes',
shared_optgroup.add_option('--shared-zlib-includes',
action='store',
dest='shared_zlib_includes',
help='directory containing zlib header files')

parser.add_option('--shared-zlib-libname',
shared_optgroup.add_option('--shared-zlib-libname',
action='store',
dest='shared_zlib_libname',
default='z',
help='alternative lib name to link to [default: %default]')

parser.add_option('--shared-zlib-libpath',
shared_optgroup.add_option('--shared-zlib-libpath',
action='store',
dest='shared_zlib_libpath',
help='a directory to search for the shared zlib DLL')

parser.add_option_group(shared_optgroup)

# TODO document when we've decided on what the tracing API and its options will
# look like
parser.add_option('--systemtap-includes',
Expand Down Expand Up @@ -225,33 +237,38 @@ parser.add_option('--with-etw',
dest='with_etw',
help='build with ETW (default is true on Windows)')

parser.add_option('--download',
parser.add_option('--with-intl',
action='store',
dest='download_list',
help=nodedownload.help())
dest='with_intl',
default='none',
choices=valid_intl_modes,
help='Intl mode (valid choices: {0}) [default: %default]'.format(
', '.join(valid_intl_modes)))

parser.add_option('--with-icu-path',
intl_optgroup.add_option('--with-icu-path',
action='store',
dest='with_icu_path',
help='Path to icu.gyp (ICU i18n, Chromium version only.)')

parser.add_option('--with-icu-locales',
intl_optgroup.add_option('--with-icu-locales',
action='store',
dest='with_icu_locales',
default='root,en',
help='Comma-separated list of locales for "small-icu". "root" is assumed. '
'[default: %default]')

parser.add_option('--with-intl',
action='store',
dest='with_intl',
help='Intl mode: none, full-icu, small-icu [default: none]')

parser.add_option('--with-icu-source',
intl_optgroup.add_option('--with-icu-source',
action='store',
dest='with_icu_source',
help='Intl mode: optional local path to icu/ dir, or path/URL of icu source archive.')

intl_optgroup.add_option('--download',
Copy link
Contributor

Choose a reason for hiding this comment

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

--download isn't necessarily an intl option though, although it is for now.

Copy link
Member Author

Choose a reason for hiding this comment

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

Do we really want to add more options here? Guessing you're thinking about npm?

Copy link
Member

Choose a reason for hiding this comment

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

Judging by the push-back I'd say that we may not be adding anything else and an objective would be to remove the one we have now. Although I'm personally in favour of download all the things.

Copy link
Member Author

Choose a reason for hiding this comment

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

@rvagg I agree. I'd prefer removing the download thing altogether and in the intl help suggest curl -O $url -- but that's for another PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair enough.

Copy link
Member

Choose a reason for hiding this comment

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

Right, --download is not Intl specific and shouldn't be described as such. In any event download is off by default, please cc me on the other PR

action='store',
dest='download_list',
help=nodedownload.help())

parser.add_option_group(intl_optgroup)

parser.add_option('--with-perfctr',
action='store_true',
dest='with_perfctr',
Expand Down Expand Up @@ -812,7 +829,7 @@ def configure_intl(o):
with_intl = options.with_intl
with_icu_source = options.with_icu_source
have_icu_path = bool(options.with_icu_path)
if have_icu_path and with_intl:
if have_icu_path and with_intl != 'none':
Copy link
Member

Choose a reason for hiding this comment

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

Good catch.

print 'Error: Cannot specify both --with-icu-path and --with-intl'
sys.exit(1)
elif have_icu_path:
Expand Down Expand Up @@ -850,11 +867,6 @@ def configure_intl(o):
# use the "system" .gyp
o['variables']['icu_gyp_path'] = 'tools/icu/icu-system.gyp'
return
else:
print 'Error: unknown value --with-intl=%s' % with_intl
sys.exit(1)
# Note: non-ICU implementations could use other 'with_intl'
# values.

# this is just the 'deps' dir. Used for unpacking.
icu_parent_path = os.path.join(root_dir, 'deps')
Expand Down