From 8b967943d3a868972b278fdd6854a95c1aa38896 Mon Sep 17 00:00:00 2001 From: Chandler Carruth Date: Wed, 11 Feb 2026 11:16:38 -0800 Subject: [PATCH] Move the `build-runtimes` option up to the top-level driver (#6720) Multiple subcommands all need the ability to disable on-demand runtime building, and this may be needed outside of using _prebuilt_ runtimes. For example, with Bazel the plan is to not build runtimes at all and have Bazel provide them as native Bazel libraries. Updates the `link` subcommand to respect this flag when running Clang to perform links. We didn't have any real testing of the `link` subcommand, in part because it was difficult -- it would try to link runtime libraries. Now that we can prevent building them on demand, we can use that to test the link command. That in turn helped uncover a couple of bugs that are fixed here. 1) The `driver_env_` member of the `Driver` was re-used across `RunCommand` invocations. Some of its fields are constant across these, others can be updated, and still more are not necessarily something we would expect to be re-used. This fixes that by removing the `driver_env_` member, and replacing it with members for just the fields of `DriverEnv` that we want to set initially based on the construction of the `Driver` object. This causes multiple, sequential `RunCommand` calls to not clobber or erroneously inherit state. 2) The temporary directory support in the driver unittest didn't allow the driver to observe the things it wrote to the temporary directory. This PR updates the test logic to create an overlay VFS so that both the in-memory test inputs are observed, but so are the real files written into the temporary directory. 3) The Clang runner, when asked to run Clang without runtimes would still attempt to include runtimes in any link command. This isn't quite what we want, as the whole reason to use this without building runtimes is to reuse ones built in some other way and potentially in some other location. For now, this PR uses a hack to suppress these issues so that we can have a basic test, but in the future we'll need a better solution here. 4) The driver test didn't include the actual driver in the install data. The test even worked around this, but it makes it impossible to link reliably as the `lld` binary isn't available. This adds the data dependency and updates the test to the available digest, etc. --- toolchain/driver/BUILD | 1 + toolchain/driver/clang_runner.cpp | 71 +++++++++----- toolchain/driver/clang_runner.h | 3 +- toolchain/driver/clang_subcommand.cpp | 23 +---- toolchain/driver/clang_subcommand.h | 2 - toolchain/driver/driver.cpp | 130 +++++++++++++++++--------- toolchain/driver/driver.h | 23 ++++- toolchain/driver/driver_env.h | 4 + toolchain/driver/driver_test.cpp | 62 +++++++++--- toolchain/driver/link_subcommand.cpp | 7 +- 10 files changed, 217 insertions(+), 109 deletions(-) diff --git a/toolchain/driver/BUILD b/toolchain/driver/BUILD index a14f4bfe5e6f..d84badb4c4f2 100644 --- a/toolchain/driver/BUILD +++ b/toolchain/driver/BUILD @@ -241,6 +241,7 @@ cc_test( name = "driver_test", size = "small", srcs = ["driver_test.cpp"], + data = ["//toolchain/install:install_data"], deps = [ ":driver", "//common:all_llvm_targets", diff --git a/toolchain/driver/clang_runner.cpp b/toolchain/driver/clang_runner.cpp index 16bb4b64ec27..10d064c466c9 100644 --- a/toolchain/driver/clang_runner.cpp +++ b/toolchain/driver/clang_runner.cpp @@ -173,23 +173,31 @@ auto ClangRunner::RunWithPrebuiltRuntimes(llvm::ArrayRef args, prebuilt_runtimes.Get(Runtimes::Libcxx)); return RunInternal(args, target, prebuilt_resource_dir_path.native(), std::move(libunwind_path), std::move(libcxx_path), - enable_leaking); + /*link_runtime_libs=*/true, enable_leaking); } auto ClangRunner::Run(llvm::ArrayRef args, Runtimes::Cache& runtimes_cache, llvm::ThreadPoolInterface& runtimes_build_thread_pool, bool enable_leaking) -> ErrorOr { + std::string target = ComputeClangTarget(args); + // Check the args to see if we have a known target-independent command. If so, // directly dispatch it to avoid the cost of building the target resource // directory. // TODO: Maybe handle response file expansion similar to the Clang CLI? if (args.empty() || args[0].starts_with("-cc1") || IsNonLinkCommand(args)) { - return RunWithNoRuntimes(args, enable_leaking); + // Note that we do allow linking default libraries here -- we want to learn + // if a command ever goes through this path and Clang thinks it needs to + // link a library as the goal here is to correctly detect that this will + // _not_ happen. Suppressing the linking of default libraries would hide a + // failure in that case. + return RunInternal(args, target, /*target_resource_dir_path=*/std::nullopt, + /*libunwind_path=*/std::nullopt, + /*libcxx_path=*/std::nullopt, /*link_runtime_libs=*/true, + enable_leaking); } - std::string target = ComputeClangTarget(args); - Runtimes::Cache::Features features = {.target = target}; CARBON_ASSIGN_OR_RETURN(Runtimes runtimes, runtimes_cache.Lookup(features)); @@ -219,7 +227,7 @@ auto ClangRunner::Run(llvm::ArrayRef args, // return. return RunInternal(args, target, resource_dir_path.native(), std::move(libunwind_path), std::move(libcxx_path), - enable_leaking); + /*link_runtime_libs=*/true, enable_leaking); } auto ClangRunner::RunWithNoRuntimes(llvm::ArrayRef args, @@ -227,7 +235,8 @@ auto ClangRunner::RunWithNoRuntimes(llvm::ArrayRef args, std::string target = ComputeClangTarget(args); return RunInternal(args, target, /*target_resource_dir_path=*/std::nullopt, /*libunwind_path=*/std::nullopt, - /*libcxx_path=*/std::nullopt, enable_leaking); + /*libcxx_path=*/std::nullopt, /*link_runtime_libs=*/false, + enable_leaking); } auto ClangRunner::RunInternal( @@ -235,8 +244,8 @@ auto ClangRunner::RunInternal( std::optional target_resource_dir_path, std::optional libunwind_path, - std::optional libcxx_path, bool enable_leaking) - -> ErrorOr { + std::optional libcxx_path, bool link_runtime_libs, + bool enable_leaking) -> ErrorOr { llvm::BumpPtrAllocator alloc; // Handle special dispatch for CC1 commands as they don't use the driver and @@ -278,19 +287,39 @@ auto ClangRunner::RunInternal( AppendDefaultClangArgs(*installation_, target, prefix_args); - // We don't have a direct way to configure the linker search paths in the - // Clang driver outside of command line flags, so we inject them here with - // flags. Note that we only inject these as _search_ paths to allow the normal - // linking rules to govern whether or not to link a given library. We also - // build our runtimes exclusively as static archives so we don't need to use - // command line flags to force static runtime linking to occur. - if (libunwind_path) { - prefix_args.push_back( - llvm::formatv("-L{0}/lib", *std::move(libunwind_path)).str()); - } - if (libcxx_path) { - prefix_args.push_back( - llvm::formatv("-L{0}/lib", std::move(libcxx_path)).str()); + if (link_runtime_libs) { + // We don't have a direct way to configure the linker search paths in the + // Clang driver outside of command line flags, so we inject them here with + // flags. Note that we only inject these as _search_ paths to allow the + // normal linking rules to govern whether or not to link a given library. We + // also build our runtimes exclusively as static archives so we don't need + // to use command line flags to force static runtime linking to occur. + if (libunwind_path) { + prefix_args.push_back( + llvm::formatv("-L{0}/lib", *std::move(libunwind_path)).str()); + } + if (libcxx_path) { + prefix_args.push_back( + llvm::formatv("-L{0}/lib", std::move(libcxx_path)).str()); + } + } else { + // If we are suppressing the linking of default libs, ensure we didn't get a + // path to add to the link for them, or an override of the resource + // directory. + CARBON_CHECK(!target_resource_dir_path); + CARBON_CHECK(!libunwind_path); + CARBON_CHECK(!libcxx_path); + + // Now suppress all the default library linking, as we don't expect to have + // any target runtimes on this code path. + // + // TODO: What we actually want here is something more like `-nostdlib++`, + // `-unwindlib=none`, `-rtlib=none`; however, the last of these doesn't + // exist in Clang and looks tricky to introduce. This is almost certainly + // wrong, as it likely suppresses the linking of the _C_ standard library, + // which isn't one of the Clang runtime libraries we're trying to control + // here. But the only user of this currently doesn't need to distinguish. + prefix_args.push_back("-nostdlib"); } prefix_args.push_back("--end-no-unused-arguments"); diff --git a/toolchain/driver/clang_runner.h b/toolchain/driver/clang_runner.h index a59a02917d5c..c957ffd0eba1 100644 --- a/toolchain/driver/clang_runner.h +++ b/toolchain/driver/clang_runner.h @@ -111,7 +111,8 @@ class ClangRunner : ToolRunnerBase { std::optional target_resource_dir_path, std::optional libunwind_path, std::optional libcxx_path, - bool enable_leaking) -> ErrorOr; + bool link_runtime_libs, bool enable_leaking) + -> ErrorOr; // Returns the target-specific source files for the builtins runtime library. auto CollectBuiltinsSrcFiles(const llvm::Triple& target_triple) diff --git a/toolchain/driver/clang_subcommand.cpp b/toolchain/driver/clang_subcommand.cpp index a76676cb4630..a31ce942ac1d 100644 --- a/toolchain/driver/clang_subcommand.cpp +++ b/toolchain/driver/clang_subcommand.cpp @@ -12,27 +12,6 @@ namespace Carbon { auto ClangOptions::Build(CommandLine::CommandBuilder& b) -> void { - b.AddFlag( - { - .name = "build-runtimes", - .help = R"""( -Enables on-demand building of target-specific runtimes. - -When enabled, any link actions using `clang` will build the necessary runtimes -on-demand. This build will use any customization it can from the link command -line flags to build the runtimes for the correct target and with any desired -features enabled. - -Note: this only has an effect when `--prebuilt-runtimes` are not provided. If -there are no prebuilt runtimes and building runtimes is disabled, then it is -assumed the installed toolchain has had the necessary target runtimes added to -the installation tree in the default searched locations. -)""", - }, - [&](auto& arg_b) { - arg_b.Default(true); - arg_b.Set(&build_runtimes_on_demand); - }); b.AddStringPositionalArg( { .name = "ARG", @@ -81,7 +60,7 @@ auto ClangSubcommand::Run(DriverEnv& driver_env) -> DriverResult { run_result = runner.RunWithPrebuiltRuntimes(options_.args, *driver_env.prebuilt_runtimes, driver_env.enable_leaking); - } else if (options_.build_runtimes_on_demand) { + } else if (driver_env.build_runtimes_on_demand) { run_result = runner.Run(options_.args, driver_env.runtimes_cache, *driver_env.thread_pool, driver_env.enable_leaking); } else { diff --git a/toolchain/driver/clang_subcommand.h b/toolchain/driver/clang_subcommand.h index 9bca679ef703..6d3f9c5e7784 100644 --- a/toolchain/driver/clang_subcommand.h +++ b/toolchain/driver/clang_subcommand.h @@ -19,8 +19,6 @@ namespace Carbon { struct ClangOptions { auto Build(CommandLine::CommandBuilder& b) -> void; - bool build_runtimes_on_demand = false; - llvm::SmallVector args; }; diff --git a/toolchain/driver/driver.cpp b/toolchain/driver/driver.cpp index 91a640575b41..16e348e2399d 100644 --- a/toolchain/driver/driver.cpp +++ b/toolchain/driver/driver.cpp @@ -34,6 +34,7 @@ struct Options { bool fuzzing = false; bool include_diagnostic_kind = false; bool threads = true; + bool build_runtimes_on_demand = false; llvm::StringRef runtimes_cache_path; llvm::StringRef prebuilt_runtimes_path; @@ -109,6 +110,27 @@ will be used instead. )""", }, [&](auto& arg_b) { arg_b.Set(&prebuilt_runtimes_path); }); + b.AddFlag( + { + .name = "build-runtimes", + .help = R"""( +Enables on-demand building of target-specific runtimes. + +When enabled (the default), any link actions using `clang` will build the +necessary runtimes on-demand. This build will use any customization it can from +the link command line flags to build the runtimes for the correct target and +with any desired features enabled. + +Note: this only has an effect when `--prebuilt-runtimes` are not provided. If +there are no prebuilt runtimes and building runtimes is disabled, then it is +assumed the installed toolchain has had the necessary target runtimes added to +the installation tree in the default searched locations. +)""", + }, + [&](auto& arg_b) { + arg_b.Default(true); + arg_b.Set(&build_runtimes_on_demand); + }); b.AddFlag( { @@ -159,87 +181,105 @@ when there are errors or other output. b.RequiresSubcommand(); } +static auto HandleRuntimesOptions(const Options& options, DriverEnv& driver_env) + -> bool { + if (!options.prebuilt_runtimes_path.empty()) { + auto result = Runtimes::OpenExisting(options.prebuilt_runtimes_path.str(), + driver_env.vlog_stream); + if (!result.ok()) { + // TODO: We should provide a better diagnostic than the raw error. + CARBON_DIAGNOSTIC(DriverPrebuiltRuntimesInvalid, Error, "{0}", + std::string); + driver_env.emitter.Emit(DriverPrebuiltRuntimesInvalid, + result.error().message()); + return false; + } + driver_env.prebuilt_runtimes = *std::move(result); + return true; + } + + if (!options.build_runtimes_on_demand) { + // Nothing else needed if we're not building runtimes on demand. + CARBON_CHECK(!driver_env.build_runtimes_on_demand); + return true; + } + driver_env.build_runtimes_on_demand = true; + + // If we don't have prebuilt runtimes and are building them on demand, we need + // to configure the runtimes cache. + auto cache_result = + options.runtimes_cache_path.empty() + ? Runtimes::Cache::MakeSystem(*driver_env.installation, + driver_env.vlog_stream) + : Runtimes::Cache::MakeCustom( + *driver_env.installation, + std::filesystem::absolute(options.runtimes_cache_path.str()), + driver_env.vlog_stream); + if (!cache_result.ok()) { + // TODO: We should provide a better diagnostic than the raw error. + CARBON_DIAGNOSTIC(DriverRuntimesCacheInvalid, Error, "{0}", std::string); + driver_env.emitter.Emit(DriverRuntimesCacheInvalid, + cache_result.error().message()); + return false; + } + driver_env.runtimes_cache = std::move(*cache_result); + return true; +} + auto Driver::RunCommand(llvm::ArrayRef args) -> DriverResult { PrettyStackTraceFunction trace_version([&](llvm::raw_ostream& out) { out << "Carbon version: " << Version::String << "\n"; }); - if (driver_env_.installation->error()) { - CARBON_DIAGNOSTIC(DriverInstallInvalid, Error, "{0}", std::string); - driver_env_.emitter.Emit(DriverInstallInvalid, - driver_env_.installation->error()->str()); - return {.success = false}; - } - Options options; + DriverEnv env(fs_, installation_, input_stream_, output_stream_, + error_stream_, fuzzing_, enable_leaking_); ErrorOr result = CommandLine::Parse( - args, *driver_env_.output_stream, Options::Info, + args, *env.output_stream, Options::Info, [&](CommandLine::CommandBuilder& b) { options.Build(b); }); // Regardless of whether the parse succeeded, try to use the diagnostic kind // flag. - driver_env_.consumer.set_include_diagnostic_kind( - options.include_diagnostic_kind); + env.consumer.set_include_diagnostic_kind(options.include_diagnostic_kind); + + if (env.installation->error()) { + CARBON_DIAGNOSTIC(DriverInstallInvalid, Error, "{0}", std::string); + env.emitter.Emit(DriverInstallInvalid, env.installation->error()->str()); + return {.success = false}; + } if (!result.ok()) { CARBON_DIAGNOSTIC(DriverCommandLineParseFailed, Error, "{0}", std::string); - driver_env_.emitter.Emit(DriverCommandLineParseFailed, - PrintToString(result.error())); + env.emitter.Emit(DriverCommandLineParseFailed, + PrintToString(result.error())); return {.success = false}; } else if (*result == CommandLine::ParseResult::MetaSuccess) { return {.success = true}; } - auto cache_result = - options.runtimes_cache_path.empty() - ? Runtimes::Cache::MakeSystem(*driver_env_.installation, - driver_env_.vlog_stream) - : Runtimes::Cache::MakeCustom( - *driver_env_.installation, - std::filesystem::absolute(options.runtimes_cache_path.str()), - driver_env_.vlog_stream); - if (!cache_result.ok()) { - // TODO: We should provide a better diagnostic than the raw error. - CARBON_DIAGNOSTIC(DriverRuntimesCacheInvalid, Error, "{0}", std::string); - driver_env_.emitter.Emit(DriverRuntimesCacheInvalid, - cache_result.error().message()); + if (!HandleRuntimesOptions(options, env)) { return {.success = false}; } - driver_env_.runtimes_cache = std::move(*cache_result); - - if (!options.prebuilt_runtimes_path.empty()) { - auto result = Runtimes::OpenExisting(options.prebuilt_runtimes_path.str(), - driver_env_.vlog_stream); - if (!result.ok()) { - // TODO: We should provide a better diagnostic than the raw error. - CARBON_DIAGNOSTIC(DriverPrebuiltRuntimesInvalid, Error, "{0}", - std::string); - driver_env_.emitter.Emit(DriverPrebuiltRuntimesInvalid, - result.error().message()); - return {.success = false}; - } - driver_env_.prebuilt_runtimes = *std::move(result); - } if (options.verbose) { // Note this implies streamed output in order to interleave. - driver_env_.vlog_stream = driver_env_.error_stream; + env.vlog_stream = env.error_stream; } if (options.fuzzing) { - driver_env_.fuzzing = true; + env.fuzzing = true; } llvm::SingleThreadExecutor single_thread({.ThreadsRequested = 1}); std::optional threads; - driver_env_.thread_pool = &single_thread; + env.thread_pool = &single_thread; if (options.threads) { threads.emplace(llvm::optimal_concurrency()); - driver_env_.thread_pool = &*threads; + env.thread_pool = &*threads; } CARBON_CHECK(options.selected_subcommand != nullptr); - return options.selected_subcommand->Run(driver_env_); + return options.selected_subcommand->Run(env); } } // namespace Carbon diff --git a/toolchain/driver/driver.h b/toolchain/driver/driver.h index 11391eb87010..f63f67b21fd2 100644 --- a/toolchain/driver/driver.h +++ b/toolchain/driver/driver.h @@ -27,8 +27,13 @@ class Driver { llvm::raw_pwrite_stream* output_stream, llvm::raw_pwrite_stream* error_stream, bool fuzzing = false, bool enable_leaking = false) - : driver_env_(std::move(fs), installation, input_stream, output_stream, - error_stream, fuzzing, enable_leaking) {} + : fs_(std::move(fs)), + installation_(installation), + input_stream_(input_stream), + output_stream_(output_stream), + error_stream_(error_stream), + fuzzing_(fuzzing), + enable_leaking_(enable_leaking) {} // Parses the given arguments into both a subcommand to select the operation // to perform and any arguments to that subcommand. @@ -39,7 +44,19 @@ class Driver { auto RunCommand(llvm::ArrayRef args) -> DriverResult; private: - DriverEnv driver_env_; + // We store the initial values in the `DriverEnv` that will be used for each + // subcommand invocation here. These are used as the _starting_ values of the + // environment, but individual `RunCommand` invocations may customize the + // `DriverEnv` instance changing these values. + // + // For details on each of these fields, see the documentation in `DriverEnv`. + llvm::IntrusiveRefCntPtr fs_; + const InstallPaths* installation_; + FILE* input_stream_; + llvm::raw_pwrite_stream* output_stream_; + llvm::raw_pwrite_stream* error_stream_; + bool fuzzing_; + bool enable_leaking_; }; } // namespace Carbon diff --git a/toolchain/driver/driver_env.h b/toolchain/driver/driver_env.h index c9cebb8ca34b..0f951db84217 100644 --- a/toolchain/driver/driver_env.h +++ b/toolchain/driver/driver_env.h @@ -58,6 +58,10 @@ struct DriverEnv { // `false` for safe and correct library execution. bool enable_leaking = false; + // Whether to build runtimes on-demand. Only used when `prebuilt_runtimes` is + // empty. + bool build_runtimes_on_demand = false; + // A diagnostic consumer, to be able to connect output. Diagnostics::StreamConsumer consumer; diff --git a/toolchain/driver/driver_test.cpp b/toolchain/driver/driver_test.cpp index 36281e058b99..b95ce388c5f4 100644 --- a/toolchain/driver/driver_test.cpp +++ b/toolchain/driver/driver_test.cpp @@ -65,6 +65,7 @@ class DriverTest : public testing::Test { std::error_code ec; auto original_dir = std::filesystem::current_path(ec); CARBON_CHECK(!ec, "{0}", ec.message()); + Driver original_driver = std::move(driver_); const auto* unit_test = ::testing::UnitTest::GetInstance(); const auto* test_info = unit_test->current_test_info(); @@ -78,12 +79,24 @@ class DriverTest : public testing::Test { std::filesystem::current_path(test_dir, ec); CARBON_CHECK(!ec, "Could not change the current working dir to '{0}': {1}", test_dir, ec.message()); - return llvm::scope_exit([original_dir, test_dir] { + + // Build an overlay filesystem between the in-memory one and this new + // directory. + llvm::IntrusiveRefCntPtr overlay_fs = + new llvm::vfs::OverlayFileSystem(llvm::vfs::getRealFileSystem()); + overlay_fs->pushOverlay(fs_); + + // Rebuild the driver around this filesystem. + driver_ = Driver(overlay_fs, &installation_, /*input_stream=*/nullptr, + &test_output_stream_, &test_error_stream_); + + return llvm::scope_exit([this, original_dir, original_driver, test_dir] { std::error_code ec; std::filesystem::current_path(original_dir, ec); CARBON_CHECK(!ec, "Could not change the current working dir to '{0}': {1}", original_dir, ec.message()); + driver_ = original_driver; std::filesystem::remove_all(test_dir, ec); CARBON_CHECK(!ec, "Could not remove the test working dir '{0}': {1}", test_dir, ec.message()); @@ -169,7 +182,7 @@ TEST_F(DriverTest, DumpParseTree) { TEST_F(DriverTest, StdoutOutput) { // Use explicit filenames so we can look for those to validate output. - MakeTestFile("fn Main() {}", "test.carbon"); + MakeTestFile("fn Run() {}", "test.carbon"); EXPECT_TRUE(driver_ .RunCommand({"compile", "--no-prelude-import", "--output=-", @@ -177,7 +190,7 @@ TEST_F(DriverTest, StdoutOutput) { .success); EXPECT_THAT(test_error_stream_.TakeStr(), StrEq("")); // The default is textual assembly. - EXPECT_THAT(test_output_stream_.TakeStr(), ContainsRegex("Main:")); + EXPECT_THAT(test_output_stream_.TakeStr(), ContainsRegex("main:")); EXPECT_TRUE(driver_ .RunCommand({"compile", "--no-prelude-import", "--output=-", @@ -198,7 +211,7 @@ TEST_F(DriverTest, FileOutput) { // Use explicit filenames as the default output filename is computed from // this, and we can use this to validate output. - MakeTestFile("fn Main() {}", "test.carbon"); + MakeTestFile("fn Run() {}", "test.carbon"); // Object output (the default) uses `.o`. // TODO: This should actually reflect the platform defaults. @@ -221,18 +234,41 @@ TEST_F(DriverTest, FileOutput) { .success); EXPECT_THAT(test_error_stream_.TakeStr(), StrEq("")); // TODO: This may need to be tailored to other assembly formats. - EXPECT_THAT(*Testing::ReadFile("test.s"), ContainsRegex("Main:")); + EXPECT_THAT(*Testing::ReadFile("test.s"), ContainsRegex("main:")); +} + +TEST_F(DriverTest, Link) { + auto scope = ScopedTempWorkingDir(); + + // First compile a file to get a linkable object. + MakeTestFile("fn Run() {}", "test.carbon"); + ASSERT_TRUE( + driver_.RunCommand({"compile", "--no-prelude-import", "test.carbon"}) + .success) + << test_error_stream_.TakeStr(); + + // Now link this into a binary. Note that we suppress building runtimes on + // demand here as no runtimes should be needed for the empty program. + EXPECT_TRUE(driver_ + .RunCommand({"--no-build-runtimes", "link", "--output=test", + "test.o"}) + .success); + EXPECT_THAT(test_error_stream_.TakeStr(), StrEq("")); + + // Ensure we wrote an executable file of some form with the correct name. + // TODO: We may need to update this if we implicitly synthesize a + // platform-specific `.exe` suffix or something similar. + auto result = llvm::object::createBinary("test"); + if (auto error = result.takeError()) { + FAIL() << toString(std::move(error)); + } + // Executables are also classified as object files. + EXPECT_TRUE(result->getBinary()->isObject()); } TEST_F(DriverTest, ConfigJson) { - // The command won't succeed because we won't find the installation digest to - // print. We can still test the overall output is valid JSON. - EXPECT_FALSE(driver_.RunCommand({"config", "--json"}).success); - - // Ensure the failure was what we expected. - EXPECT_THAT( - test_error_stream_.TakeStr(), - HasSubstr("error: unable to read the installation's digest file")); + EXPECT_TRUE(driver_.RunCommand({"config", "--json"}).success); + EXPECT_THAT(test_error_stream_.TakeStr(), StrEq("")); // Make sure the output parses as JSON. std::string output = test_output_stream_.TakeStr(); diff --git a/toolchain/driver/link_subcommand.cpp b/toolchain/driver/link_subcommand.cpp index 702fbc2df6f0..715e30013dfe 100644 --- a/toolchain/driver/link_subcommand.cpp +++ b/toolchain/driver/link_subcommand.cpp @@ -81,8 +81,11 @@ auto LinkSubcommand::Run(DriverEnv& driver_env) -> DriverResult { ? runner.RunWithPrebuiltRuntimes(clang_args, *driver_env.prebuilt_runtimes, driver_env.enable_leaking) - : runner.Run(clang_args, driver_env.runtimes_cache, - *driver_env.thread_pool, driver_env.enable_leaking); + : driver_env.build_runtimes_on_demand + ? runner.Run(clang_args, driver_env.runtimes_cache, + *driver_env.thread_pool, driver_env.enable_leaking) + : runner.RunWithNoRuntimes(clang_args, driver_env.enable_leaking); + if (!run_result.ok()) { // This is not a Clang failure, but a failure to even run Clang, so we need // to diagnose it here.