build: validate options passed to configure

[jbergstroem]

Validate some of the arguments passed to configurable options. Also, move defaults to optparse default.
I can't really vouch for options; just pulled them from comments. I've tested a few permutations, but not all.

[jbergstroem]

https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/434/

[jbergstroem]

Looks like there's some trickery going with passing linker flags. Investigating.

Edit: Had to avoid setting libraries at all.

[mscdex]

+1

[jbergstroem]

New build at https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/438/

Hopefully I didn't screw this one up..

[rvagg]

love it, lgtm with squash

[bnoordhuis]

You can shorten this to:

'CPU architecture to build for ({0})'.format(', '.join(valid_arch))

Or if you don't care about python 3 compat:

'CPU architecture to build for (%s)' % ', '.join(valid_arch)

[jbergstroem]

Thanks. My Python is weak.

[jbergstroem]

Holding off until #1331 lands; no point in writing logic for shared v8 flags when we're removing it. Agree?

[jbergstroem]

@bnoordhuis: I've looked closer at the icu branching. I'm still keen on removing the check altogether. The issue is that help text says that root,en is default - but it isn't set unless small-icu is in use. Removing the check will just make the build ignore with_icu_locales (well, it already ignores it - but it won't bail for no other reason) for anything else than small-icu. For small-icu we now set it through optparse. With your agreement I'll remove this logic.

Other than that, I've simplified the help text generation and rebased onto the v8 removal branch. PTAL.

[jbergstroem]

@bnoordhuis this is what I have in mind:

diff --git configure configure
index 1d0e65a..148b208 100755
--- configure
+++ configure
@@ -787,13 +787,7 @@ def configure_intl(o):
     return
   # --with-intl=<with_intl>
   # set the default
-  if with_intl is None:
-    with_intl = 'none'  # The default mode of Intl
-  # sanity check localelist
-  if options.with_icu_locales and (with_intl != 'small-icu'):
-    print 'Error: --with-icu-locales only makes sense with --with-intl=small-icu'
-    sys.exit(1)
-  if with_intl == 'none' or with_intl is None:
+  if with_intl in (None, 'none'):
     o['variables']['v8_enable_i18n_support'] = 0
     return  # no Intl
   elif with_intl == 'small-icu':
@@ -801,8 +795,6 @@ def configure_intl(o):
     o['variables']['v8_enable_i18n_support'] = 1
     o['variables']['icu_small'] = b(True)
     with_icu_locales = options.with_icu_locales
-    if not with_icu_locales:
-      with_icu_locales = 'root,en'
     locs = set(with_icu_locales.split(','))
     locs.add('root')  # must have root
     o['variables']['icu_locales'] = string.join(locs,',')

[bnoordhuis]

@jbergstroem Shouldn't with_icu_locales have a default in that case? Else the line that says locs = set(with_icu_locales.split(',')) is going to error.

[jbergstroem]

@bnoordhuis ah, sorry - should say options.with_icu_locales

[bnoordhuis]

What I mean is, with_icu_locales or options.with_icu_locales is going to be None, so calling .split() on it won't work.

[jbergstroem]

@bnoordhuis with_icu_locales will always be set to root,en unless you override it: https://github.com/iojs/io.js/pull/1335/files#diff-e2d5a00791bce9a01f99bc6fd613a39dR259

[bnoordhuis]

Right you are. Objection withdrawn.

[jbergstroem]

@bnoordhuis good to merge?

[bnoordhuis]

LGTM

[jbergstroem]

Merged in fd90b33. Thanks for the feedback and helping me to get it in shape. Closing.