[rsc-compile] define workflow in context instead of on fly; use workf… (

#7324)

…low in traversal

This pulls the workflow up into the context objects and draws them through to the places we classify targets. This way we're centralizing how we do classification and only doing it once per target.

It also fixes an issue with how we were handling the invalid dep traversal where we were depending on the sources of targets in the predicate, instead of the workflow

src/python/pants/backend/jvm/tasks/jvm_compile/jvm_compile.py

@@ -667,7 +667,8 @@ def format_length(self):
       # Invalidated targets are a subset of relevant targets: get the context for this one.
       compile_target = ivts.target
       invalid_dependencies = self._collect_invalid_compile_dependencies(compile_target,
-                                                                        invalid_target_set)
+                                                                        invalid_target_set,
+                                                                        compile_contexts)
 
       jobs.extend(
         self.create_compile_jobs(compile_target, compile_contexts, invalid_dependencies, ivts,
@@ -744,7 +745,8 @@ def record(k, v):
     record('sources_len', sources_len)
     record('incremental', is_incremental)
 
-  def _collect_invalid_compile_dependencies(self, compile_target, invalid_target_set):
+  def _collect_invalid_compile_dependencies(self, compile_target, invalid_target_set,
+    compile_contexts):
     # Collects all invalid dependencies that are not dependencies of other invalid dependencies
     # within the closure of compile_target.
     invalid_dependencies = OrderedSet()
@@ -757,13 +759,13 @@ def predicate(target):
         return True
       if target in invalid_target_set:
         invalid_dependencies.add(target)
-        return self._on_invalid_compile_dependency(target, compile_target)
+        return self._on_invalid_compile_dependency(target, compile_target, compile_contexts)
       return True
 
     compile_target.walk(work, predicate)
     return invalid_dependencies
 
-  def _on_invalid_compile_dependency(self, dep, compile_target):
+  def _on_invalid_compile_dependency(self, dep, compile_target, compile_contexts):
     """Decide whether to continue searching for invalid targets to use in the execution graph.
 
     By default, don't recurse because once we have an invalid dependency, we can rely on its

src/python/pants/backend/jvm/tasks/jvm_compile/rsc/rsc_compile.py

@@ -135,9 +135,11 @@ def __init__(self,
                jar_file,
                log_dir,
                zinc_args_file,
-               sources):
+               sources,
+               workflow):
     super(RscCompileContext, self).__init__(target, analysis_file, classes_dir, jar_file,
                                                log_dir, zinc_args_file, sources)
+    self.workflow = workflow
     self.rsc_jar_file = rsc_jar_file
 
   def ensure_output_dirs_exist(self):
@@ -226,9 +228,8 @@ def confify(entries):
     # Ensure that the jar/rsc jar is on the rsc_classpath.
     for target in targets:
       rsc_cc, compile_cc = compile_contexts[target]
-      target_compile_type = self._classify_compile_target(target)
-      if target_compile_type is not None:
-        cp_entries = target_compile_type.resolve_for_enum_variant({
+      if rsc_cc.workflow is not None:
+        cp_entries = rsc_cc.workflow.resolve_for_enum_variant({
           'zinc-only': lambda : confify([compile_cc.jar_file]),
           'rsc-then-zinc': lambda : confify(
             to_classpath_entries([rsc_cc.rsc_jar_file], self.context._scheduler)),
@@ -253,27 +254,25 @@ def create_empty_extra_products(self):
   def select(self, target):
     if not isinstance(target, JvmTarget):
       return False
-    if self._classify_compile_target(target) is not None:
-      return True
-    return False
+    return self._classify_compile_target(target) is not None
 
   def _classify_compile_target(self, target):
     return _JvmCompileWorkflowType.classify_target(target)
 
-  def _key_for_target_as_dep(self, target):
+  def _key_for_target_as_dep(self, target, workflow):
     # used for jobs that are either rsc jobs or zinc jobs run against rsc
-    return self._classify_compile_target(target).resolve_for_enum_variant({
-      'zinc-only': lambda: self._zinc_key_for_target(target),
+    return workflow.resolve_for_enum_variant({
+      'zinc-only': lambda: self._zinc_key_for_target(target, workflow),
       'rsc-then-zinc': lambda: self._rsc_key_for_target(target),
     })()
 
-  def _rsc_key_for_target(self, compile_target):
-    return 'rsc({})'.format(compile_target.address.spec)
+  def _rsc_key_for_target(self, target):
+    return 'rsc({})'.format(target.address.spec)
 
-  def _zinc_key_for_target(self, compile_target):
-    return self._classify_compile_target(compile_target).resolve_for_enum_variant({
-      'zinc-only': lambda: 'zinc({})'.format(compile_target.address.spec),
-      'rsc-then-zinc': lambda: 'zinc_against_rsc({})'.format(compile_target.address.spec),
+  def _zinc_key_for_target(self, target, workflow):
+    return workflow.resolve_for_enum_variant({
+      'zinc-only': lambda: 'zinc({})'.format(target.address.spec),
+      'rsc-then-zinc': lambda: 'zinc_against_rsc({})'.format(target.address.spec),
     })()
 
   def create_compile_jobs(self,
@@ -384,9 +383,10 @@ def all_zinc_rsc_invalid_dep_keys(invalid_deps):
       for tgt in invalid_deps:
         # None can occur for e.g. JarLibrary deps, which we don't need to compile as they are
         # populated in the resolve goal.
-        if self._classify_compile_target(tgt) is not None:
+        tgt_rsc_cc, tgt_z_cc = compile_contexts[tgt]
+        if tgt_rsc_cc.workflow is not None:
           # Rely on the results of zinc compiles for zinc-compatible targets
-          yield self._key_for_target_as_dep(tgt)
+          yield self._key_for_target_as_dep(tgt, tgt_rsc_cc.workflow)
 
     def make_rsc_job(target, dep_targets):
       return Job(
@@ -403,12 +403,13 @@ def make_rsc_job(target, dep_targets):
 
     def only_zinc_invalid_dep_keys(invalid_deps):
       for tgt in invalid_deps:
-        if self._classify_compile_target(tgt) is not None:
-          yield self._zinc_key_for_target(tgt)
+        rsc_cc_tgt, zinc_cc_tgt = compile_contexts[tgt]
+        if rsc_cc_tgt.workflow is not None:
+          yield self._zinc_key_for_target(tgt, rsc_cc_tgt.workflow)
 
     def make_zinc_job(target, input_product_key, output_products, dep_keys):
       return Job(
-        key=self._zinc_key_for_target(target),
+        key=self._zinc_key_for_target(target, rsc_compile_context.workflow),
         fn=functools.partial(
           self._default_work_for_vts,
           ivts,
@@ -424,7 +425,7 @@ def make_zinc_job(target, input_product_key, output_products, dep_keys):
 
     # Create the rsc job.
     # Currently, rsc only supports outlining scala.
-    workflow = self._classify_compile_target(compile_target)
+    workflow = rsc_compile_context.workflow
     workflow.resolve_for_enum_variant({
       'zinc-only': lambda: None,
       'rsc-then-zinc': lambda: rsc_jobs.append(make_rsc_job(compile_target, invalid_dependencies)),
@@ -489,6 +490,7 @@ def create_compile_context(self, target, target_workdir):
         rsc_jar_file=os.path.join(rsc_dir, 'm.jar'),
         log_dir=os.path.join(rsc_dir, 'logs'),
         sources=sources,
+        workflow=self._classify_compile_target(target),
       ),
       CompileContext(
         target=target,
@@ -607,17 +609,17 @@ def _jdk_libs_paths_and_digest(self, hermetic_dist):
   def _jdk_libs_abs(self, nonhermetic_dist):
     return nonhermetic_dist.find_libs(self._JDK_LIB_NAMES)
 
-  def _on_invalid_compile_dependency(self, dep, compile_target):
+  def _on_invalid_compile_dependency(self, dep, compile_target, contexts):
     """Decide whether to continue searching for invalid targets to use in the execution graph.
 
-    If a necessary dep is a Scala dep and the root is Java, continue to recurse because
-    otherwise we'll drop the path between Zinc compile of the Java target and a Zinc
-    compile of a transitive Scala dependency.
+    If a necessary dep is a rsc-then-zinc dep and the root is a zinc-only one, continue to recurse
+    because otherwise we'll drop the path between Zinc compile of the zinc-only target and a Zinc
+    compile of a transitive rsc-then-zinc dependency.
 
-    This is only an issue for graphs like J -> S1 -> S2, where J is a Java target,
-    S1/2 are Scala targets and S2 must be on the classpath to compile J successfully.
+    This is only an issue for graphs like J -> S1 -> S2, where J is a zinc-only target,
+    S1/2 are rsc-then-zinc targets and S2 must be on the classpath to compile J successfully.
     """
-    if dep.has_sources('.scala') and compile_target.has_sources('.java'):
-      return True
-    else:
-      return False
+    return contexts[compile_target][0].workflow.resolve_for_enum_variant({
+      'zinc-only': lambda : contexts[dep][0].workflow == _JvmCompileWorkflowType.rsc_then_zinc,
+      'rsc-then-zinc': lambda : False
+    })()

tests/python/pants_test/backend/jvm/tasks/jvm_compile/rsc/test_rsc_compile.py

@@ -7,9 +7,11 @@
 import os
 from textwrap import dedent
 
+from pants.backend.jvm.subsystems.junit import JUnit
 from pants.backend.jvm.subsystems.scala_platform import ScalaPlatform
 from pants.backend.jvm.targets.jar_library import JarLibrary
 from pants.backend.jvm.targets.java_library import JavaLibrary
+from pants.backend.jvm.targets.junit_tests import JUnitTests
 from pants.backend.jvm.targets.scala_library import ScalaLibrary
 from pants.backend.jvm.tasks.classpath_products import ClasspathProducts
 from pants.backend.jvm.tasks.jvm_compile.execution_graph import ExecutionGraph
@@ -78,7 +80,7 @@ def test_no_dependencies_between_scala_and_java_targets(self):
                      zinc_against_rsc(scala/classpath:scala_lib) -> {}""").strip(),
         dependee_graph)
 
