Support importing generated configs
Change-Id: I260851684f809e05d0b486f62950cccc61cc53bc
diff --git a/aos/BUILD b/aos/BUILD
index d35e74c..d05a086 100644
--- a/aos/BUILD
+++ b/aos/BUILD
@@ -469,27 +469,9 @@
"configuration_test.cc",
],
data = [
- "testdata/backwards.json",
- "testdata/config1.json",
- "testdata/config1_bad.json",
- "testdata/config1_multinode.json",
- "testdata/config2.json",
- "testdata/config2_multinode.json",
- "testdata/config3.json",
- "testdata/expected.json",
- "testdata/expected_merge_with.json",
- "testdata/expected_multinode.json",
- "testdata/good_multinode.json",
- "testdata/good_multinode_hostnames.json",
- "testdata/invalid_channel_name1.json",
- "testdata/invalid_channel_name2.json",
- "testdata/invalid_channel_name3.json",
- "testdata/invalid_destination_node.json",
- "testdata/invalid_nodes.json",
- "testdata/invalid_source_node.json",
- "testdata/self_forward.json",
"//aos/events:pingpong_config.json",
"//aos/events:pong.bfbs",
+ "//aos/testdata:test_configs",
],
deps = [
":configuration",
diff --git a/aos/config.bzl b/aos/config.bzl
index 86ab09e..d6dcf1e 100644
--- a/aos/config.bzl
+++ b/aos/config.bzl
@@ -1,6 +1,9 @@
load("//tools/build_rules:label.bzl", "expand_label")
-AosConfigInfo = provider(fields = ["transitive_flatbuffers", "transitive_src"])
+AosConfigInfo = provider(fields = [
+ "transitive_flatbuffers",
+ "transitive_src",
+])
def aos_config(name, src, flatbuffers = [], deps = [], visibility = None):
_aos_config(
@@ -26,7 +29,7 @@
ctx.actions.run(
outputs = [ctx.outputs.config, ctx.outputs.stripped_config],
inputs = all_files,
- arguments = [ctx.outputs.config.path, ctx.outputs.stripped_config.path, ctx.files.src[0].path] + [f.path for f in flatbuffers_depset.to_list()],
+ arguments = [ctx.outputs.config.path, ctx.outputs.stripped_config.path, ctx.files.src[0].short_path, ctx.bin_dir.path] + [f.path for f in flatbuffers_depset.to_list()],
progress_message = "Flattening config",
executable = ctx.executable._config_flattener,
)
diff --git a/aos/config_flattener.cc b/aos/config_flattener.cc
index ee2f376..d39cd50 100644
--- a/aos/config_flattener.cc
+++ b/aos/config_flattener.cc
@@ -11,19 +11,23 @@
namespace aos {
int Main(int argc, char **argv) {
- CHECK_GE(argc, 4) << ": Too few arguments";
+ CHECK_GE(argc, 5) << ": Too few arguments";
const char *full_output = argv[1];
const char *stripped_output = argv[2];
const char *config_path = argv[3];
+ // In order to support not only importing things by absolute path, but also
+ // importing the outputs of genrules (rather than just manually written
+ // files), we need to tell ReadConfig where the generated files directory is.
+ const char *bazel_outs_directory = argv[4];
VLOG(1) << "Reading " << config_path;
FlatbufferDetachedBuffer<Configuration> config =
- configuration::ReadConfig(config_path);
+ configuration::ReadConfig(config_path, {bazel_outs_directory});
std::vector<aos::FlatbufferString<reflection::Schema>> schemas;
- for (int i = 4; i < argc; ++i) {
+ for (int i = 5; i < argc; ++i) {
schemas.emplace_back(util::ReadFileToStringOrDie(argv[i]));
}
diff --git a/aos/configuration.cc b/aos/configuration.cc
index b0ff973..bcb4de6 100644
--- a/aos/configuration.cc
+++ b/aos/configuration.cc
@@ -84,14 +84,44 @@
: filename.substr(0, last_slash_pos + 1);
}
+std::string AbsolutePath(const std::string_view filename) {
+ // Uses an std::string so that we know the input will be null-terminated.
+ const std::string terminated_file(filename);
+ char buffer[PATH_MAX];
+ PCHECK(NULL != realpath(terminated_file.c_str(), buffer));
+ return buffer;
+}
+
FlatbufferDetachedBuffer<Configuration> ReadConfig(
- const std::string_view path, absl::btree_set<std::string> *visited_paths) {
+ const std::string_view path, absl::btree_set<std::string> *visited_paths,
+ const std::vector<std::string_view> &extra_import_paths) {
+ std::string raw_path(path);
+ if (!util::PathExists(path)) {
+ const bool path_is_absolute = path.size() > 0 && path[0] == '/';
+ if (path_is_absolute) {
+ CHECK(extra_import_paths.empty())
+ << "Can't specify extra import paths if attempting to read a config "
+ "file from an absolute path (path is "
+ << path << ").";
+ }
+
+ bool found_path = false;
+ for (const auto &import_path : extra_import_paths) {
+ raw_path = std::string(import_path) + "/" + std::string(path);
+ if (util::PathExists(raw_path)) {
+ found_path = true;
+ break;
+ }
+ }
+ CHECK(found_path) << ": Failed to find file " << path << ".";
+ }
flatbuffers::DetachedBuffer buffer = JsonToFlatbuffer(
- util::ReadFileToStringOrDie(path), ConfigurationTypeTable());
+ util::ReadFileToStringOrDie(raw_path), ConfigurationTypeTable());
CHECK_GT(buffer.size(), 0u) << ": Failed to parse JSON file";
FlatbufferDetachedBuffer<Configuration> config(std::move(buffer));
+
// Depth first. Take the following example:
//
// config1.json:
@@ -123,8 +153,16 @@
// config. That means that it needs to be merged into the imported configs,
// not the other way around.
+ const std::string absolute_path = AbsolutePath(raw_path);
// Track that we have seen this file before recursing.
- visited_paths->insert(::std::string(path));
+ if (!visited_paths->insert(absolute_path).second) {
+ for (const auto &visited_path : *visited_paths) {
+ LOG(INFO) << "Already visited: " << visited_path;
+ }
+ LOG(FATAL)
+ << "Already imported " << path << " (i.e. " << absolute_path
+ << "). See above for the files that have already been processed.";
+ }
if (config.message().has_imports()) {
// Capture the imports.
@@ -138,18 +176,15 @@
FlatbufferDetachedBuffer<Configuration> merged_config =
FlatbufferDetachedBuffer<Configuration>::Empty();
- const ::std::string folder(ExtractFolder(path));
-
+ const std::string path_folder(ExtractFolder(path));
for (const flatbuffers::String *str : *v) {
- const ::std::string included_config = folder + str->c_str();
- // Abort on any paths we have already seen.
- CHECK(visited_paths->find(included_config) == visited_paths->end())
- << ": Found duplicate file " << included_config << " while reading "
- << path;
+ const std::string included_config =
+ path_folder + "/" + std::string(str->string_view());
// And them merge everything in.
merged_config = MergeFlatBuffers(
- merged_config, ReadConfig(included_config, visited_paths));
+ merged_config,
+ ReadConfig(included_config, visited_paths, extra_import_paths));
}
// Finally, merge this file in.
@@ -446,10 +481,11 @@
}
FlatbufferDetachedBuffer<Configuration> ReadConfig(
- const std::string_view path) {
+ const std::string_view path,
+ const std::vector<std::string_view> &import_paths) {
// We only want to read a file once. So track the visited files in a set.
absl::btree_set<std::string> visited_paths;
- return MergeConfiguration(ReadConfig(path, &visited_paths));
+ return MergeConfiguration(ReadConfig(path, &visited_paths, import_paths));
}
FlatbufferDetachedBuffer<Configuration> MergeWithConfig(
diff --git a/aos/configuration.h b/aos/configuration.h
index a01a902..4ada459 100644
--- a/aos/configuration.h
+++ b/aos/configuration.h
@@ -20,7 +20,8 @@
// Reads a json configuration. This includes all imports and merges. Note:
// duplicate imports will result in a CHECK.
FlatbufferDetachedBuffer<Configuration> ReadConfig(
- const std::string_view path);
+ const std::string_view path,
+ const std::vector<std::string_view> &extra_import_paths = {});
// Sorts and merges entries in a config.
FlatbufferDetachedBuffer<Configuration> MergeConfiguration(
diff --git a/aos/testdata/BUILD b/aos/testdata/BUILD
new file mode 100644
index 0000000..816ad7e
--- /dev/null
+++ b/aos/testdata/BUILD
@@ -0,0 +1,55 @@
+load("//aos:config.bzl", "aos_config")
+
+filegroup(
+ name = "test_configs",
+ srcs = [
+ "backwards.json",
+ "config1.json",
+ "config1_bad.json",
+ "config1_multinode.json",
+ "config2.json",
+ "config2_multinode.json",
+ "config3.json",
+ "expected.json",
+ "expected_merge_with.json",
+ "expected_multinode.json",
+ "good_multinode.json",
+ "good_multinode_hostnames.json",
+ "invalid_channel_name1.json",
+ "invalid_channel_name2.json",
+ "invalid_channel_name3.json",
+ "invalid_destination_node.json",
+ "invalid_nodes.json",
+ "invalid_source_node.json",
+ "self_forward.json",
+ ],
+ visibility = ["//aos:__pkg__"],
+)
+
+genrule(
+ name = "generated_test_config",
+ outs = ["generated.json"],
+ cmd = "echo '" +
+ """{
+ "imports": [
+ "empty.json"
+ ]
+}""" + "'> $@",
+)
+
+aos_config(
+ name = "empty_config",
+ src = "empty.json",
+)
+
+aos_config(
+ name = "generated_config",
+ src = "generated.json",
+ deps = [":empty_config"],
+)
+
+aos_config(
+ name = "generated_config_flattener_test",
+ src = "generated_config_test.json",
+ deps = [":generated_config"],
+)
diff --git a/aos/testdata/empty.json b/aos/testdata/empty.json
new file mode 100644
index 0000000..2c63c08
--- /dev/null
+++ b/aos/testdata/empty.json
@@ -0,0 +1,2 @@
+{
+}
diff --git a/aos/testdata/generated_config_test.json b/aos/testdata/generated_config_test.json
new file mode 100644
index 0000000..4a06f9f
--- /dev/null
+++ b/aos/testdata/generated_config_test.json
@@ -0,0 +1,5 @@
+{
+ "imports": [
+ "generated.json"
+ ]
+}
diff --git a/frc971/control_loops/drivetrain/drivetrain_config.json b/frc971/control_loops/drivetrain/drivetrain_config.json
index bafedff..e505c82 100644
--- a/frc971/control_loops/drivetrain/drivetrain_config.json
+++ b/frc971/control_loops/drivetrain/drivetrain_config.json
@@ -43,8 +43,5 @@
"type": "frc971.control_loops.drivetrain.LocalizerControl",
"frequency": 200
}
- ],
- "imports": [
- "../../../aos/robot_state/robot_state_config.json"
]
}
diff --git a/frc971/control_loops/drivetrain/drivetrain_simulation_config.json b/frc971/control_loops/drivetrain/drivetrain_simulation_config.json
index aa67f2e..c113bc0 100644
--- a/frc971/control_loops/drivetrain/drivetrain_simulation_config.json
+++ b/frc971/control_loops/drivetrain/drivetrain_simulation_config.json
@@ -1,5 +1,6 @@
{
"imports": [
+ "../../../aos/robot_state/robot_state_config.json",
"drivetrain_config.json",
"drivetrain_simulation_channels.json"
]