Skip to content

Commit

Permalink
Zinc compiles can execute hermetically
Browse files Browse the repository at this point in the history
There are a LOT of TODOs here, but it fundamentally works.

Notable things which are missing:
 * Cached vts won't provide files on the classpath
 * Rebase map is not set up properly
 * use-classpath-jars doesn't work
 * workdir must be a child of buildroot
 * classpath entries in the java dist don't work
 * Upstream analysis may or may not work properly in all cases
 * Remote execution will fail because .jdk isn't created yet
 * -zinc-cache-dir is in a homedir, so won't be written if that homedir
   doesn't exist
 * Many things (e.g. jar_library) don't populate ClasspathEntry
   DirectoryDigests
 * A big pile of inefficiencies
  • Loading branch information
illicitonion committed Aug 15, 2018
1 parent 8ce6297 commit b135ad9
Show file tree
Hide file tree
Showing 5 changed files with 153 additions and 19 deletions.
20 changes: 20 additions & 0 deletions src/python/pants/backend/jvm/subsystems/zinc.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@
from pants.backend.jvm.tasks.classpath_products import ClasspathEntry
from pants.backend.jvm.tasks.classpath_util import ClasspathUtil
from pants.base.build_environment import get_buildroot
from pants.engine.fs import PathGlobs, PathGlobsAndRoot
from pants.java.jar.jar_dependency import JarDependency
from pants.subsystem.subsystem import Subsystem
from pants.util.dirutil import fast_relpath
from pants.util.memo import memoized_method, memoized_property


Expand Down Expand Up @@ -118,6 +120,7 @@ def create(self, products):
def __init__(self, zinc_factory, products):
self._zinc_factory = zinc_factory
self._products = products
self._snapshot = None

