Просмотр исходного кода

Add remaining clang symlinks and test them (#5050)

We only had the single `clang` symlink, but in case its useful to use
the toolchain with some other build system that expects `clang++`, or
even `clang-cl` or `clang-cpp`, fill in the rest of the symlinks.

The different `clang` flavors don't really need anything to support in
the subcommand as there is already an excellent way to get the exact
behavior of these names using Clang's `--driver-mode` flag, so these
just use that. That makes this change really *only* about busybox
behavior.

We don't really have a dedicated test path for things that are only
exposed via the symlinks, so I've added a simple Python integration test
we can use for that. I can backfill some testing of other symlinks if
useful (the `ld.lld` one might be worthwhile), although there is minimal
interesting logic to cover there.
Chandler Carruth 1 год назад
Родитель
Сommit
ca2ef22476

+ 4 - 1
scripts/target_determinator.py

@@ -34,9 +34,12 @@ def log(s: str) -> None:
 
 
 def filter_targets(bazel: Path, targets: str) -> str:
+    # Need to quote targets for inclusion in another query.
+    quoted_targets = "\n".join([f'"{t}"' for t in targets.splitlines()])
+
     with tempfile.NamedTemporaryFile(mode="w+") as tmp:
         query = (
-            f"let t = set({targets}) in "
+            f"let t = set({quoted_targets}) in "
             "kind(rule, $t) except attr(tags, manual, $t)\n"
         )
         query_lines = query.splitlines()

+ 17 - 8
toolchain/install/BUILD

@@ -7,6 +7,7 @@ load(
     "LLVM_VERSION_MAJOR",
 )
 load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_library", "cc_test")
+load("@rules_python//python:defs.bzl", "py_test")
 load("//bazel/cc_toolchains:defs.bzl", "cc_env")
 load("//bazel/manifest:defs.bzl", "manifest")
 load("//toolchain/base:llvm_tools.bzl", "LLVM_MAIN_TOOLS", "LLVM_TOOL_ALIASES")
@@ -119,6 +120,13 @@ cc_binary(
     ],
 )
 
