Hand over post-compile resources to zinc wrapper to keep classes/ and z.jar consistent

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

@@ -15,13 +15,14 @@ class CompileContext:
   """
 
   def __init__(self, target, analysis_file, classes_dir, jar_file,
-               log_dir, args_file, sources):
+               log_dir, args_file, post_compile_merge_dir, sources):
     self.target = target
     self.analysis_file = analysis_file
     self.classes_dir = classes_dir
     self.jar_file = jar_file
     self.log_dir = log_dir
     self.args_file = args_file
+    self.post_compile_merge_dir = post_compile_merge_dir
     self.sources = sources
 
   @contextmanager

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

@@ -172,7 +172,7 @@ def compile(self, ctx, args, dependency_classpath, upstream_analysis,
           if return_code:
             raise TaskError('javac exited with return code {rc}'.format(rc=return_code))
         self.context._scheduler.materialize_directories((
-          DirectoryToMaterialize(text_type(ctx.classes_dir.path), self.extra_resources_digest(ctx)),
+          DirectoryToMaterialize(text_type(ctx.classes_dir.path), self.extra_post_compile_resources_digest(ctx)),
         ))
 
     self._create_context_jar(ctx)
@@ -232,7 +232,7 @@ def _execute_hermetic_compile(self, cmd, ctx):
     # Dump the output to the .pants.d directory where it's expected by downstream tasks.
     merged_directories = self.context._scheduler.merge_directories([
         exec_result.output_directory_digest,
-        self.extra_resources_digest(ctx),
+        self.extra_post_compile_resources_digest(ctx),
       ])
     classes_directory = ctx.classes_dir.path
     self.context._scheduler.materialize_directories((

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

@@ -262,7 +262,7 @@ def scalac_plugin_classpath_elements(self):
     """Classpath entries containing scalac plugins."""
     return []
 
-  def extra_resources(self, compile_context):
+  def post_compile_extra_resources(self, compile_context):
     """Produces a dictionary of any extra, out-of-band resources for a target.
 
     E.g., targets that produce scala compiler plugins or annotation processor files
@@ -279,18 +279,26 @@ def extra_resources(self, compile_context):
 
     return result
 
-  def extra_resources_digest(self, compile_context):
-    """Compute a Digest for the extra_resources for the given context."""
+  def extra_post_compile_resources_digest(self, compile_context):
+    """Compute a Digest for the post_compile_extra_resources for the given context."""
     # TODO: Switch to using #7739 once it is available.
-    extra_resources = self.extra_resources(compile_context)
+    extra_resources = self.post_compile_extra_resources(compile_context)
     if not extra_resources:
       return EMPTY_DIRECTORY_DIGEST
+
     with temporary_dir() as tmpdir:
+      rel_post_compile_merge_dir = fast_relpath(compile_context.post_compile_merge_dir, get_buildroot())
+      root_dir = os.path.join(tmpdir, rel_post_compile_merge_dir)
+      safe_mkdir(root_dir)
+
       for filename, filecontent in extra_resources.items():
-        safe_file_dump(os.path.join(tmpdir, filename), filecontent)
+        safe_file_dump(os.path.join(root_dir, filename), filecontent)
+
+      extra_resources_relative_to_rootdir = {os.path.join(rel_post_compile_merge_dir, k): v for k, v in
+                                             extra_resources.items()}
       snapshot, = self.context._scheduler.capture_snapshots([
-          PathGlobsAndRoot(PathGlobs(extra_resources), tmpdir)
-        ])
+        PathGlobsAndRoot(PathGlobs(extra_resources_relative_to_rootdir), tmpdir)
+      ])
       return snapshot.directory_digest
 
   def write_argsfile(self, ctx, args):
@@ -367,13 +375,15 @@ def _missing_deps_finder(self):
       self.get_options().class_not_found_error_patterns))
 
   def create_compile_context(self, target, target_workdir):
