From 6963da616bc26708061878788240dbbb11cddc39 Mon Sep 17 00:00:00 2001 From: Nirbheek Chauhan Date: Thu, 30 Mar 2017 23:38:28 +0530 Subject: [PATCH 1/4] Don't add dependencies recursively while linking We were doing this on the basis of an old comment, but there was no test for it and I couldn't reproduce the issue with clang on Linux at all. Let's add a (somewhat comprehensive) test and see if it breaks anywhere if we stop doing this. Halves the size of gstreamer's build.ninja from 20M to 8.7M Closes https://github.com/mesonbuild/meson/issues/1057 --- mesonbuild/backend/backends.py | 12 ++--- .../3rdorderdeps/lib.c.in | 8 ++++ .../3rdorderdeps/main.c.in | 16 +++++++ .../3rdorderdeps/meson.build | 44 ++++++++++++++++++ test cases/common/153 recursive linking/lib.h | 12 +++++ .../common/153 recursive linking/main.c | 46 +++++++++++++++++++ .../common/153 recursive linking/meson.build | 22 +++++++++ .../153 recursive linking/shnodep/lib.c | 6 +++ .../153 recursive linking/shnodep/meson.build | 1 + .../153 recursive linking/shshdep/lib.c | 8 ++++ .../153 recursive linking/shshdep/meson.build | 1 + .../153 recursive linking/shstdep/lib.c | 8 ++++ .../153 recursive linking/shstdep/meson.build | 1 + .../153 recursive linking/stnodep/lib.c | 6 +++ .../153 recursive linking/stnodep/meson.build | 1 + .../153 recursive linking/stshdep/lib.c | 8 ++++ .../153 recursive linking/stshdep/meson.build | 1 + .../153 recursive linking/ststdep/lib.c | 8 ++++ .../153 recursive linking/ststdep/meson.build | 1 + test cases/frameworks/7 gnome/gir/meson.build | 2 +- test cases/vala/11 generated vapi/meson.build | 2 +- 21 files changed, 204 insertions(+), 10 deletions(-) create mode 100644 test cases/common/153 recursive linking/3rdorderdeps/lib.c.in create mode 100644 test cases/common/153 recursive linking/3rdorderdeps/main.c.in create mode 100644 test cases/common/153 recursive linking/3rdorderdeps/meson.build create mode 100644 test cases/common/153 recursive linking/lib.h create mode 100644 test cases/common/153 recursive linking/main.c create mode 100644 test cases/common/153 recursive linking/meson.build create mode 100644 test cases/common/153 recursive linking/shnodep/lib.c create mode 100644 test cases/common/153 recursive linking/shnodep/meson.build create mode 100644 test cases/common/153 recursive linking/shshdep/lib.c create mode 100644 test cases/common/153 recursive linking/shshdep/meson.build create mode 100644 test cases/common/153 recursive linking/shstdep/lib.c create mode 100644 test cases/common/153 recursive linking/shstdep/meson.build create mode 100644 test cases/common/153 recursive linking/stnodep/lib.c create mode 100644 test cases/common/153 recursive linking/stnodep/meson.build create mode 100644 test cases/common/153 recursive linking/stshdep/lib.c create mode 100644 test cases/common/153 recursive linking/stshdep/meson.build create mode 100644 test cases/common/153 recursive linking/ststdep/lib.c create mode 100644 test cases/common/153 recursive linking/ststdep/meson.build diff --git a/mesonbuild/backend/backends.py b/mesonbuild/backend/backends.py index 4cfdd9ebd..3044ce617 100644 --- a/mesonbuild/backend/backends.py +++ b/mesonbuild/backend/backends.py @@ -453,15 +453,11 @@ class Backend: for d in deps: if not isinstance(d, (build.StaticLibrary, build.SharedLibrary)): raise RuntimeError('Tried to link with a non-library target "%s".' % d.get_basename()) - if isinstance(compiler, compilers.LLVMDCompiler) or isinstance(compiler, compilers.DmdDCompiler): - args += ['-L' + self.get_target_filename_for_linking(d)] + if isinstance(compiler, (compilers.LLVMDCompiler, compilers.DmdDCompiler)): + d_arg = '-L' + self.get_target_filename_for_linking(d) else: - args.append(self.get_target_filename_for_linking(d)) - # If you have executable e that links to shared lib s1 that links to shared library s2 - # you have to specify s2 as well as s1 when linking e even if e does not directly use - # s2. Gcc handles this case fine but Clang does not for some reason. Thus we need to - # explictly specify all libraries every time. - args += self.build_target_link_arguments(compiler, d.get_dependencies()) + d_arg = self.get_target_filename_for_linking(d) + args.append(d_arg) return args def determine_windows_extra_paths(self, target): diff --git a/test cases/common/153 recursive linking/3rdorderdeps/lib.c.in b/test cases/common/153 recursive linking/3rdorderdeps/lib.c.in new file mode 100644 index 000000000..461f85995 --- /dev/null +++ b/test cases/common/153 recursive linking/3rdorderdeps/lib.c.in @@ -0,0 +1,8 @@ +#include "../lib.h" + +int get_@DEPENDENCY@dep_value (void); + +SYMBOL_EXPORT +int get_@LIBTYPE@@DEPENDENCY@dep_value (void) { + return get_@DEPENDENCY@dep_value (); +} diff --git a/test cases/common/153 recursive linking/3rdorderdeps/main.c.in b/test cases/common/153 recursive linking/3rdorderdeps/main.c.in new file mode 100644 index 000000000..8155f357e --- /dev/null +++ b/test cases/common/153 recursive linking/3rdorderdeps/main.c.in @@ -0,0 +1,16 @@ +#include + +#include "../lib.h" + +SYMBOL_IMPORT int get_@LIBTYPE@@DEPENDENCY@dep_value (void); + +int main(int argc, char *argv[]) { + int val; + + val = get_@LIBTYPE@@DEPENDENCY@dep_value (); + if (val != @VALUE@) { + printf("@LIBTYPE@@DEPENDENCY@ was %i instead of @VALUE@\n", val); + return -1; + } + return 0; +} diff --git a/test cases/common/153 recursive linking/3rdorderdeps/meson.build b/test cases/common/153 recursive linking/3rdorderdeps/meson.build new file mode 100644 index 000000000..a2907f383 --- /dev/null +++ b/test cases/common/153 recursive linking/3rdorderdeps/meson.build @@ -0,0 +1,44 @@ +dep3_libs = [] + +# Permutate all combinations of shared and static libraries up to three levels +# executable -> shared -> static -> shared (etc) +foreach dep2 : ['sh', 'st'] + foreach dep1 : ['sh', 'st'] + foreach libtype : ['sh', 'st'] + name = libtype + dep1 + dep2 + if dep2 == 'sh' + libret = 1 + elif dep2 == 'st' + libret = 2 + else + error('Unknown dep2 "@0@"'.format(dep2)) + endif + + if libtype == 'sh' + target = 'shared_library' + elif libtype == 'st' + target = 'static_library' + else + error('Unknown libtype "@0@"'.format(libtype)) + endif + + cdata = configuration_data() + cdata.set('DEPENDENCY', dep1 + dep2) + cdata.set('LIBTYPE', libtype) + cdata.set('VALUE', libret) + + lib_c = configure_file(input : 'lib.c.in', + output : name + '-lib.c', + configuration : cdata) + dep = get_variable(dep1 + dep2 + 'dep') + dep3_lib = build_target(name, lib_c, link_with : dep, target_type : target) + dep3_libs += [dep3_lib] + + main_c = configure_file(input : 'main.c.in', + output : name + '-main.c', + configuration : cdata) + dep3_bin = executable(name, main_c, link_with : dep3_lib) + test(name + 'test', dep3_bin) + endforeach + endforeach +endforeach diff --git a/test cases/common/153 recursive linking/lib.h b/test cases/common/153 recursive linking/lib.h new file mode 100644 index 000000000..b6b75fa05 --- /dev/null +++ b/test cases/common/153 recursive linking/lib.h @@ -0,0 +1,12 @@ +#if defined _WIN32 + #define SYMBOL_IMPORT __declspec(dllimport) + #define SYMBOL_EXPORT __declspec(dllexport) +#else + #define SYMBOL_IMPORT + #if defined __GNUC__ + #define SYMBOL_EXPORT __attribute__ ((visibility("default"))) + #else + #pragma message ("Compiler does not support symbol visibility.") + #define SYMBOL_EXPORT + #endif +#endif diff --git a/test cases/common/153 recursive linking/main.c b/test cases/common/153 recursive linking/main.c new file mode 100644 index 000000000..f1219a649 --- /dev/null +++ b/test cases/common/153 recursive linking/main.c @@ -0,0 +1,46 @@ +#include + +#include "lib.h" + +SYMBOL_IMPORT int get_stnodep_value (void); +SYMBOL_IMPORT int get_shnodep_value (void); +SYMBOL_IMPORT int get_shshdep_value (void); +SYMBOL_IMPORT int get_shstdep_value (void); +SYMBOL_IMPORT int get_stshdep_value (void); +SYMBOL_IMPORT int get_ststdep_value (void); + +int main(int argc, char *argv[]) { + int val; + + val = get_shnodep_value (); + if (val != 1) { + printf("shnodep was %i instead of 1\n", val); + return -1; + } + val = get_stnodep_value (); + if (val != 2) { + printf("stnodep was %i instead of 2\n", val); + return -2; + } + val = get_shshdep_value (); + if (val != 1) { + printf("shshdep was %i instead of 1\n", val); + return -3; + } + val = get_shstdep_value (); + if (val != 2) { + printf("shstdep was %i instead of 2\n", val); + return -4; + } + val = get_stshdep_value (); + if (val != 1) { + printf("shstdep was %i instead of 1\n", val); + return -5; + } + val = get_ststdep_value (); + if (val != 2) { + printf("ststdep was %i instead of 2\n", val); + return -6; + } + return 0; +} diff --git a/test cases/common/153 recursive linking/meson.build b/test cases/common/153 recursive linking/meson.build new file mode 100644 index 000000000..f17fc4a6e --- /dev/null +++ b/test cases/common/153 recursive linking/meson.build @@ -0,0 +1,22 @@ +project('recursive dependencies', 'c') + +# Test that you can link a shared executable to: +# - A shared library with no other deps +subdir('shnodep') +# - A static library with no other deps +subdir('stnodep') +# - A shared library with a shared library dep +subdir('shshdep') +# - A shared library with a static library dep +subdir('shstdep') +# - A static library with a shared library dep +subdir('stshdep') +# - A static library with a static library dep +subdir('ststdep') + +test('alldeps', + executable('alldeps', 'main.c', + link_with : [shshdep, shstdep, ststdep, stshdep])) + +# More combinations of static and shared libraries +subdir('3rdorderdeps') diff --git a/test cases/common/153 recursive linking/shnodep/lib.c b/test cases/common/153 recursive linking/shnodep/lib.c new file mode 100644 index 000000000..a3b7993c2 --- /dev/null +++ b/test cases/common/153 recursive linking/shnodep/lib.c @@ -0,0 +1,6 @@ +#include "../lib.h" + +SYMBOL_EXPORT +int get_shnodep_value (void) { + return 1; +} diff --git a/test cases/common/153 recursive linking/shnodep/meson.build b/test cases/common/153 recursive linking/shnodep/meson.build new file mode 100644 index 000000000..796f0ddcb --- /dev/null +++ b/test cases/common/153 recursive linking/shnodep/meson.build @@ -0,0 +1 @@ +shnodep = shared_library('shnodep', 'lib.c') diff --git a/test cases/common/153 recursive linking/shshdep/lib.c b/test cases/common/153 recursive linking/shshdep/lib.c new file mode 100644 index 000000000..715d12092 --- /dev/null +++ b/test cases/common/153 recursive linking/shshdep/lib.c @@ -0,0 +1,8 @@ +#include "../lib.h" + +int get_shnodep_value (void); + +SYMBOL_EXPORT +int get_shshdep_value (void) { + return get_shnodep_value (); +} diff --git a/test cases/common/153 recursive linking/shshdep/meson.build b/test cases/common/153 recursive linking/shshdep/meson.build new file mode 100644 index 000000000..020b481cf --- /dev/null +++ b/test cases/common/153 recursive linking/shshdep/meson.build @@ -0,0 +1 @@ +shshdep = shared_library('shshdep', 'lib.c', link_with : shnodep) diff --git a/test cases/common/153 recursive linking/shstdep/lib.c b/test cases/common/153 recursive linking/shstdep/lib.c new file mode 100644 index 000000000..5da8d0b2e --- /dev/null +++ b/test cases/common/153 recursive linking/shstdep/lib.c @@ -0,0 +1,8 @@ +#include "../lib.h" + +int get_stnodep_value (void); + +SYMBOL_EXPORT +int get_shstdep_value (void) { + return get_stnodep_value (); +} diff --git a/test cases/common/153 recursive linking/shstdep/meson.build b/test cases/common/153 recursive linking/shstdep/meson.build new file mode 100644 index 000000000..008f9f893 --- /dev/null +++ b/test cases/common/153 recursive linking/shstdep/meson.build @@ -0,0 +1 @@ +shstdep = shared_library('shstdep', 'lib.c', link_with : stnodep) diff --git a/test cases/common/153 recursive linking/stnodep/lib.c b/test cases/common/153 recursive linking/stnodep/lib.c new file mode 100644 index 000000000..4bc50bea2 --- /dev/null +++ b/test cases/common/153 recursive linking/stnodep/lib.c @@ -0,0 +1,6 @@ +#include "../lib.h" + +SYMBOL_EXPORT +int get_stnodep_value (void) { + return 2; +} diff --git a/test cases/common/153 recursive linking/stnodep/meson.build b/test cases/common/153 recursive linking/stnodep/meson.build new file mode 100644 index 000000000..0334f23ca --- /dev/null +++ b/test cases/common/153 recursive linking/stnodep/meson.build @@ -0,0 +1 @@ +stnodep = static_library('stnodep', 'lib.c') diff --git a/test cases/common/153 recursive linking/stshdep/lib.c b/test cases/common/153 recursive linking/stshdep/lib.c new file mode 100644 index 000000000..3cfa12bcb --- /dev/null +++ b/test cases/common/153 recursive linking/stshdep/lib.c @@ -0,0 +1,8 @@ +#include "../lib.h" + +int get_shnodep_value (void); + +SYMBOL_EXPORT +int get_stshdep_value (void) { + return get_shnodep_value (); +} diff --git a/test cases/common/153 recursive linking/stshdep/meson.build b/test cases/common/153 recursive linking/stshdep/meson.build new file mode 100644 index 000000000..7e8e7c82b --- /dev/null +++ b/test cases/common/153 recursive linking/stshdep/meson.build @@ -0,0 +1 @@ +stshdep = static_library('stshdep', 'lib.c', link_with : shnodep) diff --git a/test cases/common/153 recursive linking/ststdep/lib.c b/test cases/common/153 recursive linking/ststdep/lib.c new file mode 100644 index 000000000..fca870669 --- /dev/null +++ b/test cases/common/153 recursive linking/ststdep/lib.c @@ -0,0 +1,8 @@ +#include "../lib.h" + +int get_stnodep_value (void); + +SYMBOL_EXPORT +int get_ststdep_value (void) { + return get_stnodep_value (); +} diff --git a/test cases/common/153 recursive linking/ststdep/meson.build b/test cases/common/153 recursive linking/ststdep/meson.build new file mode 100644 index 000000000..92b0230a7 --- /dev/null +++ b/test cases/common/153 recursive linking/ststdep/meson.build @@ -0,0 +1 @@ +ststdep = static_library('ststdep', 'lib.c', link_with : stnodep) diff --git a/test cases/frameworks/7 gnome/gir/meson.build b/test cases/frameworks/7 gnome/gir/meson.build index f3a453433..27585410c 100644 --- a/test cases/frameworks/7 gnome/gir/meson.build +++ b/test cases/frameworks/7 gnome/gir/meson.build @@ -12,7 +12,7 @@ girlib = shared_library( girexe = executable( 'girprog', sources : 'prog.c', - dependencies : [glib, gobj, gir], + dependencies : [glib, gobj, gir, dep1_dep], link_with : girlib ) diff --git a/test cases/vala/11 generated vapi/meson.build b/test cases/vala/11 generated vapi/meson.build index 82f0c44e5..d5f38cad6 100644 --- a/test cases/vala/11 generated vapi/meson.build +++ b/test cases/vala/11 generated vapi/meson.build @@ -6,7 +6,7 @@ subdir('libbar') vapiexe = executable('vapigen-test', 'main.vala', - dependencies: [dependency('gobject-2.0'), libbar_vapi], + dependencies: [dependency('gobject-2.0'), libfoo_vapi, libbar_vapi], install: true, ) From d2dc38abd45bf7427ef27043847cf9c89513bd1e Mon Sep 17 00:00:00 2001 From: Nirbheek Chauhan Date: Fri, 2 Jun 2017 05:23:47 +0530 Subject: [PATCH 2/4] ninja: Use a set for target deps and ordered deps This significantly reduces the size of build.ninja for GStreamer. --- mesonbuild/backend/ninjabackend.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/mesonbuild/backend/ninjabackend.py b/mesonbuild/backend/ninjabackend.py index 6bd963319..a794ba81d 100644 --- a/mesonbuild/backend/ninjabackend.py +++ b/mesonbuild/backend/ninjabackend.py @@ -58,22 +58,22 @@ class NinjaBuildElement: self.infilenames = [infilenames] else: self.infilenames = infilenames - self.deps = [] - self.orderdeps = [] + self.deps = set() + self.orderdeps = set() self.elems = [] self.all_outputs = all_outputs def add_dep(self, dep): if isinstance(dep, list): - self.deps += dep + self.deps.update(dep) else: - self.deps.append(dep) + self.deps.add(dep) def add_orderdep(self, dep): if isinstance(dep, list): - self.orderdeps += dep + self.orderdeps.update(dep) else: - self.orderdeps.append(dep) + self.orderdeps.add(dep) def add_item(self, name, elems): if isinstance(elems, str): From ae9b23832e3ef4064e5735265ce7008794ab0491 Mon Sep 17 00:00:00 2001 From: Nirbheek Chauhan Date: Fri, 2 Jun 2017 05:25:17 +0530 Subject: [PATCH 3/4] ninja: De-dup libraries and use --start/end-group Now we aggressively de-dup the list of libraries used while linking, and when linking with GNU ld we have to enclose all static libraries with -Wl,--start-group and -Wl,--end-group to force the linker to resolve all symbols recursively. This is needed when static libraries have circular deps on each other (see included test). The --start/end-group change is also needed for circular dependencies between static libraries because we no longer recursively list out all library dependencies. The size of build.ninja for GStreamer is now down to 6.1M from 20M, and yields a net reduction in configuration time of 10% --- mesonbuild/compilers.py | 50 +++++++++++++++++-- run_unittests.py | 15 +++++- .../3rdorderdeps/meson.build | 9 +++- .../153 recursive linking/circular/lib1.c | 6 +++ .../153 recursive linking/circular/lib2.c | 6 +++ .../153 recursive linking/circular/lib3.c | 6 +++ .../153 recursive linking/circular/main.c | 28 +++++++++++ .../circular/meson.build | 5 ++ .../153 recursive linking/circular/prop1.c | 3 ++ .../153 recursive linking/circular/prop2.c | 3 ++ .../153 recursive linking/circular/prop3.c | 3 ++ test cases/common/153 recursive linking/lib.h | 9 +++- .../common/153 recursive linking/main.c | 6 +-- .../common/153 recursive linking/meson.build | 4 ++ .../153 recursive linking/stnodep/meson.build | 3 +- .../153 recursive linking/stshdep/meson.build | 3 +- .../153 recursive linking/ststdep/meson.build | 3 +- 17 files changed, 146 insertions(+), 16 deletions(-) create mode 100644 test cases/common/153 recursive linking/circular/lib1.c create mode 100644 test cases/common/153 recursive linking/circular/lib2.c create mode 100644 test cases/common/153 recursive linking/circular/lib3.c create mode 100644 test cases/common/153 recursive linking/circular/main.c create mode 100644 test cases/common/153 recursive linking/circular/meson.build create mode 100644 test cases/common/153 recursive linking/circular/prop1.c create mode 100644 test cases/common/153 recursive linking/circular/prop2.c create mode 100644 test cases/common/153 recursive linking/circular/prop3.c diff --git a/mesonbuild/compilers.py b/mesonbuild/compilers.py index e7c02b272..d0f33496e 100644 --- a/mesonbuild/compilers.py +++ b/mesonbuild/compilers.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import re import shutil import contextlib import subprocess, os.path @@ -375,10 +376,15 @@ class CompilerArgs(list): # Arg prefixes that override by prepending instead of appending prepend_prefixes = ('-I', '-L') # Arg prefixes and args that must be de-duped by returning 2 - dedup2_prefixes = ('-I', '-L', '-D') + dedup2_prefixes = ('-I', '-L', '-D', '-U') + dedup2_suffixes = () dedup2_args = () # Arg prefixes and args that must be de-duped by returning 1 - dedup1_prefixes = () + dedup1_prefixes = ('-l',) + dedup1_suffixes = ('.lib', '.dll', '.so', '.dylib', '.a') + # Match a .so of the form path/to/libfoo.so.0.1.0 + # Only UNIX shared libraries require this. Others have a fixed extension. + dedup1_regex = re.compile(r'([\/\\]|\A)lib.*\.so(\.[0-9]+)?(\.[0-9]+)?(\.[0-9]+)?$') dedup1_args = ('-c', '-S', '-E', '-pipe', '-pthread') compiler = None @@ -416,7 +422,7 @@ class CompilerArgs(list): def _can_dedup(cls, arg): ''' Returns whether the argument can be safely de-duped. This is dependent - on two things: + on three things: a) Whether an argument can be 'overriden' by a later argument. For example, -DFOO defines FOO and -UFOO undefines FOO. In this case, we @@ -430,10 +436,20 @@ class CompilerArgs(list): a particular argument is present. This can matter for symbol resolution in static or shared libraries, so we cannot de-dup or reorder them. For these we return `0`. This is the default. + + In addition to these, we handle library arguments specially. + With GNU ld, we surround library arguments with -Wl,--start/end-group + to recursively search for symbols in the libraries. This is not needed + with other linkers. ''' - if arg.startswith(cls.dedup2_prefixes) or arg in cls.dedup2_args: + if arg in cls.dedup2_args or \ + arg.startswith(cls.dedup2_prefixes) or \ + arg.endswith(cls.dedup2_suffixes): return 2 - if arg.startswith(cls.dedup1_prefixes) or arg in cls.dedup1_args: + if arg in cls.dedup1_args or \ + arg.startswith(cls.dedup1_prefixes) or \ + arg.endswith(cls.dedup1_suffixes) or \ + re.search(cls.dedup1_regex, arg): return 1 return 0 @@ -444,6 +460,21 @@ class CompilerArgs(list): return False def to_native(self): + # Check if we need to add --start/end-group for circular dependencies + # between static libraries. + if get_compiler_uses_gnuld(self.compiler): + group_started = False + for each in self: + if not each.startswith('-l') and not each.endswith('.a'): + continue + i = self.index(each) + if not group_started: + # First occurance of a library + self.insert(i, '-Wl,--start-group') + group_started = True + # Last occurance of a library + if group_started: + self.insert(i + 1, '-Wl,--end-group') return self.compiler.unix_args_to_native(self) def __add__(self, args): @@ -2402,6 +2433,15 @@ def get_compiler_is_linuxlike(compiler): return True return False +def get_compiler_uses_gnuld(c): + # FIXME: Perhaps we should detect the linker in the environment? + # FIXME: Assumes that *BSD use GNU ld, but they might start using lld soon + if (getattr(c, 'gcc_type', None) in (GCC_STANDARD, GCC_MINGW, GCC_CYGWIN)) or \ + (getattr(c, 'clang_type', None) in (CLANG_STANDARD, CLANG_WIN)) or \ + (getattr(c, 'icc_type', None) in (ICC_STANDARD, ICC_WIN)): + return True + return False + def get_largefile_args(compiler): ''' Enable transparent large-file-support for 32-bit UNIX systems diff --git a/run_unittests.py b/run_unittests.py index e3b7c5cc3..71448f941 100755 --- a/run_unittests.py +++ b/run_unittests.py @@ -139,7 +139,7 @@ class InternalTests(unittest.TestCase): self.assertEqual(a, ['-Ibar', '-Ifoo', '-Ibaz', '-I..', '-I.', '-O3', '-O2', '-Wall']) ## Test that reflected addition works - # Test that adding to a list with just one old arg works and DOES NOT yield the same array + # Test that adding to a list with just one old arg works and yields the same array a = ['-Ifoo'] + a self.assertEqual(a, ['-Ibar', '-Ifoo', '-Ibaz', '-I..', '-I.', '-O3', '-O2', '-Wall']) # Test that adding to a list with just one new arg that is not pre-pended works @@ -148,6 +148,19 @@ class InternalTests(unittest.TestCase): # Test that adding to a list with two new args preserves the order a = ['-Ldir', '-Lbah'] + a self.assertEqual(a, ['-Ibar', '-Ifoo', '-Ibaz', '-I..', '-I.', '-Ldir', '-Lbah', '-Werror', '-O3', '-O2', '-Wall']) + # Test that adding to a list with old args does nothing + a = ['-Ibar', '-Ibaz', '-Ifoo'] + a + self.assertEqual(a, ['-Ibar', '-Ifoo', '-Ibaz', '-I..', '-I.', '-Ldir', '-Lbah', '-Werror', '-O3', '-O2', '-Wall']) + + ## Test that adding libraries works + l = cargsfunc(c, ['-Lfoodir', '-lfoo']) + self.assertEqual(l, ['-Lfoodir', '-lfoo']) + # Adding a library and a libpath appends both correctly + l += ['-Lbardir', '-lbar'] + self.assertEqual(l, ['-Lbardir', '-Lfoodir', '-lfoo', '-lbar']) + # Adding the same library again does nothing + l += ['-lbar'] + self.assertEqual(l, ['-Lbardir', '-Lfoodir', '-lfoo', '-lbar']) def test_commonpath(self): from os.path import sep diff --git a/test cases/common/153 recursive linking/3rdorderdeps/meson.build b/test cases/common/153 recursive linking/3rdorderdeps/meson.build index a2907f383..d4ef7453d 100644 --- a/test cases/common/153 recursive linking/3rdorderdeps/meson.build +++ b/test cases/common/153 recursive linking/3rdorderdeps/meson.build @@ -16,8 +16,10 @@ foreach dep2 : ['sh', 'st'] if libtype == 'sh' target = 'shared_library' + build_args = [] elif libtype == 'st' target = 'static_library' + build_args = ['-DMESON_STATIC_BUILD'] else error('Unknown libtype "@0@"'.format(libtype)) endif @@ -31,13 +33,16 @@ foreach dep2 : ['sh', 'st'] output : name + '-lib.c', configuration : cdata) dep = get_variable(dep1 + dep2 + 'dep') - dep3_lib = build_target(name, lib_c, link_with : dep, target_type : target) + dep3_lib = build_target(name, lib_c, link_with : dep, + target_type : target, + c_args : build_args) dep3_libs += [dep3_lib] main_c = configure_file(input : 'main.c.in', output : name + '-main.c', configuration : cdata) - dep3_bin = executable(name, main_c, link_with : dep3_lib) + dep3_bin = executable(name, main_c, link_with : dep3_lib, + c_args : build_args) test(name + 'test', dep3_bin) endforeach endforeach diff --git a/test cases/common/153 recursive linking/circular/lib1.c b/test cases/common/153 recursive linking/circular/lib1.c new file mode 100644 index 000000000..38889cfa0 --- /dev/null +++ b/test cases/common/153 recursive linking/circular/lib1.c @@ -0,0 +1,6 @@ +int get_st2_prop (void); +int get_st3_prop (void); + +int get_st1_value (void) { + return get_st2_prop () + get_st3_prop (); +} diff --git a/test cases/common/153 recursive linking/circular/lib2.c b/test cases/common/153 recursive linking/circular/lib2.c new file mode 100644 index 000000000..31cd37cc1 --- /dev/null +++ b/test cases/common/153 recursive linking/circular/lib2.c @@ -0,0 +1,6 @@ +int get_st1_prop (void); +int get_st3_prop (void); + +int get_st2_value (void) { + return get_st1_prop () + get_st3_prop (); +} diff --git a/test cases/common/153 recursive linking/circular/lib3.c b/test cases/common/153 recursive linking/circular/lib3.c new file mode 100644 index 000000000..67d473aac --- /dev/null +++ b/test cases/common/153 recursive linking/circular/lib3.c @@ -0,0 +1,6 @@ +int get_st1_prop (void); +int get_st2_prop (void); + +int get_st3_value (void) { + return get_st1_prop () + get_st2_prop (); +} diff --git a/test cases/common/153 recursive linking/circular/main.c b/test cases/common/153 recursive linking/circular/main.c new file mode 100644 index 000000000..5f797a5ff --- /dev/null +++ b/test cases/common/153 recursive linking/circular/main.c @@ -0,0 +1,28 @@ +#include + +#include "../lib.h" + +int get_st1_value (void); +int get_st2_value (void); +int get_st3_value (void); + +int main(int argc, char *argv[]) { + int val; + + val = get_st1_value (); + if (val != 5) { + printf("st1 value was %i instead of 5\n", val); + return -1; + } + val = get_st2_value (); + if (val != 4) { + printf("st2 value was %i instead of 4\n", val); + return -2; + } + val = get_st3_value (); + if (val != 3) { + printf("st3 value was %i instead of 3\n", val); + return -3; + } + return 0; +} diff --git a/test cases/common/153 recursive linking/circular/meson.build b/test cases/common/153 recursive linking/circular/meson.build new file mode 100644 index 000000000..b7a70a86b --- /dev/null +++ b/test cases/common/153 recursive linking/circular/meson.build @@ -0,0 +1,5 @@ +st1 = static_library('st1', 'lib1.c', 'prop1.c') +st2 = static_library('st2', 'lib2.c', 'prop2.c') +st3 = static_library('st3', 'lib3.c', 'prop3.c') + +test('circular', executable('circular', 'main.c', link_with : [st1, st2, st3])) diff --git a/test cases/common/153 recursive linking/circular/prop1.c b/test cases/common/153 recursive linking/circular/prop1.c new file mode 100644 index 000000000..4e571f5ee --- /dev/null +++ b/test cases/common/153 recursive linking/circular/prop1.c @@ -0,0 +1,3 @@ +int get_st1_prop (void) { + return 1; +} diff --git a/test cases/common/153 recursive linking/circular/prop2.c b/test cases/common/153 recursive linking/circular/prop2.c new file mode 100644 index 000000000..ceabba055 --- /dev/null +++ b/test cases/common/153 recursive linking/circular/prop2.c @@ -0,0 +1,3 @@ +int get_st2_prop (void) { + return 2; +} diff --git a/test cases/common/153 recursive linking/circular/prop3.c b/test cases/common/153 recursive linking/circular/prop3.c new file mode 100644 index 000000000..246206c2c --- /dev/null +++ b/test cases/common/153 recursive linking/circular/prop3.c @@ -0,0 +1,3 @@ +int get_st3_prop (void) { + return 3; +} diff --git a/test cases/common/153 recursive linking/lib.h b/test cases/common/153 recursive linking/lib.h index b6b75fa05..b54bf3673 100644 --- a/test cases/common/153 recursive linking/lib.h +++ b/test cases/common/153 recursive linking/lib.h @@ -1,6 +1,11 @@ #if defined _WIN32 - #define SYMBOL_IMPORT __declspec(dllimport) - #define SYMBOL_EXPORT __declspec(dllexport) + #ifdef MESON_STATIC_BUILD + #define SYMBOL_EXPORT + #define SYMBOL_IMPORT + #else + #define SYMBOL_IMPORT __declspec(dllimport) + #define SYMBOL_EXPORT __declspec(dllexport) + #endif #else #define SYMBOL_IMPORT #if defined __GNUC__ diff --git a/test cases/common/153 recursive linking/main.c b/test cases/common/153 recursive linking/main.c index f1219a649..085161115 100644 --- a/test cases/common/153 recursive linking/main.c +++ b/test cases/common/153 recursive linking/main.c @@ -2,12 +2,12 @@ #include "lib.h" -SYMBOL_IMPORT int get_stnodep_value (void); +int get_stnodep_value (void); +int get_stshdep_value (void); +int get_ststdep_value (void); SYMBOL_IMPORT int get_shnodep_value (void); SYMBOL_IMPORT int get_shshdep_value (void); SYMBOL_IMPORT int get_shstdep_value (void); -SYMBOL_IMPORT int get_stshdep_value (void); -SYMBOL_IMPORT int get_ststdep_value (void); int main(int argc, char *argv[]) { int val; diff --git a/test cases/common/153 recursive linking/meson.build b/test cases/common/153 recursive linking/meson.build index f17fc4a6e..4cecd5727 100644 --- a/test cases/common/153 recursive linking/meson.build +++ b/test cases/common/153 recursive linking/meson.build @@ -20,3 +20,7 @@ test('alldeps', # More combinations of static and shared libraries subdir('3rdorderdeps') + +# Circular dependencies between static libraries +# This requires the use of --start/end-group with GNU ld +subdir('circular') diff --git a/test cases/common/153 recursive linking/stnodep/meson.build b/test cases/common/153 recursive linking/stnodep/meson.build index 0334f23ca..77f7129b6 100644 --- a/test cases/common/153 recursive linking/stnodep/meson.build +++ b/test cases/common/153 recursive linking/stnodep/meson.build @@ -1 +1,2 @@ -stnodep = static_library('stnodep', 'lib.c') +stnodep = static_library('stnodep', 'lib.c', + c_args : '-DMESON_STATIC_BUILD') diff --git a/test cases/common/153 recursive linking/stshdep/meson.build b/test cases/common/153 recursive linking/stshdep/meson.build index 7e8e7c82b..0967c1ce4 100644 --- a/test cases/common/153 recursive linking/stshdep/meson.build +++ b/test cases/common/153 recursive linking/stshdep/meson.build @@ -1 +1,2 @@ -stshdep = static_library('stshdep', 'lib.c', link_with : shnodep) +stshdep = static_library('stshdep', 'lib.c', link_with : shnodep, + c_args : '-DMESON_STATIC_BUILD') diff --git a/test cases/common/153 recursive linking/ststdep/meson.build b/test cases/common/153 recursive linking/ststdep/meson.build index 92b0230a7..3602442aa 100644 --- a/test cases/common/153 recursive linking/ststdep/meson.build +++ b/test cases/common/153 recursive linking/ststdep/meson.build @@ -1 +1,2 @@ -ststdep = static_library('ststdep', 'lib.c', link_with : stnodep) +ststdep = static_library('ststdep', 'lib.c', link_with : stnodep, + c_args : '-DMESON_STATIC_BUILD') From 4b428053f496720ec437eb5d455c86ada2de7977 Mon Sep 17 00:00:00 2001 From: Nirbheek Chauhan Date: Fri, 2 Jun 2017 10:34:19 +0530 Subject: [PATCH 4/4] ninja: Use shlex.quote for quoting on non-Windows This is more reliable, and more accurate. For instance, this means arguments in commands aren't surrounded by `'` on Linux unless that is actually needed by that specific argument. There is no equivalent helper for Windows, so we keep the old behaviour for that. --- mesonbuild/backend/ninjabackend.py | 46 +++++++++++++++--------------- run_unittests.py | 16 +++++------ 2 files changed, 31 insertions(+), 31 deletions(-) diff --git a/mesonbuild/backend/ninjabackend.py b/mesonbuild/backend/ninjabackend.py index a794ba81d..29179aa2b 100644 --- a/mesonbuild/backend/ninjabackend.py +++ b/mesonbuild/backend/ninjabackend.py @@ -12,6 +12,11 @@ # See the License for the specific language governing permissions and # limitations under the License. +import shlex +import os, sys, pickle, re +import subprocess, shutil +from collections import OrderedDict + from . import backends from .. import modules from .. import environment, mesonlib @@ -24,16 +29,13 @@ from ..mesonlib import File, MesonException, OrderedSet from ..mesonlib import get_meson_script, get_compiler_for_source from .backends import CleanTrees, InstallData from ..build import InvalidArguments -import os, sys, pickle, re -import subprocess, shutil -from collections import OrderedDict if mesonlib.is_windows(): - quote_char = '"' + quote_func = lambda s: '"{}"'.format(s) execute_wrapper = 'cmd /c' rmfile_prefix = 'del /f /s /q {} &&' else: - quote_char = "'" + quote_func = shlex.quote execute_wrapper = '' rmfile_prefix = 'rm -f {} &&' @@ -105,18 +107,17 @@ class NinjaBuildElement: (name, elems) = e should_quote = name not in raw_names line = ' %s = ' % name - q_templ = quote_char + "%s" + quote_char noq_templ = "%s" newelems = [] for i in elems: if not should_quote or i == '&&': # Hackety hack hack - templ = noq_templ + quoter = ninja_quote else: - templ = q_templ + quoter = lambda x: ninja_quote(quote_func(x)) i = i.replace('\\', '\\\\') - if quote_char == '"': + if quote_func('') == '""': i = i.replace('"', '\\"') - newelems.append(templ % ninja_quote(i)) + newelems.append(quoter(i)) line += ' '.join(newelems) line += '\n' outfile.write(line) @@ -854,12 +855,12 @@ int dummy; outfile.write(' depfile = $DEPFILE\n') outfile.write(' restat = 1\n\n') outfile.write('rule REGENERATE_BUILD\n') - c = (quote_char + ninja_quote(sys.executable) + quote_char, - quote_char + ninja_quote(self.environment.get_build_command()) + quote_char, + c = (ninja_quote(quote_func(sys.executable)), + ninja_quote(quote_func(self.environment.get_build_command())), '--internal', 'regenerate', - quote_char + ninja_quote(self.environment.get_source_dir()) + quote_char, - quote_char + ninja_quote(self.environment.get_build_dir()) + quote_char) + ninja_quote(quote_func(self.environment.get_source_dir())), + ninja_quote(quote_func(self.environment.get_build_dir()))) outfile.write(" command = %s %s %s %s %s %s --backend ninja\n" % c) outfile.write(' description = Regenerating build files.\n') outfile.write(' generator = 1\n\n') @@ -1515,7 +1516,7 @@ rule FORTRAN_DEP_HACK pass return [] - def generate_compile_rule_for(self, langname, compiler, qstr, is_cross, outfile): + def generate_compile_rule_for(self, langname, compiler, is_cross, outfile): if langname == 'java': if not is_cross: self.generate_java_compile_rule(compiler, outfile) @@ -1547,7 +1548,7 @@ rule FORTRAN_DEP_HACK quoted_depargs = [] for d in depargs: if d != '$out' and d != '$in': - d = qstr % d + d = quote_func(d) quoted_depargs.append(d) cross_args = self.get_cross_info_lang_args(langname, is_cross) if mesonlib.is_windows(): @@ -1576,7 +1577,7 @@ rule FORTRAN_DEP_HACK outfile.write(description) outfile.write('\n') - def generate_pch_rule_for(self, langname, compiler, qstr, is_cross, outfile): + def generate_pch_rule_for(self, langname, compiler, is_cross, outfile): if langname != 'c' and langname != 'cpp': return if is_cross: @@ -1595,7 +1596,7 @@ rule FORTRAN_DEP_HACK quoted_depargs = [] for d in depargs: if d != '$out' and d != '$in': - d = qstr % d + d = quote_func(d) quoted_depargs.append(d) if compiler.get_id() == 'msvc': output = '' @@ -1621,12 +1622,11 @@ rule FORTRAN_DEP_HACK outfile.write('\n') def generate_compile_rules(self, outfile): - qstr = quote_char + "%s" + quote_char for langname, compiler in self.build.compilers.items(): if compiler.get_id() == 'clang': self.generate_llvm_ir_compile_rule(compiler, False, outfile) - self.generate_compile_rule_for(langname, compiler, qstr, False, outfile) - self.generate_pch_rule_for(langname, compiler, qstr, False, outfile) + self.generate_compile_rule_for(langname, compiler, False, outfile) + self.generate_pch_rule_for(langname, compiler, False, outfile) if self.environment.is_cross_build(): # In case we are going a target-only build, make the native compilers # masquerade as cross compilers. @@ -1637,8 +1637,8 @@ rule FORTRAN_DEP_HACK for langname, compiler in cclist.items(): if compiler.get_id() == 'clang': self.generate_llvm_ir_compile_rule(compiler, True, outfile) - self.generate_compile_rule_for(langname, compiler, qstr, True, outfile) - self.generate_pch_rule_for(langname, compiler, qstr, True, outfile) + self.generate_compile_rule_for(langname, compiler, True, outfile) + self.generate_pch_rule_for(langname, compiler, True, outfile) outfile.write('\n') def generate_generator_list_rules(self, target, outfile): diff --git a/run_unittests.py b/run_unittests.py index 71448f941..a610e6b85 100755 --- a/run_unittests.py +++ b/run_unittests.py @@ -1266,15 +1266,15 @@ class LinuxlikeTests(BasePlatformTests): self.assertIsNotNone(vala_command) self.assertIsNotNone(c_command) # -w suppresses all warnings, should be there in Vala but not in C - self.assertIn("'-w'", vala_command) - self.assertNotIn("'-w'", c_command) + self.assertIn(" -w ", vala_command) + self.assertNotIn(" -w ", c_command) # -Wall enables all warnings, should be there in C but not in Vala - self.assertNotIn("'-Wall'", vala_command) - self.assertIn("'-Wall'", c_command) + self.assertNotIn(" -Wall ", vala_command) + self.assertIn(" -Wall ", c_command) # -Werror converts warnings to errors, should always be there since it's # injected by an unrelated piece of code and the project has werror=true - self.assertIn("'-Werror'", vala_command) - self.assertIn("'-Werror'", c_command) + self.assertIn(" -Werror ", vala_command) + self.assertIn(" -Werror ", c_command) def test_qt5dependency_pkgconfig_detection(self): ''' @@ -1405,7 +1405,7 @@ class LinuxlikeTests(BasePlatformTests): self.init(testdir, ['-D' + std_opt]) cmd = self.get_compdb()[0]['command'] if v != 'none': - cmd_std = "'-std={}'".format(v) + cmd_std = " -std={} ".format(v) self.assertIn(cmd_std, cmd) try: self.build() @@ -1420,7 +1420,7 @@ class LinuxlikeTests(BasePlatformTests): os.environ[env_flags] = cmd_std self.init(testdir) cmd = self.get_compdb()[0]['command'] - qcmd_std = "'{}'".format(cmd_std) + qcmd_std = " {} ".format(cmd_std) self.assertIn(qcmd_std, cmd) with self.assertRaises(subprocess.CalledProcessError, msg='{} should have failed'.format(qcmd_std)):