From f60d32167fff6b3152c2778b648e256a179f2deb Mon Sep 17 00:00:00 2001 From: Zach Anderson Date: Fri, 8 Mar 2024 20:47:47 +0000 Subject: [PATCH] [gn] Deprecate copy_trees() Part of https://github.com/flutter/flutter/issues/144430 Previously, this code did backflips to accomplish two goals 1. Collect the paths of all files that would be copied from one place to another after applying regex filters so that they could be listed as the "inputs" to a GN action. 2. Arrange so that the python script doing the above would only be invoked once during GN to reduce the cost of calling out to python. However, this is exactly the use-case for the "depfile" parameter to a GN action. Instead of running the script during GN to populate the "inputs" list, when we run the copy_tree.py script, we can instead ask it to generate a depfile to collect all the input files that were actually copied after applying the regex filter. Using that, we don't have to run the python script at all during GN. TEST=it builds Change-Id: I41a251ce4659119cdc3997bb2d6fc7ee0831bb6d Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/356481 Reviewed-by: Siva Annamalai Commit-Queue: Zach Anderson Reviewed-by: Alexander Aprelev --- build/dart/copy_tree.gni | 48 +++++------------- runtime/observatory/BUILD.gn | 38 +++++--------- sdk/BUILD.gn | 76 ++++++++++------------------ tools/copy_tree.py | 96 ++++++++++++------------------------ utils/dartdev/BUILD.gn | 15 ++---- 5 files changed, 88 insertions(+), 185 deletions(-) diff --git a/build/dart/copy_tree.gni b/build/dart/copy_tree.gni index a17e52976af..284b106f103 100644 --- a/build/dart/copy_tree.gni +++ b/build/dart/copy_tree.gni @@ -10,13 +10,11 @@ _dart_root = rebase_path("../..") # Optional parameters: # exclude - A comma separated list that is passed to shutil.ignore_patterns() # in tools/copy_tree.py. -template("_copy_tree") { +template("copy_tree") { assert(defined(invoker.source), "copy_tree must define 'source'") assert(defined(invoker.dest), "copy_tree must define 'dest'") - assert(defined(invoker.inputs), "copy_tree must define 'inputs'") source = invoker.source dest = invoker.dest - inputs = invoker.inputs action(target_name) { if (defined(invoker.visibility)) { visibility = invoker.visibility @@ -27,11 +25,18 @@ template("_copy_tree") { deps += invoker.deps } + depfile = "$target_gen_dir/$target_name.d" + stampfile = "$target_gen_dir/$target_name.stamp" + common_args = [ "--from", rebase_path(source), "--to", rebase_path(dest), + "--depfile", + rebase_path(depfile), + "--stamp", + rebase_path(stampfile), ] if (defined(invoker.exclude)) { common_args += [ @@ -40,33 +45,14 @@ template("_copy_tree") { ] } - relative_files = rebase_path(inputs, rebase_path(source)) - - output_files = [] - foreach(input, relative_files) { - output_files += [ "$dest/$input" ] - } - - outputs = output_files + outputs = [ stampfile ] script = "$_dart_root/tools/copy_tree.py" args = common_args } } -# copy_trees() arranges to invoke copy_tree.py only once to gather the list of -# input source files for every _copy_tree() target. It takes a list of scopes as -# a parameter. The scopes should contain the following mappings. -# -# target: The target name for the _copy_tree() target. -# visibility: The visibility for the _copy_tree() target. -# source: The source directory relative to this directory. -# dest: The destination directory for the _copy_tree() target. -# deps: Any deps needed for the _copy_tree() target. -# ignore_patterns: Patterns to ignore when walking the directory tree. -# This should be '{}' if nothing should be ignored. -# -# copy_trees() will then make sure each invocation of _copy_tree() has the -# correct 'inputs' parameter +# DEPRECATED: This can be removed after the uses in the flutter/engine tree +# are migrated to use copy_tree(). template("copy_trees") { assert(defined(invoker.sources), "$target_name must define 'source'") sources = invoker.sources @@ -78,21 +64,12 @@ template("copy_trees") { ] } - # Evaluate script output as GN, producing a scope containing a single value - # "sources" - copy_tree_inputs_scope = exec_script("$_dart_root/tools/copy_tree.py", - [ "--gn" ] + copy_tree_source_paths, - "scope") - # A list of lists of input source files for copy_tree. - copy_tree_inputs = copy_tree_inputs_scope.sources - copy_tree_inputs_index = 0 foreach(copy_tree_spec, sources) { - _copy_tree(copy_tree_spec.target) { + copy_tree(copy_tree_spec.target) { visibility = copy_tree_spec.visibility source = copy_tree_spec.source dest = copy_tree_spec.dest - inputs = copy_tree_inputs[copy_tree_inputs_index] if (defined(copy_tree_spec.deps)) { deps = copy_tree_spec.deps } @@ -100,6 +77,5 @@ template("copy_trees") { exclude = copy_tree_spec.ignore_patterns } } - copy_tree_inputs_index = copy_tree_inputs_index + 1 } } diff --git a/runtime/observatory/BUILD.gn b/runtime/observatory/BUILD.gn index 518b6322048..df4d91693c7 100644 --- a/runtime/observatory/BUILD.gn +++ b/runtime/observatory/BUILD.gn @@ -74,39 +74,25 @@ if (!is_debug) { observatory_ignore_patterns += [ "*.map" ] } -# The ignore_patterns entry in the scopes accepted by copy_trees() is a +# The ignore_patterns entry in the scopes accepted by copy_tree() is a # string of comma delimited patterns. observatory_ignore_string = "\$sdk" foreach(pattern, observatory_ignore_patterns) { observatory_ignore_string = "$observatory_ignore_string,$pattern" } -copy_tree_specs = [] +copy_tree("copy_web_package") { + visibility = [ ":deploy_observatory" ] + source = "web" + dest = "$target_out_dir/observatory/deployed/web" + exclude = observatory_ignore_string +} -copy_tree_specs += [ - { - target = "copy_web_package" - visibility = [ ":deploy_observatory" ] - source = "web" - dest = "$target_out_dir/observatory/deployed/web" - ignore_patterns = observatory_ignore_string - }, -] - -copy_tree_specs += [ - { - target = "copy_observatory_package" - visibility = [ ":deploy_observatory" ] - source = "lib" - dest = "$target_out_dir/observatory/deployed/web/packages/observatory" - ignore_patterns = observatory_ignore_string - }, -] - -# This is not a rule, rather, it generates rules with names of the form: -# "copy_$package_package" for the packages in observatory_pub_packages. -copy_trees("copy_observatory_packages") { - sources = copy_tree_specs +copy_tree("copy_observatory_package") { + visibility = [ ":deploy_observatory" ] + source = "lib" + dest = "$target_out_dir/observatory/deployed/web/packages/observatory" + exclude = observatory_ignore_string } copy("copy_main_dart_js") { diff --git a/sdk/BUILD.gn b/sdk/BUILD.gn index 50fa5e0da5d..ed3fcbf98c9 100644 --- a/sdk/BUILD.gn +++ b/sdk/BUILD.gn @@ -212,66 +212,44 @@ _full_sdk_libraries = [ # ] _platform_sdk_libraries = _full_sdk_libraries -# From here down to the copy_trees() invocation, we collect all the information -# about trees that need to be copied in the list of scopes, copy_tree_specs. -copy_tree_specs = [] - # This rule copies dartdoc templates to # bin/resources/dartdoc/templates -copy_tree_specs += [ - { - target = "copy_dartdoc_templates" - visibility = [ ":copy_dartdoc_files" ] - source = "../third_party/pkg/dartdoc/lib/templates" - dest = "$root_out_dir/$dart_sdk_output/bin/resources/dartdoc/templates" - ignore_patterns = "{}" - }, -] +copy_tree("copy_dartdoc_templates") { + visibility = [ ":copy_dartdoc_files" ] + source = "../third_party/pkg/dartdoc/lib/templates" + dest = "$root_out_dir/$dart_sdk_output/bin/resources/dartdoc/templates" + exclude = "{}" +} # This rule copies dartdoc resources to # bin/resources/dartdoc/resources -copy_tree_specs += [ - { - target = "copy_dartdoc_resources" - visibility = [ ":copy_dartdoc_files" ] - source = "../third_party/pkg/dartdoc/lib/resources" - dest = "$root_out_dir/$dart_sdk_output/bin/resources/dartdoc/resources" - ignore_patterns = "{}" - }, -] +copy_tree("copy_dartdoc_resources") { + visibility = [ ":copy_dartdoc_files" ] + source = "../third_party/pkg/dartdoc/lib/resources" + dest = "$root_out_dir/$dart_sdk_output/bin/resources/dartdoc/resources" + exclude = "{}" +} # This rule copies the pre-built DevTools application to # bin/resources/devtools/ -copy_tree_specs += [ - { - target = "copy_prebuilt_devtools" - visibility = [ ":create_common_sdk" ] - source = "../third_party/devtools/web" - dest = "$root_out_dir/$dart_sdk_output/bin/resources/devtools" - ignore_patterns = "{}" - }, -] +copy_tree("copy_prebuilt_devtools") { + visibility = [ ":create_common_sdk" ] + source = "../third_party/devtools/web" + dest = "$root_out_dir/$dart_sdk_output/bin/resources/devtools" + exclude = "{}" +} # This loop generates rules to copy libraries to lib/ foreach(library, _full_sdk_libraries) { - copy_tree_specs += [ - { - target = "copy_${library}_library" - visibility = [ - ":copy_full_sdk_libraries", - ":copy_platform_sdk_libraries", - ] - source = "lib/$library" - dest = "$root_out_dir/$dart_sdk_output/lib/$library" - ignore_patterns = "*.svn,doc,*.py,*.gypi,*.sh,.gitignore" - }, - ] -} - -# This generates targets for everything in copy_tree_specs. The targets have the -# same name as the "target" fields in the scopes of copy_tree_specs. -copy_trees("copy_trees") { - sources = copy_tree_specs + copy_tree("copy_${library}_library") { + visibility = [ + ":copy_full_sdk_libraries", + ":copy_platform_sdk_libraries", + ] + source = "lib/$library" + dest = "$root_out_dir/$dart_sdk_output/lib/$library" + exclude = "*.svn,doc,*.py,*.gypi,*.sh,.gitignore" + } } _has_dot_sym = !is_win && rebase_path(".") == rebase_path("//sdk") diff --git a/tools/copy_tree.py b/tools/copy_tree.py index a3ef4c2ebdf..155ce490009 100755 --- a/tools/copy_tree.py +++ b/tools/copy_tree.py @@ -16,6 +16,10 @@ def ParseArgs(args): parser = argparse.ArgumentParser( description='A script to copy a file tree somewhere') + parser.add_argument('--depfile', + '-d', + type=str, + help='Path to a depfile to write when copying.') parser.add_argument( '--exclude_patterns', '-e', @@ -24,33 +28,16 @@ def ParseArgs(args): parser.add_argument( '--from', '-f', dest="copy_from", type=str, help='Source directory') parser.add_argument( - '--gn', - '-g', - dest='gn', - default=False, - action='store_true', - help='Output for GN for multiple sources, but do not copy anything.') - parser.add_argument( - 'gn_paths', - metavar='name path ignore_pattern', + '--stamp', + '-s', type=str, - nargs='*', - default=None, - help='When --gn is given, the specification of source paths to list.') + help='The path to a stamp file to output when finished.') parser.add_argument('--to', '-t', type=str, help='Destination directory') return parser.parse_args(args) def ValidateArgs(args): - if args.gn: - if args.exclude_patterns or args.copy_from or args.to: - print("--gn mode does not accept other switches") - return False - if not args.gn_paths: - print("--gn mode requires a list of source specifications") - return False - return True if not args.copy_from or not os.path.isdir(args.copy_from): print("--from argument must refer to a directory") return False @@ -60,7 +47,10 @@ def ValidateArgs(args): return True +# Returns a list of the files under 'src' that were copied. def CopyTree(src, dst, ignore=None): + copied_files = [] + # Recursive helper method to collect errors but keep processing. def copy_tree(src, dst, ignore, errors): names = os.listdir(src) @@ -80,6 +70,7 @@ def CopyTree(src, dst, ignore=None): if os.path.isdir(srcname): copy_tree(srcname, dstname, ignore, errors) else: + copied_files.append(srcname) shutil.copy(srcname, dstname) except (IOError, os.error) as why: errors.append((srcname, dstname, str(why))) @@ -106,45 +97,18 @@ def CopyTree(src, dst, ignore=None): msg = '\n'.join(parts) raise RuntimeError(msg) - -def ListTree(src, ignore=None): - names = os.listdir(src) - if ignore is not None: - ignored_names = ignore(src, names) - else: - ignored_names = set() - - srcnames = [] - for name in names: - if name in ignored_names: - continue - srcname = os.path.join(src, name) - if os.path.isdir(srcname): - srcnames.extend(ListTree(srcname, ignore)) - else: - srcnames.append(srcname) - return srcnames + return copied_files -# source_dirs is organized such that sources_dirs[n] is the path for the source -# directory, and source_dirs[n+1] is a list of ignore patterns. -def SourcesToGN(source_dirs): - if len(source_dirs) % 2 != 0: - print("--gn list length should be a multiple of 2.") - return False - data = [] - for i in range(0, len(source_dirs), 2): - path = source_dirs[i] - ignores = source_dirs[i + 1] - if ignores in ["{}"]: - sources = ListTree(path) - else: - patterns = ignores.split(',') - sources = ListTree(path, ignore=shutil.ignore_patterns(*patterns)) - data.append(sources) - scope_data = {"sources": data} - print(gn_helpers.ToGNString(scope_data)) - return True +def WriteDepfile(depfile, stamp, dep_list): + os.makedirs(os.path.dirname(depfile), exist_ok=True) + # Paths in the depfile must be relative to the root build output directory, + # which is the cwd that ninja invokes the script from. + cwd = os.getcwd() + relstamp = os.path.relpath(stamp, cwd) + reldep_list = [os.path.relpath(d, cwd) for d in dep_list] + with open(depfile, 'w') as f: + print("{0}: {1}".format(relstamp, " ".join(reldep_list)), file=f) def Main(argv): @@ -152,16 +116,20 @@ def Main(argv): if not ValidateArgs(args): return -1 - if args.gn: - SourcesToGN(args.gn_paths) - return 0 - if args.exclude_patterns == None: - CopyTree(args.copy_from, args.to) + copied_files = CopyTree(args.copy_from, args.to) else: patterns = args.exclude_patterns.split(',') - CopyTree( - args.copy_from, args.to, ignore=shutil.ignore_patterns(*patterns)) + copied_files = CopyTree(args.copy_from, + args.to, + ignore=shutil.ignore_patterns(*patterns)) + + if args.depfile and args.stamp: + WriteDepfile(args.depfile, args.stamp, copied_files) + + if args.stamp: + open(args.stamp, 'w').close() + return 0 diff --git a/utils/dartdev/BUILD.gn b/utils/dartdev/BUILD.gn index ad9756b2a8f..338c04320c1 100644 --- a/utils/dartdev/BUILD.gn +++ b/utils/dartdev/BUILD.gn @@ -37,14 +37,9 @@ application_snapshot("generate_dartdev_snapshot") { output = "$root_gen_dir/dartdev.dart.snapshot" } -copy_trees("copy_prebuilt_devtools") { - sources = [ - { - target = "copy_prebuilt_devtools" - visibility = [ ":dartdev" ] - source = "../../third_party/devtools/web" - dest = "$root_out_dir/devtools" - ignore_patterns = "{}" - }, - ] +copy_tree("copy_prebuilt_devtools") { + visibility = [ ":dartdev" ] + source = "../../third_party/devtools/web" + dest = "$root_out_dir/devtools" + exclude = "{}" }