Blob Blame History Raw
From ed831fc8624d33252771c49f4369e7262d4685ff Mon Sep 17 00:00:00 2001
From: Xavier Claessens <xavier.claessens@collabora.com>
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 <build directory>' % 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