-    return CompileContext(target,
-                          os.path.join(target_workdir, 'z.analysis'),
-                          ClasspathEntry(os.path.join(target_workdir, 'classes')),
-                          ClasspathEntry(os.path.join(target_workdir, 'z.jar')),
-                          os.path.join(target_workdir, 'logs'),
-                          os.path.join(target_workdir, 'zinc_args'),
-                          self._compute_sources_for_target(target))
+    return CompileContext(target=target,
+                          analysis_file=os.path.join(target_workdir, 'z.analysis'),
+                          classes_dir=ClasspathEntry(os.path.join(target_workdir, 'classes')),
+                          jar_file=ClasspathEntry(os.path.join(target_workdir, 'z.jar')),
+                          log_dir=os.path.join(target_workdir, 'logs'),
+                          args_file=os.path.join(target_workdir, 'zinc_args'),
+                          post_compile_merge_dir=os.path.join(target_workdir,
+                                                              'post_compile_merge_dir'),
+                          sources=self._compute_sources_for_target(target))
 
   def execute(self):
     if JvmPlatform.global_instance().get_options().compiler != self.compiler_name:

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

@@ -548,6 +548,7 @@ def create_compile_context(self, target, target_workdir):
         jar_file=ClasspathEntry(os.path.join(zinc_dir, 'z.jar'), None),
         log_dir=os.path.join(zinc_dir, 'logs'),
         args_file=os.path.join(zinc_dir, 'zinc_args'),
+        post_compile_merge_dir=os.path.join(zinc_dir, 'post_compile_merge_dir'),
         sources=sources,
       ))
 

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

@@ -227,9 +227,9 @@ def create_empty_extra_products(self):
     if self.context.products.is_required_data('zinc_args'):
       self.context.products.safe_create_data('zinc_args', lambda: defaultdict(list))
 
-  def extra_resources(self, compile_context):
-    """Override `extra_resources` to additionally include scalac_plugin info."""
-    result = super().extra_resources(compile_context)
+  def post_compile_extra_resources(self, compile_context):
+    """Override `post_compile_extra_resources` to additionally include scalac_plugin info."""
+    result = super().post_compile_extra_resources(compile_context)
     target = compile_context.target
 
     if isinstance(target, ScalacPlugin):
@@ -289,6 +289,10 @@ def relative_to_exec_root(path):
     if not self.get_options().colors:
       zinc_args.append('-no-color')
 
+    if self.post_compile_extra_resources(ctx):
+      post_compile_merge_dir = relative_to_exec_root(ctx.post_compile_merge_dir)
+      zinc_args.extend(['--post-compile-merge-dir', post_compile_merge_dir])
+
     compiler_bridge_classpath_entry = self._zinc.compile_compiler_bridge(self.context)
     zinc_args.extend(['-compiled-bridge-jar', relative_to_exec_root(compiler_bridge_classpath_entry.path)])
     zinc_args.extend(['-scala-path', ':'.join(scala_path)])
@@ -359,7 +363,15 @@ class ZincCompileError(TaskError):
     """An exception type specifically to signal a failed zinc execution."""
 
   def _compile_nonhermetic(self, jvm_options, ctx, classes_directory):
