From 2d370c24ec83de889c83511c4a32e52e75a38aca Mon Sep 17 00:00:00 2001 From: Nirbheek Chauhan Date: Wed, 25 Jul 2018 12:59:41 +0530 Subject: [PATCH] Fix linking of shared/static libs with static libs This commit contains the following fixes: 1. When a shared library A does `link_with:` to static library B, the parts of B used by A will be added to A, and so we don't need to return B in A.get_dependencies() for targets that link to A. This already is the behaviour when a shared library A does `link_whole:` on B. 2. In situation (1), when generating a pkg-config file for A, we must also not add B to Libs.private for A. This already is the behaviour when a shared library A does `link_whole:` on B. 3. When a static library A does `link_whole:` to static library B, we must add the objects in B to A. 4. When a static library A does `link_with:` to static library B, and B is not installed (which makes it an internal static library), we must add the objects in B to A, otherwise nothing can use A. 5. In situation (4), when generating a pkg-config file for A, we must also not add B to Libs.private for A. All these situations are tested by the unit test added in this commit. Closes https://github.com/mesonbuild/meson/issues/3934 Closes https://github.com/mesonbuild/meson/issues/3937 --- mesonbuild/backend/ninjabackend.py | 2 +- mesonbuild/backend/vs2010backend.py | 2 +- mesonbuild/build.py | 61 ++++++++++--- run_unittests.py | 85 ++++++++++++++++++- .../common/143 C and CPP link/meson.build | 4 +- .../consumer/meson.build | 11 +++ .../consumer/tester.c | 17 ++++ .../35 both library usability/provider/both.c | 20 +++++ .../provider/meson.build | 36 ++++++++ .../provider/otherlib/installed.c | 7 ++ .../provider/otherlib/internal.c | 7 ++ .../provider/otherlib/meson.build | 3 + .../provider/tester.c | 14 +++ 13 files changed, 253 insertions(+), 16 deletions(-) create mode 100644 test cases/unit/35 both library usability/consumer/meson.build create mode 100644 test cases/unit/35 both library usability/consumer/tester.c create mode 100644 test cases/unit/35 both library usability/provider/both.c create mode 100644 test cases/unit/35 both library usability/provider/meson.build create mode 100644 test cases/unit/35 both library usability/provider/otherlib/installed.c create mode 100644 test cases/unit/35 both library usability/provider/otherlib/internal.c create mode 100644 test cases/unit/35 both library usability/provider/otherlib/meson.build create mode 100644 test cases/unit/35 both library usability/provider/tester.c diff --git a/mesonbuild/backend/ninjabackend.py b/mesonbuild/backend/ninjabackend.py index 6b2a00a19040..80edce5d0695 100644 --- a/mesonbuild/backend/ninjabackend.py +++ b/mesonbuild/backend/ninjabackend.py @@ -2412,7 +2412,7 @@ def generate_link(self, target, outfile, outname, obj_list, linker, extra_args=[ # line where the static library is used. dependencies = [] else: - dependencies = target.get_dependencies() + dependencies = target.get_dependencies(link_whole=True) internal = self.build_target_link_arguments(linker, dependencies) commands += internal # Only non-static built targets need link args and link dependencies diff --git a/mesonbuild/backend/vs2010backend.py b/mesonbuild/backend/vs2010backend.py index 2e86ca9aa2ac..09b074c60b4f 100644 --- a/mesonbuild/backend/vs2010backend.py +++ b/mesonbuild/backend/vs2010backend.py @@ -1032,7 +1032,7 @@ def gen_vcxproj(self, target, ofname, guid): (additional_libpaths, additional_links, extra_link_args) = self.split_link_args(extra_link_args.to_native()) # Add more libraries to be linked if needed - for t in target.get_dependencies(): + for t in target.get_dependencies(link_whole=True): lobj = self.build.targets[t.get_id()] linkname = os.path.join(down, self.get_target_filename_for_linking(lobj)) if t in target.link_whole_targets: diff --git a/mesonbuild/build.py b/mesonbuild/build.py index ec6e1e656f9e..c26db764abd6 100644 --- a/mesonbuild/build.py +++ b/mesonbuild/build.py @@ -856,22 +856,43 @@ def get_outputs(self): def get_extra_args(self, language): return self.extra_args.get(language, []) - def get_dependencies(self, exclude=None, internal=True): + def is_internal(self): + if isinstance(self, StaticLibrary) and not self.need_install: + return True + return False + + def get_dependencies(self, exclude=None, internal=True, link_whole=False): transitive_deps = [] if exclude is None: exclude = [] - if internal: - link_targets = itertools.chain(self.link_targets, self.link_whole_targets) - else: - # We don't want the 'internal' libraries when generating the - # `Libs:` and `Libs.private:` lists in pkg-config files. - link_targets = self.link_targets - for t in link_targets: + for t in self.link_targets: if t in transitive_deps or t in exclude: continue + # When we don't want internal libraries, f.ex. when we're + # generating the list of private installed libraries for use in + # a pkg-config file, don't include static libraries that aren't + # installed because those get directly included in the static + # or shared library already. See: self.link() + if not internal and t.is_internal(): + continue transitive_deps.append(t) if isinstance(t, StaticLibrary): - transitive_deps += t.get_dependencies(transitive_deps + exclude, internal) + transitive_deps += t.get_dependencies(transitive_deps + exclude, + internal, link_whole) + for t in self.link_whole_targets: + if t in transitive_deps or t in exclude: + continue + if not internal and t.is_internal(): + continue + # self.link_whole_targets are not included by default here because + # the objects from those will already be in the library. They are + # only needed while generating backend (ninja) target dependencies. + if link_whole: + transitive_deps.append(t) + # However, the transitive dependencies are still needed + if isinstance(t, StaticLibrary): + transitive_deps += t.get_dependencies(transitive_deps + exclude, + internal, link_whole) return transitive_deps def get_source_subdir(self): @@ -958,7 +979,17 @@ def link(self, target): raise InvalidArguments(msg) if self.is_cross != t.is_cross: raise InvalidArguments('Tried to mix cross built and native libraries in target {!r}'.format(self.name)) - self.link_targets.append(t) + # When linking to a static library that's not installed, we + # transparently add that target's objects to ourselves. + # Static libraries that are installed will either be linked through + # self.link_targets or using the pkg-config file. + if isinstance(self, StaticLibrary) and isinstance(t, StaticLibrary) and not t.need_install: + self.objects.append(t.extract_all_objects()) + # Add internal and external deps + self.external_deps += t.external_deps + self.link_targets += t.link_targets + else: + self.link_targets.append(t) def link_whole(self, target): for t in listify(target, unholder=True): @@ -970,7 +1001,15 @@ def link_whole(self, target): raise InvalidArguments(msg) if self.is_cross != t.is_cross: raise InvalidArguments('Tried to mix cross built and native libraries in target {!r}'.format(self.name)) - self.link_whole_targets.append(t) + # When we're a static library and we link_whole: to another static + # library, we need to add that target's objects to ourselves. + if isinstance(self, StaticLibrary): + self.objects.append(t.extract_all_objects()) + # Add internal and external deps + self.external_deps += t.external_deps + self.link_targets += t.link_targets + else: + self.link_whole_targets.append(t) def add_pch(self, language, pchlist): if not pchlist: diff --git a/run_unittests.py b/run_unittests.py index 96802cc8cc9a..87518a8e90e6 100755 --- a/run_unittests.py +++ b/run_unittests.py @@ -96,6 +96,14 @@ def _git_init(project_dir): subprocess.check_call(['git', 'commit', '-a', '-m', 'I am a project'], cwd=project_dir, stdout=subprocess.DEVNULL) +def can_use_pkgconfig(): + # CI provides pkg-config, and we should fail the test if it isn't found + if is_ci(): + return True + if shutil.which('pkg-config'): + return True + return False + def skipIfNoPkgconfig(f): ''' Skip this test if no pkg-config is found, unless we're on Travis or @@ -106,7 +114,7 @@ def skipIfNoPkgconfig(f): Note: Yes, we provide pkg-config even while running Windows CI ''' def wrapped(*args, **kwargs): - if not is_ci() and shutil.which('pkg-config') is None: + if not can_use_pkgconfig(): raise unittest.SkipTest('pkg-config not found') return f(*args, **kwargs) return wrapped @@ -2744,6 +2752,81 @@ def test_buildtype_setting(self): self.assertEqual(opts['debug'], True) self.assertEqual(opts['optimization'], '0') + def test_static_and_shared_library_usability(self): + ''' + Test that static and shared libraries with various kinds of static + library internal dependencies are usable after installation, and that + the pkg-config files generated for such libraries have the correct + Libs: and Libs.private: lines. + ''' + env = get_fake_env('', self.builddir, self.prefix) + cc = env.detect_c_compiler(False) + if cc.get_id() == 'msvc': + static_args = '-DPROVIDER_STATIC' + # FIXME: Can't reliably test mixed shared/static because of + # __declspec linkage issues and because it will greatly complicate + # the build files. Waiting for static_c_args support. + libtypes = ('static',) + else: + static_args = '' + libtypes = ('static', 'shared', 'both') + # Test + for libtype in libtypes: + oldprefix = self.prefix + # Install external library so we can find it + testdir = os.path.join(self.unit_test_dir, '35 both library usability', 'provider') + # install into installdir without using DESTDIR + installdir = self.installdir + self.prefix = installdir + if libtype == 'static': + c_args = static_args + else: + c_args = '' + self.init(testdir, extra_args=['--default-library=' + libtype, '-Dc_args=' + c_args]) + self.prefix = oldprefix + for each in ('whole-installed', 'whole-internal', 'with-installed', 'with-internal'): + pc = os.path.join(self.privatedir, '{}.pc'.format(each)) + with open(pc, 'r') as f: + for l in f: + l = l.strip() + if l.startswith('Libs:'): + if libtype == 'static' and each == 'with-installed': + self.assertEqual(l, 'Libs: -L${libdir} -linstalled-some -l' + each) + else: + self.assertEqual(l, 'Libs: -L${libdir} -l' + each) + if l.startswith('Libs.private:'): + if each == 'with-installed': + self.assertEqual(l, 'Libs.private: -L${libdir} -linstalled-some') + else: + self.assertNotIn('internal-some', l) + self.build() + self.run_tests() + # Rest of the test requires pkg-config + if not can_use_pkgconfig(): + ## New builddir for the next iteration + self.new_builddir() + continue + self.install(use_destdir=False) + if is_windows() or is_cygwin(): + os.environ['PATH'] += os.pathsep + os.path.join(installdir, 'bin') + os.environ['PKG_CONFIG_PATH'] = os.path.join(installdir, self.libdir, 'pkgconfig') + testdir = os.path.join(self.unit_test_dir, '35 both library usability', 'consumer') + for _libtype in libtypes: + if _libtype == 'static': + _c_args = static_args + else: + _c_args = '' + ## New builddir for the consumer + self.new_builddir() + self.init(testdir, extra_args=['--default-library=' + _libtype, '-Dc_args=' + _c_args]) + self.build() + self.run_tests() + ## New builddir for the next iteration + self.new_builddir() + # Deliver a skip status to signal incomplete test + if not can_use_pkgconfig(): + raise unittest.SkipTest('pkg-config not found, test incomplete') + class FailureTests(BasePlatformTests): ''' diff --git a/test cases/common/143 C and CPP link/meson.build b/test cases/common/143 C and CPP link/meson.build index 55c1b87a507e..519fe22cd30c 100644 --- a/test cases/common/143 C and CPP link/meson.build +++ b/test cases/common/143 C and CPP link/meson.build @@ -15,8 +15,8 @@ project('C and C++ static link test', ['c', 'cpp']) # Verify that adding link arguments works. -add_global_link_arguments('', language : 'c') -add_project_link_arguments('', language : 'c') +add_global_link_arguments('-DMESON_UNUSED', language : 'c') +add_project_link_arguments('-DMESON_UNUSED', language : 'c') libc = static_library('cfoo', ['foo.c', 'foo.h']) diff --git a/test cases/unit/35 both library usability/consumer/meson.build b/test cases/unit/35 both library usability/consumer/meson.build new file mode 100644 index 000000000000..2d8d2587cbfa --- /dev/null +++ b/test cases/unit/35 both library usability/consumer/meson.build @@ -0,0 +1,11 @@ +project('both libraries consumer', 'c') + +d1 = dependency('whole-installed') +d2 = dependency('whole-internal') +d3 = dependency('with-installed') +d4 = dependency('with-internal') + +test('both-whole-installed', executable('tester1', 'tester.c', dependencies : d1)) +test('both-whole-internal', executable('tester2', 'tester.c', dependencies : d2)) +test('both-with-installed', executable('tester3', 'tester.c', dependencies : d3)) +test('both-with-internal', executable('tester4', 'tester.c', dependencies : d4)) diff --git a/test cases/unit/35 both library usability/consumer/tester.c b/test cases/unit/35 both library usability/consumer/tester.c new file mode 100644 index 000000000000..9a2538b31141 --- /dev/null +++ b/test cases/unit/35 both library usability/consumer/tester.c @@ -0,0 +1,17 @@ +#include + +#if defined(_MSC_VER) && !defined(PROVIDER_STATIC) +__declspec(dllimport) +#endif +int both_get_dat_value (void); + +int main (int argc, char *argv[]) +{ + int got = both_get_dat_value (); + + if (got != 111) { + printf ("Got %i instead of 111\n", got); + return 2; + } + return 0; +} diff --git a/test cases/unit/35 both library usability/provider/both.c b/test cases/unit/35 both library usability/provider/both.c new file mode 100644 index 000000000000..db6015140186 --- /dev/null +++ b/test cases/unit/35 both library usability/provider/both.c @@ -0,0 +1,20 @@ +#if defined(_MSC_VER) && !defined(PROVIDER_STATIC) +__declspec(dllimport) +#endif +int get_dat_value (void); + +#ifdef INSTALLED_LIBRARY + #define EXPECTED_VALUE 69 +#else + #define EXPECTED_VALUE 42 +#endif + +#if defined(_MSC_VER) && !defined(PROVIDER_STATIC) +__declspec(dllexport) +#endif +int both_get_dat_value (void) +{ + if (get_dat_value () != EXPECTED_VALUE) + return 666; + return 111; +} diff --git a/test cases/unit/35 both library usability/provider/meson.build b/test cases/unit/35 both library usability/provider/meson.build new file mode 100644 index 000000000000..b7f87d782628 --- /dev/null +++ b/test cases/unit/35 both library usability/provider/meson.build @@ -0,0 +1,36 @@ +project('both library provider', 'c') + +pkg = import('pkgconfig') + +subdir('otherlib') + +# Both libraries with a link_whole dependency on an installed static library +l1 = library('whole-installed', 'both.c', + c_args : ['-DINSTALLED_LIBRARY'], + link_whole : installed_lib, + install: true) +pkg.generate(l1) + +# Both libraries with a link_whole dependency on a not-installed static library +l2 = library('whole-internal', 'both.c', + link_whole : internal_lib, + install: true) +pkg.generate(l2) + +# Both libraries with a link_with dependency on an installed static library +l3 = library('with-installed', 'both.c', + c_args : ['-DINSTALLED_LIBRARY'], + link_with : installed_lib, + install: true) +pkg.generate(l3) + +# Both libraries with a link_with dependency on a not-installed static library +l4 = library('with-internal', 'both.c', + link_with : internal_lib, + install: true) +pkg.generate(l4) + +test('test-both-whole-installed', executable('tester1', 'tester.c', link_with : l1)) +test('test-both-whole-internal', executable('tester2', 'tester.c', link_with : l2)) +test('test-both-with-installed', executable('tester3', 'tester.c', link_with : l3)) +test('test-both-with-internal', executable('tester4', 'tester.c', link_with : l4)) diff --git a/test cases/unit/35 both library usability/provider/otherlib/installed.c b/test cases/unit/35 both library usability/provider/otherlib/installed.c new file mode 100644 index 000000000000..08cfcb125416 --- /dev/null +++ b/test cases/unit/35 both library usability/provider/otherlib/installed.c @@ -0,0 +1,7 @@ +#if defined(_MSC_VER) && !defined(PROVIDER_STATIC) +__declspec(dllexport) +#endif +int get_dat_value (void) +{ + return 69; +} diff --git a/test cases/unit/35 both library usability/provider/otherlib/internal.c b/test cases/unit/35 both library usability/provider/otherlib/internal.c new file mode 100644 index 000000000000..c70fd9807986 --- /dev/null +++ b/test cases/unit/35 both library usability/provider/otherlib/internal.c @@ -0,0 +1,7 @@ +#if defined(_MSC_VER) && !defined(PROVIDER_STATIC) +__declspec(dllexport) +#endif +int get_dat_value (void) +{ + return 42; +} diff --git a/test cases/unit/35 both library usability/provider/otherlib/meson.build b/test cases/unit/35 both library usability/provider/otherlib/meson.build new file mode 100644 index 000000000000..2f2cf678ce22 --- /dev/null +++ b/test cases/unit/35 both library usability/provider/otherlib/meson.build @@ -0,0 +1,3 @@ +internal_lib = static_library('internal-some', 'internal.c') + +installed_lib = static_library('installed-some', 'installed.c', install: true) diff --git a/test cases/unit/35 both library usability/provider/tester.c b/test cases/unit/35 both library usability/provider/tester.c new file mode 100644 index 000000000000..5946099e2d8a --- /dev/null +++ b/test cases/unit/35 both library usability/provider/tester.c @@ -0,0 +1,14 @@ +#include + +int both_get_dat_value (void); + +int main (int argc, char *argv[]) +{ + int got = both_get_dat_value (); + + if (got != 111) { + printf ("Got %i instead of 111\n", got); + return 2; + } + return 0; +}