From 24d2c90a28a1761423bf3958bd96c2f2b5123975 Mon Sep 17 00:00:00 2001 From: Daniel Bertalan Date: Sat, 5 Nov 2022 11:37:33 +0100 Subject: [PATCH] BindingsGenerator+CMake: Keep track of IDL dependencies This commit teaches BindingsGenerator to generate depfiles, which can be used by CMake to ensure that bindings are properly regenerated when imported IDL files change. Two new options, `--depfile` and `--depfile-target` are added. - `--depfile` sets the path for the dependency file. - `--depfile-target` lets us set a target name different than the output file in the depfile. This option is needed because generated files are first written to a temporary file, but depfiles have to refer to the final location. These are analogous to GCC's `-MF` and `-MT` options respectively. The depfile's syntax matches the ones generated by GCC. Note: This changes the minimal required CMake version to 3.20 if the Make generator is used, and to 3.21 for the Xcode generator. Ninja is not affected. --- Meta/CMake/common_options.cmake | 5 +++++ Meta/CMake/libweb_generators.cmake | 15 +++++++++++++- .../LibWeb/BindingsGenerator/main.cpp | 20 ++++++++++++++++++- Userland/Libraries/LibIDL/IDLParser.cpp | 4 ++++ Userland/Libraries/LibIDL/IDLParser.h | 2 ++ 5 files changed, 44 insertions(+), 2 deletions(-) diff --git a/Meta/CMake/common_options.cmake b/Meta/CMake/common_options.cmake index 8ca08c1a76..f6155fb0a9 100644 --- a/Meta/CMake/common_options.cmake +++ b/Meta/CMake/common_options.cmake @@ -2,6 +2,11 @@ # Options common for the Serenity (target) and Lagom (host) builds # +# Make relative paths in depfiles be relative to CMAKE_CURRENT_BINARY_DIR rather than to CMAKE_BINARY_DIR +if (POLICY CMP0116) + cmake_policy(SET CMP0116 NEW) +endif() + serenity_option(ENABLE_COMPILETIME_FORMAT_CHECK ON CACHE BOOL "Enable compiletime format string checks") serenity_option(ENABLE_UNDEFINED_SANITIZER OFF CACHE BOOL "Enable undefined behavior sanitizer testing in gcc/clang") diff --git a/Meta/CMake/libweb_generators.cmake b/Meta/CMake/libweb_generators.cmake index d593f82537..63ff5b2cc0 100644 --- a/Meta/CMake/libweb_generators.cmake +++ b/Meta/CMake/libweb_generators.cmake @@ -111,14 +111,27 @@ function (generate_js_bindings target) list(GET BINDINGS_TYPES ${iter} bindings_type) get_property(include_paths DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} PROPERTY INCLUDE_DIRECTORIES) list(TRANSFORM include_paths PREPEND -i) + + # Ninja expects the target name in depfiles to be relative to CMAKE_BINARY_DIR, but ${bindings_src} is + # relative to CMAKE_CURRENT_BINARY_DIR. CMake >= 3.20 can do the rewriting transparently (CMP0116). + if(CMAKE_GENERATOR MATCHES "^Ninja" AND NOT POLICY CMP0116) + # FIXME: Drop this branch for cmake_minimum_required(3.20) + get_filename_component(full_path ${bindings_src} ABSOLUTE BASE_DIR ${CMAKE_CURRENT_BINARY_DIR}) + file(RELATIVE_PATH depfile_target ${CMAKE_BINARY_DIR} ${full_path}) + else() + set(depfile_target ${bindings_src}) + endif() + add_custom_command( OUTPUT "${bindings_src}" - COMMAND "$" "--${bindings_type}" -o "${bindings_src}.tmp" ${include_paths} "${LIBWEB_INPUT_FOLDER}/${class}.idl" "${LIBWEB_INPUT_FOLDER}" + COMMAND "$" "--${bindings_type}" -o "${bindings_src}.tmp" --depfile "${bindings_src}.d" + --depfile-target "${depfile_target}" ${include_paths} "${LIBWEB_INPUT_FOLDER}/${class}.idl" "${LIBWEB_INPUT_FOLDER}" COMMAND "${CMAKE_COMMAND}" -E copy_if_different "${bindings_src}.tmp" "${bindings_src}" COMMAND "${CMAKE_COMMAND}" -E remove "${bindings_src}.tmp" VERBATIM DEPENDS Lagom::BindingsGenerator MAIN_DEPENDENCY ${class}.idl + DEPFILE ${CMAKE_CURRENT_BINARY_DIR}/${bindings_src}.d ) endforeach() diff --git a/Meta/Lagom/Tools/CodeGenerators/LibWeb/BindingsGenerator/main.cpp b/Meta/Lagom/Tools/CodeGenerators/LibWeb/BindingsGenerator/main.cpp index 07de0db313..5dda94b574 100644 --- a/Meta/Lagom/Tools/CodeGenerators/LibWeb/BindingsGenerator/main.cpp +++ b/Meta/Lagom/Tools/CodeGenerators/LibWeb/BindingsGenerator/main.cpp @@ -33,6 +33,8 @@ ErrorOr serenity_main(Main::Arguments arguments) StringView path; StringView import_base_path; StringView output_path = "-"sv; + StringView depfile_path; + StringView depfile_target; bool constructor_header_mode = false; bool constructor_implementation_mode = false; bool prototype_header_mode = false; @@ -57,6 +59,8 @@ ErrorOr serenity_main(Main::Arguments arguments) }, }); args_parser.add_option(output_path, "Path to output generated file into", "output-path", 'o', "output-path"); + args_parser.add_option(depfile_path, "Path to write dependency file to", "depfile", 'd', "depfile-path"); + args_parser.add_option(depfile_target, "Name of target in the depfile (default: output path)", "depfile-target", 't', "target"); args_parser.add_positional_argument(path, "IDL file", "idl-file"); args_parser.add_positional_argument(import_base_path, "Import base path", "import-base-path", Core::ArgsParser::Required::No); args_parser.parse(arguments); @@ -66,7 +70,7 @@ ErrorOr serenity_main(Main::Arguments arguments) LexicalPath lexical_path(path); auto& namespace_ = lexical_path.parts_view().at(lexical_path.parts_view().size() - 2); - auto data = TRY(file->read_all()); + auto data = TRY(file->read_until_eof()); if (import_base_path.is_null()) import_base_path = lexical_path.dirname(); @@ -146,5 +150,19 @@ ErrorOr serenity_main(Main::Arguments arguments) IDL::generate_iterator_prototype_implementation(interface, output_builder); TRY(output_file->write(output_builder.string_view().bytes())); + + if (!depfile_path.is_null()) { + auto depfile = TRY(Core::Stream::File::open_file_or_standard_stream(depfile_path, Core::Stream::OpenMode::Write)); + + StringBuilder depfile_builder; + depfile_builder.append(depfile_target.is_null() ? output_path : depfile_target); + depfile_builder.append(':'); + for (auto const& path : parser.imported_files()) { + depfile_builder.append(" \\\n "sv); + depfile_builder.append(path); + } + depfile_builder.append('\n'); + TRY(depfile->write(depfile_builder.string_view().bytes())); + } return 0; } diff --git a/Userland/Libraries/LibIDL/IDLParser.cpp b/Userland/Libraries/LibIDL/IDLParser.cpp index e54d78861c..5fb0a95c05 100644 --- a/Userland/Libraries/LibIDL/IDLParser.cpp +++ b/Userland/Libraries/LibIDL/IDLParser.cpp @@ -1068,4 +1068,8 @@ HashTable>& Parser::top_level_interfaces() return top_level_parser()->interfaces; } +Vector Parser::imported_files() const +{ + return const_cast(this)->top_level_resolved_imports().keys(); +} } diff --git a/Userland/Libraries/LibIDL/IDLParser.h b/Userland/Libraries/LibIDL/IDLParser.h index 1233af275b..e1239c54f7 100644 --- a/Userland/Libraries/LibIDL/IDLParser.h +++ b/Userland/Libraries/LibIDL/IDLParser.h @@ -20,6 +20,8 @@ public: Parser(DeprecatedString filename, StringView contents, DeprecatedString import_base_path); Interface& parse(); + Vector imported_files() const; + private: // https://webidl.spec.whatwg.org/#dfn-special-operation // A special operation is a getter, setter or deleter.