From ed831fc8624d33252771c49f4369e7262d4685ff Mon Sep 17 00:00:00 2001 From: Xavier Claessens Date: Thu, 26 Apr 2018 22:18:46 -0400 Subject: [PATCH 12/16] Fix --warnlevel being renamed to --warning-level in latest release --- mesonbuild/coredata.py | 54 ++++++++++++++++----- mesonbuild/mconf.py | 15 +----- mesonbuild/mesonmain.py | 9 ++-- run_unittests.py | 39 +++++++++++++++ test cases/unit/30 command line/meson.build | 1 + 5 files changed, 87 insertions(+), 31 deletions(-) create mode 100644 test cases/unit/30 command line/meson.build diff --git a/mesonbuild/coredata.py b/mesonbuild/coredata.py index 93a9e718..52d3c7d1 100644 --- a/mesonbuild/coredata.py +++ b/mesonbuild/coredata.py @@ -368,12 +368,6 @@ def get_builtin_option_action(optname): return 'store_true' return None -def get_builtin_option_destination(optname): - optname = optname.replace('-', '_') - if optname == 'warnlevel': - return 'warning_level' - return optname - def get_builtin_option_default(optname, prefix='', noneIfSuppress=False): if is_builtin_option(optname): o = builtin_options[optname] @@ -391,24 +385,32 @@ def get_builtin_option_default(optname, prefix='', noneIfSuppress=False): else: raise RuntimeError('Tried to get the default value for an unknown builtin option \'%s\'.' % optname) +def get_builtin_option_cmdline_name(name): + if name == 'warning_level': + return '--warnlevel' + else: + return '--' + name.replace('_', '-') + def add_builtin_argument(p, name): kwargs = {} - k = get_builtin_option_destination(name) - c = get_builtin_option_choices(k) - b = get_builtin_option_action(k) - h = get_builtin_option_description(k) + c = get_builtin_option_choices(name) + b = get_builtin_option_action(name) + h = get_builtin_option_description(name) if not b: - h = h.rstrip('.') + ' (default: %s).' % get_builtin_option_default(k) + h = h.rstrip('.') + ' (default: %s).' % get_builtin_option_default(name) else: kwargs['action'] = b if c and not b: kwargs['choices'] = c - default = get_builtin_option_default(k, noneIfSuppress=True) + default = get_builtin_option_default(name, noneIfSuppress=True) if default is not None: kwargs['default'] = default else: kwargs['default'] = argparse.SUPPRESS - p.add_argument('--' + name.replace('_', '-'), help=h, **kwargs) + kwargs['dest'] = name + + cmdline_name = get_builtin_option_cmdline_name(name) + p.add_argument(cmdline_name, help=h, **kwargs) def register_builtin_arguments(parser): for n in builtin_options: @@ -416,6 +418,32 @@ def register_builtin_arguments(parser): parser.add_argument('-D', action='append', dest='projectoptions', default=[], metavar="option", help='Set the value of an option, can be used several times to set multiple options.') +def filter_builtin_options(args, original_args): + """Filter out any builtin arguments passed as -- instead of -D. + + Error if an argument is passed with -- and -D + """ + for name in builtin_options: + # Check if user passed --option. Cannot use hasattr(args, name) here + # because they are all set with default value if user didn't pass it. + cmdline_name = get_builtin_option_cmdline_name(name) + has_dashdash = any([a.startswith(cmdline_name) for a in original_args]) + + # Chekc if user passed -Doption=value + has_dashd = any([a.startswith('{}='.format(name)) for a in args.projectoptions]) + + # Passing both is ambigous, abort + if has_dashdash and has_dashd: + raise MesonException( + 'Got argument {0} as both -D{0} and {1}. Pick one.'.format(name, cmdline_name)) + + # Pretend --option never existed + if has_dashdash: + args.projectoptions.append('{}={}'.format(name, getattr(args, name))) + if hasattr(args, name): + delattr(args, name) + + builtin_options = { 'buildtype': [UserComboOption, 'Build type to use.', ['plain', 'debug', 'debugoptimized', 'release', 'minsize'], 'debug'], 'strip': [UserBooleanOption, 'Strip targets on install.', False], diff --git a/mesonbuild/mconf.py b/mesonbuild/mconf.py index f907d752..1375b2e0 100644 --- a/mesonbuild/mconf.py +++ b/mesonbuild/mconf.py @@ -28,19 +28,6 @@ def buildparser(): return parser -def filter_builtin_options(args, original_args): - """Filter out any args passed with -- instead of -D.""" - for arg in original_args: - if not arg.startswith('--') or arg == '--clearcache': - continue - name = arg.lstrip('--').split('=', 1)[0] - if any([a.startswith(name + '=') for a in args.projectoptions]): - raise mesonlib.MesonException( - 'Got argument {0} as both -D{0} and --{0}. Pick one.'.format(name)) - args.projectoptions.append('{}={}'.format(name, getattr(args, name))) - delattr(args, name) - - class ConfException(mesonlib.MesonException): pass @@ -242,7 +229,7 @@ def run(args): if not args: args = [os.getcwd()] options = buildparser().parse_args(args) - filter_builtin_options(options, args) + coredata.filter_builtin_options(options, args) if len(options.directory) > 1: print('%s ' % args[0]) print('If you omit the build directory, the current directory is substituted.') diff --git a/mesonbuild/mesonmain.py b/mesonbuild/mesonmain.py index 9dda4af5..2b6281d7 100644 --- a/mesonbuild/mesonmain.py +++ b/mesonbuild/mesonmain.py @@ -61,11 +61,12 @@ def filter_builtin_options(args, original_args): if meson_opts: for arg in meson_opts: value = arguments[arg] - if any([a.startswith('--{}'.format(arg)) for a in original_args]): + cmdline_name = coredata.get_builtin_option_cmdline_name(arg) + if any([a.startswith(cmdline_name) for a in original_args]): raise MesonException( - 'Argument "{0}" passed as both --{0} and -D{0}, but only ' - 'one is allowed'.format(arg)) - setattr(args, coredata.get_builtin_option_destination(arg), value) + 'Argument "{0}" passed as both {1} and -D{0}, but only ' + 'one is allowed'.format(arg, cmdline_name)) + setattr(args, arg, value) # Remove the builtin option from the project args values args.projectoptions.remove('{}={}'.format(arg, value)) diff --git a/run_unittests.py b/run_unittests.py index 3f80f74f..2a466db0 100755 --- a/run_unittests.py +++ b/run_unittests.py @@ -2020,6 +2020,45 @@ recommended as it can lead to undefined behaviour on some platforms''') self.builddir = exebuilddir self.assertRebuiltTarget('app') + def test_command_line(self): + testdir = os.path.join(self.unit_test_dir, '30 command line') + + # Verify default values when passing no args + self.init(testdir) + obj = mesonbuild.coredata.load(self.builddir) + self.assertEqual(obj.builtins['warning_level'].value, '1') + self.wipe() + + # warning_level is special, it's --warnlevel instead of --warning-level + # for historical reasons + self.init(testdir, extra_args=['--warnlevel=2']) + obj = mesonbuild.coredata.load(self.builddir) + self.assertEqual(obj.builtins['warning_level'].value, '2') + self.setconf('--warnlevel=3') + obj = mesonbuild.coredata.load(self.builddir) + self.assertEqual(obj.builtins['warning_level'].value, '3') + self.wipe() + + # But when using -D syntax, it should be 'warning_level' + self.init(testdir, extra_args=['-Dwarning_level=2']) + obj = mesonbuild.coredata.load(self.builddir) + self.assertEqual(obj.builtins['warning_level'].value, '2') + self.setconf('-Dwarning_level=3') + obj = mesonbuild.coredata.load(self.builddir) + self.assertEqual(obj.builtins['warning_level'].value, '3') + self.wipe() + + # Mixing --option and -Doption is forbidden + with self.assertRaises(subprocess.CalledProcessError) as e: + self.init(testdir, extra_args=['--warnlevel=1', '-Dwarning_level=3']) + self.assertNotEqual(0, e.returncode) + self.assertIn('passed as both', e.stderr) + with self.assertRaises(subprocess.CalledProcessError) as e: + self.setconf('--warnlevel=1', '-Dwarning_level=3') + self.assertNotEqual(0, e.returncode) + self.assertIn('passed as both', e.stderr) + self.wipe() + class FailureTests(BasePlatformTests): ''' diff --git a/test cases/unit/30 command line/meson.build b/test cases/unit/30 command line/meson.build new file mode 100644 index 00000000..2ab21b6e --- /dev/null +++ b/test cases/unit/30 command line/meson.build @@ -0,0 +1 @@ +project('command line test', 'c') -- 2.17.0