@memoized_property
def zinc(self):
Expand Down Expand Up @@ -151,6 +154,23 @@ def compiler_interface(self):
"""
return self._zinc_factory._compiler_interface(self._products)

def snapshot(self, scheduler):
if self._snapshot is None:
buildroot = get_buildroot()
self._snapshot = scheduler.capture_snapshots((
PathGlobsAndRoot(
PathGlobs(
tuple(
fast_relpath(a, buildroot)
for a in (self.zinc, self.compiler_bridge, self.compiler_interface)
)
),
buildroot,
),
))[0]
return self._snapshot

# TODO: Relativise (and sort)
@memoized_property
def rebase_map_args(self):
"""We rebase known stable paths in zinc analysis to make it portable across machines."""
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/backend/jvm/tasks/classpath_products.py
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ def _add_excludes_for_target(self, target):
self._excludes.add_for_target(target, target.excludes)

def _wrap_path_elements(self, classpath_elements):
return [(element[0], ClasspathEntry(element[1])) for element in classpath_elements]
return [(element[0], ClasspathEntry(*element[1:])) for element in classpath_elements]

def _add_elements_for_target(self, target, elements):
self._validate_classpath_tuples(elements, target)
Expand Down
10 changes: 7 additions & 3 deletions src/python/pants/backend/jvm/tasks/jvm_compile/jvm_compile.py
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,7 @@ def execute(self):
classpath_product,
)

# TODO: Pass around DirectoryDigests in these ClasspathEntries
if not self.get_options().use_classpath_jars:
# Once compilation has completed, replace the classpath entry for each target with
# its jar'd representation.
Expand Down Expand Up @@ -418,6 +419,8 @@ def do_compile(self, invalidation_check, compile_contexts, classpath_product):
valid_targets = [vt.target for vt in invalidation_check.all_vts if vt.valid]

# Register classpaths and products for valid targets.
# TODO: Store the DirectoryDigest for successful compiles in the target workdir as text,
# and hydrate them into the ClasspathEntries here when we get cache hits.
for valid_target in valid_targets:
cc = self.select_runtime_context(compile_contexts[valid_target])
classpath_product.add_for_target(
Expand Down Expand Up @@ -496,7 +499,7 @@ def _compile_vts(self, vts, ctx, upstream_analysis, dependency_classpath, progre
').')
with self.context.new_workunit('compile', labels=[WorkUnitLabel.COMPILER]) as compile_workunit:
try:
self.compile(
directory_digest = self.compile(
ctx,
self._args,
dependency_classpath,
Expand All @@ -508,6 +511,7 @@ def _compile_vts(self, vts, ctx, upstream_analysis, dependency_classpath, progre
self._get_plugin_map('scalac', ScalaPlatform.global_instance(), ctx.target),
)
self._capture_logs(compile_workunit, ctx.log_dir)
return directory_digest
except TaskError:
if self.get_options().suggest_missing_deps:
logs = [path
Expand Down Expand Up @@ -698,7 +702,7 @@ def work_for_vts(vts, ctx):
compiler_option_sets = dep_context.defaulted_property(tgt, lambda x: x.compiler_option_sets)
zinc_file_manager = dep_context.defaulted_property(tgt, lambda x: x.zinc_file_manager)
with Timer() as timer:
self._compile_vts(vts,
directory_digest = self._compile_vts(vts,
ctx,
upstream_analysis,
dependency_cp_entries,
Expand All @@ -723,7 +727,7 @@ def work_for_vts(vts, ctx):
# Update the products with the latest classes.
classpath_product.add_for_target(
ctx.target,
[(conf, self._classpath_for_context(ctx)) for conf in self._confs],
[(conf, self._classpath_for_context(ctx), directory_digest) for conf in self._confs],
)
self.register_extra_products_from_contexts([ctx.target], all_compile_contexts)

Expand Down
104 changes: 89 additions & 15 deletions src/python/pants/backend/jvm/tasks/jvm_compile/zinc/zinc_compile.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
from hashlib import sha1
from xml.etree import ElementTree

from future.utils import PY3
from future.utils import PY3, text_type

from pants.backend.jvm.subsystems.java import Java
from pants.backend.jvm.subsystems.jvm_platform import JvmPlatform
Expand All @@ -31,6 +31,8 @@
from pants.base.exceptions import TaskError
from pants.base.hash_utils import hash_file
from pants.base.workunit import WorkUnitLabel
from pants.engine.fs import DirectoryToMaterialize, PathGlobs, PathGlobsAndRoot
from pants.engine.isolated_process import ExecuteProcessRequest
from pants.java.distribution.distribution import DistributionLocator
from pants.util.contextutil import open_zip
from pants.util.dirutil import fast_relpath, safe_open
Expand Down Expand Up @@ -207,6 +209,20 @@ def __init__(self, *args, **kwargs):
# Validate zinc options.
ZincCompile.validate_arguments(self.context.log, self.get_options().whitelisted_args,
self._args)
if self.execution_strategy == self.HERMETIC:
if fast_relpath(self.get_options().pants_workdir, get_buildroot()).startswith('..'):
raise TaskError(
"Hermetic zinc execution currently requires the workdir to be a child of the buildroot but "
"workdir was {} and buildroot was {}".format(
self.get_options().pants_workdir,
get_buildroot(),
)
)

if self.get_options().use_classpath_jars:
# TODO: Make this work by capturing the correct DirectoryDigest and passing them around the
# right places.
raise TaskError("Hermetic zinc execution currently doesn't work with classpath jars")

def select(self, target):
raise NotImplementedError()
Expand Down Expand Up @@ -284,10 +300,13 @@ def compile(self, ctx, args, dependency_classpath, upstream_analysis,
if self.get_options().capture_classpath:
self._record_compile_classpath(absolute_classpath, ctx.target, ctx.classes_dir)

self._verify_zinc_classpath(absolute_classpath)
self._verify_zinc_classpath(list(upstream_analysis.keys()))
# TODO: Allow use of absolute classpath entries with hermetic execution, specifically by putting in $JAVA_HOME placeholders
self._verify_zinc_classpath(absolute_classpath, allow_dist=(self.execution_strategy != self.HERMETIC))
# TODO: Investigate upstream_analysis for hermetic compiles
self._verify_zinc_classpath(upstream_analysis.keys())

def relative_to_exec_root(path):
# TODO: Support workdirs not nested under buildroot by path-rewriting.
return fast_relpath(path, get_buildroot())

scala_path = self.scalac_classpath()
Expand Down Expand Up @@ -316,7 +335,8 @@ def relative_to_exec_root(path):

zinc_args.extend(['-compiler-interface', compiler_interface])
zinc_args.extend(['-compiler-bridge', compiler_bridge])
# TODO: Move this to live inside the workdir: https://github.com/pantsbuild/pants/issues/6155
# TODO: Kill zinc-cache-dir: https://github.com/pantsbuild/pants/issues/6155
# But for now, this will probably fail remotely because the homedir probably doesn't exist.
zinc_args.extend(['-zinc-cache-dir', self._zinc_cache_dir])
zinc_args.extend(['-scala-path', ':'.join(scala_path)])

Expand Down Expand Up @@ -388,16 +408,70 @@ def relative_to_exec_root(path):
fp.write(arg)
fp.write(b'\n')

if self.runjava(classpath=[self._zinc.zinc],
main=Zinc.ZINC_COMPILE_MAIN,
jvm_options=jvm_options,
args=zinc_args,
workunit_name=self.name(),
workunit_labels=[WorkUnitLabel.COMPILER],
dist=self._zinc.dist):
raise TaskError('Zinc compile failed.')

def _verify_zinc_classpath(self, classpath):
if self.execution_strategy == self.HERMETIC:
zinc_relpath = fast_relpath(self._zinc.zinc, get_buildroot())

snapshots = [
self._zinc.snapshot(self.context._scheduler),
ctx.target.sources_snapshot(self.context._scheduler),
]

directory_digests = tuple(
entry.directory_digest for entry in dependency_classpath if entry.directory_digest
)
if len(directory_digests) != len(dependency_classpath):
for dep in dependency_classpath:
if dep.directory_digest is None:
logger.warning(
"ClasspathEntry {} didn't have a DirectoryDigest, so won't be present for hermetic "
"execution".format(dep)
)

if scala_path:
snapshots.append(
self.context._scheduler.capture_snapshots((PathGlobsAndRoot(
PathGlobs(scala_path),
get_buildroot(),
),))[0]
)

merged_input_digest = self.context._scheduler.merge_directories(
tuple(s.directory_digest for s in (snapshots)) + directory_digests
)

# TODO: Extract something common from Executor._create_command to make the command line
# TODO: Lean on distribution for the bin/java appending here
argv = tuple(['.jdk/bin/java'] + jvm_options + ['-cp', zinc_relpath, Zinc.ZINC_COMPILE_MAIN] + zinc_args)
req = ExecuteProcessRequest(
argv=argv,
# TODO: https://github.com/pantsbuild/pants/issues/6160
env={'PATH': '/bin'},
input_files=merged_input_digest,
output_files=(analysis_cache,),
output_directories=(classes_dir,),
description="zinc compile for {}".format(ctx.target.address.spec),
# TODO: These should always be unicodes
jdk_home=text_type(self._zinc.dist.home),
)
res = self.context.execute_process_synchronously(req, self.name(), [WorkUnitLabel.COMPILER])
# TODO: Materialize as a batch in do_compile or somewhere
self.context._scheduler.materialize_directories((
DirectoryToMaterialize(get_buildroot(), res.output_directory_digest),
))

# TODO: This should probably return a ClasspathEntry rather than a DirectoryDigest
return res.output_directory_digest
else:
if self.runjava(classpath=[self._zinc.zinc],
main=Zinc.ZINC_COMPILE_MAIN,
jvm_options=jvm_options,
args=zinc_args,
workunit_name=self.name(),
workunit_labels=[WorkUnitLabel.COMPILER],
dist=self._zinc.dist):
raise TaskError('Zinc compile failed.')

def _verify_zinc_classpath(self, classpath, allow_dist=True):
def is_outside(path, putative_parent):
return os.path.relpath(path, putative_parent).startswith(os.pardir)

Expand All @@ -407,7 +481,7 @@ def is_outside(path, putative_parent):
raise TaskError('Classpath entries provided to zinc should be absolute. '
'{} is not.'.format(path))

if is_outside(path, self.get_options().pants_workdir) and is_outside(path, dist.home):
if is_outside(path, self.get_options().pants_workdir) and (not allow_dist or is_outside(path, dist.home)):
raise TaskError('Classpath entries provided to zinc should be in working directory or '
'part of the JDK. {} is not.'.format(path))
if path != os.path.normpath(path):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

from pants.build_graph.address import Address
from pants.build_graph.target import Target
from pants.util.contextutil import temporary_dir
from pants_test.backend.jvm.tasks.jvm_compile.base_compile_integration_test import BaseCompileIT
from pants_test.backend.jvm.tasks.missing_jvm_check import is_missing_jvm

Expand Down Expand Up @@ -218,3 +219,38 @@ def test_no_zinc_file_manager(self):
with self.temporary_cachedir() as cachedir:
pants_run = self.run_test_compile(workdir, cachedir, target_spec, clean_all=True)
self.assertEqual(0, pants_run.returncode)

def test_hermetic_binary_with_dependencies(self):
with temporary_dir() as cache_dir:
config = {
'cache.compile.zinc': {'write_to': [cache_dir]},
'compile.zinc': {
'execution_strategy': 'hermetic',
'use_classpath_jars': False,
}
}

with self.temporary_workdir(cleanup=False) as workdir:
pants_run = self.run_pants_with_workdir(
[
'-q',
'run',
'examples/src/scala/org/pantsbuild/example/hello/exe',
],
workdir,
config,
)
self.assert_success(pants_run)
self.assertIn(
'Num args passed: 0. Stand by for welcome...\nHello, Resource World!',
pants_run.stdout_data,
)

compile_dir = os.path.join(workdir, 'compile', 'zinc', 'current')

for path_suffix in [
'examples.src.scala.org.pantsbuild.example.hello.exe.exe/current/classes/org/pantsbuild/example/hello/exe/Exe.class',
'examples.src.scala.org.pantsbuild.example.hello.welcome.welcome/current/classes/org/pantsbuild/example/hello/welcome/WelcomeEverybody.class',
]:
path = os.path.join(compile_dir, path_suffix)
self.assertTrue(os.path.exists(path), "Want path {} to exist".format(path))

0 comments on commit b135ad9

Please sign in to comment.