From e34e2402633e2d3113385f242258666eeb8bfeed Mon Sep 17 00:00:00 2001 From: Chandler Carruth Date: Sat, 1 Jun 2024 07:16:44 +0200 Subject: [PATCH] Move the core standard library to the installation. (#3995) This removes the `data_dir` from the driver favoring the installation abstraction for the both locating the prelude and linking utilities. With this, an installed toolchain should also be able to compile and link Carbon successfully, and the build of the examples should exercise this path almost exactly. (The only difference is using the driver `cc_binary` directly rather than relying on the symlink from inside the install tree.) --- bazel/carbon_rules/defs.bzl | 15 +++++++-------- core/BUILD | 2 +- toolchain/check/check_fuzzer.cpp | 2 +- toolchain/driver/BUILD | 1 - toolchain/driver/driver.cpp | 13 +++++++------ toolchain/driver/driver.h | 9 ++------- toolchain/driver/driver_fuzzer.cpp | 2 +- toolchain/driver/driver_main.cpp | 10 +--------- toolchain/driver/driver_test.cpp | 3 +-- toolchain/install/BUILD | 14 ++++++++++++++ toolchain/install/install_paths.cpp | 8 ++++++++ toolchain/install/install_paths.h | 1 + toolchain/install/install_paths_test.cpp | 5 +++++ toolchain/sem_ir/yaml_test.cpp | 2 +- toolchain/testing/file_test.cpp | 11 ++++++----- 15 files changed, 56 insertions(+), 42 deletions(-) diff --git a/bazel/carbon_rules/defs.bzl b/bazel/carbon_rules/defs.bzl index 31d7fd103acd..c37bfe94e812 100644 --- a/bazel/carbon_rules/defs.bzl +++ b/bazel/carbon_rules/defs.bzl @@ -4,8 +4,6 @@ """Provides rules for building Carbon files using the toolchain.""" -load("@bazel_skylib//rules:run_binary.bzl", "run_binary") - def carbon_binary(name, srcs): """Compiles a Carbon binary. @@ -27,13 +25,14 @@ def carbon_binary(name, srcs): # the prelude moves there. out = src + ".o" srcs_reordered = [s for s in srcs if s != src] + [src] - run_binary( + native.genrule( name = src + ".compile", - tool = "//toolchain/driver:carbon", - args = (["compile"] + - ["$(location %s)" % s for s in srcs_reordered] + - ["--output=$(location %s)" % out]), - srcs = srcs, + tools = [ + "//toolchain/install:prefix_root/bin/carbon", + "//toolchain/install:install_data", + ], + cmd = "$(execpath //toolchain/install:prefix_root/bin/carbon) compile --output=$@ $(SRCS)", + srcs = srcs_reordered, outs = [out], ) diff --git a/core/BUILD b/core/BUILD index a66c142e9104..8cbc4a3ba923 100644 --- a/core/BUILD +++ b/core/BUILD @@ -5,6 +5,6 @@ # The prelude, and all of its dependencies. filegroup( name = "prelude", - data = ["prelude.carbon"] + glob(["prelude/**/*.carbon"]), + srcs = ["prelude.carbon"] + glob(["prelude/**/*.carbon"]), visibility = ["//visibility:public"], ) diff --git a/toolchain/check/check_fuzzer.cpp b/toolchain/check/check_fuzzer.cpp index 305ca2fcfe64..e7113865fa8a 100644 --- a/toolchain/check/check_fuzzer.cpp +++ b/toolchain/check/check_fuzzer.cpp @@ -29,7 +29,7 @@ extern "C" int LLVMFuzzerTestOneInput(const unsigned char* data, // TODO: We should try to thread the executable path into here. const auto install_paths = InstallPaths::Make(""); llvm::raw_null_ostream null_ostream; - Driver driver(fs, &install_paths, "", null_ostream, null_ostream); + Driver driver(fs, &install_paths, null_ostream, null_ostream); // TODO: Get checking to a point where it can handle invalid parse trees // without crashing. diff --git a/toolchain/driver/BUILD b/toolchain/driver/BUILD index 92519a6848dc..7e1a9a90d7eb 100644 --- a/toolchain/driver/BUILD +++ b/toolchain/driver/BUILD @@ -57,7 +57,6 @@ cc_library( srcs = ["driver.cpp"], hdrs = ["driver.h"], data = [ - "//core:prelude", "//toolchain/install:install_lib_data", ], textual_hdrs = ["flags.def"], diff --git a/toolchain/driver/driver.cpp b/toolchain/driver/driver.cpp index caf1d2bd16de..80319dd07341 100644 --- a/toolchain/driver/driver.cpp +++ b/toolchain/driver/driver.cpp @@ -32,7 +32,7 @@ namespace Carbon { -auto Driver::FindPreludeFiles(llvm::StringRef data_dir, +auto Driver::FindPreludeFiles(llvm::StringRef core_package_dir, llvm::raw_ostream& error_stream) -> llvm::SmallVector { llvm::SmallVector result; @@ -40,16 +40,16 @@ auto Driver::FindPreludeFiles(llvm::StringRef data_dir, // Include /core/prelude.carbon, which is the entry point into the // prelude. { - llvm::SmallString<256> prelude_file(data_dir); + llvm::SmallString<256> prelude_file(core_package_dir); llvm::sys::path::append(prelude_file, llvm::sys::path::Style::posix, - "core/prelude.carbon"); + "prelude.carbon"); result.push_back(prelude_file.str().str()); } // Glob for /core/prelude/**/*.carbon and add all the files we find. - llvm::SmallString<256> prelude_dir(data_dir); + llvm::SmallString<256> prelude_dir(core_package_dir); llvm::sys::path::append(prelude_dir, llvm::sys::path::Style::posix, - "core/prelude"); + "prelude"); std::error_code ec; for (llvm::sys::fs::recursive_directory_iterator prelude_files_it( prelude_dir, ec, /*follow_symlinks=*/false); @@ -790,7 +790,8 @@ auto Driver::Compile(const CompileOptions& options, // package-specific search path based on the library name. bool want_prelude = options.prelude_import && options.phase >= CompileOptions::Phase::Check; - auto prelude = want_prelude ? FindPreludeFiles(data_dir_, error_stream_) + auto prelude = want_prelude ? FindPreludeFiles(installation_->core_package(), + error_stream_) : llvm::SmallVector{}; if (want_prelude && prelude.empty()) { return {.success = false}; diff --git a/toolchain/driver/driver.h b/toolchain/driver/driver.h index a16060e64a3f..846833c1c31f 100644 --- a/toolchain/driver/driver.h +++ b/toolchain/driver/driver.h @@ -34,11 +34,10 @@ class Driver { // Constructs a driver with any error or informational output directed to a // specified stream. Driver(llvm::vfs::FileSystem& fs, const InstallPaths* installation, - llvm::StringRef data_dir, llvm::raw_pwrite_stream& output_stream, + llvm::raw_pwrite_stream& output_stream, llvm::raw_pwrite_stream& error_stream) : fs_(fs), installation_(installation), - data_dir_(data_dir), output_stream_(output_stream), error_stream_(error_stream) {} @@ -53,7 +52,7 @@ class Driver { // Finds the source files that define the prelude and returns a list of their // filenames. On error, writes a message to `error_stream` and returns an // empty list. - static auto FindPreludeFiles(llvm::StringRef data_dir, + static auto FindPreludeFiles(llvm::StringRef core_package_dir, llvm::raw_ostream& error_stream) -> llvm::SmallVector; @@ -87,10 +86,6 @@ class Driver { // Helper to locate the toolchain installation's files. const InstallPaths* installation_; - // The path within fs for data files. - // TODO: Replace with use of `installation_` once everything is moved over. - std::string data_dir_; - // Standard output; stdout. llvm::raw_pwrite_stream& output_stream_; // Error output; stderr. diff --git a/toolchain/driver/driver_fuzzer.cpp b/toolchain/driver/driver_fuzzer.cpp index da1f2e917997..4d8cad1ae345 100644 --- a/toolchain/driver/driver_fuzzer.cpp +++ b/toolchain/driver/driver_fuzzer.cpp @@ -71,7 +71,7 @@ extern "C" auto LLVMFuzzerTestOneInput(const unsigned char* data, size_t size) const auto install_paths = InstallPaths::Make(""); TestRawOstream error_stream; llvm::raw_null_ostream dest; - Driver d(fs, &install_paths, "", dest, error_stream); + Driver d(fs, &install_paths, dest, error_stream); if (!d.RunCommand(args).success) { if (error_stream.TakeStr().find("ERROR:") == std::string::npos) { llvm::errs() << "No error message on a failure!\n"; diff --git a/toolchain/driver/driver_main.cpp b/toolchain/driver/driver_main.cpp index 4a1f2b341592..bddf0617887a 100644 --- a/toolchain/driver/driver_main.cpp +++ b/toolchain/driver/driver_main.cpp @@ -9,7 +9,6 @@ #include "common/init_llvm.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringRef.h" -#include "llvm/Support/Path.h" #include "toolchain/driver/driver.h" #include "toolchain/install/install_paths.h" @@ -29,14 +28,7 @@ auto main(int argc, char** argv) -> int { const auto install_paths = Carbon::InstallPaths::MakeExeRelative(exe_path); - // Construct the data directory relative to the executable location. - // TODO: Will be removed when everything moves to the install_paths. - llvm::SmallString<256> data_dir(llvm::sys::path::parent_path(exe_path)); - llvm::sys::path::append(data_dir, llvm::sys::path::Style::posix, - "carbon.runfiles/_main/"); - - Carbon::Driver driver(*fs, &install_paths, data_dir, llvm::outs(), - llvm::errs()); + Carbon::Driver driver(*fs, &install_paths, llvm::outs(), llvm::errs()); bool success = driver.RunCommand(args).success; return success ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/toolchain/driver/driver_test.cpp b/toolchain/driver/driver_test.cpp index 6cc58917f0a6..d751a2f749ad 100644 --- a/toolchain/driver/driver_test.cpp +++ b/toolchain/driver/driver_test.cpp @@ -44,8 +44,7 @@ class DriverTest : public testing::Test { DriverTest() : installation_( InstallPaths::MakeForBazelRunfiles(Testing::GetTestExePath())), - driver_(fs_, &installation_, "", test_output_stream_, - test_error_stream_) { + driver_(fs_, &installation_, test_output_stream_, test_error_stream_) { char* tmpdir_env = getenv("TEST_TMPDIR"); CARBON_CHECK(tmpdir_env != nullptr); test_tmpdir_ = tmpdir_env; diff --git a/toolchain/install/BUILD b/toolchain/install/BUILD index 67c400036e8a..9f4bc7835391 100644 --- a/toolchain/install/BUILD +++ b/toolchain/install/BUILD @@ -31,6 +31,12 @@ write_file( ], ) +symlink_filegroup( + name = "core_data", + srcs = ["//core:prelude"], + out_prefix = "prefix_root/lib/carbon/", +) + # Copy Clang and LLVM toolchain files into a synthetic LLVM installation under # `prefix_root/lib/carbon/llvm` so that parts of Clang that expect to find an LLVM # installation at relative paths work correctly without exposing these in an @@ -72,6 +78,7 @@ filegroup( filegroup( name = "install_lib_data", srcs = [ + ":core_data", ":install_marker", ":llvm_link_data", ], @@ -171,9 +178,16 @@ pkg_files( for bin_name in lld_bin_names ] +pkg_files( + name = "packaging_core_files", + srcs = [":core_data"], + strip_prefix = strip_prefix.from_pkg("prefix_root"), +) + pkg_filegroup( name = "packaging_files", srcs = [ + ":packaging_core_files", ":packaging_exe_files", ] + [ ":packaging_link_lld_alias_" + bin_name diff --git a/toolchain/install/install_paths.cpp b/toolchain/install/install_paths.cpp index 0f658ba4c7fe..e867af01e81e 100644 --- a/toolchain/install/install_paths.cpp +++ b/toolchain/install/install_paths.cpp @@ -98,6 +98,14 @@ auto InstallPaths::driver() const -> std::string { return path.str().str(); } +auto InstallPaths::core_package() const -> std::string { + llvm::SmallString<256> path(prefix_); + // TODO: Adjust this to work equally well on Windows. + llvm::sys::path::append(path, llvm::sys::path::Style::posix, + "lib/carbon/core"); + return path.str().str(); +} + auto InstallPaths::llvm_install_bin() const -> std::string { llvm::SmallString<256> path(prefix_); // TODO: Adjust this to work equally well on Windows. diff --git a/toolchain/install/install_paths.h b/toolchain/install/install_paths.h index b9e9bdf7b33d..3816f65a7806 100644 --- a/toolchain/install/install_paths.h +++ b/toolchain/install/install_paths.h @@ -101,6 +101,7 @@ class InstallPaths { auto prefix() const -> llvm::StringRef { return prefix_; } auto driver() const -> std::string; + auto core_package() const -> std::string; auto llvm_install_bin() const -> std::string; private: diff --git a/toolchain/install/install_paths_test.cpp b/toolchain/install/install_paths_test.cpp index ac390050fb14..ba42b2975303 100644 --- a/toolchain/install/install_paths_test.cpp +++ b/toolchain/install/install_paths_test.cpp @@ -53,6 +53,11 @@ class InstallPathsTest : public ::testing::Test { EXPECT_TRUE(llvm::sys::fs::can_execute(driver_path)) << "path: " << driver_path; + std::string core_package_path = paths.core_package(); + ASSERT_THAT(core_package_path, StartsWith(prefix)); + EXPECT_TRUE(llvm::sys::fs::exists(core_package_path + "/prelude.carbon")) + << "path: " << core_package_path; + std::string llvm_bin_path = paths.llvm_install_bin(); ASSERT_THAT(llvm_bin_path, StartsWith(prefix)); EXPECT_TRUE(llvm::sys::fs::exists(llvm_bin_path)) diff --git a/toolchain/sem_ir/yaml_test.cpp b/toolchain/sem_ir/yaml_test.cpp index 30e94e3fd3f1..17dffd6caae0 100644 --- a/toolchain/sem_ir/yaml_test.cpp +++ b/toolchain/sem_ir/yaml_test.cpp @@ -38,7 +38,7 @@ TEST(SemIRTest, YAML) { const auto install_paths = InstallPaths::MakeForBazelRunfiles(Testing::GetTestExePath()); TestRawOstream print_stream; - Driver d(fs, &install_paths, "", print_stream, llvm::errs()); + Driver d(fs, &install_paths, print_stream, llvm::errs()); auto run_result = d.RunCommand({"compile", "--no-prelude-import", "--phase=check", "--dump-raw-sem-ir", "test.carbon"}); diff --git a/toolchain/testing/file_test.cpp b/toolchain/testing/file_test.cpp index 7dd82e494398..0b5daa53a6d6 100644 --- a/toolchain/testing/file_test.cpp +++ b/toolchain/testing/file_test.cpp @@ -32,9 +32,8 @@ class ToolchainFileTest : public FileTestBase { auto Run(const llvm::SmallVector& test_args, llvm::vfs::InMemoryFileSystem& fs, llvm::raw_pwrite_stream& stdout, llvm::raw_pwrite_stream& stderr) -> ErrorOr override { - const llvm::StringRef data_dir = ""; - - auto prelude = Driver::FindPreludeFiles(data_dir, stderr); + auto prelude = + Driver::FindPreludeFiles(installation_.core_package(), stderr); if (prelude.empty()) { return Error("Could not find prelude"); } @@ -42,7 +41,7 @@ class ToolchainFileTest : public FileTestBase { CARBON_RETURN_IF_ERROR(AddFile(fs, file)); } - Driver driver(fs, &installation_, data_dir, stdout, stderr); + Driver driver(fs, &installation_, stdout, stderr); auto driver_result = driver.RunCommand(test_args); RunResult result{ @@ -84,7 +83,9 @@ class ToolchainFileTest : public FileTestBase { args.push_back("--no-prelude-import"); } - args.insert(args.end(), {"--exclude-dump-file-prefix=core/", "%s"}); + args.insert( + args.end(), + {"--exclude-dump-file-prefix=" + installation_.core_package(), "%s"}); return args; }