Change analyzer to search for build_targets from supplied targets

pylib/gyp/generator/analyzer.py

Index: pylib/gyp/generator/analyzer.py
diff --git a/pylib/gyp/generator/analyzer.py b/pylib/gyp/generator/analyzer.py
index f403d4e266b048e59b81512ad43c2a95e37dcc74..298c6635e035e74f57a00bf26791d370c0709ac8 100644
--- a/pylib/gyp/generator/analyzer.py
+++ b/pylib/gyp/generator/analyzer.py
@@ -15,13 +15,32 @@ targets: the set of targets passed in via targets that either directly or
   indirectly depend upon the set of paths supplied in files.
 build_targets: minimal set of targets that directly depend on the changed
   files and need to be built. The expectation is this set of targets is passed
-  into a build step.
+  into a build step. The returned values are either values in the supplied
+  targets, or have a dependency on one of the supplied targets.
 status: outputs one of three values: none of the supplied files were found,
   one of the include files changed so that it should be assumed everything
   changed (in this case targets and build_targets are not output) or at
   least one file was found.
 invalid_targets: list of supplied targets thare were not found.
 
+|targets| and |build_targets| are very closely related but subtly different.
+Consider a graph like the following:
+  A     D
+ / \
+B   C
+A depends upon both B and C, A is of type none and B and C are executables.
+D is an executable, has no dependencies and nothing depends on it.
+If |targets| = ["A"] and files = ["b.cc", "d.cc"] (B depends upon b.cc and D
+depends upon d.cc), then the following is output:
+|targets| = ["A"] A is output because A depends upon B and B has the changed
+file b.cc.
+|build_targets| = ["B"] B must built as it depends upon the changed file b.cc
+and the supplied target A depends upon it. A is not output as a build_target
+as it is of type none with no rules and actions.

[Dirk Pranke, 2015/10/12 19:01:28]