+clang_aliases = [
+    "clang",
+    "clang++",
+    "clang-cl",
+    "clang-cpp",
+]
+
 # TODO: Add remaining aliases of LLD for Windows and WASM when we have support
 # for them wired up through the busybox.
 lld_aliases = [
@@ -126,7 +134,7 @@ lld_aliases = [
     "ld64.lld",
 ]
 
-llvm_binaries = lld_aliases + [
+llvm_binaries = clang_aliases + lld_aliases + [
     tool.bin_name
     for tool in LLVM_MAIN_TOOLS.values()
 ] + [
@@ -167,13 +175,7 @@ install_dirs = {
         ),
         install_filegroup("core", "//core:prelude"),
     ],
-    "lib/carbon/llvm/bin": [
-        install_symlink(
-            "clang",
-            "../../carbon-busybox",
-            is_driver = True,
-        ),
-    ] + [install_symlink(
+    "lib/carbon/llvm/bin": [install_symlink(
         name,
         "../../carbon-busybox",
         is_driver = True,
@@ -191,6 +193,13 @@ make_install_filegroups(
     prefix = "prefix_root",
 )
 
+py_test(
+    name = "llvm_symlinks_test",
+    size = "small",
+    srcs = ["llvm_symlinks_test.py"],
+    data = [":install_data"],
+)
+
 manifest(
     name = "install_data_manifest.txt",
     srcs = [":install_data"],

+ 10 - 0
toolchain/install/busybox_main.cpp

@@ -52,6 +52,16 @@ static auto Main(int argc, char** argv) -> ErrorOr<int> {
     auto subcommand_args =
         llvm::StringSwitch<llvm::SmallVector<llvm::StringRef>>(
             *busybox_info.mode)
+            // The `clang` program name used configures the default for its
+            // `--driver-mode` flag. The first of these is redundant with the
+            // default, but we group it here for clarity.
+            .Case("clang", {"clang", "--"})
+            .Case("clang++", {"clang", "--", "--driver-mode=g++"})
+            .Case("clang-cl", {"clang", "--", "--driver-mode=cl"})
+            .Case("clang-cpp", {"clang", "--", "--driver-mode=cpp"})
+
+            // LLD has platform-specific program names that we translate into
+            // platform flags.
             .Case("ld.lld", {"lld", "--platform=gnu", "--"})
             .Case("ld64.lld", {"lld", "--platform=darwin", "--"})
 

+ 101 - 0
toolchain/install/llvm_symlinks_test.py

@@ -0,0 +1,101 @@
+#!/usr/bin/env python3
+
+"""Checks various LLVM tool symlinks behave as expected."""
+
+__copyright__ = """
+Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+Exceptions. See /LICENSE for license information.
+SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+"""
+
+from pathlib import Path
+import subprocess
+import os
+import sys
+import unittest
+
+
+class LLVMSymlinksTest(unittest.TestCase):
+    def setUp(self) -> None:
+        # The install root is adjacent to the test script
+        self.install_root = Path(sys.argv[0]).parent / "prefix_root"
+        self.tmpdir = Path(os.environ["TEST_TMPDIR"])
+        self.test_o_file = self.tmpdir / "test.o"
+        self.test_o_file.touch()
+
+    def get_link_cmd(self, clang: Path) -> list[str | Path]:
+        return [
+            clang,
+            # We pick an arbitrary linux target to get stable results.
+            "--target=aarch64-unknown-linux-gnu",
+            # Verbose printing to help with debugging.
+            "-v",
+            # Print out the link command rather than running it.
+            "-###",
+            # Give the link command an output.
+            "-o",
+            self.tmpdir / "test",
+            # A test input file. This won't be read though.
+            self.test_o_file,
+        ]
+
+    def test_clang(self) -> None:
+        bin = self.install_root / "lib/carbon/llvm/bin/clang"
+        run = subprocess.run(
+            self.get_link_cmd(bin), check=True, capture_output=True, text=True
+        )
+        # Check that we do have a plausible link command.
+        self.assertRegex(run.stderr, r'"-m" "aarch64linux"')
+
+        # Ensure it doesn't contain the C++ standard library.
+        self.assertNotRegex(run.stderr, r'"-lstdc++"')
+
+    def test_clangplusplus(self) -> None:
+        bin = self.install_root / "lib/carbon/llvm/bin/clang++"
+        run = subprocess.run(
+            self.get_link_cmd(bin), check=True, capture_output=True, text=True
+        )
+        # Check that we do have a plausible link command.
+        self.assertRegex(run.stderr, r'"-m" "aarch64linux"')
+
+        # Ensure it doesn't contain the C++ standard library.
+        self.assertNotRegex(run.stderr, r'"-lstdc++"')
+
+    def test_clang_cl(self) -> None:
+        bin = self.install_root / "lib/carbon/llvm/bin/clang-cl"
+        run = subprocess.run(
+            # Use the `cl.exe`-specific help flag to test the mode.
+            [bin, "/?"],
+            check=True,
+            capture_output=True,
+            text=True,
+        )
+
+        # This should print the help string, including `cl.exe` specifics.
+        self.assertRegex(run.stdout, r"CL.EXE COMPATIBILITY OPTIONS:")
+
+    def test_clang_cpp(self) -> None:
+        # Note that this is a test of the C-preprocessor mode, not C++ mode.
+
+        # Create a test file that we'll preprocess.
+        text_file = self.tmpdir / "test.txt"
+        with open(text_file, "w") as f:
+            f.write("TEST\n")
+
+        # Run the preprocessor using a CPP-specific command line reading from
+        # the test file and writing to stdout. We define a macro that we'll
+        # check is expanded.
+        bin = self.install_root / "lib/carbon/llvm/bin/clang-cpp"
+        run = subprocess.run(
+            [bin, "-D", "TEST=SUCCESS", text_file, "-"],
+            check=True,
+            capture_output=True,
+            text=True,
+        )
+
+        self.assertEqual(run.stderr, "")
+        self.assertRegex(run.stdout, r"(^|\n)SUCCESS\n")
+
+
+if __name__ == "__main__":
+    unittest.main()