[gn] Deprecate copy_trees()

Part of flutter/flutter#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 <[email protected]>
Commit-Queue: Zach Anderson <[email protected]>
Reviewed-by: Alexander Aprelev <[email protected]>

Author: zanderso

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,28 +64,18 @@ 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
       }
       if (copy_tree_spec.ignore_patterns != "{}") {
         exclude = copy_tree_spec.ignore_patterns
       }
     }
-    copy_tree_inputs_index = copy_tree_inputs_index + 1
   }
 }

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_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
-  },
-]
+copy_tree("copy_web_package") {
+  visibility = [ ":deploy_observatory" ]
+  source = "web"
+  dest = "$target_out_dir/observatory/deployed/web"
+  exclude = 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") {

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")

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 copy_tree(src, dst, ignore, errors):
                 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,62 +97,39 @@ def format_error(error):
         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):
     args = ParseArgs(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