I thought we agreed that we were going to get rid of 'targets' as an output that
is different than 'build_targets' (or at least make them both return the same
thing?

[sky, 2015/10/12 19:05:18]

I'm fine with that, but wasn't sure about the staging of the changes. What is
recipes consuming now?

+
+Even though the file d.cc, which D depends upon, has changed D is not output
+as none of the supplied targets (A) depend upon D.
+
 If the generator flag analyzer_output_path is specified, output is written
 there. Otherwise output is written to stdout.
 """
@@ -259,13 +278,12 @@ def _WasBuildFileModified(build_file, data, files, toplevel_dir):
 
 def _GetOrCreateTargetByName(targets, target_name):
   """Creates or returns the Target at targets[target_name]. If there is no
-  Target for |target_name| one is created. Returns a tuple of whether a new
-  Target was created and the Target."""
+  Target for |target_name| one is created."""
   if target_name in targets:
-    return False, targets[target_name]
+    return targets[target_name]
   target = Target(target_name)
   targets[target_name] = target
-  return True, target
+  return target
 
 
 def _DoesTargetTypeRequireBuild(target_dict):
@@ -280,7 +298,6 @@ def _GenerateTargets(data, target_list, target_dicts, toplevel_dir, files,
   """Returns a tuple of the following:
   . A dictionary mapping from fully qualified name to Target.
   . A list of the targets that have a source file in |files|.
-  . Set of root Targets reachable from the the files |build_files|.
   This sets the |match_status| of the targets that contain any of the source
   files in |files| to MATCH_STATUS_MATCHES.
   |toplevel_dir| is the root of the source tree."""
@@ -297,18 +314,10 @@ def _GenerateTargets(data, target_list, target_dicts, toplevel_dir, files,
   # |files|.
   build_file_in_files = {}
 
-  # Root targets across all files.
-  roots = set()
-
-  # Set of Targets in |build_files|.
-  build_file_targets = set()
-
   while len(targets_to_visit) > 0:
     target_name = targets_to_visit.pop()
-    created_target, target = _GetOrCreateTargetByName(targets, target_name)
-    if created_target:
-      roots.add(target)
-    elif target.visited:
+    target = _GetOrCreateTargetByName(targets, target_name)
+    if target.visited:
       continue
 
     target.visited = True
@@ -325,9 +334,6 @@ def _GenerateTargets(data, target_list, target_dicts, toplevel_dir, files,
       build_file_in_files[build_file] = \
           _WasBuildFileModified(build_file, data, files, toplevel_dir)
 
-    if build_file in build_files:
-      build_file_targets.add(target)
-
     # If a build file (or any of its included files) is modified we assume all
     # targets in the file are modified.
     if build_file_in_files[build_file]:
@@ -348,14 +354,12 @@ def _GenerateTargets(data, target_list, target_dicts, toplevel_dir, files,
     for dep in target_dicts[target_name].get('dependencies', []):
       targets_to_visit.append(dep)
 
-      created_dep_target, dep_target = _GetOrCreateTargetByName(targets, dep)
-      if not created_dep_target:
-        roots.discard(dep_target)
+      dep_target = _GetOrCreateTargetByName(targets, dep)
 
       target.deps.add(dep_target)
       dep_target.back_deps.add(target)
 
-  return targets, matching_targets, roots & build_file_targets
+  return targets, matching_targets
 
 
 def _GetUnqualifiedToTargetMapping(all_targets, to_find):
@@ -405,21 +409,19 @@ def _GetTargetsDependingOn(possible_targets):
   return found
 
 
-def _AddBuildTargets(target, roots, add_if_no_ancestor, result):
+def _AddBuildTargets(target, roots, result):
   """Recurses through all targets that depend on |target|, adding all targets
   that need to be built (and are in |roots|) to |result|.
   roots: set of root targets.
-  add_if_no_ancestor: If true and there are no ancestors of |target| then add
-  |target| to |result|. |target| must still be in |roots|.
   result: targets that need to be built are added here."""
   if target.visited:
     return
 
   target.visited = True
-  target.in_roots = not target.back_deps and target in roots
+  target.in_roots = target in roots
 
   for back_dep_target in target.back_deps:
-    _AddBuildTargets(back_dep_target, roots, False, result)
+    _AddBuildTargets(back_dep_target, roots, result)
     target.added_to_compile_targets |= back_dep_target.added_to_compile_targets
     target.in_roots |= back_dep_target.in_roots
     target.is_or_has_linked_ancestor |= (
@@ -433,16 +435,14 @@ def _AddBuildTargets(target, roots, add_if_no_ancestor, result):
   # static libraries themselves, which are not compile time dependencies.
   if target.in_roots and \
         (target.is_executable or
-         (not target.added_to_compile_targets and
-          (add_if_no_ancestor or target.requires_build)) or
-         (target.is_static_library and add_if_no_ancestor and
-          not target.is_or_has_linked_ancestor)):
+         (not target.added_to_compile_targets and target.requires_build) or
+         (target.is_static_library and not target.is_or_has_linked_ancestor)):
     print '\t\tadding to build targets', target.name, 'executable', \
            target.is_executable, 'added_to_compile_targets', \
-           target.added_to_compile_targets, 'add_if_no_ancestor', \
-           add_if_no_ancestor, 'requires_build', target.requires_build, \
-           'is_static_library', target.is_static_library, \
-           'is_or_has_linked_ancestor', target.is_or_has_linked_ancestor
+           target.added_to_compile_targets, 'requires_build', \
+           target.requires_build, 'is_static_library', \
+           target.is_static_library, 'is_or_has_linked_ancestor', \
+           target.is_or_has_linked_ancestor
     result.add(target)
     target.added_to_compile_targets = True
 
@@ -454,7 +454,7 @@ def _GetBuildTargets(matching_targets, roots):
   result = set()
   for target in matching_targets:
     print '\tfinding build targets for match', target.name
-    _AddBuildTargets(target, roots, True, result)
+    _AddBuildTargets(target, roots, result)
   return result
 
 
@@ -557,14 +557,10 @@ def GenerateOutput(target_list, target_dicts, data, params):
       _WriteOutput(params, **result_dict)
       return
 
-    all_targets, matching_targets, roots = _GenerateTargets(
+    all_targets, matching_targets = _GenerateTargets(
       data, target_list, target_dicts, toplevel_dir, frozenset(config.files),
       params['build_files'])
 
-    print 'roots:'
-    for root in roots:
-      print '\t', root.name
-
     unqualified_mapping = _GetUnqualifiedToTargetMapping(all_targets,
                                                          config.targets)
     invalid_targets = None
@@ -587,7 +583,7 @@ def GenerateOutput(target_list, target_dicts, data, params):
       for target in all_targets.itervalues():
         target.visited = False
       print 'Finding build targets'
-      build_targets = _GetBuildTargets(matching_targets, roots)
+      build_targets = _GetBuildTargets(matching_targets, search_targets)
       matched_search_targets = [gyp.common.ParseQualifiedTarget(target.name)[1]
                                 for target in matched_search_targets]
       build_targets = [gyp.common.ParseQualifiedTarget(target.name)[1]