From 40c197d524181713968ac5a1f052671744915c62 Mon Sep 17 00:00:00 2001 From: Xavier Claessens Date: Tue, 21 Jul 2020 23:05:17 -0400 Subject: pkgconfig: Fix various corner cases See unit tests for the exact scenarios this PR fixes. --- mesonbuild/build.py | 13 +--- mesonbuild/modules/pkgconfig.py | 91 ++++++++++++++++++++------ run_unittests.py | 13 ++++ test cases/common/47 pkgconfig-gen/meson.build | 36 ++++++++++ test cases/common/47 pkgconfig-gen/simple5.c | 6 ++ test cases/common/47 pkgconfig-gen/test.json | 6 +- 6 files changed, 135 insertions(+), 30 deletions(-) create mode 100644 test cases/common/47 pkgconfig-gen/simple5.c diff --git a/mesonbuild/build.py b/mesonbuild/build.py index a06979c..5e6db73 100644 --- a/mesonbuild/build.py +++ b/mesonbuild/build.py @@ -1011,23 +1011,16 @@ This will become a hard error in a future Meson release.''') def get_extra_args(self, language): return self.extra_args.get(language, []) - def get_dependencies(self, exclude=None, for_pkgconfig=False): + def get_dependencies(self, exclude=None): transitive_deps = [] if exclude is None: exclude = [] for t in itertools.chain(self.link_targets, self.link_whole_targets): if t in transitive_deps or t in exclude: continue - # When generating `Libs:` and `Libs.private:` lists in pkg-config - # files we don't want to include static libraries that we link_whole - # or are uninstalled (they're implicitly promoted to link_whole). - # But we still need to include their transitive dependencies, - # a static library we link_whole would itself link to a shared - # library or an installed static library. - if not for_pkgconfig or (not t.is_internal() and t not in self.link_whole_targets): - transitive_deps.append(t) + transitive_deps.append(t) if isinstance(t, StaticLibrary): - transitive_deps += t.get_dependencies(transitive_deps + exclude, for_pkgconfig) + transitive_deps += t.get_dependencies(transitive_deps + exclude) return transitive_deps def get_source_subdir(self): diff --git a/mesonbuild/modules/pkgconfig.py b/mesonbuild/modules/pkgconfig.py index b7a12ff..1cb7698 100644 --- a/mesonbuild/modules/pkgconfig.py +++ b/mesonbuild/modules/pkgconfig.py @@ -36,6 +36,7 @@ class DependenciesHelper: self.priv_reqs = [] self.cflags = [] self.version_reqs = {} + self.link_whole_targets = [] def add_pub_libs(self, libs): libs, reqs, cflags = self._process_libs(libs, True) @@ -130,10 +131,7 @@ class DependenciesHelper: if obj.found(): processed_libs += obj.get_link_args() processed_cflags += obj.get_compile_args() - if public: - self.add_pub_libs(obj.libraries) - else: - self.add_priv_libs(obj.libraries) + self._add_lib_dependencies(obj.libraries, obj.whole_libraries, obj.ext_deps, public) elif isinstance(obj, dependencies.Dependency): if obj.found(): processed_libs += obj.get_link_args() @@ -148,12 +146,13 @@ class DependenciesHelper: processed_libs.append(obj) elif isinstance(obj, (build.SharedLibrary, build.StaticLibrary)): processed_libs.append(obj) - if isinstance(obj, build.StaticLibrary) and public: - self.add_pub_libs(obj.get_dependencies(for_pkgconfig=True)) - self.add_pub_libs(obj.get_external_deps()) - else: - self.add_priv_libs(obj.get_dependencies(for_pkgconfig=True)) - self.add_priv_libs(obj.get_external_deps()) + # If there is a static library in `Libs:` all its deps must be + # public too, otherwise the generated pc file will never be + # usable without --static. + self._add_lib_dependencies(obj.link_targets, + obj.link_whole_targets, + obj.external_deps, + isinstance(obj, build.StaticLibrary) and public) elif isinstance(obj, str): processed_libs.append(obj) else: @@ -161,6 +160,31 @@ class DependenciesHelper: return processed_libs, processed_reqs, processed_cflags + def _add_lib_dependencies(self, link_targets, link_whole_targets, external_deps, public): + add_libs = self.add_pub_libs if public else self.add_priv_libs + # Recursively add all linked libraries + for t in link_targets: + # Internal libraries (uninstalled static library) will be promoted + # to link_whole, treat them as such here. + if t.is_internal(): + self._add_link_whole(t, public) + else: + add_libs([t]) + for t in link_whole_targets: + self._add_link_whole(t, public) + # And finally its external dependencies + add_libs(external_deps) + + def _add_link_whole(self, t, public): + # Don't include static libraries that we link_whole. But we still need to + # include their dependencies: a static library we link_whole + # could itself link to a shared library or an installed static library. + # Keep track of link_whole_targets so we can remove them from our + # lists in case a library is link_with and link_whole at the same time. + # See remove_dups() below. + self.link_whole_targets.append(t) + self._add_lib_dependencies(t.link_targets, t.link_whole_targets, t.external_deps, public) + def add_version_reqs(self, name, version_reqs): if version_reqs: if name not in self.version_reqs: @@ -196,6 +220,32 @@ class DependenciesHelper: return ', '.join(result) def remove_dups(self): + # Set of ids that have already been handled and should not be added any more + exclude = set() + + # We can't just check if 'x' is excluded because we could have copies of + # the same SharedLibrary object for example. + def _ids(x): + if hasattr(x, 'generated_pc'): + yield x.generated_pc + if isinstance(x, build.Target): + yield x.get_id() + yield x + + # Exclude 'x' in all its forms and return if it was already excluded + def _add_exclude(x): + was_excluded = False + for i in _ids(x): + if i in exclude: + was_excluded = True + else: + exclude.add(i) + return was_excluded + + # link_whole targets are already part of other targets, exclude them all. + for t in self.link_whole_targets: + _add_exclude(t) + def _fn(xs, libs=False): # Remove duplicates whilst preserving original order result = [] @@ -206,19 +256,21 @@ class DependenciesHelper: cannot_dedup = libs and isinstance(x, str) and \ not x.startswith(('-l', '-L')) and \ x not in known_flags - if x not in result or cannot_dedup: - result.append(x) + if not cannot_dedup and _add_exclude(x): + continue + result.append(x) return result - self.pub_libs = _fn(self.pub_libs, True) + + # Handle lists in priority order: public items can be excluded from + # private and Requires can excluded from Libs. self.pub_reqs = _fn(self.pub_reqs) - self.priv_libs = _fn(self.priv_libs, True) + self.pub_libs = _fn(self.pub_libs, True) self.priv_reqs = _fn(self.priv_reqs) + self.priv_libs = _fn(self.priv_libs, True) + # Reset exclude list just in case some values can be both cflags and libs. + exclude = set() self.cflags = _fn(self.cflags) - # Remove from private libs/reqs if they are in public already - self.priv_libs = [i for i in self.priv_libs if i not in self.pub_libs] - self.priv_reqs = [i for i in self.priv_reqs if i not in self.pub_reqs] - class PkgConfigModule(ExtensionModule): def _get_lname(self, l, msg, pcfile): @@ -267,7 +319,6 @@ class PkgConfigModule(ExtensionModule): def generate_pkgconfig_file(self, state, deps, subdirs, name, description, url, version, pcfile, conflicts, variables, uninstalled=False, dataonly=False): - deps.remove_dups() coredata = state.environment.get_coredata() if uninstalled: outdir = os.path.join(state.environment.build_dir, 'meson-uninstalled') @@ -460,6 +511,8 @@ class PkgConfigModule(ExtensionModule): if compiler: deps.add_cflags(compiler.get_feature_args({'versions': dversions}, None)) + deps.remove_dups() + def parse_variable_list(stringlist): reserved = ['prefix', 'libdir', 'includedir'] variables = [] diff --git a/run_unittests.py b/run_unittests.py index 73131c7..2b0e4e1 100755 --- a/run_unittests.py +++ b/run_unittests.py @@ -5836,6 +5836,19 @@ class LinuxlikeTests(BasePlatformTests): out = self._run(cmd + ['--libs'], override_envvars=env).strip().split() self.assertEqual(out, ['-llibmain2', '-llibinternal']) + # See common/47 pkgconfig-gen/meson.build for description of the case this test + with open(os.path.join(privatedir1, 'simple2.pc')) as f: + content = f.read() + self.assertIn('Libs: -L${libdir} -lsimple2 -lz -lsimple1', content) + + with open(os.path.join(privatedir1, 'simple3.pc')) as f: + content = f.read() + self.assertEqual(1, content.count('-lsimple3')) + + with open(os.path.join(privatedir1, 'simple5.pc')) as f: + content = f.read() + self.assertNotIn('-lstat2', content) + def test_pkgconfig_uninstalled(self): testdir = os.path.join(self.common_test_dir, '47 pkgconfig-gen') self.init(testdir) diff --git a/test cases/common/47 pkgconfig-gen/meson.build b/test cases/common/47 pkgconfig-gen/meson.build index eb2afe4..8c16cd5 100644 --- a/test cases/common/47 pkgconfig-gen/meson.build +++ b/test cases/common/47 pkgconfig-gen/meson.build @@ -1,5 +1,12 @@ project('pkgconfig-gen', 'c') +# Some CI runners does not have zlib, just skip them as we need some common +# external dependency. +cc = meson.get_compiler('c') +if not cc.find_library('z', required: false).found() + error('MESON_SKIP_TEST: zlib missing') +endif + # First check we have pkg-config >= 0.29 pkgconfig = find_program('pkg-config', required: false) if not pkgconfig.found() @@ -59,3 +66,32 @@ pkgg.generate( version : libver, dataonly: true ) + +# Regression test for 2 cases: +# - link_whole from InternalDependency used to be ignored, but we should still +# recurse to add libraries they link to. In this case it must add `-lsimple1` +# in generated pc file. +# - dependencies from InternalDependency used to be ignored. In this it must add +# `-lz` in generated pc file. +simple1 = shared_library('simple1', 'simple.c') +stat1 = static_library('stat1', 'simple.c', link_with: simple1) +dep = declare_dependency(link_whole: stat1, dependencies: cc.find_library('z')) +simple2 = library('simple2', 'simple.c') +pkgg.generate(simple2, libraries: dep) + +# Regression test: as_system() does a deepcopy() of the InternalDependency object +# which caused `-lsimple3` to be duplicated because generator used to compare +# Target instances instead of their id. +simple3 = shared_library('simple3', 'simple.c') +dep1 = declare_dependency(link_with: simple3) +dep2 = dep1.as_system() +pkgg.generate(libraries: [dep1, dep2], + name: 'simple3', + description: 'desc') + +# Regression test: stat2 is both link_with and link_whole, it should not appear +# in generated pc file. +stat2 = static_library('stat2', 'simple.c', install: true) +simple4 = library('simple4', 'simple.c', link_with: stat2) +simple5 = library('simple5', 'simple5.c', link_with: simple4, link_whole: stat2) +pkgg.generate(simple5) diff --git a/test cases/common/47 pkgconfig-gen/simple5.c b/test cases/common/47 pkgconfig-gen/simple5.c new file mode 100644 index 0000000..9f924bd --- /dev/null +++ b/test cases/common/47 pkgconfig-gen/simple5.c @@ -0,0 +1,6 @@ +int simple5(void); + +int simple5(void) +{ + return 0; +} diff --git a/test cases/common/47 pkgconfig-gen/test.json b/test cases/common/47 pkgconfig-gen/test.json index 1c6a452..702e7fe 100644 --- a/test cases/common/47 pkgconfig-gen/test.json +++ b/test cases/common/47 pkgconfig-gen/test.json @@ -1,9 +1,13 @@ { "installed": [ {"type": "file", "file": "usr/include/simple.h"}, + {"type": "file", "file": "usr/lib/libstat2.a"}, {"type": "file", "file": "usr/lib/pkgconfig/simple.pc"}, {"type": "file", "file": "usr/lib/pkgconfig/libfoo.pc"}, {"type": "file", "file": "usr/lib/pkgconfig/libhello.pc"}, - {"type": "file", "file": "usr/lib/pkgconfig/libhello_nolib.pc"} + {"type": "file", "file": "usr/lib/pkgconfig/libhello_nolib.pc"}, + {"type": "file", "file": "usr/lib/pkgconfig/simple2.pc"}, + {"type": "file", "file": "usr/lib/pkgconfig/simple3.pc"}, + {"type": "file", "file": "usr/lib/pkgconfig/simple5.pc"} ] } -- cgit v1.1