-    exit_code = self.runjava(classpath=self.get_zinc_compiler_classpath(),
+    # Populate the resources to merge post compile onto disk for the nonhermetic case,
+    # where `--post-compile-merge-dir` was added is the relevant part.
+    self.context._scheduler.materialize_directories((
+      DirectoryToMaterialize(get_buildroot(), self.extra_post_compile_resources_digest(ctx)),
+    ))
+
+    exit_code = self.runjava(
+                             # classpath=self.get_zinc_compiler_classpath(),
+                             classpath=['/Users/yic/workspace/pants/dist/bin.jar'],
                              main=Zinc.ZINC_COMPILE_MAIN,
                              jvm_options=jvm_options,
                              args=['@{}'.format(ctx.args_file)],
@@ -368,14 +380,12 @@ def _compile_nonhermetic(self, jvm_options, ctx, classes_directory):
                              dist=self._zinc.dist)
     if exit_code != 0:
       raise self.ZincCompileError('Zinc compile failed.', exit_code=exit_code)
-    self.context._scheduler.materialize_directories((
-      DirectoryToMaterialize(text_type(classes_directory), self.extra_resources_digest(ctx)),
-    ))
 
   def _compile_hermetic(self, jvm_options, ctx, classes_dir, jar_file,
                         compiler_bridge_classpath_entry, dependency_classpath,
                         scalac_classpath_entries):
-    zinc_relpath = fast_relpath(self._zinc.zinc, get_buildroot())
+    # zinc_relpath = fast_relpath(self._zinc.zinc, get_buildroot())
+    zinc_relpath = '/Users/yic/workspace/pants/dist/bin.jar'
 
     snapshots = [
       self._zinc.snapshot(self.context._scheduler),
@@ -457,7 +467,7 @@ def _compile_hermetic(self, jvm_options, ctx, classes_dir, jar_file,
       tuple(s.directory_digest for s in snapshots) +
       directory_digests +
       native_image_snapshots +
-      (self.extra_resources_digest(ctx), argfile_snapshot.directory_digest)
+      (self.extra_post_compile_resources_digest(ctx), argfile_snapshot.directory_digest)
     )
 
     req = ExecuteProcessRequest(

tests/python/pants_test/backend/jvm/tasks/jvm_compile/java/test_zinc_compile_integration.py

@@ -57,75 +57,6 @@ def test_unicode_source_symbol(self):
         )
         self.assert_success(pants_run)
 
-  def test_apt_compile(self):
-    with self.do_test_compile('testprojects/src/java/org/pantsbuild/testproject/annotation/processor',
-                              expected_files=['ResourceMappingProcessor.class',
-                                              'javax.annotation.processing.Processor']) as found:
-
-      self.assertTrue(
-          self.get_only(found, 'ResourceMappingProcessor.class').endswith(
-              'org/pantsbuild/testproject/annotation/processor/ResourceMappingProcessor.class'))
-
-      processor_service_files = found['javax.annotation.processing.Processor']
-      # There should be only a per-target service info file.
-      self.assertEqual(1, len(processor_service_files))
-      processor_service_file = list(processor_service_files)[0]
-      self.assertTrue(processor_service_file.endswith(
-          'META-INF/services/javax.annotation.processing.Processor'))
-      with open(processor_service_file, 'r') as fp:
-        self.assertEqual('org.pantsbuild.testproject.annotation.processor.ResourceMappingProcessor',
-                          fp.read().strip())
-
-  def test_apt_compile_and_run(self):
-    with self.do_test_compile('testprojects/src/java/org/pantsbuild/testproject/annotation/main',
-                              expected_files=['Main.class',
-                                              'deprecation_report.txt']) as found:
-
-      self.assertTrue(
-          self.get_only(found, 'Main.class').endswith(
-              'org/pantsbuild/testproject/annotation/main/Main.class'))
-
-      # This is the proof that the ResourceMappingProcessor annotation processor was compiled in a
-      # round and then the Main was compiled in a later round with the annotation processor and its
-      # service info file from on its compile classpath.
-      with open(self.get_only(found, 'deprecation_report.txt'), 'r') as fp:
-        self.assertIn('org.pantsbuild.testproject.annotation.main.Main', fp.read().splitlines())
-
-  def test_stale_apt_with_deps(self):
-    """An annotation processor with a dependency doesn't pollute other annotation processors.
-
-    At one point, when you added an annotation processor, it stayed configured for all subsequent
-    compiles.  Meaning that if that annotation processor had a dep that wasn't on the classpath,
-    subsequent compiles would fail with missing symbols required by the stale annotation processor.
-    """
-
-    # Demonstrate that the annotation processor is working
-    with self.do_test_compile(
-        'testprojects/src/java/org/pantsbuild/testproject/annotation/processorwithdep/main',
-        expected_files=['Main.class', 'Main_HelloWorld.class', 'Main_HelloWorld.java']) as found:
-      gen_file = self.get_only(found, 'Main_HelloWorld.java')
-      self.assertTrue(gen_file.endswith(
-        'org/pantsbuild/testproject/annotation/processorwithdep/main/Main_HelloWorld.java'),
-        msg='{} does not match'.format(gen_file))
-
-
-    # Try to reproduce second compile that fails with missing symbol
-    with self.temporary_workdir() as workdir:
-      with self.temporary_cachedir() as cachedir:
-        # This annotation processor has a unique external dependency
-        self.assert_success(self.run_test_compile(
-          workdir,
-          cachedir,
-          'testprojects/src/java/org/pantsbuild/testproject/annotation/processorwithdep::'))
-
-        # When we run a second compile with annotation processors, make sure the previous annotation
-        # processor doesn't stick around to spoil the compile
-        self.assert_success(self.run_test_compile(
-          workdir,
-          cachedir,
-          'testprojects/src/java/org/pantsbuild/testproject/annotation/processor::',
-          clean_all=False))
-
   def test_fatal_warning(self):
     def test_combination(target, expect_success):
       with self.temporary_workdir() as workdir:

tests/python/pants_test/backend/jvm/tasks/jvm_compile/zinc/zinc_compile_integration_base.py

@@ -106,6 +106,77 @@ def test_scala_with_java_sources_compile(self):
           self.get_only(found, 'JavaSource.class').endswith(
               'org/pantsbuild/testproject/javasources/JavaSource.class'))
 
+  def test_apt_compile(self):
+    with self.do_test_compile('testprojects/src/java/org/pantsbuild/testproject/annotation/processor',
+                              expected_files=['ResourceMappingProcessor.class',
+                                              'javax.annotation.processing.Processor']) as found:
+
+      self.assertTrue(
+          self.get_only(found, 'ResourceMappingProcessor.class').endswith(
+              'org/pantsbuild/testproject/annotation/processor/ResourceMappingProcessor.class'))
+
+      # processor info file under classes/ dir
+      processor_service_files = list(filter(lambda x: 'classes' in x,
+                                            found['javax.annotation.processing.Processor']))
+      # There should be only a per-target service info file.
+      self.assertEqual(1, len(processor_service_files))
+      processor_service_file = list(processor_service_files)[0]
+      self.assertTrue(processor_service_file.endswith(
+          'META-INF/services/javax.annotation.processing.Processor'))
+      with open(processor_service_file, 'r') as fp:
+        self.assertEqual('org.pantsbuild.testproject.annotation.processor.ResourceMappingProcessor',
+                          fp.read().strip())
+
+  def test_apt_compile_and_run(self):
+    with self.do_test_compile('testprojects/src/java/org/pantsbuild/testproject/annotation/main',
+                              expected_files=['Main.class',
+                                              'deprecation_report.txt']) as found:
+
+      self.assertTrue(
+          self.get_only(found, 'Main.class').endswith(
+              'org/pantsbuild/testproject/annotation/main/Main.class'))
+
+      # This is the proof that the ResourceMappingProcessor annotation processor was compiled in a
+      # round and then the Main was compiled in a later round with the annotation processor and its
+      # service info file from on its compile classpath.
+      with open(self.get_only(found, 'deprecation_report.txt'), 'r') as fp:
+        self.assertIn('org.pantsbuild.testproject.annotation.main.Main', fp.read().splitlines())
+
+  def test_stale_apt_with_deps(self):
+    """An annotation processor with a dependency doesn't pollute other annotation processors.
+
+    At one point, when you added an annotation processor, it stayed configured for all subsequent
+    compiles.  Meaning that if that annotation processor had a dep that wasn't on the classpath,
+    subsequent compiles would fail with missing symbols required by the stale annotation processor.
+    """
+
+    # Demonstrate that the annotation processor is working
+    with self.do_test_compile(
+        'testprojects/src/java/org/pantsbuild/testproject/annotation/processorwithdep/main',
+        expected_files=['Main.class', 'Main_HelloWorld.class', 'Main_HelloWorld.java']) as found:
+      gen_file = self.get_only(found, 'Main_HelloWorld.java')
+      self.assertTrue(gen_file.endswith(
+        'org/pantsbuild/testproject/annotation/processorwithdep/main/Main_HelloWorld.java'),
+        msg='{} does not match'.format(gen_file))
+
+
+    # Try to reproduce second compile that fails with missing symbol
+    with self.temporary_workdir() as workdir:
+      with self.temporary_cachedir() as cachedir:
+        # This annotation processor has a unique external dependency
+        self.assert_success(self.run_test_compile(
+          workdir,
+          cachedir,
+          'testprojects/src/java/org/pantsbuild/testproject/annotation/processorwithdep::'))
+
+        # When we run a second compile with annotation processors, make sure the previous annotation
+        # processor doesn't stick around to spoil the compile
+        self.assert_success(self.run_test_compile(
+          workdir,
+          cachedir,
+          'testprojects/src/java/org/pantsbuild/testproject/annotation/processor::',
+          clean_all=False))
+
   def test_scalac_plugin_compile(self):
     with self.do_test_compile(
         'examples/src/scala/org/pantsbuild/example/scalac/plugin:other_simple_scalac_plugin',