-  def test_scala_dep_for_scala_and_java_targets(self):
+  def test_rsc_dep_for_scala_java_and_test_targets(self):
     self.init_dependencies_for_scala_libraries()
 
     scala_dep = self.make_target(
@@ -99,10 +101,17 @@ def test_scala_dep_for_scala_and_java_targets(self):
       dependencies=[scala_dep]
     )
 
+    test_target = self.make_target(
+      'scala/classpath:scala_test',
+      target_type=JUnitTests,
+      sources=['com/example/Test.scala'],
+      dependencies=[scala_target]
+    )
+
     with temporary_dir() as tmp_dir:
-      invalid_targets = [java_target, scala_target, scala_dep]
+      invalid_targets = [java_target, scala_target, scala_dep, test_target]
       task = self.create_task_with_target_roots(
-        target_roots=[java_target, scala_target]
+        target_roots=[java_target, scala_target, test_target]
       )
 
       jobs = task._create_compile_jobs(
@@ -118,15 +127,19 @@ def test_scala_dep_for_scala_and_java_targets(self):
                      rsc(scala/classpath:scala_lib) -> {
                        zinc_against_rsc(scala/classpath:scala_lib)
                      }
-                     zinc_against_rsc(scala/classpath:scala_lib) -> {}
+                     zinc_against_rsc(scala/classpath:scala_lib) -> {
+                       zinc(scala/classpath:scala_test)
+                     }
                      rsc(scala/classpath:scala_dep) -> {
                        rsc(scala/classpath:scala_lib),
                        zinc_against_rsc(scala/classpath:scala_lib),
                        zinc_against_rsc(scala/classpath:scala_dep)
                      }
                      zinc_against_rsc(scala/classpath:scala_dep) -> {
-                       zinc(java/classpath:java_lib)
-                     }""").strip(),
+                       zinc(java/classpath:java_lib),
+                       zinc(scala/classpath:scala_test)
+                     }
+                     zinc(scala/classpath:scala_test) -> {}""").strip(),
         dependee_graph)
 
   def test_scala_lib_with_java_sources_not_passed_to_rsc(self):
