[jvm-compile][bug] Fixes other target's class dir ending up on classpath

https://rbcommons.com/s/twitter/r/3635/ introduced a regression where classpath entries for targets other than the target being compiled could end up on the compile classpath if there are multiple targets being compiled.

An example.
You tell pants to compile targets A, B and C.
When jvm_compile creates the compile jobs, it does it in the order A, B and C.
Then, for A and B instead of having their own class directories on the classpath, they'll have C's. This is because compile_context will point at C and we were using compile_context to find the class directory.

This fixes it by replacing the usages of the closed over iteration variable with the passed ctx variable.

It also adds a regression test.

Testing Done:
Wrote failing regression test and made it pass. CI away on the linked PR.

Bugs closed: 3824

Reviewed at https://rbcommons.com/s/twitter/r/4198/

closes pantsbuild#3824

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

@@ -680,7 +680,7 @@ def check_cache(vts):
       counter()
       return True
 
-    def should_compile_incrementally(vts):
+    def should_compile_incrementally(vts, ctx):
       """Check to see if the compile should try to re-use the existing analysis.
 
       Returns true if we should try to compile the target incrementally.
@@ -689,7 +689,7 @@ def should_compile_incrementally(vts):
         return False
       if not self._clear_invalid_analysis:
         return True
-      return os.path.exists(compile_context.analysis_file)
+      return os.path.exists(ctx.analysis_file)
 
     def work_for_vts(vts, ctx):
       progress_message = ctx.target.address.spec
@@ -702,7 +702,7 @@ def work_for_vts(vts, ctx):
 
       if not hit_cache:
         # Compute the compile classpath for this target.
-        cp_entries = [compile_context.classes_dir]
+        cp_entries = [ctx.classes_dir]
         cp_entries.extend(ClasspathUtil.compute_classpath(ctx.dependencies(self._dep_context),
                                                           classpath_products,
                                                           extra_compile_time_classpath,
@@ -712,7 +712,7 @@ def work_for_vts(vts, ctx):
 
         # Write analysis to a temporary file, and move it to the final location on success.
         tmp_analysis_file = "{}.tmp".format(ctx.analysis_file)
-        if should_compile_incrementally(vts):
+        if should_compile_incrementally(vts, ctx):
           # If this is an incremental compile, rebase the analysis to our new classes directory.
           self._analysis_tools.rebase_from_path(ctx.analysis_file,
                                                 tmp_analysis_file,

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

@@ -155,6 +155,31 @@ def test_combination(target, default_fatal_warnings, expect_success):
     test_combination('nonfatal', default_fatal_warnings=True, expect_success=True)
     test_combination('nonfatal', default_fatal_warnings=False, expect_success=True)
 
+  def test_classpath_does_not_include_extra_classes_dirs(self):
+    target_rel_spec = 'testprojects/src/java/org/pantsbuild/testproject/phrases:'
+    classpath_file_by_target_id = {}
+    for target_name in ['there-was-a-duck',
+      'lesser-of-two',
+      'once-upon-a-time',
+      'ten-thousand']:
+      target_id = Target.compute_target_id(Address.parse('{}{}'
+        .format(target_rel_spec, target_name)))
+      classpath_file_by_target_id[target_id] = '{}.txt'.format(target_id)
+
+    with self.do_test_compile(target_rel_spec,
+      expected_files=classpath_file_by_target_id.values(),
+      extra_args=['--compile-zinc-capture-classpath']) as found:
+      for target_id, filename in classpath_file_by_target_id.items():
+        found_classpath_file = self.get_only(found, filename)
+        with open(found_classpath_file, 'r') as f:
+          contents = f.read()
+
+          self.assertIn(target_id, contents)
+
+          other_target_ids = set(classpath_file_by_target_id.keys()) - {target_id}
+          for other_id in other_target_ids:
+            self.assertNotIn(other_id, contents)
+
   def test_record_classpath(self):
     target_spec = 'testprojects/src/java/org/pantsbuild/testproject/printversion:printversion'
     target_id = Target.compute_target_id(Address.parse(target_spec))