From 78ebdcd5ed645f018c8e4c8483b96a6f71689578 Mon Sep 17 00:00:00 2001
From: Yifei Teng <yifeit@google.com>
Date: Fri, 26 Apr 2019 23:40:32 +0000
Subject: [PATCH] [fidl] [llcpp] Validate that checked in bindings are up to
date
Implements fx build tools/fidlgen_llcpp_zircon:validate.
In addition, adds the validation pass to the fuchsia build.
Test: Use fuchsia-mem:llcpp somewhere, then change it. Observe that the
build fails with an error prompting the user to re-run bindings
generation.
Change-Id: I376516c794120fa5faaaf4e2afaffc0283604d09
---
BUILD.gn | 7 ++
tools/fidlgen_llcpp_zircon/BUILD.gn | 6 +-
tools/fidlgen_llcpp_zircon/README.md | 4 +-
tools/fidlgen_llcpp_zircon/llcpp_codegen.cc | 88 ++++++++++++++++++++-
tools/fidlgen_llcpp_zircon/llcpp_codegen.h | 1 +
tools/fidlgen_llcpp_zircon/main.cc | 17 +++-
zircon/public/gn/fidl/llcpp.gni | 9 ++-
7 files changed, 120 insertions(+), 12 deletions(-)
diff --git a/BUILD.gn b/BUILD.gn
index 4e5d5a2d0d9..e4a3c73f2dd 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -68,6 +68,7 @@ group("default") {
deps = [
"//build/images:packages",
"//sdk",
+ ":build_time_checks",
]
if (base_package_labels != [] || cache_package_labels != []) {
deps += [ "//build/images" ]
@@ -77,6 +78,12 @@ group("default") {
}
}
+group("build_time_checks") {
+ deps = [
+ "//tools/fidlgen_llcpp_zircon:validate",
+ ]
+}
+
group("recovery_image") {
deps = [
"build/images/recovery",
diff --git a/tools/fidlgen_llcpp_zircon/BUILD.gn b/tools/fidlgen_llcpp_zircon/BUILD.gn
index ee567c8e1b5..8c4fc1eaed6 100644
--- a/tools/fidlgen_llcpp_zircon/BUILD.gn
+++ b/tools/fidlgen_llcpp_zircon/BUILD.gn
@@ -23,7 +23,8 @@ compiled_action("validate") {
rebase_path(zircon_root_build_dir, root_build_dir),
rebase_path("$root_build_dir/tools/fidlgen_llcpp", root_build_dir),
rebase_path("$target_gen_dir/fidlgen_llcpp_zircon_validated.stamp", root_build_dir),
- rebase_path("$target_gen_dir/fidlgen_llcpp_zircon_validated.d", root_build_dir)
+ rebase_path("$target_gen_dir/fidlgen_llcpp_zircon_validated.d", root_build_dir),
+ rebase_path("$target_gen_dir/validator_tmp", root_build_dir)
]
depfile = "$target_gen_dir/fidlgen_llcpp_zircon_validated.d"
inputs = [ "$zircon_root_build_dir/fidl_gen.json" ]
@@ -37,7 +38,8 @@ compiled_action("update") {
rebase_path(zircon_root_build_dir, root_build_dir),
rebase_path("$root_build_dir/tools/fidlgen_llcpp", root_build_dir),
rebase_path("$target_gen_dir/fidlgen_llcpp_zircon_updated.stamp", root_build_dir),
- rebase_path("$target_gen_dir/fidlgen_llcpp_zircon_updated.d", root_build_dir)
+ rebase_path("$target_gen_dir/fidlgen_llcpp_zircon_updated.d", root_build_dir),
+ rebase_path("$target_gen_dir/validator_tmp", root_build_dir)
]
depfile = "$target_gen_dir/fidlgen_llcpp_zircon_updated.d"
inputs = [ "$zircon_root_build_dir/fidl_gen.json" ]
diff --git a/tools/fidlgen_llcpp_zircon/README.md b/tools/fidlgen_llcpp_zircon/README.md
index 35da4c4e90e..04ddc6a17f9 100644
--- a/tools/fidlgen_llcpp_zircon/README.md
+++ b/tools/fidlgen_llcpp_zircon/README.md
@@ -18,8 +18,8 @@ fx build -k 0 tools/fidlgen_llcpp_zircon:update
```
The `-k 0` switches would keep the build going even if parts of zircon failed to build.
-[TODO(yifeit): Implement] As extra precaution measure, the full build will validate that the
-generated bindings are up-to-date. You may manually run the same check with the following command:
+As an extra precaution measure, the full build will validate that the generated bindings are
+up to date. You can manually run the same check with the following command:
```bash
fx build tools/fidlgen_llcpp_zircon:validate
```
diff --git a/tools/fidlgen_llcpp_zircon/llcpp_codegen.cc b/tools/fidlgen_llcpp_zircon/llcpp_codegen.cc
index 9441db0bc0c..6db10f1c6ee 100644
--- a/tools/fidlgen_llcpp_zircon/llcpp_codegen.cc
+++ b/tools/fidlgen_llcpp_zircon/llcpp_codegen.cc
@@ -4,6 +4,7 @@
#include <cstdio>
#include <cstdlib>
+#include <filesystem>
#include <fstream>
#include <iostream>
#include <memory>
@@ -48,8 +49,13 @@ rapidjson::Document ReadMetadata(const fs::path& path) {
struct Target {
fs::path gen_dir;
+ std::string name;
std::vector<fs::path> fidl_sources;
std::vector<std::string> args;
+ fs::path json;
+ fs::path header;
+ fs::path source;
+ fs::path include_base;
};
std::vector<Target> AllTargets(const fs::path& zircon_build_root) {
@@ -76,8 +82,13 @@ std::vector<Target> AllTargets(const fs::path& zircon_build_root) {
}
targets_vector.push_back(Target {
.gen_dir = fs::path(target["target_gen_dir"].GetString()),
+ .name = target["name"].GetString(),
.args = std::move(args_vector),
- .fidl_sources = std::move(fidl_sources_vector)
+ .fidl_sources = std::move(fidl_sources_vector),
+ .json = fs::path(target["json"].GetString()),
+ .header = fs::path(target["header"].GetString()),
+ .source = fs::path(target["source"].GetString()),
+ .include_base = fs::path(target["include_base"].GetString()),
});
}
return targets_vector;
@@ -118,17 +129,88 @@ void RunCommand(const std::string& cmd, const std::string& working_directory,
}
}
+fs::path FindCommonPath(fs::path a, fs::path b) {
+ auto a_it = a.begin();
+ auto b_it = b.begin();
+ fs::path result;
+ while (a_it != a.end() && b_it != b.end()) {
+ auto& a_part = *a_it;
+ auto& b_part = *b_it;
+ if (a_part != b_part) {
+ break;
+ }
+ result /= a_part;
+ a_it++;
+ b_it++;
+ }
+ return result;
+}
+
+bool Diff(fs::path a, fs::path b) {
+ std::ifstream a_stream(a.c_str(), std::ios::binary | std::ios::ate);
+ std::ifstream b_stream(b.c_str(), std::ios::binary | std::ios::ate);
+ if (!a_stream) {
+ return false;
+ }
+ if (!b_stream) {
+ return false;
+ }
+ if (a_stream.tellg() != b_stream.tellg()) {
+ return false;
+ }
+ a_stream.seekg(0, std::ifstream::beg);
+ b_stream.seekg(0, std::ifstream::beg);
+ return std::equal(std::istreambuf_iterator<char>(a_stream.rdbuf()),
+ std::istreambuf_iterator<char>(),
+ std::istreambuf_iterator<char>(b_stream.rdbuf()));
+}
+
} // namespace
bool DoValidate(std::filesystem::path zircon_build_root,
std::filesystem::path fidlgen_llcpp_path,
+ std::filesystem::path tmp_dir,
std::vector<fs::path>* out_dependencies) {
+ fs::remove_all(tmp_dir);
+ fs::create_directories(tmp_dir);
auto all_targets = AllTargets(zircon_build_root);
+ auto normalize = [&zircon_build_root](fs::path path) {
+ return fs::weakly_canonical(zircon_build_root / path);
+ };
for (const auto& target : all_targets) {
for (const auto& source : target.fidl_sources) {
out_dependencies->push_back(zircon_build_root / source);
}
- // TODO(yifeit): Implement.
+ fs::path json = normalize(target.json);
+ fs::path header = normalize(target.header);
+ fs::path source = normalize(target.source);
+ fs::path include_base = normalize(target.include_base);
+ fs::path common = FindCommonPath(header,
+ FindCommonPath(include_base, source));
+ // Generate in an alternative location
+ fs::path tmp = fs::absolute(tmp_dir) / target.name;
+ fs::path alt_header = tmp / fs::relative(header, common);
+ fs::path alt_source = tmp / fs::relative(source, common);
+ fs::path alt_include_base = tmp / fs::relative(include_base, common);
+ std::vector<std::string> args = {
+ "-json",
+ json,
+ "-include-base",
+ alt_include_base,
+ "-header",
+ alt_header,
+ "-source",
+ alt_source
+ };
+ RunCommand(fidlgen_llcpp_path, zircon_build_root, args);
+ if (!Diff(header, alt_header)) {
+ std::cerr << header << " is different from " << alt_header << std::endl;
+ return false;
+ }
+ if (!Diff(source, alt_source)) {
+ std::cerr << source << " is different from " << alt_source << std::endl;
+ return false;
+ }
}
return true;
}
@@ -141,6 +223,8 @@ void DoUpdate(fs::path zircon_build_root,
for (const auto& source : target.fidl_sources) {
out_dependencies->push_back(zircon_build_root / source);
}
+ std::cout << "Generating low-level C++ bindings for " << target.name
+ << std::endl;
RunCommand(fidlgen_llcpp_path, zircon_build_root, target.args);
}
}
diff --git a/tools/fidlgen_llcpp_zircon/llcpp_codegen.h b/tools/fidlgen_llcpp_zircon/llcpp_codegen.h
index 997addb8932..c43a9de71af 100644
--- a/tools/fidlgen_llcpp_zircon/llcpp_codegen.h
+++ b/tools/fidlgen_llcpp_zircon/llcpp_codegen.h
@@ -13,6 +13,7 @@
// Returns true when sources are up-to-date.
bool DoValidate(std::filesystem::path zircon_build_root,
std::filesystem::path fidlgen_llcpp_path,
+ std::filesystem::path tmp_dir,
std::vector<std::filesystem::path>* out_dependencies);
// Update the checked-in sources.
diff --git a/tools/fidlgen_llcpp_zircon/main.cc b/tools/fidlgen_llcpp_zircon/main.cc
index 1b5685d9345..3156812e432 100644
--- a/tools/fidlgen_llcpp_zircon/main.cc
+++ b/tools/fidlgen_llcpp_zircon/main.cc
@@ -19,12 +19,14 @@ static void Usage(const char* exe_name) {
stderr,
"Generate or validate the checked-in low-level C++ bindings in zircon.\n"
"Usage: %s (validate|update) ZIRCON_BUILDROOT FIDLGEN_LLCPP_PATH "
- "STAMP DEPFILE\n"
+ "STAMP DEPFILE TMP_DIR\n"
"ZIRCON_BUILDROOT is the root build directory of the Zircon GN build.\n"
"FIDLGEN_LLCPP_PATH is the path to the fidlgen_llcpp executable.\n"
"STAMP is the output path to a file indicating the success of the tool.\n"
"DEPFILE is the output path to a depfile describing the FIDL files\n"
"which when updated should trigger a re-run of this tool.\n"
+ "TMP_DIR is a temporary directory for the validator to store generated\n"
+ "bindings. It will be cleared on each run.\n"
"\n"
"When validate is specified, it will validate that the generated\n"
"bindings are up-to-date, exiting with an error if not so.\n"
@@ -37,7 +39,7 @@ static void Usage(const char* exe_name) {
}
int main(int argc, char** argv) {
- if (argc != 6) {
+ if (argc != 7) {
std::cerr << argv[0] << ": Invalid arguments" << "\n\n";
Usage(argv[0]);
return -1;
@@ -51,10 +53,19 @@ int main(int argc, char** argv) {
fs::remove(stamp_path);
fs::path depfile_path = fs::absolute(argv[5]);
fs::remove(depfile_path);
+ fs::path tmp_dir = fs::absolute(argv[6]);
std::vector<fs::path> dependencies;
if (strcmp(argv[1], "validate") == 0) {
- DoValidate(zircon_build_root, fidlgen_llcpp_path, &dependencies);
+ bool ok = DoValidate(zircon_build_root, fidlgen_llcpp_path, tmp_dir,
+ &dependencies);
+ if (!ok) {
+ std::cerr << "========================================================\n"
+ << "Out-of-date checked-in low-level C++ bindings in Zircon.\n"
+ "Re-run fx build -k 0 tools/fidlgen_llcpp_zircon:update\n"
+ << "========================================================\n";
+ return -1;
+ }
} else if (strcmp(argv[1], "update") == 0) {
DoUpdate(zircon_build_root, fidlgen_llcpp_path, &dependencies);
} else {
diff --git a/zircon/public/gn/fidl/llcpp.gni b/zircon/public/gn/fidl/llcpp.gni
index 93b75462169..93604495277 100644
--- a/zircon/public/gn/fidl/llcpp.gni
+++ b/zircon/public/gn/fidl/llcpp.gni
@@ -127,6 +127,9 @@ template("fidl_llcpp_library") {
name = invoker.fidl_name
target_gen_dir = rebase_path("gen/llcpp", root_build_dir)
json = rebase_path(fidlc_outputs[0], root_build_dir)
+ header = "$target_gen_dir/include/${invoker.fidl_path}/llcpp/fidl.h"
+ source = "$target_gen_dir/fidl.cc"
+ include_base = "$target_gen_dir/include"
# TODO(BLD-442): Could generate an individual response file
# here that could be used very simply to drive running fidlgen.
@@ -138,11 +141,11 @@ template("fidl_llcpp_library") {
"-json",
json,
"-include-base",
- "$target_gen_dir/include",
+ include_base,
"-header",
- "$target_gen_dir/include/${invoker.fidl_path}/llcpp/fidl.h",
+ header,
"-source",
- "$target_gen_dir/fidl.cc",
+ source
]
},
]
--
GitLab