diff --git a/BUILD.gn b/BUILD.gn index 4e5d5a2d0d9e76a320ae383d1506db0095ce9ec2..e4a3c73f2dd24fee49ad850f6cf433c85b1250ae 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 ee567c8e1b52e1cc03f69cf70f26e4189702ee22..8c4fc1eaed6e53463ca459c69757018612b2edd0 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 35da4c4e90efd9e461673d56dc280500559aee0b..04ddc6a17f96f969236c394450ecbfb3e706c101 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 9441db0bc0c3ed243211103dd9beda1ca5337038..6db10f1c6ee1e9b5b894ccb2fb769dc40eeece78 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 997addb8932ffe85fe0cb71d41bce8205790bf14..c43a9de71af07464acac40257bf4ccd24b8a61a3 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 1b5685d934510724c85515f0b279b3b631b07506..3156812e432f20b86bc6a4947fa104a7ebd6fd5c 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 93b7546216970d361c3949d4e6f0c4e3e338e107..93604495277c389769c8e6ad5fde4c995d4f6b47 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 ] }, ]