From 8541f2945425d5244768ad6c3078907c799d5cc7 Mon Sep 17 00:00:00 2001 From: Ashrafahmed9 Date: Sun, 30 Aug 2026 21:13:35 +0530 Subject: [PATCH] fix(ninja): link shared libraries through the link_shared rule Shared library support landed twice, in two incompatible shapes, and the halves were never reconciled. The generator defines a link_shared rule, but nothing ever refers to it: the build statement for a shared_library uses the plain link rule and pushes -shared (or -dynamiclib) into that target's ldflags instead. So link_shared is dead, and because it is emitted unconditionally its hardcoded -shared lands in the build.ninja of projects that build nothing but static archives. Each half also brought its own test, and they contradict each other. tests/ebuild asserts the shared library is built by link_shared; tests/unit asserts it is built by link. Both cannot hold, and on master three tests fail on every platform in the CI matrix. Use the rule. It is the one that produces a correct LINK_SHARED line in ninja's output, and putting the flag in the rule keeps it out of every static target's ldflags. Emit it only when a shared_library target exists, so a static-only project's build.ninja does not carry a rule for a link it never performs, and give it the platform's spelling rather than a hardcoded -shared, since the matrix includes macos-13. Two test assertions move with it. The tests/unit one asserted ": link ", whose stated purpose in its own comment is that the target goes through a compiler driver rather than ar_rule; that now checks exactly that. The tests/ebuild one hardcoded -shared and would have failed on the macOS runner once the rule was reached at all. The suite goes from 3 failures to 201 passed, 1 skipped. --- ebuild/build/ninja_backend.py | 26 +++++++++++++++++--------- tests/ebuild/test_ninja_backend.py | 4 +++- tests/unit/test_ninja_backend.py | 5 +++-- 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/ebuild/build/ninja_backend.py b/ebuild/build/ninja_backend.py index 5aa2f37..a3434d9 100644 --- a/ebuild/build/ninja_backend.py +++ b/ebuild/build/ninja_backend.py @@ -123,16 +123,25 @@ def _write_ninja(self) -> None: " command = $cc $ldflags $in -o $out $libs", " description = LINK $out", "", - "rule link_shared", - " command = $cc -shared $ldflags $in -o $out $libs", - " description = LINK_SHARED $out", - "", "rule ar_rule", " command = $ar rcs $out $in", " description = AR $out", "", ] + # The flag that turns a link into a shared object is spelled + # differently on Darwin. Emit the rule only when something needs it, + # so a purely static project's build.ninja does not carry a rule for + # a link it never performs. + shared_flag = "-dynamiclib" if sys.platform == "darwin" else "-shared" + if any(t.target_type == "shared_library" for t in self.config.targets): + lines += [ + "rule link_shared", + f" command = $cc {shared_flag} $ldflags $in -o $out $libs", + " description = LINK_SHARED $out", + "", + ] + toolchain_ldflags = self._get_toolchain_ldflags() for target in self.config.targets: @@ -190,11 +199,10 @@ def _write_ninja(self) -> None: if target.target_type == "static_library": lines.append(f"build {out}: ar_rule {' '.join(obj_files)}") else: - # Shared libraries need the platform's "build a shared - # object" flag and the same -L/-l wiring executables get, - # neither of which the generic `link` rule provides. + # link_shared carries the platform's "build a shared + # object" flag; the -L/-l wiring is the same as for + # executables and still goes through ldflags/libs. ldflags = list(target.ldflags) - ldflags.insert(0, "-dynamiclib" if sys.platform == "darwin" else "-shared") libs = [] for pkg_name in target.uses: pkg = self.package_paths.get(pkg_name) @@ -204,7 +212,7 @@ def _write_ninja(self) -> None: for lib in pkg.libraries: libs.append(f"-l{lib}") - lines.append(f"build {out}: link {' '.join(obj_files)}") + lines.append(f"build {out}: link_shared {' '.join(obj_files)}") if ldflags: lines.append(f" ldflags = {' '.join(ldflags)}") if libs: diff --git a/tests/ebuild/test_ninja_backend.py b/tests/ebuild/test_ninja_backend.py index cca5812..5d06fb3 100644 --- a/tests/ebuild/test_ninja_backend.py +++ b/tests/ebuild/test_ninja_backend.py @@ -45,7 +45,9 @@ def test_shared_library_uses_shared_link_rule(tmp_path): NinjaBackend(config, tmp_path / "build", toolchain).generate() ninja_file = (tmp_path / "build" / "build.ninja").read_text(encoding="utf-8") - assert "rule link_shared\n command = $cc -shared" in ninja_file + # Darwin spells the flag -dynamiclib; the CI matrix covers macos-13. + shared_flag = "-dynamiclib" if sys.platform == "darwin" else "-shared" + assert f"rule link_shared\n command = $cc {shared_flag}" in ninja_file assert "build " in ninja_file assert ": link_shared " in ninja_file diff --git a/tests/unit/test_ninja_backend.py b/tests/unit/test_ninja_backend.py index e973ca0..ff68f09 100644 --- a/tests/unit/test_ninja_backend.py +++ b/tests/unit/test_ninja_backend.py @@ -40,9 +40,10 @@ def test_shared_library_gets_shared_flag(self): shared_flag = "-dynamiclib" if sys.platform == "darwin" else "-shared" self.assertIn(shared_flag, ninja) - # It must use the `link` rule (compiler driver), not `ar_rule`. + # It must go through a compiler-driver link rule, not `ar_rule`. lib_line = next(line for line in ninja.splitlines() if "libmylib" in line and line.startswith("build")) - self.assertIn(": link ", lib_line) + self.assertNotIn(": ar_rule", lib_line) + self.assertIn(": link_shared ", lib_line) def test_shared_library_gets_lib_dirs_and_libs(self): target = TargetConfig(