@@ -175,12 +188,6 @@ def test_scala_lib_with_java_sources_not_passed_to_rsc(self):
                      zinc(scala/classpath:scala_with_indirect_java_sources) -> {}""").strip(),
         dependee_graph)
 
-  def construct_dependee_graph_str(self, jobs, task):
-    exec_graph = ExecutionGraph(jobs, task.get_options().print_exception_stacktrace)
-    dependee_graph = exec_graph.format_dependee_graph()
-    print(dependee_graph)
-    return dependee_graph
-
   def test_desandbox_fn(self):
     # TODO remove this after https://github.com/scalameta/scalameta/issues/1791 is released
     desandbox = _create_desandboxify_fn(['.pants.d/cool/beans.*', '.pants.d/c/r/c/.*'])
@@ -199,6 +206,12 @@ def test_desandbox_fn(self):
     self.assertEqual(desandbox('/some/path/.pants.d/exec-location/.pants.d/tmp.pants.d/cool/beans'),
                                '.pants.d/tmp.pants.d/cool/beans')
 
+  def construct_dependee_graph_str(self, jobs, task):
+    exec_graph = ExecutionGraph(jobs, task.get_options().print_exception_stacktrace)
+    dependee_graph = exec_graph.format_dependee_graph()
+    print(dependee_graph)
+    return dependee_graph
+
   def wrap_in_vts(self, invalid_targets):
     return [LightWeightVTS(t) for t in invalid_targets]
 
@@ -212,11 +225,19 @@ def init_dependencies_for_scala_libraries(self):
         }
       }
     )
+    init_subsystem(
+      JUnit,
+    )
     self.make_target(
       '//:scala-library',
       target_type=JarLibrary,
       jars=[JarDependency(org='com.example', name='scala', rev='0.0.0')]
     )
+    self.make_target(
+      '//:junit-library',
+      target_type=JarLibrary,
+      jars=[JarDependency(org='com.example', name='scala', rev='0.0.0')]
+    )
 
   def create_task_with_target_roots(self, target_roots):
     context = self.context(target_roots=target_roots)