GN: Schedule config loads for sub-configs.
Previously the Builder did not schedule file loads for configs of configs.
This meant that if a BUILD file contained only configs referenced by other
configs (rather than configs referenced by targets, or targets referenced by
other targets), it would never be loaded. The result will be a failed run with
an undefined object.
BUG=536844
Review URL: https://codereview.chromium.org/1373903002
Cr-Original-Commit-Position: refs/heads/master@{#351089}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 1b1e7e8c8f3105b6c9811ff28947961eb96153c3
diff --git a/tools/gn/builder.cc b/tools/gn/builder.cc
index fbd4d57..3a0c1f5 100644
--- a/tools/gn/builder.cc
+++ b/tools/gn/builder.cc
@@ -240,6 +240,17 @@
Config* config = record->item()->AsConfig();
if (!AddDeps(record, config->configs(), err))
return false;
+
+ // Make sure all deps of this config are scheduled to be loaded. For other
+ // item types like targets, the "should generate" flag is propogated around
+ // to mark whether this should happen. We could call
+ // RecursiveSetShouldGenerate to do this step here, but since configs nor
+ // anything they depend on is actually written, the "generate" flag isn't
+ // relevant and means extra book keeping. Just force load any deps of this
+ // config.
+ for (const auto& cur : record->all_deps())
+ ScheduleItemLoadIfNecessary(cur);
+
return true;
}
diff --git a/tools/gn/builder_unittest.cc b/tools/gn/builder_unittest.cc
index ffd1a9d..a5356a2 100644
--- a/tools/gn/builder_unittest.cc
+++ b/tools/gn/builder_unittest.cc
@@ -4,6 +4,7 @@
#include "testing/gtest/include/gtest/gtest.h"
#include "tools/gn/builder.h"
+#include "tools/gn/config.h"
#include "tools/gn/loader.h"
#include "tools/gn/target.h"
#include "tools/gn/test_with_scope.h"
@@ -32,8 +33,17 @@
return files_.empty();
}
- // Returns true if two loads have been requested and they match the given
- // file. This will clear the records so it will be empty for the next call.
+ // Returns true if one/two loads have been requested and they match the given
+ // file(s). This will clear the records so it will be empty for the next call.
+ bool HasLoadedOne(const SourceFile& file) {
+ if (files_.size() != 1u) {
+ files_.clear();
+ return false;
+ }
+ bool match = (files_[0] == file);
+ files_.clear();
+ return match;
+ }
bool HasLoadedTwo(const SourceFile& a, const SourceFile& b) {
if (files_.size() != 2u) {
files_.clear();
@@ -216,3 +226,24 @@
// It should have gotten pushed to B.
EXPECT_TRUE(b_record->should_generate());
}
+
+// Tests that configs applied to a config get loaded (bug 536844).
+TEST_F(BuilderTest, ConfigLoad) {
+ SourceDir toolchain_dir = settings_.toolchain_label().dir();
+ std::string toolchain_name = settings_.toolchain_label().name();
+
+ // Construct a dependency chain: A -> B -> C. Define A first with a
+ // forward-reference to B, then C, then B to test the different orders that
+ // the dependencies are hooked up.
+ Label a_label(SourceDir("//a/"), "a", toolchain_dir, toolchain_name);
+ Label b_label(SourceDir("//b/"), "b", toolchain_dir, toolchain_name);
+ Label c_label(SourceDir("//c/"), "c", toolchain_dir, toolchain_name);
+
+ // The builder will take ownership of the pointers.
+ Config* a = new Config(&settings_, a_label);
+ a->configs().push_back(LabelConfigPair(b_label));
+ builder_->ItemDefined(scoped_ptr<Item>(a));
+
+ // Should have requested that B is loaded.
+ EXPECT_TRUE(loader_->HasLoadedOne(SourceFile("//b/BUILD.gn")));
+}
