From 926c3a69195385bd5c16acd269f4dae5322e0c03 Mon Sep 17 00:00:00 2001 From: Xavier Claessens Date: Tue, 18 Jul 2023 21:20:32 -0400 Subject: [PATCH] Error when an installed static library links to internal custom target When an installed static library A links to an internal static library B built using a custom_target(), raise an error instead of a warning. This is because to be usable, A needs to contain B which would require to extract the archive to get its objects files. This used to work, but was printing a warning and was installing a broken static library, because we used to overlink in many cases, and that got fixed in Meson 1.2.0. It now fails at link time with symbols from the custom target not being defined. It's better to turn the warning into a hard error at configure time. While at it, noticed this situation can happen for any internal custom or rust target we link to, recursively. get_internal_static_libraries_recurse() could be called on CustomTarget objects which do not implement it, and even if we did not call that method, it would still fail when trying to call extract_all_objects() on it. Fixes: #12006 --- mesonbuild/build.py | 59 ++++++++++--------- test cases/rust/5 polyglot static/meson.build | 2 +- .../unit/113 complex link cases/meson.build | 51 ++++++++++++++++ unittests/linuxliketests.py | 3 + 4 files changed, 86 insertions(+), 29 deletions(-) diff --git a/mesonbuild/build.py b/mesonbuild/build.py index d6ca8c11c..a0a38d0ac 100644 --- a/mesonbuild/build.py +++ b/mesonbuild/build.py @@ -1380,17 +1380,6 @@ class BuildTarget(Target): def link(self, targets): for t in targets: - if isinstance(self, StaticLibrary) and self.install: - if isinstance(t, (CustomTarget, CustomTargetIndex)): - if not t.should_install(): - mlog.warning(f'Try to link an installed static library target {self.name} with a' - 'custom target that is not installed, this might cause problems' - 'when you try to use this static library') - elif t.is_internal(): - # When we're a static library and we link_with to an - # internal/convenience library, promote to link_whole. - self.link_whole([t]) - continue if not isinstance(t, (Target, CustomTargetIndex)): if isinstance(t, dependencies.ExternalLibrary): raise MesonException(textwrap.dedent('''\ @@ -1403,6 +1392,11 @@ class BuildTarget(Target): raise InvalidArguments(f'{t!r} is not a target.') if not t.is_linkable_target(): raise InvalidArguments(f"Link target '{t!s}' is not linkable.") + if isinstance(self, StaticLibrary) and self.install and t.is_internal(): + # When we're a static library and we link_with to an + # internal/convenience library, promote to link_whole. + self.link_whole([t], promoted=True) + continue if isinstance(self, SharedLibrary) and isinstance(t, StaticLibrary) and not t.pic: msg = f"Can't link non-PIC static library {t.name!r} into shared library {self.name!r}. " msg += "Use the 'pic' option to static_library to build with PIC." @@ -1415,7 +1409,7 @@ class BuildTarget(Target): mlog.warning(msg + ' This will fail in cross build.') self.link_targets.append(t) - def link_whole(self, targets): + def link_whole(self, targets, promoted: bool = False): for t in targets: if isinstance(t, (CustomTarget, CustomTargetIndex)): if not t.is_linkable_target(): @@ -1435,40 +1429,49 @@ class BuildTarget(Target): else: mlog.warning(msg + ' This will fail in cross build.') if isinstance(self, StaticLibrary) and not self.uses_rust(): - if isinstance(t, (CustomTarget, CustomTargetIndex)) or t.uses_rust(): - # There are cases we cannot do this, however. In Rust, for - # example, this can't be done with Rust ABI libraries, though - # it could be done with C ABI libraries, though there are - # several meson issues that need to be fixed: - # https://github.com/mesonbuild/meson/issues/10722 - # https://github.com/mesonbuild/meson/issues/10723 - # https://github.com/mesonbuild/meson/issues/10724 - # FIXME: We could extract the .a archive to get object files - raise InvalidArguments('Cannot link_whole a custom or Rust target into a static library') # When we're a static library and we link_whole: to another static # library, we need to add that target's objects to ourselves. + self.check_can_extract_objects(t, origin=self, promoted=promoted) self.objects += [t.extract_all_objects()] # If we install this static library we also need to include objects # from all uninstalled static libraries it depends on. if self.install: - for lib in t.get_internal_static_libraries(): + for lib in t.get_internal_static_libraries(origin=self): self.objects += [lib.extract_all_objects()] self.link_whole_targets.append(t) @lru_cache(maxsize=None) - def get_internal_static_libraries(self) -> OrderedSet[Target]: + def get_internal_static_libraries(self, origin: StaticLibrary) -> OrderedSet[Target]: result: OrderedSet[Target] = OrderedSet() - self.get_internal_static_libraries_recurse(result) + self.get_internal_static_libraries_recurse(result, origin) return result - def get_internal_static_libraries_recurse(self, result: OrderedSet[Target]) -> None: + def get_internal_static_libraries_recurse(self, result: OrderedSet[Target], origin: StaticLibrary) -> None: for t in self.link_targets: if t.is_internal() and t not in result: + self.check_can_extract_objects(t, origin, promoted=True) result.add(t) - t.get_internal_static_libraries_recurse(result) + t.get_internal_static_libraries_recurse(result, origin) for t in self.link_whole_targets: if t.is_internal(): - t.get_internal_static_libraries_recurse(result) + t.get_internal_static_libraries_recurse(result, origin) + + def check_can_extract_objects(self, t: T.Union[Target, CustomTargetIndex], origin: StaticLibrary, promoted: bool = False) -> None: + if isinstance(t, (CustomTarget, CustomTargetIndex)) or t.uses_rust(): + # To extract objects from a custom target we would have to extract + # the archive, WIP implementation can be found in + # https://github.com/mesonbuild/meson/pull/9218. + # For Rust C ABI we could in theory have access to objects, but there + # are several meson issues that need to be fixed: + # https://github.com/mesonbuild/meson/issues/10722 + # https://github.com/mesonbuild/meson/issues/10723 + # https://github.com/mesonbuild/meson/issues/10724 + m = (f'Cannot link_whole a custom or Rust target {t.name!r} into a static library {origin.name!r}. ' + 'Instead, pass individual object files with the "objects:" keyword argument if possible.') + if promoted: + m += (f' Meson had to promote link to link_whole because {origin.name!r} is installed but not {t.name!r},' + f' and thus has to include objects from {t.name!r} to be usable.') + raise InvalidArguments(m) def add_pch(self, language: str, pchlist: T.List[str]) -> None: if not pchlist: diff --git a/test cases/rust/5 polyglot static/meson.build b/test cases/rust/5 polyglot static/meson.build index 5d1f02368..54f383cd3 100644 --- a/test cases/rust/5 polyglot static/meson.build +++ b/test cases/rust/5 polyglot static/meson.build @@ -7,7 +7,7 @@ r = static_library('stuff', 'stuff.rs', rust_crate_type : 'staticlib') # as it would do with C libraries, but then cannot extract objects from stuff and # thus should error out. # FIXME: We should support this use-case in the future. -testcase expect_error('Cannot link_whole a custom or Rust target into a static library') +testcase expect_error('Cannot link_whole a custom or Rust target \'stuff\' into a static library \'clib\'. Instead, pass individual object files with the "objects:" keyword argument if possible. Meson had to promote link to link_whole because \'clib\' is installed but not \'stuff\', and thus has to include objects from \'stuff\' to be usable.') l = static_library('clib', 'clib.c', link_with : r, install : true) endtestcase diff --git a/test cases/unit/113 complex link cases/meson.build b/test cases/unit/113 complex link cases/meson.build index 04e628177..3b4b898df 100644 --- a/test cases/unit/113 complex link cases/meson.build +++ b/test cases/unit/113 complex link cases/meson.build @@ -1,5 +1,7 @@ project('complex link cases', 'c') +cc = meson.get_compiler('c') + # In all tests, e1 uses s3 which uses s2 which uses s1. # Executable links with s3 and s1 but not s2 because it is included in s3. @@ -58,3 +60,52 @@ e = executable('t8-e1', 'main.c', link_with: [s1, s2], dependencies: declare_dependency(link_with: s3), ) + +if cc.get_argument_syntax() == 'gcc' + # s1 is an internal static library, using custom target. + s1_o = custom_target( + input: 's1.c', + output: 's1.c.o', + command: [cc.cmd_array(), '-c', '-o', '@OUTPUT@', '@INPUT@'] + ) + s1 = custom_target( + output: 'libt9-s1.a', + command: ['ar', 'rcs', '@OUTPUT@', s1_o], + ) + + # Executable needs to link with s1, s2 and s3. + s2 = static_library('t9-s2', 's2.c', link_with: s1) + s3 = static_library('t9-s3', 's3.c', link_with: s2) + e = executable('t9-e1', 'main.c', link_with: s3) + + # s2 cannot be installed because s1 is not being installed and Meson cannot + # extract object files from the custom target. + testcase expect_error('Cannot link_whole a custom or Rust target \'libt9-s1.a\' into a static library \'t10-s2\'. Instead, pass individual object files with the "objects:" keyword argument if possible. Meson had to promote link to link_whole because \'t10-s2\' is installed but not \'libt9-s1.a\', and thus has to include objects from \'libt9-s1.a\' to be usable.') + s2 = static_library('t10-s2', 's2.c', link_with: s1, install: true) + endtestcase + + # s3 cannot be installed because s1 is not being installed and Meson cannot + # extract object files from the custom target. + testcase expect_error('Cannot link_whole a custom or Rust target \'libt9-s1.a\' into a static library \'t11-s3\'. Instead, pass individual object files with the "objects:" keyword argument if possible. Meson had to promote link to link_whole because \'t11-s3\' is installed but not \'libt9-s1.a\', and thus has to include objects from \'libt9-s1.a\' to be usable.') + s2 = static_library('t11-s2', 's2.c', link_with: s1) + s3 = static_library('t11-s3', 's3.c', link_with: s2, install: true) + endtestcase + + # s1 is an installed static library, using custom target. + s1 = custom_target( + output: 'libt12-s1.a', + command: ['ar', 'rcs', '@OUTPUT@', s1_o], + install: true, + install_dir: get_option('libdir'), + ) + + # Executable needs to link with s1, s2 and s3. + s2 = static_library('t12-s2', 's2.c', link_with: s1, install: true) + s3 = static_library('t12-s3', 's3.c', link_with: s2) + e = executable('t12-e1', 'main.c', link_with: s3) + + # Executable links with s3 and s1 but not s2 because it is included in s3. + s2 = static_library('t13-s2', 's2.c', link_with: s1) + s3 = static_library('t13-s3', 's3.c', link_with: s2, install: true) + e = executable('t13-e1', 'main.c', link_with: s3) +endif diff --git a/unittests/linuxliketests.py b/unittests/linuxliketests.py index bd73857e9..2de4dbcf0 100644 --- a/unittests/linuxliketests.py +++ b/unittests/linuxliketests.py @@ -1863,3 +1863,6 @@ class LinuxlikeTests(BasePlatformTests): self.assertIn('build t6-e1: c_LINKER t6-e1.p/main.c.o | libt6-s2.a libt6-s3.a\n', content) self.assertIn('build t7-e1: c_LINKER t7-e1.p/main.c.o | libt7-s3.a\n', content) self.assertIn('build t8-e1: c_LINKER t8-e1.p/main.c.o | libt8-s1.a libt8-s2.a libt8-s3.a\n', content) + self.assertIn('build t9-e1: c_LINKER t9-e1.p/main.c.o | libt9-s1.a libt9-s2.a libt9-s3.a\n', content) + self.assertIn('build t12-e1: c_LINKER t12-e1.p/main.c.o | libt12-s1.a libt12-s2.a libt12-s3.a\n', content) + self.assertIn('build t13-e1: c_LINKER t13-e1.p/main.c.o | libt12-s1.a libt13-s3.a\n', content)