diff --git a/src/python/pants/backend/jvm/tasks/jvm_compile/zinc/zinc_compile.py b/src/python/pants/backend/jvm/tasks/jvm_compile/zinc/zinc_compile.py index 4cdef91fb23..a447cdb0af1 100644 --- a/src/python/pants/backend/jvm/tasks/jvm_compile/zinc/zinc_compile.py +++ b/src/python/pants/backend/jvm/tasks/jvm_compile/zinc/zinc_compile.py @@ -386,71 +386,90 @@ def relative_to_exec_root(path): with open(ctx.zinc_args_file, 'w') as fp: for arg in zinc_args: # NB: in Python 2, options are stored sometimes as bytes and sometimes as unicode in the OptionValueContainer. - # This is due to how Python 2 natively stores attributes as a map of `str` (aka `bytes`) to their value. So, + # This is due to how Python 2 natively stores attributes as a map of `str` (aka `bytes`) to their value. So, # the setattr() and getattr() functions sometimes use bytes. if PY2: arg = ensure_text(arg) fp.write(arg) fp.write('\n') - 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), - ] - - relevant_classpath_entries = dependency_classpath + [compiler_bridge_classpath_entry] - directory_digests = tuple( - entry.directory_digest for entry in relevant_classpath_entries if entry.directory_digest - ) - if len(directory_digests) != len(relevant_classpath_entries): - for dep in relevant_classpath_entries: - if dep.directory_digest is None: - logger.warning( - "ClasspathEntry {} didn't have a Digest, so won't be present for hermetic " - "execution".format(dep) - ) - - snapshots.extend( - classpath_entry.directory_digest for classpath_entry in scalac_classpath_entries - ) - - 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, - input_files=merged_input_digest, - output_directories=(classes_dir,), - description="zinc compile for {}".format(ctx.target.address.spec), - # TODO: These should always be unicodes - # Since this is always hermetic, we need to use `underlying_dist` - jdk_home=text_type(self._zinc.underlying_dist.home), - ) - res = self.context.execute_process_synchronously_or_raise(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 Digest - return res.output_directory_digest - else: - if self.runjava(classpath=self.get_zinc_compiler_classpath(), - 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.') + return self.execution_strategy_enum.resolve_for_enum_variant({ + self.HERMETIC: lambda: self._compile_hermetic( + jvm_options, ctx, classes_dir, zinc_args, compiler_bridge_classpath_entry, + dependency_classpath, scalac_classpath_entries), + self.SUBPROCESS: lambda: self._compile_nonhermetic(jvm_options, zinc_args), + self.NAILGUN: lambda: self._compile_nonhermetic(jvm_options, zinc_args), + })() + + class ZincCompileError(TaskError): + """An exception type specifically to signal a failed zinc execution.""" + + def _compile_nonhermetic(self, jvm_options, zinc_args): + exit_code = self.runjava(classpath=self.get_zinc_compiler_classpath(), + main=Zinc.ZINC_COMPILE_MAIN, + jvm_options=jvm_options, + args=zinc_args, + workunit_name=self.name(), + workunit_labels=[WorkUnitLabel.COMPILER], + dist=self._zinc.dist) + if exit_code != 0: + raise self.ZincCompileError('Zinc compile failed.', exit_code=exit_code) + + def _compile_hermetic(self, jvm_options, ctx, classes_dir, zinc_args, + compiler_bridge_classpath_entry, dependency_classpath, + scalac_classpath_entries): + zinc_relpath = fast_relpath(self._zinc.zinc, get_buildroot()) + + snapshots = [ + self._zinc.snapshot(self.context._scheduler), + ctx.target.sources_snapshot(self.context._scheduler), + ] + + relevant_classpath_entries = dependency_classpath + [compiler_bridge_classpath_entry] + directory_digests = tuple( + entry.directory_digest for entry in relevant_classpath_entries if entry.directory_digest + ) + if len(directory_digests) != len(relevant_classpath_entries): + for dep in relevant_classpath_entries: + if dep.directory_digest is None: + logger.warning( + "ClasspathEntry {} didn't have a Digest, so won't be present for hermetic " + "execution".format(dep) + ) + + snapshots.extend( + classpath_entry.directory_digest for classpath_entry in scalac_classpath_entries + ) + + # TODO: Extract something common from Executor._create_command to make the command line + # TODO: Lean on distribution for the bin/java appending here + merged_input_digest = self.context._scheduler.merge_directories( + tuple(s.directory_digest for s in snapshots) + directory_digests + ) + argv = ['.jdk/bin/java'] + jvm_options + [ + '-cp', zinc_relpath, + Zinc.ZINC_COMPILE_MAIN + ] + zinc_args + + req = ExecuteProcessRequest( + argv=tuple(argv), + input_files=merged_input_digest, + output_directories=(classes_dir,), + description="zinc compile for {}".format(ctx.target.address.spec), + # TODO: These should always be unicodes + # Since this is always hermetic, we need to use `underlying_dist` + jdk_home=text_type(self._zinc.underlying_dist.home), + ) + res = self.context.execute_process_synchronously_or_raise( + 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 Digest + return res.output_directory_digest def get_zinc_compiler_classpath(self): """Get the classpath for the zinc compiler JVM tool. diff --git a/tests/python/pants_test/backend/jvm/tasks/jvm_compile/java/test_zinc_compile_integration.py b/tests/python/pants_test/backend/jvm/tasks/jvm_compile/java/test_zinc_compile_integration.py index ce342b50d0a..8d10295183f 100644 --- a/tests/python/pants_test/backend/jvm/tasks/jvm_compile/java/test_zinc_compile_integration.py +++ b/tests/python/pants_test/backend/jvm/tasks/jvm_compile/java/test_zinc_compile_integration.py @@ -235,6 +235,7 @@ def test_failed_hermetic_incremental_compile(self): config, ) self.assert_failure(pants_run) + self.assertIn('Please use --no-compile-zinc-incremental', pants_run.stdout_data) def test_failed_compile_with_hermetic(self): with temporary_dir() as cache_dir: @@ -258,6 +259,36 @@ def test_failed_compile_with_hermetic(self): config, ) self.assert_failure(pants_run) + self.assertIn('package System2 does not exist', pants_run.stderr_data) + self.assertIn( + 'Failed jobs: compile(testprojects/src/java/org/pantsbuild/testproject/dummies:' + 'compilation_failure_target)', + pants_run.stdout_data) + + def test_failed_compile_with_subprocess(self): + with temporary_dir() as cache_dir: + config = { + 'cache.compile.zinc': {'write_to': [cache_dir]}, + 'compile.zinc': { + 'execution_strategy': 'subprocess', + 'use_classpath_jars': False, + 'incremental': False, + } + } + + with self.temporary_workdir() as workdir: + pants_run = self.run_pants_with_workdir( + [ + # NB: We don't use -q here because subprocess squashes the error output + # See https://github.com/pantsbuild/pants/issues/5646 + 'compile', + 'testprojects/src/java/org/pantsbuild/testproject/dummies:compilation_failure_target', + ], + workdir, + config, + ) + self.assert_failure(pants_run) + self.assertIn('package System2 does not exist', pants_run.stdout_data) self.assertIn( 'Failed jobs: compile(testprojects/src/java/org/pantsbuild/testproject/dummies:' 'compilation_failure_target)',