From a9a84dfe1e154def7fcc7b7cb0ea27386489a8fc Mon Sep 17 00:00:00 2001 From: Corentin Wallez Date: Thu, 19 Sep 2019 23:30:42 +0000 Subject: [PATCH] BUILD.gn: Delete stale generated files in Dawn's gen dir. This normalizes even more the directory structure of generated files in Dawn and removes stale autogenerated files that could be included wrongly using a GN action. See comment on top of dawn_generator.gni in this commit for more context. BUG=dawn:22 Change-Id: I8ec038f949c048431b2b643af4462f98c4ae610b Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/11361 Reviewed-by: Austin Eng Reviewed-by: Kai Ninomiya Commit-Queue: Corentin Wallez --- generator/BUILD.gn | 59 ++++++++++++++++++++ generator/dawn_generator.gni | 35 +++++++++++- generator/generator_lib.gni | 5 ++ generator/remove_files.py | 70 ++++++++++++++++++++++++ scripts/dawn_overrides_with_defaults.gni | 1 + src/dawn/BUILD.gn | 4 -- 6 files changed, 167 insertions(+), 7 deletions(-) create mode 100644 generator/BUILD.gn create mode 100644 generator/remove_files.py diff --git a/generator/BUILD.gn b/generator/BUILD.gn new file mode 100644 index 0000000000..2970e2277c --- /dev/null +++ b/generator/BUILD.gn @@ -0,0 +1,59 @@ +# Copyright 2019 The Dawn Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import("../scripts/dawn_overrides_with_defaults.gni") +import("dawn_generator.gni") + +# The list of directories in which to check for stale autogenerated files. +# It should include the list of all directories in which we ever generated +# files but we can't just put dawn_gen_root because there are more than +# autogenerated sources there. +_stale_dirs = [ + "dawn", + "dawn_native", + "dawn_wire", + "mock", + "src" +] + +_allowed_output_dirs_file = "${dawn_gen_root}/removed_stale_autogen_files.allowed_output_dirs" +write_file(_allowed_output_dirs_file, dawn_allowed_gen_output_dirs) + +_stale_dirs_file = "${dawn_gen_root}/removed_stale_autogen_files.stale_dirs" +write_file(_stale_dirs_file, _stale_dirs) + +_stamp_file = "${dawn_gen_root}/removed_stale_autogen_files.stamp" + +# An action that removes autogenerated files that aren't in allowed directories +# see dawn_generator.gni for more details. +action("remove_stale_autogen_files") { + script = "remove_files.py" + args = [ + "--root-dir", + rebase_path(dawn_gen_root, root_build_dir), + "--allowed-output-dirs-file", + rebase_path(_allowed_output_dirs_file, root_build_dir), + "--stale-dirs-file", + rebase_path(_stale_dirs_file, root_build_dir), + "--stamp", + rebase_path(_stamp_file, root_build_dir), + ] + + # Have the "list of file" inputs as a dependency so that the action reruns + # as soon as they change. + inputs = [_allowed_output_dirs_file, _stale_dirs_file] + + # Output a stamp file so we don't re-run this action on every build. + outputs = [_stamp_file] +} diff --git a/generator/dawn_generator.gni b/generator/dawn_generator.gni index fc728832c9..370fc2c70b 100644 --- a/generator/dawn_generator.gni +++ b/generator/dawn_generator.gni @@ -15,6 +15,28 @@ import("../scripts/dawn_overrides_with_defaults.gni") import("generator_lib.gni") +# Dawn used to put autogenerated files in a lot of different places. When we +# started to move them around, some compilation issues arised because some +# stale include files stayed in the build directory and were picked up. +# To counter this, now Dawn does the following: +# +# 1. The generated output file directory structure has to match the structure +# of the source tree, starting at dawn_gen_root (gen/ or +# gen/third_party/dawn depending on where we are). +# 2. src/include and dawn_gen_root/src/include has to match the structure of +# the source tree too. +# 3. Dawn files must use include relative to src/ or src/include such as +# "dawn/dawn.h" or "dawn_native/backend/BackendStuff.h". +# +# The allowed list below ensure 1). Include directory rules for Dawn ensure 3) +# and 2) is something we need to enforce in code review. +# +# However GN's toolchains automatically add some include directories for us +# which breaks 3) slightly. To avoid stale headers in for example +# dawn_gen_root/src/dawn/dawn/ to be picked up (instead of +# dawn_gen_root/src/dawn), we have a special action that removes files in +# disallowed gen directories. + dawn_allowed_gen_output_dirs = [ "src/dawn/", "src/dawn_native/", @@ -40,9 +62,6 @@ dawn_allowed_gen_output_dirs = [ # "MyAwesomeTarget.cpp", # "MyAwesomeTarget.h", # ] -# -# # Optional, use a custom generated file directory. -# custom_gen_dir = "${target_gen_dir}/.." # } # # Using the generated files is done like so: @@ -55,9 +74,19 @@ dawn_allowed_gen_output_dirs = [ template("dawn_generator") { generator_lib_action(target_name) { forward_variables_from(invoker, "*") + + # Set arguments required to find the python libraries for the generator generator_lib_dir = "${dawn_root}/generator" jinja2_path = dawn_jinja2_dir + + # Force Dawn's autogenerated file structure to mirror exactly the source + # tree but start at ${dawn_gen_root} instead of ${dawn_root} allowed_output_dirs = dawn_allowed_gen_output_dirs + custom_gen_dir = dawn_gen_root + + # Make sure that we delete stale autogenerated file in directories that are + # no longer used by code generation to avoid include conflicts. + deps = [ "${dawn_root}/generator:remove_stale_autogen_files" ] } } diff --git a/generator/generator_lib.gni b/generator/generator_lib.gni index 10c560d926..36ac72f2f6 100644 --- a/generator/generator_lib.gni +++ b/generator/generator_lib.gni @@ -24,6 +24,8 @@ # outputs: List of expected outputs, generation will fail if there is a # mistmatch. # +# deps: additional deps for the code generation targets. +# # generator_lib_dir: directory where generator_lib.py is located. # # custom_gen_dir: Optional custom target gen dir. Defaults to $target_gen_dir @@ -138,6 +140,9 @@ template("generator_lib_action") { ] depfile = _json_tarball_depfile args = _generator_args + if (defined(invoker.deps)) { + deps = invoker.deps + } } # Extract the JSON tarball into the gen_dir diff --git a/generator/remove_files.py b/generator/remove_files.py new file mode 100644 index 0000000000..b143abee11 --- /dev/null +++ b/generator/remove_files.py @@ -0,0 +1,70 @@ +# Copyright 2019 The Dawn Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import argparse, glob, os, sys + +def check_in_subdirectory(path, directory): + return path.startswith(directory) and not '/' in path[len(directory):] + +def check_is_allowed(path, allowed_dirs): + return any(check_in_subdirectory(path, directory) for directory in allowed_dirs) + +def get_all_files_in_dir(find_directory): + result = [] + for (directory, _, files) in os.walk(find_directory): + result += [os.path.join(directory, filename) for filename in files] + return result + +def run(): + # Parse command line arguments + parser = argparse.ArgumentParser( + description = "Removes stale autogenerated files from gen/ directories." + ) + parser.add_argument('--root-dir', type=str, help='The root directory, all other paths in files are relative to it.') + parser.add_argument('--allowed-output-dirs-file', type=str, help='The file containing a list of allowed directories') + parser.add_argument('--stale-dirs-file', type=str, help='The file containing a list of directories to check for stale files') + parser.add_argument('--stamp', type=str, help='A stamp written once this script completes') + args = parser.parse_args() + + root_dir = args.root_dir + stamp_file = args.stamp + + # Load the list of allowed and stale directories + with open(args.allowed_output_dirs_file) as f: + allowed_dirs = set([os.path.join(root_dir, line.strip()) for line in f.readlines()]) + + for directory in allowed_dirs: + if not directory.endswith('/'): + print('Allowed directory entry "{}" doesn\'t end with /'.format(directory)) + return 1 + + with open(args.stale_dirs_file) as f: + stale_dirs = set([line.strip() for line in f.readlines()]) + + # Remove all files in stale dirs that aren't in the allowed dirs. + for stale_dir in stale_dirs: + stale_dir = os.path.join(root_dir, stale_dir) + + for candidate in get_all_files_in_dir(stale_dir): + if not check_is_allowed(candidate, allowed_dirs): + os.remove(candidate) + + # Finished! Write the stamp file so ninja knows to not run this again. + with open(stamp_file, "w") as f: + f.write("") + + return 0 + +if __name__ == "__main__": + sys.exit(run()) diff --git a/scripts/dawn_overrides_with_defaults.gni b/scripts/dawn_overrides_with_defaults.gni index 196a448b55..2a65be7b6e 100644 --- a/scripts/dawn_overrides_with_defaults.gni +++ b/scripts/dawn_overrides_with_defaults.gni @@ -27,6 +27,7 @@ if (!defined(dawn_standalone)) { if (!defined(dawn_root)) { dawn_root = get_path_info("..", "abspath") } +dawn_gen_root = get_path_info("${dawn_root}", "gen_dir") if (!defined(dawn_jinja2_dir)) { dawn_jinja2_dir = "//third_party/jinja2" diff --git a/src/dawn/BUILD.gn b/src/dawn/BUILD.gn index 1d50b9da22..2b94ce540e 100644 --- a/src/dawn/BUILD.gn +++ b/src/dawn/BUILD.gn @@ -23,10 +23,6 @@ import("${dawn_root}/scripts/dawn_component.gni") dawn_json_generator("dawn_headers_gen") { target = "dawn_headers" - - # Generate as if we were in the main BUILD.gn because that was historically - # the case and we can't move generated files without clobbering the build. - custom_gen_dir = "${target_gen_dir}/../.." outputs = [ "src/include/dawn/dawncpp.h", "src/include/dawn/dawn.h",