From 867de719ac60e880b72272dfc205ad9ef9db4626 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johan=20Bergstr=C3=B6m?= Date: Wed, 15 Apr 2015 09:16:54 +1000 Subject: [PATCH 1/5] build: improve shared library flags logic Move all logic for headers/libraries for each shared library into the shared library scope - similar to how OpenSSL already works. Also, don't assume that b() returns a boolean. --- configure | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/configure b/configure index 19ec28f92f1d01..36852b1fced0a2 100755 --- a/configure +++ b/configure @@ -658,37 +658,37 @@ def configure_node(o): def configure_libz(o): o['variables']['node_shared_zlib'] = b(options.shared_zlib) - if b(options.shared_zlib) == True: + if b(options.shared_zlib) == 'true': o['libraries'] += ['-l%s' % options.shared_zlib_libname] - if options.shared_zlib_libpath: - o['libraries'] += ['-L%s' % options.shared_zlib_libpath] - if options.shared_zlib_includes: - o['include_dirs'] += [options.shared_zlib_includes] + if options.shared_zlib_libpath: + o['libraries'] += ['-L%s' % options.shared_zlib_libpath] + if options.shared_zlib_includes: + o['include_dirs'] += [options.shared_zlib_includes] def configure_http_parser(o): o['variables']['node_shared_http_parser'] = b(options.shared_http_parser) - - if b(options.shared_http_parser) == True: + + if b(options.shared_http_parser) == 'true': o['libraries'] += ['-l%s' % options.shared_http_parser_libname] - if options.shared_http_parser_libpath: + if options.shared_http_parser_libpath: o['libraries'] += ['-L%s' % options.shared_http_parser_libpath] - if options.shared_http_parser_includes: + if options.shared_http_parser_includes: o['include_dirs'] += [options.shared_http_parser_includes] def configure_libuv(o): o['variables']['node_shared_libuv'] = b(options.shared_libuv) - if b(options.shared_libuv) == True: + if b(options.shared_libuv) == 'true': o['libraries'] += ['-l%s' % options.shared_libuv_libname] - if options.shared_libuv_libpath: - o['libraries'] += ['-L%s' % options.shared_libuv_libpath] - else: - o['variables']['uv_library'] = 'static_library' + if options.shared_libuv_libpath: + o['libraries'] += ['-L%s' % options.shared_libuv_libpath] + else: + o['variables']['uv_library'] = 'static_library' - if options.shared_libuv_includes: - o['include_dirs'] += [options.shared_libuv_includes] + if options.shared_libuv_includes: + o['include_dirs'] += [options.shared_libuv_includes] def configure_v8(o): @@ -707,7 +707,7 @@ def configure_openssl(o): if options.without_ssl: return - if options.shared_openssl: + if b(options.shared_openssl) == 'true': (libs, cflags) = pkg_config('openssl') or ('-lssl -lcrypto', '') libnames = options.shared_openssl_libname.split(',') From 984317c3bc395e57bcd9a09d9838502f04c3e29b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johan=20Bergstr=C3=B6m?= Date: Wed, 15 Apr 2015 10:16:59 +1000 Subject: [PATCH 2/5] build: fetch shared library info from pkg-config Attempt to fetch information from pkg-config if available. Also, let the user provide a custom PKG_CONFIG path, based on a floating patch by Paul McClave . Additionally, make library input parsing more robust as well as deduplicating what we provide to the linker. --- configure | 40 ++++++++++++++++++++++++++++++++-------- 1 file changed, 32 insertions(+), 8 deletions(-) diff --git a/configure b/configure index 36852b1fced0a2..b0e324df2e8dcf 100755 --- a/configure +++ b/configure @@ -326,12 +326,13 @@ def b(value): def pkg_config(pkg): - cmd = os.popen('pkg-config --libs %s' % pkg, 'r') + pkg_config = os.environ.get('PKG_CONFIG', 'pkg-config') + cmd = os.popen(pkg_config + ' --silence-errors --libs %s' % pkg, 'r') libs = cmd.readline().strip() ret = cmd.close() if (ret): return None - cmd = os.popen('pkg-config --cflags %s' % pkg, 'r') + cmd = os.popen(pkg_config + ' --silence-errors --cflags %s' % pkg, 'r') cflags = cmd.readline().strip() ret = cmd.close() if (ret): return None @@ -339,6 +340,12 @@ def pkg_config(pkg): return (libs, cflags) +def format_libraries(list): + """Returns string of space separated libraries""" + list = list.replace(' ', '') + set = list.split(',') + return ' '.join('-l{0}'.format(i) for i in set) + def try_check_compiler(cc, lang): try: proc = subprocess.Popen(shlex.split(cc) + ['-E', '-P', '-x', lang, '-'], @@ -659,17 +666,23 @@ def configure_libz(o): o['variables']['node_shared_zlib'] = b(options.shared_zlib) if b(options.shared_zlib) == 'true': - o['libraries'] += ['-l%s' % options.shared_zlib_libname] + libraries = format_libraries(options.shared_zlib_libname) + (libs, cflags) = pkg_config('zlib') or (libraries, '') + o['libraries'] += libs.split() + if options.shared_zlib_libpath: o['libraries'] += ['-L%s' % options.shared_zlib_libpath] if options.shared_zlib_includes: o['include_dirs'] += [options.shared_zlib_includes] + else: + o['cflags'] += cflags.split() def configure_http_parser(o): o['variables']['node_shared_http_parser'] = b(options.shared_http_parser) if b(options.shared_http_parser) == 'true': + # no .pc-file for http_parser o['libraries'] += ['-l%s' % options.shared_http_parser_libname] if options.shared_http_parser_libpath: o['libraries'] += ['-L%s' % options.shared_http_parser_libpath] @@ -681,7 +694,10 @@ def configure_libuv(o): o['variables']['node_shared_libuv'] = b(options.shared_libuv) if b(options.shared_libuv) == 'true': - o['libraries'] += ['-l%s' % options.shared_libuv_libname] + libraries = format_libraries(options.shared_libuv_libname) + (libs, cflags) = pkg_config('libuv') or (libraries, '') + o['libraries'] += libs.split() + if options.shared_libuv_libpath: o['libraries'] += ['-L%s' % options.shared_libuv_libpath] else: @@ -689,6 +705,8 @@ def configure_libuv(o): if options.shared_libuv_includes: o['include_dirs'] += [options.shared_libuv_includes] + else: + o['cflags'] += cflags.split() def configure_v8(o): @@ -708,10 +726,9 @@ def configure_openssl(o): return if b(options.shared_openssl) == 'true': - (libs, cflags) = pkg_config('openssl') or ('-lssl -lcrypto', '') - - libnames = options.shared_openssl_libname.split(',') - o['libraries'] += ['-l%s' % s for s in libnames] + libraries = format_libraries(options.shared_openssl_libname) + (libs, cflags) = pkg_config('openssl') or (libraries, '') + o['libraries'] += libs.split() if options.shared_openssl_libpath: o['libraries'] += ['-L%s' % options.shared_openssl_libpath] @@ -1017,6 +1034,13 @@ configure_winsdk(output) configure_intl(output) configure_fullystatic(output) +# remove duplicates from libraries +unique_libraries = [] +for library in output['libraries']: + if library not in unique_libraries: + unique_libraries.append(library) +output['libraries'] = unique_libraries + # variables should be a root level element, # move everything else to target_defaults variables = output['variables'] From 464a98681c6a1bbfaeb7d58e9ad394aed7d81745 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johan=20Bergstr=C3=B6m?= Date: Wed, 15 Apr 2015 12:03:44 +1000 Subject: [PATCH 3/5] build: Introduce option groups in configure Minor edits to current build flags and its help texts as well as grouping shared and i18n options into separate option groups. Hopefully this will improve the readability of `./configure` output. --- configure | 91 +++++++++++++++++++++++++++++++------------------------ 1 file changed, 52 insertions(+), 39 deletions(-) diff --git a/configure b/configure index b0e324df2e8dcf..fe82be3e602182 100755 --- a/configure +++ b/configure @@ -32,6 +32,15 @@ valid_mips_arch = ('loongson', 'r1', 'r2', 'r6', 'rx') valid_mips_fpu = ('fp32', 'fp64', 'fpxx') valid_mips_float_abi = ('soft', 'hard') +# 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. parser.add_option('--prefix', @@ -78,89 +87,91 @@ 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') + help='link to a shared http_parser library') -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') + help='path to http_parser headers') -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') + help='http_parser shared library directory path') -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') + help='link to a shared libuv library') -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') + help='path to libuv headers') -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') + help='libuv shared library directory path') -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') + help='link to a shared OpenSSL library') -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') + help='path to OpenSSL headers') -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') + help='OpenSSL shared library path') -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') + help='link to a shared zlib library') -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') + help='path to zlib headers') -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') + help='zlib shared library path') + +parser.add_option_group(shared_optgroup) # TODO document when we've decided on what the tracing API and its options will # look like @@ -225,33 +236,35 @@ parser.add_option('--with-etw', dest='with_etw', help='build with ETW (default is true on Windows)') -parser.add_option('--download', +intl_optgroup.add_option('--with-intl', action='store', - dest='download_list', - help=nodedownload.help()) + dest='with_intl', + help='Intl mode: none, full-icu, small-icu (default is none)') -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', + 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', From eff3ec32093bfcfb7f54ea9d5eee7af1b953c97f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johan=20Bergstr=C3=B6m?= Date: Wed, 15 Apr 2015 12:12:00 +1000 Subject: [PATCH 4/5] build: Switch --with-intl to optparse format Avoid custom validation as well as simplify output generated by configure. Add 'system-icu' to the list of valid choices. --- configure | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/configure b/configure index fe82be3e602182..cb8ca8001d0d52 100755 --- a/configure +++ b/configure @@ -31,6 +31,7 @@ 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", @@ -239,7 +240,10 @@ parser.add_option('--with-etw', intl_optgroup.add_option('--with-intl', action='store', dest='with_intl', - help='Intl mode: none, full-icu, small-icu (default is none)') + default='none', + choices=valid_intl_modes, + help='Intl mode (valid choices: {0}) [default: %default]'.format( + ', '.join(valid_intl_modes))) intl_optgroup.add_option('--with-icu-path', action='store', @@ -842,7 +846,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': print 'Error: Cannot specify both --with-icu-path and --with-intl' sys.exit(1) elif have_icu_path: @@ -880,11 +884,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') From 88fd3f23363015dd56a7dfbf824542ffe53ac6ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johan=20Bergstr=C3=B6m?= Date: Thu, 16 Apr 2015 11:27:01 +1000 Subject: [PATCH 5/5] fixup! simplify shared checks --- configure | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/configure b/configure index cb8ca8001d0d52..07600193e5d2ad 100755 --- a/configure +++ b/configure @@ -682,7 +682,7 @@ def configure_node(o): def configure_libz(o): o['variables']['node_shared_zlib'] = b(options.shared_zlib) - if b(options.shared_zlib) == 'true': + if options.shared_zlib: libraries = format_libraries(options.shared_zlib_libname) (libs, cflags) = pkg_config('zlib') or (libraries, '') o['libraries'] += libs.split() @@ -698,7 +698,7 @@ def configure_libz(o): def configure_http_parser(o): o['variables']['node_shared_http_parser'] = b(options.shared_http_parser) - if b(options.shared_http_parser) == 'true': + if options.shared_http_parser: # no .pc-file for http_parser o['libraries'] += ['-l%s' % options.shared_http_parser_libname] if options.shared_http_parser_libpath: @@ -710,7 +710,7 @@ def configure_http_parser(o): def configure_libuv(o): o['variables']['node_shared_libuv'] = b(options.shared_libuv) - if b(options.shared_libuv) == 'true': + if options.shared_libuv: libraries = format_libraries(options.shared_libuv_libname) (libs, cflags) = pkg_config('libuv') or (libraries, '') o['libraries'] += libs.split() @@ -742,7 +742,7 @@ def configure_openssl(o): if options.without_ssl: return - if b(options.shared_openssl) == 'true': + if options.shared_openssl: libraries = format_libraries(options.shared_openssl_libname) (libs, cflags) = pkg_config('openssl') or (libraries, '') o['libraries'] += libs.split()