Skip to content

Commit

Permalink
[compile.rsc] fix key error; ensure java compiles get necessary zinc …
Browse files Browse the repository at this point in the history
…scala deps (pantsbuild#7038)

### Problem
- the dependency list for java compiles used the wrong keys in some cases
- collecting the invalid deps for the graph for java zinc compiles requires traversing transitive scala deps

### Solution
- add a helper for metacp deps
- override collecting invalid deps to ensure the walk recurses when the compile target has java deps and the current is scala

### Description

For a plain zinc compile, all of the work has the same flavor, so we can assume that only depending on a direct dependency will give us the outputs needed. But, the Rsc graph has multiple flavors of nodes, so to ensure the right dependency flavors have already been calculated, we need to traverse more of the invalidated graph.
  • Loading branch information
baroquebobcat authored Jan 9, 2019
1 parent 64073b0 commit 93cbcfb
Show file tree
Hide file tree
Showing 9 changed files with 102 additions and 2 deletions.
11 changes: 10 additions & 1 deletion src/python/pants/backend/jvm/tasks/jvm_compile/jvm_compile.py
Original file line number Diff line number Diff line change
Expand Up @@ -757,12 +757,21 @@ def predicate(target):
return True
if target in invalid_target_set:
invalid_dependencies.add(target)
return False
return self._on_invalid_compile_dependency(target, compile_target)
return True

compile_target.walk(work, predicate)
return invalid_dependencies

def _on_invalid_compile_dependency(self, dep, compile_target):
"""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
dependencies having been compiled already.
Override to adjust this behavior."""
return False

def _create_context_jar(self, compile_context):
"""Jar up the compile_context to its output jar location.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,17 @@ def _rsc_key_for_target(self, compile_target):
else:
raise TaskError('unexpected target for compiling with rsc .... {}'.format(compile_target))

def _metacp_dep_key_for_target(self, compile_target):
if self._only_zinc_compileable(compile_target):
# rsc outlining with java dependencies depends on the output of a second metacp job.
return self._metacp_key_for_target(compile_target)
elif self._rsc_compilable(compile_target):
return self._compile_against_rsc_key_for_target(compile_target)
elif self._metacpable(compile_target):
return self._metacp_key_for_target(compile_target)
else:
raise TaskError('unexpected target for compiling with rsc .... {}'.format(compile_target))

def _metacp_key_for_target(self, compile_target):
return "metacp({})".format(compile_target.address.spec)

Expand Down Expand Up @@ -684,7 +695,7 @@ def add_for_target(self, *args, **kwargs):
ivts,
compile_context_pair[0],
'runtime_classpath'),
[self._metacp_key_for_target(target) for target in invalid_dependencies] + [
[self._metacp_dep_key_for_target(target) for target in invalid_dependencies] + [
'metacp(jdk)',
zinc_key,
],
Expand Down Expand Up @@ -902,3 +913,18 @@ def _rehome(self, l):
return HermeticDistribution('.jdk', local_distribution)
else:
return local_distribution

def _on_invalid_compile_dependency(self, dep, compile_target):
"""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.
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.
"""
if dep.has_sources('.scala') and compile_target.has_sources('.java'):
return True
else:
return False
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# Copyright 2014 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

java_library(
dependencies = [
'testprojects/src/scala/org/pantsbuild/testproject/javadepsonscalatransitive:scala'
],
sources = rglobs('*.java'),
strict_deps = True
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright 2014 Pants project contributors (see CONTRIBUTORS.md).
// Licensed under the Apache License, Version 2.0 (see LICENSE).

package org.pantsbuild.testproject.javadepsonscalatransitive;

public class Java {

public String doStuff() {
// This should not trigger a missing dependency warning
// since we actually depend on the scala library.
Scala scala = new Scala();
Scala2 scala2 = new Scala2();
return scala.toString() + scala2.toString();
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# Copyright 2014 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

scala_library(
name = 'scala',
sources = ['Scala.scala'],
dependencies = [':scala-2'],
exports = [':scala-2']
)

scala_library(
name = 'scala-2',
sources = ['Scala2.scala']
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
If a java_library depends on a scala_library and a scala_library exported by that scala_library, it
should still be compilable.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package org.pantsbuild.testproject.javadepsonscalatransitive

class Scala {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package org.pantsbuild.testproject.javadepsonscalatransitive

class Scala2 {}
Original file line number Diff line number Diff line change
Expand Up @@ -96,3 +96,20 @@ def test_executing_multi_target_binary_hermetic(self):
workdir, config)
self.assert_success(pants_run)
self.assertIn('Hello, Resource World!', pants_run.stdout_data)

def test_java_with_transitive_exported_scala_dep(self):
with temporary_dir() as cache_dir:
config = {
'cache.compile.rsc': {'write_to': [cache_dir]},
'jvm-platform': {'compiler': 'rsc'},
'compile.rsc': {
'execution_strategy': 'subprocess',
'worker_count': 1}
}
with self.temporary_workdir() as workdir:
pants_run = self.run_pants_with_workdir(
['compile',
'testprojects/src/scala/org/pantsbuild/testproject/javadepsonscalatransitive:scala',
],
workdir, config)
self.assert_success(pants_run)

0 comments on commit 93cbcfb

Please sign in to comment.