From cee9638cc43682b6e371cc2becae745876c73bda Mon Sep 17 00:00:00 2001 From: Nirbheek Chauhan Date: Tue, 29 Nov 2016 03:15:03 +0530 Subject: [PATCH 1/4] Compiler check and extra args should always override We want compiler check arguments (-O0, -fpermissive, etc) to override all other arguments, and we want extra_args passed in by the build file to always override everything. To do this properly, we must split include arguments out, append them first, append all other arguments as usual, and then append the rest. As part of this, we also add the compiler check flags to the cc.compiles() and cc.links() helper functions since they also most likely need them. Also includes a unit test for all this. --- mesonbuild/compilers.py | 59 ++++++++++++++----- run_unittests.py | 28 +++++++++ test cases/common/43 has function/meson.build | 32 +++++++--- 3 files changed, 96 insertions(+), 23 deletions(-) diff --git a/mesonbuild/compilers.py b/mesonbuild/compilers.py index 8f8851f4d..ef37226a8 100644 --- a/mesonbuild/compilers.py +++ b/mesonbuild/compilers.py @@ -701,8 +701,28 @@ int main () {{ #endif return 0; }}''' - args = extra_args + self.get_compiler_check_args() - return self.compiles(templ.format(hname, symbol, prefix), env, args, dependencies) + return self.compiles(templ.format(hname, symbol, prefix), env, + extra_args, dependencies) + + @staticmethod + def _override_args(args, override): + ''' + Add @override to @args in such a way that arguments are overrided + correctly. + + We want the include directories to be added first (since they are + chosen left-to-right) and all other arguments later (since they + override previous arguments or add to a list that's chosen + right-to-left). + ''' + before_args = [] + after_args = [] + for arg in override: + if arg.startswith(('-I', '/I')): + before_args.append(arg) + else: + after_args.append(arg) + return before_args + args + after_args def compiles(self, code, env, extra_args=None, dependencies=None): if extra_args is None: @@ -713,9 +733,10 @@ int main () {{ dependencies = [] elif not isinstance(dependencies, list): dependencies = [dependencies] + # Add compile flags needed by dependencies after converting to the + # native type of the selected compiler cargs = [a for d in dependencies for a in d.get_compile_args()] - # Convert flags to the native type of the selected compiler - args = self.unix_link_flags_to_native(cargs + extra_args) + args = self.unix_link_flags_to_native(cargs) # Read c_args/cpp_args/etc from the cross-info file (if needed) args += self.get_cross_extra_flags(env, compile=True, link=False) # Add CFLAGS/CXXFLAGS/OBJCFLAGS/OBJCXXFLAGS from the env @@ -723,6 +744,11 @@ int main () {{ args += env.coredata.external_args[self.language] # We only want to compile; not link args += self.get_compile_only_args() + # Append extra_args to the compiler check args such that it overrides + extra_args = self._override_args(self.get_compiler_check_args(), extra_args) + extra_args = self.unix_link_flags_to_native(extra_args) + # Append both to the compiler args such that they override them + args = self._override_args(args, extra_args) with self.compile(code, args) as p: return p.returncode == 0 @@ -736,17 +762,24 @@ int main () {{ dependencies = [] elif not isinstance(dependencies, list): dependencies = [dependencies] + # Add compile and link flags needed by dependencies after converting to + # the native type of the selected compiler cargs = [a for d in dependencies for a in d.get_compile_args()] link_args = [a for d in dependencies for a in d.get_link_args()] - # Convert flags to the native type of the selected compiler - args = self.unix_link_flags_to_native(cargs + link_args + extra_args) + args = self.unix_link_flags_to_native(cargs + link_args) # Select a CRT if needed since we're linking args += self.get_linker_debug_crt_args() - # Read c_args/c_link_args/cpp_args/cpp_link_args/etc from the cross-info file (if needed) + # Read c_args/c_link_args/cpp_args/cpp_link_args/etc from the + # cross-info file (if needed) args += self.get_cross_extra_flags(env, compile=True, link=True) # Add LDFLAGS from the env. We assume that the user has ensured these # are compiler-specific args += env.coredata.external_link_args[self.language] + # Append extra_args to the compiler check args such that it overrides + extra_args = self._override_args(self.get_compiler_check_args(), extra_args) + extra_args = self.unix_link_flags_to_native(extra_args) + # Append both to the compiler args such that they override them + args = self._override_args(args, extra_args) return self.compile(code, args) def links(self, code, env, extra_args=None, dependencies=None): @@ -795,7 +828,6 @@ int main(int argc, char **argv) {{ %s int temparray[%d-sizeof(%s)]; ''' - args = extra_args + self.get_compiler_check_args() if not self.compiles(element_exists_templ.format(prefix, element), env, args, dependencies): return -1 for i in range(1, 1024): @@ -844,7 +876,6 @@ struct tmp { int testarray[%d-offsetof(struct tmp, target)]; ''' - args = extra_args + self.get_compiler_check_args() if not self.compiles(type_exists_templ.format(typename), env, args, dependencies): return -1 for i in range(1, 1024): @@ -980,14 +1011,14 @@ int main(int argc, char **argv) { head, main = self._no_prototype_templ() templ = head + stubs_fail + main - args = extra_args + self.get_compiler_check_args() - if self.links(templ.format(prefix, funcname), env, args, dependencies): + if self.links(templ.format(prefix, funcname), env, extra_args, dependencies): return True # Some functions like alloca() are defined as compiler built-ins which # are inlined by the compiler, so test for that instead. Built-ins are # special functions that ignore all includes and defines, so we just # directly try to link via main(). - return self.links('int main() {{ {0}; }}'.format('__builtin_' + funcname), env, args, dependencies) + return self.links('int main() {{ {0}; }}'.format('__builtin_' + funcname), + env, extra_args, dependencies) def has_members(self, typename, membernames, prefix, env, extra_args=None, dependencies=None): if extra_args is None: @@ -1071,8 +1102,8 @@ class CPPCompiler(CCompiler): #include <{0}> using {1}; int main () {{ return 0; }}''' - args = extra_args + self.get_compiler_check_args() - return self.compiles(templ.format(hname, symbol, prefix), env, args, dependencies) + return self.compiles(templ.format(hname, symbol, prefix), env, + extra_args, dependencies) class ObjCCompiler(CCompiler): def __init__(self, exelist, version, is_cross, exe_wrap): diff --git a/run_unittests.py b/run_unittests.py index 8b1f13f2e..b8d23b88a 100755 --- a/run_unittests.py +++ b/run_unittests.py @@ -94,6 +94,16 @@ class LinuxlikeTests(unittest.TestCase): with open(os.path.join(self.builddir, 'meson-logs', 'meson-log.txt')) as f: return f.readlines() + def get_meson_log_compiler_checks(self): + ''' + Fetch a list command-lines run by meson for compiler checks. + Each command-line is returned as a list of arguments. + ''' + log = self.get_meson_log() + prefix = 'Command line:' + cmds = [l[len(prefix):].split() for l in log if l.startswith(prefix)] + return cmds + def introspect(self, arg): out = subprocess.check_output(self.mintro_command + [arg, self.builddir]) return json.loads(out.decode('utf-8')) @@ -262,5 +272,23 @@ class LinuxlikeTests(unittest.TestCase): self.assertEqual(self.get_soname(bothset), 'libbothset.so.1.2.3') self.assertEqual(len(glob(bothset[:-3] + '*')), 3) + def test_compiler_check_flags_order(self): + ''' + Test that compiler check flags override all other flags. This can't be + an ordinary test case because it needs the environment to be set. + ''' + Oflag = '-O3' + os.environ['CFLAGS'] = os.environ['CXXFLAGS'] = Oflag + testdir = os.path.join(self.common_test_dir, '43 has function') + self.init(testdir) + cmds = self.get_meson_log_compiler_checks() + for cmd in cmds: + # Verify that -I flags from the `args` kwarg are first + # This is set in the '43 has function' test case + self.assertEqual(cmd[2], '-I/tmp') + # Verify that -O3 set via the environment is overriden by -O0 + Oargs = [arg for arg in cmd if arg.startswith('-O')] + self.assertEqual(Oargs, [Oflag, '-O0']) + if __name__ == '__main__': unittest.main() diff --git a/test cases/common/43 has function/meson.build b/test cases/common/43 has function/meson.build index 61f96e11e..e0d3344f6 100644 --- a/test cases/common/43 has function/meson.build +++ b/test cases/common/43 has function/meson.build @@ -1,9 +1,12 @@ project('has function', 'c', 'cpp') +# This is used in the `test_compiler_check_flags_order` unit test +unit_test_args = '-I/tmp' compilers = [meson.get_compiler('c'), meson.get_compiler('cpp')] foreach cc : compilers - if not cc.has_function('printf', prefix : '#include') + if not cc.has_function('printf', prefix : '#include', + args : unit_test_args) error('"printf" function not found (should always exist).') endif @@ -13,12 +16,16 @@ foreach cc : compilers # On MSVC fprintf is defined as an inline function in the header, so it cannot # be found without the include. if cc.get_id() != 'msvc' - assert(cc.has_function('fprintf'), '"fprintf" function not found without include (on !msvc).') + assert(cc.has_function('fprintf', args : unit_test_args), + '"fprintf" function not found without include (on !msvc).') else - assert(cc.has_function('fprintf', prefix : '#include '), '"fprintf" function not found with include (on msvc).') + assert(cc.has_function('fprintf', prefix : '#include ', + args : unit_test_args), + '"fprintf" function not found with include (on msvc).') endif - if cc.has_function('hfkerhisadf', prefix : '#include') + if cc.has_function('hfkerhisadf', prefix : '#include', + args : unit_test_args) error('Found non-existent function "hfkerhisadf".') endif @@ -28,16 +35,23 @@ foreach cc : compilers # implemented in glibc it's probably not implemented in any other 'slimmer' # C library variants either, so the check should be safe either way hopefully. if host_machine.system() == 'linux' and cc.get_id() == 'gcc' - assert (cc.has_function('poll', prefix : '#include '), 'couldn\'t detect "poll" when defined by a header') - assert (not cc.has_function('lchmod', prefix : '''#include - #include '''), '"lchmod" check should have failed') + assert (cc.has_function('poll', prefix : '#include ', + args : unit_test_args), + 'couldn\'t detect "poll" when defined by a header') + lchmod_prefix = '#include \n#include ' + assert (not cc.has_function('lchmod', prefix : lchmod_prefix, + args : unit_test_args), + '"lchmod" check should have failed') endif # For some functions one needs to define _GNU_SOURCE before including the # right headers to get them picked up. Make sure we can detect these functions # as well without any prefix - if cc.has_header_symbol('sys/socket.h', 'recvmmsg', prefix : '#define _GNU_SOURCE') + if cc.has_header_symbol('sys/socket.h', 'recvmmsg', + prefix : '#define _GNU_SOURCE', + args : unit_test_args) # We assume that if recvmmsg exists sendmmsg does too - assert (cc.has_function('sendmmsg'), 'Failed to detect function "sendmmsg" (should always exist).') + assert (cc.has_function('sendmmsg', args : unit_test_args), + 'Failed to detect function "sendmmsg" (should always exist).') endif endforeach From 025d699c002ab9e0c1823dce3e526b70c0fc2280 Mon Sep 17 00:00:00 2001 From: Nirbheek Chauhan Date: Tue, 29 Nov 2016 03:36:35 +0530 Subject: [PATCH 2/4] unit tests: Use universal_newlines everywhere This approach is locale-independent and more correct. For instance, this will work with UTF-16 while the previous approach with binary comparison would not. This also removes the need for doing an explicit decode to utf-8 which is lossy and can fail by yielding no output at all. --- run_unittests.py | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/run_unittests.py b/run_unittests.py index b8d23b88a..cc105f297 100755 --- a/run_unittests.py +++ b/run_unittests.py @@ -24,9 +24,10 @@ from mesonbuild.dependencies import PkgConfigDependency, Qt5Dependency def get_soname(fname): # HACK, fix to not use shell. - raw_out = subprocess.check_output(['readelf', '-a', fname]) - pattern = re.compile(b'soname: \[(.*?)\]') - for line in raw_out.split(b'\n'): + raw_out = subprocess.check_output(['readelf', '-a', fname], + universal_newlines=True) + pattern = re.compile('soname: \[(.*?)\]') + for line in raw_out.split('\n'): m = pattern.search(line) if m is not None: return m.group(1) @@ -105,8 +106,9 @@ class LinuxlikeTests(unittest.TestCase): return cmds def introspect(self, arg): - out = subprocess.check_output(self.mintro_command + [arg, self.builddir]) - return json.loads(out.decode('utf-8')) + out = subprocess.check_output(self.mintro_command + [arg, self.builddir], + universal_newlines=True) + return json.loads(out) def test_basic_soname(self): testdir = os.path.join(self.common_test_dir, '4 shared') @@ -114,7 +116,7 @@ class LinuxlikeTests(unittest.TestCase): self.build() lib1 = os.path.join(self.builddir, 'libmylib.so') soname = get_soname(lib1) - self.assertEqual(soname, b'libmylib.so') + self.assertEqual(soname, 'libmylib.so') def test_custom_soname(self): testdir = os.path.join(self.common_test_dir, '27 library versions') @@ -122,7 +124,7 @@ class LinuxlikeTests(unittest.TestCase): self.build() lib1 = os.path.join(self.builddir, 'prefixsomelib.suffix') soname = get_soname(lib1) - self.assertEqual(soname, b'prefixsomelib.suffix') + self.assertEqual(soname, 'prefixsomelib.suffix') def test_pic(self): testdir = os.path.join(self.common_test_dir, '3 static') @@ -225,8 +227,9 @@ class LinuxlikeTests(unittest.TestCase): self.assertTrue(msg in mesonlog or msg2 in mesonlog) def get_soname(self, fname): - output = subprocess.check_output(['readelf', '-a', fname]) - for line in output.decode('utf-8', errors='ignore').split('\n'): + output = subprocess.check_output(['readelf', '-a', fname], + universal_newlines=True) + for line in output.split('\n'): if 'SONAME' in line: return line.split('[')[1].split(']')[0] raise RuntimeError('Readelf gave no SONAME.') From ae8c4f5a3078ad74d9d05eeddeb9c726b83863b8 Mon Sep 17 00:00:00 2001 From: Nirbheek Chauhan Date: Tue, 29 Nov 2016 03:38:06 +0530 Subject: [PATCH 3/4] run_unittests: Document the purpose of each test --- run_unittests.py | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/run_unittests.py b/run_unittests.py index cc105f297..03ce0df6c 100755 --- a/run_unittests.py +++ b/run_unittests.py @@ -111,6 +111,12 @@ class LinuxlikeTests(unittest.TestCase): return json.loads(out) def test_basic_soname(self): + ''' + Test that the soname is set correctly for shared libraries. This can't + be an ordinary test case because we need to run `readelf` and actually + check the soname. + https://github.com/mesonbuild/meson/issues/785 + ''' testdir = os.path.join(self.common_test_dir, '4 shared') self.init(testdir) self.build() @@ -119,6 +125,12 @@ class LinuxlikeTests(unittest.TestCase): self.assertEqual(soname, 'libmylib.so') def test_custom_soname(self): + ''' + Test that the soname is set correctly for shared libraries when + a custom prefix and/or suffix is used. This can't be an ordinary test + case because we need to run `readelf` and actually check the soname. + https://github.com/mesonbuild/meson/issues/785 + ''' testdir = os.path.join(self.common_test_dir, '27 library versions') self.init(testdir) self.build() @@ -127,6 +139,11 @@ class LinuxlikeTests(unittest.TestCase): self.assertEqual(soname, 'prefixsomelib.suffix') def test_pic(self): + ''' + Test that -fPIC is correctly added to static libraries when b_staticpic + is true and not when it is false. This can't be an ordinary test case + because we need to inspect the compiler database. + ''' testdir = os.path.join(self.common_test_dir, '3 static') self.init(testdir) compdb = self.get_compdb() @@ -142,6 +159,12 @@ class LinuxlikeTests(unittest.TestCase): self.assertTrue('-fPIC' not in compdb[0]['command']) def test_pkgconfig_gen(self): + ''' + Test that generated pkg-config files can be found and have the correct + version and link args. This can't be an ordinary test case because we + need to run pkg-config outside of a Meson build file. + https://github.com/mesonbuild/meson/issues/889 + ''' testdir = os.path.join(self.common_test_dir, '51 pkgconfig-gen') self.init(testdir) env = FakeEnvironment() @@ -153,6 +176,12 @@ class LinuxlikeTests(unittest.TestCase): self.assertTrue('-lfoo' in simple_dep.get_link_args()) def test_vala_c_warnings(self): + ''' + Test that no warnings are emitted for C code generated by Vala. This + can't be an ordinary test case because we need to inspect the compiler + database. + https://github.com/mesonbuild/meson/issues/864 + ''' testdir = os.path.join(self.vala_test_dir, '5 target glib') self.init(testdir) compdb = self.get_compdb() @@ -180,6 +209,12 @@ class LinuxlikeTests(unittest.TestCase): self.assertTrue('-Werror' in c_command) def test_static_compile_order(self): + ''' + Test that the order of files in a compiler command-line while compiling + and linking statically is deterministic. This can't be an ordinary test + case because we need to inspect the compiler database. + https://github.com/mesonbuild/meson/pull/951 + ''' testdir = os.path.join(self.common_test_dir, '5 linkstatic') self.init(testdir) compdb = self.get_compdb() @@ -191,6 +226,10 @@ class LinuxlikeTests(unittest.TestCase): # FIXME: We don't have access to the linker command def test_install_introspection(self): + ''' + Tests that the Meson introspection API exposes install filenames correctly + https://github.com/mesonbuild/meson/issues/829 + ''' testdir = os.path.join(self.common_test_dir, '8 install') self.init(testdir) intro = self.introspect('--targets') @@ -200,11 +239,19 @@ class LinuxlikeTests(unittest.TestCase): self.assertEqual(intro[1]['install_filename'], '/usr/local/bin/prog') def test_run_target_files_path(self): + ''' + Test that run_targets are run from the correct directory + https://github.com/mesonbuild/meson/issues/957 + ''' testdir = os.path.join(self.common_test_dir, '58 run target') self.init(testdir) self.run_target('check_exists') def test_qt5dependency_qmake_detection(self): + ''' + Test that qt5 detection with qmake works. This can't be an ordinary + test case because it involves setting the environment. + ''' # Verify that qmake is for Qt5 if not shutil.which('qmake-qt5'): if not shutil.which('qmake'): From 24be8b847443282c3f675266f3e63f2f1e1c9e90 Mon Sep 17 00:00:00 2001 From: Nirbheek Chauhan Date: Tue, 29 Nov 2016 04:28:36 +0530 Subject: [PATCH 4/4] compilers.py: Fix typo in function documentation --- mesonbuild/compilers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mesonbuild/compilers.py b/mesonbuild/compilers.py index ef37226a8..2534a47fb 100644 --- a/mesonbuild/compilers.py +++ b/mesonbuild/compilers.py @@ -707,7 +707,7 @@ int main () {{ @staticmethod def _override_args(args, override): ''' - Add @override to @args in such a way that arguments are overrided + Add @override to @args in such a way that arguments are overriden correctly. We want the include directories to be added first (since they are