Forbid Rust source_sets.
These are not currently used by either Fuchsia or Chromium,
as current best practice is to use groups instead. It's not clear
what the purpose of a Rust source_set would be, since only the
crate_root is fed to rustc anyway; any other sources just act
as implicit dependencies.
A subsequent commit will allow Rust targets to depend upon C++
source sets, and that change is much less invasive if Rust
source sets are not possible.
Change-Id: I28f5c931c793aa80fbf7116519cc22ae3631efc2
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/8240
Reviewed-by: Brett Wilson <brettw@chromium.org>
Commit-Queue: Brett Wilson <brettw@chromium.org>
diff --git a/src/gn/functions_target.cc b/src/gn/functions_target.cc
index 5ce0901..0864db5 100644
--- a/src/gn/functions_target.cc
+++ b/src/gn/functions_target.cc
@@ -740,8 +740,7 @@
const char kSourceSet_Help[] =
R"(source_set: Declare a source set target.
- The language of a source_set target is determined by the extensions present
- in its sources.
+ Only C-language source sets are supported at the moment.
C-language source_sets
@@ -766,13 +765,6 @@
not from the intermediate targets." There is no way to express this concept
when linking multiple static libraries into a shared library.
-Rust-language source_sets
-
- A Rust source set is a collection of sources that get passed along to the
- final target that depends on it. No compilation is performed, and the source
- files are simply added as dependencies on the eventual rustc invocation that
- would produce a binary.
-
Variables
)" CONFIG_VALUES_VARS_HELP DEPS_VARS DEPENDENT_CONFIG_VARS GENERAL_TARGET_VARS;
diff --git a/src/gn/ninja_binary_target_writer.cc b/src/gn/ninja_binary_target_writer.cc
index 1a3e9b9..cde6be6 100644
--- a/src/gn/ninja_binary_target_writer.cc
+++ b/src/gn/ninja_binary_target_writer.cc
@@ -196,9 +196,49 @@
void NinjaBinaryTargetWriter::AddSourceSetFiles(
const Target* source_set,
UniqueVector<OutputFile>* obj_files) const {
- // Just add all sources to the list.
+ std::vector<OutputFile> tool_outputs; // Prevent allocation in loop.
+
+ // Compute object files for all sources. Only link the first output from
+ // the tool if there are more than one.
for (const auto& source : source_set->sources()) {
- obj_files->push_back(OutputFile(settings_->build_settings(), source));
+ const char* tool_name = Tool::kToolNone;
+ if (source_set->GetOutputFilesForSource(source, &tool_name, &tool_outputs))
+ obj_files->push_back(tool_outputs[0]);
+ }
+
+ // Add MSVC precompiled header object files. GCC .gch files are not object
+ // files so they are omitted.
+ if (source_set->config_values().has_precompiled_headers()) {
+ if (source_set->source_types_used().Get(SourceFile::SOURCE_C)) {
+ const CTool* tool = source_set->toolchain()->GetToolAsC(CTool::kCToolCc);
+ if (tool && tool->precompiled_header_type() == CTool::PCH_MSVC) {
+ GetPCHOutputFiles(source_set, CTool::kCToolCc, &tool_outputs);
+ obj_files->Append(tool_outputs.begin(), tool_outputs.end());
+ }
+ }
+ if (source_set->source_types_used().Get(SourceFile::SOURCE_CPP)) {
+ const CTool* tool = source_set->toolchain()->GetToolAsC(CTool::kCToolCxx);
+ if (tool && tool->precompiled_header_type() == CTool::PCH_MSVC) {
+ GetPCHOutputFiles(source_set, CTool::kCToolCxx, &tool_outputs);
+ obj_files->Append(tool_outputs.begin(), tool_outputs.end());
+ }
+ }
+ if (source_set->source_types_used().Get(SourceFile::SOURCE_M)) {
+ const CTool* tool =
+ source_set->toolchain()->GetToolAsC(CTool::kCToolObjC);
+ if (tool && tool->precompiled_header_type() == CTool::PCH_MSVC) {
+ GetPCHOutputFiles(source_set, CTool::kCToolObjC, &tool_outputs);
+ obj_files->Append(tool_outputs.begin(), tool_outputs.end());
+ }
+ }
+ if (source_set->source_types_used().Get(SourceFile::SOURCE_MM)) {
+ const CTool* tool =
+ source_set->toolchain()->GetToolAsC(CTool::kCToolObjCxx);
+ if (tool && tool->precompiled_header_type() == CTool::PCH_MSVC) {
+ GetPCHOutputFiles(source_set, CTool::kCToolObjCxx, &tool_outputs);
+ obj_files->Append(tool_outputs.begin(), tool_outputs.end());
+ }
+ }
}
}
diff --git a/src/gn/ninja_binary_target_writer.h b/src/gn/ninja_binary_target_writer.h
index 7539ebb..69f5b50 100644
--- a/src/gn/ninja_binary_target_writer.h
+++ b/src/gn/ninja_binary_target_writer.h
@@ -65,8 +65,8 @@
void WriteLibs(std::ostream& out, const Tool* tool);
void WriteFrameworks(std::ostream& out, const Tool* tool);
- virtual void AddSourceSetFiles(const Target* source_set,
- UniqueVector<OutputFile>* obj_files) const;
+ void AddSourceSetFiles(const Target* source_set,
+ UniqueVector<OutputFile>* obj_files) const;
// Cached version of the prefix used for rule types for this toolchain.
std::string rule_prefix_;
diff --git a/src/gn/ninja_c_binary_target_writer.cc b/src/gn/ninja_c_binary_target_writer.cc
index ebc2d17..5fa646f 100644
--- a/src/gn/ninja_c_binary_target_writer.cc
+++ b/src/gn/ninja_c_binary_target_writer.cc
@@ -639,54 +639,3 @@
}
return true;
}
-
-// Appends the object files generated by the given source set to the given
-// output vector.
-void NinjaCBinaryTargetWriter::AddSourceSetFiles(
- const Target* source_set,
- UniqueVector<OutputFile>* obj_files) const {
- std::vector<OutputFile> tool_outputs; // Prevent allocation in loop.
-
- // Compute object files for all sources. Only link the first output from
- // the tool if there are more than one.
- for (const auto& source : source_set->sources()) {
- const char* tool_name = Tool::kToolNone;
- if (source_set->GetOutputFilesForSource(source, &tool_name, &tool_outputs))
- obj_files->push_back(tool_outputs[0]);
- }
-
- // Add MSVC precompiled header object files. GCC .gch files are not object
- // files so they are omitted.
- if (source_set->config_values().has_precompiled_headers()) {
- if (source_set->source_types_used().Get(SourceFile::SOURCE_C)) {
- const CTool* tool = source_set->toolchain()->GetToolAsC(CTool::kCToolCc);
- if (tool && tool->precompiled_header_type() == CTool::PCH_MSVC) {
- GetPCHOutputFiles(source_set, CTool::kCToolCc, &tool_outputs);
- obj_files->Append(tool_outputs.begin(), tool_outputs.end());
- }
- }
- if (source_set->source_types_used().Get(SourceFile::SOURCE_CPP)) {
- const CTool* tool = source_set->toolchain()->GetToolAsC(CTool::kCToolCxx);
- if (tool && tool->precompiled_header_type() == CTool::PCH_MSVC) {
- GetPCHOutputFiles(source_set, CTool::kCToolCxx, &tool_outputs);
- obj_files->Append(tool_outputs.begin(), tool_outputs.end());
- }
- }
- if (source_set->source_types_used().Get(SourceFile::SOURCE_M)) {
- const CTool* tool =
- source_set->toolchain()->GetToolAsC(CTool::kCToolObjC);
- if (tool && tool->precompiled_header_type() == CTool::PCH_MSVC) {
- GetPCHOutputFiles(source_set, CTool::kCToolObjC, &tool_outputs);
- obj_files->Append(tool_outputs.begin(), tool_outputs.end());
- }
- }
- if (source_set->source_types_used().Get(SourceFile::SOURCE_MM)) {
- const CTool* tool =
- source_set->toolchain()->GetToolAsC(CTool::kCToolObjCxx);
- if (tool && tool->precompiled_header_type() == CTool::PCH_MSVC) {
- GetPCHOutputFiles(source_set, CTool::kCToolObjCxx, &tool_outputs);
- obj_files->Append(tool_outputs.begin(), tool_outputs.end());
- }
- }
- }
-}
diff --git a/src/gn/ninja_c_binary_target_writer.h b/src/gn/ninja_c_binary_target_writer.h
index 55cc0fc..ee6b6ac 100644
--- a/src/gn/ninja_c_binary_target_writer.h
+++ b/src/gn/ninja_c_binary_target_writer.h
@@ -22,11 +22,6 @@
void Run() override;
- protected:
- // Adds source_set files to the list of object files.
- void AddSourceSetFiles(const Target* source_set,
- UniqueVector<OutputFile>* obj_files) const override;
-
private:
using OutputFileSet = std::set<OutputFile>;
diff --git a/src/gn/ninja_rust_binary_target_writer.cc b/src/gn/ninja_rust_binary_target_writer.cc
index 8a907bb..abe903f 100644
--- a/src/gn/ninja_rust_binary_target_writer.cc
+++ b/src/gn/ninja_rust_binary_target_writer.cc
@@ -122,72 +122,68 @@
// -Ldependency rustdeps, and non-public source_sets get passed in as normal
// source files
UniqueVector<OutputFile> deps;
- AddSourceSetFiles(target_, &deps);
- if (target_->output_type() == Target::SOURCE_SET) {
- WriteSharedVars(target_->toolchain()->substitution_bits());
- WriteSourceSetStamp(deps.vector());
- } else {
- WriteCompilerVars();
- UniqueVector<const Target*> linkable_deps;
- UniqueVector<const Target*> non_linkable_deps;
- UniqueVector<const Target*> framework_deps;
- GetDeps(&deps, &linkable_deps, &non_linkable_deps, &framework_deps);
+ DCHECK(target_->output_type() != Target::SOURCE_SET);
+ WriteCompilerVars();
+ UniqueVector<const Target*> linkable_deps;
+ UniqueVector<const Target*> non_linkable_deps;
+ UniqueVector<const Target*> framework_deps;
+ GetDeps(&deps, &linkable_deps, &non_linkable_deps, &framework_deps);
+ AppendSourcesToImplicitDeps(&deps);
- if (!input_dep.value().empty())
- order_only_deps.push_back(input_dep);
+ if (!input_dep.value().empty())
+ order_only_deps.push_back(input_dep);
- std::vector<OutputFile> rustdeps;
- std::vector<OutputFile> nonrustdeps;
- for (const auto* framework_dep : framework_deps) {
- order_only_deps.push_back(framework_dep->dependency_output_file());
- }
- for (const auto* non_linkable_dep : non_linkable_deps) {
- if (non_linkable_dep->source_types_used().RustSourceUsed() &&
- non_linkable_dep->output_type() != Target::SOURCE_SET) {
- rustdeps.push_back(non_linkable_dep->dependency_output_file());
- }
- order_only_deps.push_back(non_linkable_dep->dependency_output_file());
- }
- for (const auto* linkable_dep : linkable_deps) {
- if (linkable_dep->source_types_used().RustSourceUsed()) {
- rustdeps.push_back(linkable_dep->link_output_file());
- } else {
- nonrustdeps.push_back(linkable_dep->link_output_file());
- }
- deps.push_back(linkable_dep->dependency_output_file());
- }
-
- // Rust libraries specified by paths.
- for (ConfigValuesIterator iter(target_); !iter.done(); iter.Next()) {
- const ConfigValues& cur = iter.cur();
- for (const auto& e : cur.externs()) {
- if (e.second.is_source_file()) {
- deps.push_back(
- OutputFile(settings_->build_settings(), e.second.source_file()));
- }
- }
- }
-
- std::vector<OutputFile> transitive_rustlibs;
- for (const auto* dep :
- target_->rust_values().transitive_libs().GetOrdered()) {
- if (dep->source_types_used().RustSourceUsed()) {
- transitive_rustlibs.push_back(dep->dependency_output_file());
- }
- }
-
- std::vector<OutputFile> tool_outputs;
- SubstitutionWriter::ApplyListToLinkerAsOutputFile(
- target_, tool_, tool_->outputs(), &tool_outputs);
- WriteCompilerBuildLine(target_->rust_values().crate_root(), deps.vector(),
- order_only_deps, tool_->name(), tool_outputs);
-
- std::vector<const Target*> extern_deps(linkable_deps.vector());
- std::copy(non_linkable_deps.begin(), non_linkable_deps.end(),
- std::back_inserter(extern_deps));
- WriteExterns(extern_deps);
- WriteRustdeps(transitive_rustlibs, rustdeps, nonrustdeps);
+ std::vector<OutputFile> rustdeps;
+ std::vector<OutputFile> nonrustdeps;
+ for (const auto* framework_dep : framework_deps) {
+ order_only_deps.push_back(framework_dep->dependency_output_file());
}
+ for (const auto* non_linkable_dep : non_linkable_deps) {
+ if (non_linkable_dep->source_types_used().RustSourceUsed() &&
+ non_linkable_dep->output_type() != Target::SOURCE_SET) {
+ rustdeps.push_back(non_linkable_dep->dependency_output_file());
+ }
+ order_only_deps.push_back(non_linkable_dep->dependency_output_file());
+ }
+ for (const auto* linkable_dep : linkable_deps) {
+ if (linkable_dep->source_types_used().RustSourceUsed()) {
+ rustdeps.push_back(linkable_dep->link_output_file());
+ } else {
+ nonrustdeps.push_back(linkable_dep->link_output_file());
+ }
+ deps.push_back(linkable_dep->dependency_output_file());
+ }
+
+ // Rust libraries specified by paths.
+ for (ConfigValuesIterator iter(target_); !iter.done(); iter.Next()) {
+ const ConfigValues& cur = iter.cur();
+ for (const auto& e : cur.externs()) {
+ if (e.second.is_source_file()) {
+ deps.push_back(
+ OutputFile(settings_->build_settings(), e.second.source_file()));
+ }
+ }
+ }
+
+ std::vector<OutputFile> transitive_rustlibs;
+ for (const auto* dep :
+ target_->rust_values().transitive_libs().GetOrdered()) {
+ if (dep->source_types_used().RustSourceUsed()) {
+ transitive_rustlibs.push_back(dep->dependency_output_file());
+ }
+ }
+
+ std::vector<OutputFile> tool_outputs;
+ SubstitutionWriter::ApplyListToLinkerAsOutputFile(
+ target_, tool_, tool_->outputs(), &tool_outputs);
+ WriteCompilerBuildLine(target_->rust_values().crate_root(), deps.vector(),
+ order_only_deps, tool_->name(), tool_outputs);
+
+ std::vector<const Target*> extern_deps(linkable_deps.vector());
+ std::copy(non_linkable_deps.begin(), non_linkable_deps.end(),
+ std::back_inserter(extern_deps));
+ WriteExterns(extern_deps);
+ WriteRustdeps(transitive_rustlibs, rustdeps, nonrustdeps);
}
void NinjaRustBinaryTargetWriter::WriteCompilerVars() {
@@ -205,6 +201,18 @@
WriteSharedVars(subst);
}
+void NinjaRustBinaryTargetWriter::AppendSourcesToImplicitDeps(
+ UniqueVector<OutputFile>* deps) const {
+ // Only the crate_root file needs to be given to rustc as input.
+ // Any other 'sources' are just implicit deps.
+ // Most Rust targets won't bother specifying the "sources =" line
+ // because it is handled sufficiently by crate_root and the generation
+ // of depfiles by rustc. But for those which do...
+ for (const auto& source : target_->sources()) {
+ deps->push_back(OutputFile(settings_->build_settings(), source));
+ }
+}
+
void NinjaRustBinaryTargetWriter::WriteExterns(
const std::vector<const Target*>& deps) {
out_ << " externs =";
diff --git a/src/gn/ninja_rust_binary_target_writer.h b/src/gn/ninja_rust_binary_target_writer.h
index cd845d5..4d6e146 100644
--- a/src/gn/ninja_rust_binary_target_writer.h
+++ b/src/gn/ninja_rust_binary_target_writer.h
@@ -26,9 +26,10 @@
const std::vector<OutputFile>& order_only_deps);
void WriteExterns(const std::vector<const Target*>& deps);
void WriteRustdeps(const std::vector<OutputFile>& transitive_rustdeps,
- const std::vector<OutputFile>& rustdeps,
- const std::vector<OutputFile>& nonrustdeps);
+ const std::vector<OutputFile>& rustdeps,
+ const std::vector<OutputFile>& nonrustdeps);
void WriteEdition();
+ void AppendSourcesToImplicitDeps(UniqueVector<OutputFile>* deps) const;
const RustTool* tool_;
diff --git a/src/gn/ninja_rust_binary_target_writer_unittest.cc b/src/gn/ninja_rust_binary_target_writer_unittest.cc
index ded290a..70a7d00 100644
--- a/src/gn/ninja_rust_binary_target_writer_unittest.cc
+++ b/src/gn/ninja_rust_binary_target_writer_unittest.cc
@@ -26,39 +26,13 @@
target.sources().push_back(SourceFile("//foo/main.rs"));
target.source_types_used().Set(SourceFile::SOURCE_RS);
target.SetToolchain(setup.toolchain());
- ASSERT_TRUE(target.OnResolved(&err));
-
- // Source set itself.
- {
- std::ostringstream out;
- NinjaRustBinaryTargetWriter writer(&target, out);
- writer.Run();
-
- const char expected[] =
- "root_out_dir = .\n"
- "target_out_dir = obj/foo\n"
- "target_output_name = bar\n"
- "\n"
- "build obj/foo/bar.stamp: stamp ../../foo/input1.rs "
- "../../foo/main.rs\n";
- std::string out_str = out.str();
- EXPECT_EQ(expected, out_str) << out_str;
- }
+ ASSERT_FALSE(target.OnResolved(&err));
}
TEST_F(NinjaRustBinaryTargetWriterTest, RustExecutable) {
Err err;
TestWithScope setup;
- Target source_set(setup.settings(), Label(SourceDir("//foo/"), "sources"));
- source_set.set_output_type(Target::SOURCE_SET);
- source_set.visibility().SetPublic();
- source_set.sources().push_back(SourceFile("//foo/input1.rs"));
- source_set.sources().push_back(SourceFile("//foo/input2.rs"));
- source_set.source_types_used().Set(SourceFile::SOURCE_RS);
- source_set.SetToolchain(setup.toolchain());
- ASSERT_TRUE(source_set.OnResolved(&err));
-
Target target(setup.settings(), Label(SourceDir("//foo/"), "bar"));
target.set_output_type(Target::EXECUTABLE);
target.visibility().SetPublic();
@@ -68,7 +42,6 @@
target.source_types_used().Set(SourceFile::SOURCE_RS);
target.rust_values().set_crate_root(main);
target.rust_values().crate_name() = "foo_bar";
- target.private_deps().push_back(LabelTargetPair(&source_set));
target.SetToolchain(setup.toolchain());
ASSERT_TRUE(target.OnResolved(&err));
@@ -89,8 +62,7 @@
"target_output_name = bar\n"
"\n"
"build ./foo_bar: rust_bin ../../foo/main.rs | ../../foo/input3.rs "
- "../../foo/main.rs ../../foo/input1.rs ../../foo/input2.rs || "
- "obj/foo/sources.stamp\n"
+ "../../foo/main.rs\n"
" externs =\n"
" rustdeps =\n";
std::string out_str = out.str();
@@ -495,15 +467,6 @@
Err err;
TestWithScope setup;
- Target source_set(setup.settings(), Label(SourceDir("//foo/"), "sources"));
- source_set.set_output_type(Target::SOURCE_SET);
- source_set.visibility().SetPublic();
- source_set.sources().push_back(SourceFile("//foo/input1.rs"));
- source_set.sources().push_back(SourceFile("//foo/input2.rs"));
- source_set.source_types_used().Set(SourceFile::SOURCE_RS);
- source_set.SetToolchain(setup.toolchain());
- ASSERT_TRUE(source_set.OnResolved(&err));
-
Target target(setup.settings(), Label(SourceDir("//foo/"), "bar"));
target.set_output_type(Target::EXECUTABLE);
target.visibility().SetPublic();
@@ -515,7 +478,6 @@
target.set_output_dir(SourceDir("//out/Debug/foo/"));
target.rust_values().set_crate_root(main);
target.rust_values().crate_name() = "foo_bar";
- target.private_deps().push_back(LabelTargetPair(&source_set));
target.SetToolchain(setup.toolchain());
ASSERT_TRUE(target.OnResolved(&err));
@@ -536,8 +498,7 @@
"target_output_name = bar\n"
"\n"
"build ./foo_bar.exe: rust_bin ../../foo/main.rs | ../../foo/input3.rs "
- "../../foo/main.rs ../../foo/input1.rs ../../foo/input2.rs || "
- "obj/foo/sources.stamp\n"
+ "../../foo/main.rs\n"
" externs =\n"
" rustdeps =\n";
std::string out_str = out.str();
diff --git a/src/gn/target.cc b/src/gn/target.cc
index 93ad7c0..ef91167 100644
--- a/src/gn/target.cc
+++ b/src/gn/target.cc
@@ -383,6 +383,8 @@
if (!FillOutputFiles(err))
return false;
+ if (!CheckSourceSetLanguages(err))
+ return false;
if (!CheckVisibility(err))
return false;
if (!CheckTestonly(err))
@@ -934,6 +936,17 @@
return true;
}
+bool Target::CheckSourceSetLanguages(Err* err) const {
+ if (output_type() == Target::SOURCE_SET &&
+ source_types_used().RustSourceUsed()) {
+ *err = Err(defined_from(), "source_set contained Rust code.",
+ label().GetUserVisibleName(false) +
+ " has Rust code. Only C/C++ source_sets are supported.");
+ return false;
+ }
+ return true;
+}
+
bool Target::CheckTestonly(Err* err) const {
// If the current target is marked testonly, it can include both testonly
// and non-testonly targets, so there's nothing to check.
diff --git a/src/gn/target.h b/src/gn/target.h
index 700859c..44f7e17 100644
--- a/src/gn/target.h
+++ b/src/gn/target.h
@@ -407,6 +407,7 @@
bool CheckAssertNoDeps(Err* err) const;
void CheckSourcesGenerated() const;
void CheckSourceGenerated(const SourceFile& source) const;
+ bool CheckSourceSetLanguages(Err* err) const;
OutputType output_type_ = UNKNOWN;
std::string output_name_;
