From 2ed875e1b49d06d677d299534f2f8290bfbd3b35 Mon Sep 17 00:00:00 2001 From: Jon Turney Date: Sun, 31 Dec 2017 16:50:52 +0000 Subject: [PATCH 1/3] Consolidate warning location formatting in mlog.warning() Also use .format() rather than % Also use build.environment rather than hardcoding 'meson.build' --- mesonbuild/interpreter.py | 2 +- mesonbuild/mlog.py | 7 +++++++ mesonbuild/mparser.py | 5 +++-- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/mesonbuild/interpreter.py b/mesonbuild/interpreter.py index e8fb08154..8170357e0 100644 --- a/mesonbuild/interpreter.py +++ b/mesonbuild/interpreter.py @@ -1935,7 +1935,7 @@ to directly access options of other subprojects.''') @noKwargs def func_warning(self, node, args, kwargs): argstr = self.get_message_string_arg(node) - mlog.warning('%s in file %s, line %d' % (argstr, os.path.join(node.subdir, 'meson.build'), node.lineno)) + mlog.warning(argstr, location=node) @noKwargs def func_error(self, node, args, kwargs): diff --git a/mesonbuild/mlog.py b/mesonbuild/mlog.py index a0d07ec02..aa2ac20ec 100644 --- a/mesonbuild/mlog.py +++ b/mesonbuild/mlog.py @@ -103,6 +103,13 @@ def log(*args, **kwargs): force_print(*arr, **kwargs) def warning(*args, **kwargs): + from . import environment + + if kwargs.get('location'): + location = kwargs['location'] + del kwargs['location'] + args += ('in file {}, line {}.'.format(os.path.join(location.subdir, environment.build_filename), location.lineno),) + log(yellow('WARNING:'), *args, **kwargs) # Format a list for logging purposes as a string. It separates diff --git a/mesonbuild/mparser.py b/mesonbuild/mparser.py index 782b7a7b1..eb03393fc 100644 --- a/mesonbuild/mparser.py +++ b/mesonbuild/mparser.py @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -import os, re +import re from .mesonlib import MesonException from . import mlog @@ -368,7 +368,8 @@ class ArgumentNode: def set_kwarg(self, name, value): if name in self.kwargs: - mlog.warning('Keyword argument "%s" defined multiple times in file %s, line %d. This will be an error in future Meson releases.' % (name, os.path.join(self.subdir, 'meson.build'), self.lineno)) + mlog.warning('Keyword argument "{}" defined multiple times'.format(name), location=self) + mlog.warning('This will be an error in future Meson releases.') self.kwargs[name] = value def num_args(self): From f85fde743a292e24c9aed81c23f6af466054aee3 Mon Sep 17 00:00:00 2001 From: Jon Turney Date: Sun, 31 Dec 2017 17:16:28 +0000 Subject: [PATCH 2/3] Wire up locations in a couple more warnings These are the remaining warnings in the parser, where we have the location to hand. --- mesonbuild/interpreter.py | 4 ++-- run_unittests.py | 13 +++++++++---- test cases/unit/20 warning location/conf.in | 1 + test cases/unit/20 warning location/meson.build | 6 +++++- 4 files changed, 17 insertions(+), 7 deletions(-) create mode 100644 test cases/unit/20 warning location/conf.in diff --git a/mesonbuild/interpreter.py b/mesonbuild/interpreter.py index 8170357e0..85c3f7711 100644 --- a/mesonbuild/interpreter.py +++ b/mesonbuild/interpreter.py @@ -1574,7 +1574,7 @@ class Interpreter(InterpreterBase): modname = args[0] if modname.startswith('unstable-'): plainname = modname.split('-', 1)[1] - mlog.warning('Module %s has no backwards or forwards compatibility and might not exist in future releases.' % modname) + mlog.warning('Module %s has no backwards or forwards compatibility and might not exist in future releases' % modname, location=node) modname = 'unstable_' + plainname if modname not in self.environment.coredata.modules: try: @@ -2732,7 +2732,7 @@ root and issuing %s. mlog.warning( "The variable(s) %s in the input file %s are not " "present in the given configuration data" % ( - var_list, inputfile)) + var_list, inputfile), location=node) else: mesonlib.dump_conf_header(ofile_abs, conf.held_object) conf.mark_used() diff --git a/run_unittests.py b/run_unittests.py index dc2429af0..11f16a24d 100755 --- a/run_unittests.py +++ b/run_unittests.py @@ -1710,10 +1710,15 @@ int main(int argc, char **argv) { def test_warning_location(self): tdir = os.path.join(self.unit_test_dir, '20 warning location') out = self.init(tdir) - self.assertRegex(out, r'WARNING: Keyword argument "link_with" defined multiple times in file meson.build, line 4') - self.assertRegex(out, r'WARNING: Keyword argument "link_with" defined multiple times in file sub' + re.escape(os.path.sep) + r'meson.build, line 3') - self.assertRegex(out, r'WARNING: a warning of some sort in file meson.build, line 6') - self.assertRegex(out, r'WARNING: subdir warning in file sub' + re.escape(os.path.sep) + r'meson.build, line 4') + for expected in [ + r'WARNING: Keyword argument "link_with" defined multiple times in file meson.build, line 4', + r'WARNING: Keyword argument "link_with" defined multiple times in file sub' + os.path.sep + r'meson.build, line 3', + r'WARNING: a warning of some sort in file meson.build, line 6', + r'WARNING: subdir warning in file sub' + os.path.sep + r'meson.build, line 4', + r'WARNING: Module unstable-simd has no backwards or forwards compatibility and might not exist in future releases in file meson.build, line 7', + r"WARNING: The variable(s) 'MISSING' in the input file conf.in are not present in the given configuration data in file meson.build, line 10", + ]: + self.assertRegex(out, re.escape(expected)) def test_templates(self): ninja = detect_ninja() diff --git a/test cases/unit/20 warning location/conf.in b/test cases/unit/20 warning location/conf.in new file mode 100644 index 000000000..a2903ed38 --- /dev/null +++ b/test cases/unit/20 warning location/conf.in @@ -0,0 +1 @@ +@MISSING@ diff --git a/test cases/unit/20 warning location/meson.build b/test cases/unit/20 warning location/meson.build index e26c6c9df..0b14b8be9 100644 --- a/test cases/unit/20 warning location/meson.build +++ b/test cases/unit/20 warning location/meson.build @@ -1,6 +1,10 @@ -project('duplicate kwarg', 'c') +project('warning location', 'c') a = library('liba', 'a.c') b = library('libb', 'b.c') executable('main', 'main.c', link_with: a, link_with: b) subdir('sub') warning('a warning of some sort') +import('unstable-simd') + +conf_data = configuration_data() +configure_file(input: 'conf.in' , output: 'conf', configuration: conf_data) From bcc95d7dd703779228ec81b92197e010d0e5a1ea Mon Sep 17 00:00:00 2001 From: Jon Turney Date: Sun, 31 Dec 2017 17:33:50 +0000 Subject: [PATCH 3/3] Use location formatting in mlog.warning() for invalid kwarg warning This already reports the location (in a slightly different format), but using mlog.warning() will make it easier if we want to change the location format in future. --- mesonbuild/interpreterbase.py | 18 ++++++++++-------- run_unittests.py | 1 + .../unit/20 warning location/meson.build | 2 +- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/mesonbuild/interpreterbase.py b/mesonbuild/interpreterbase.py index 91f4bd3e9..9dc6b0f95 100644 --- a/mesonbuild/interpreterbase.py +++ b/mesonbuild/interpreterbase.py @@ -18,7 +18,7 @@ from . import mparser, mesonlib, mlog from . import environment, dependencies -import os, copy, re +import os, copy, re, types from functools import wraps # Decorators for method calls. @@ -63,17 +63,19 @@ class permittedKwargs: def __call__(self, f): @wraps(f) def wrapped(s, node_or_state, args, kwargs): + loc = types.SimpleNamespace() if hasattr(s, 'subdir'): - subdir = s.subdir - lineno = s.current_lineno + loc.subdir = s.subdir + loc.lineno = s.current_lineno elif hasattr(node_or_state, 'subdir'): - subdir = node_or_state.subdir - lineno = node_or_state.current_lineno + loc.subdir = node_or_state.subdir + loc.lineno = node_or_state.current_lineno + else: + loc = None for k in kwargs: if k not in self.permitted: - fname = os.path.join(subdir, environment.build_filename) - mlog.warning('''Passed invalid keyword argument "%s" in %s line %d. -This will become a hard error in the future.''' % (k, fname, lineno)) + mlog.warning('''Passed invalid keyword argument "{}"'''.format(k), location=loc) + mlog.warning('This will become a hard error in the future.') return f(s, node_or_state, args, kwargs) return wrapped diff --git a/run_unittests.py b/run_unittests.py index 11f16a24d..c634b8b2b 100755 --- a/run_unittests.py +++ b/run_unittests.py @@ -1717,6 +1717,7 @@ int main(int argc, char **argv) { r'WARNING: subdir warning in file sub' + os.path.sep + r'meson.build, line 4', r'WARNING: Module unstable-simd has no backwards or forwards compatibility and might not exist in future releases in file meson.build, line 7', r"WARNING: The variable(s) 'MISSING' in the input file conf.in are not present in the given configuration data in file meson.build, line 10", + r'WARNING: Passed invalid keyword argument "invalid" in file meson.build, line 1' ]: self.assertRegex(out, re.escape(expected)) diff --git a/test cases/unit/20 warning location/meson.build b/test cases/unit/20 warning location/meson.build index 0b14b8be9..15295a95c 100644 --- a/test cases/unit/20 warning location/meson.build +++ b/test cases/unit/20 warning location/meson.build @@ -1,4 +1,4 @@ -project('warning location', 'c') +project('warning location', 'c', invalid: 'cheese') a = library('liba', 'a.c') b = library('libb', 'b.c') executable('main', 'main.c', link_with: a, link_with: b)