Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[rsc-compile] support explicit tagging of targets in rsc compile #7362

Merged
merged 14 commits into from Mar 16, 2019
@@ -89,6 +89,13 @@ def add_for_target(self, *args, **kwargs):
product.add_for_target(*args, **kwargs)


class _CompileWorkflowChoice(enum(['zinc-only', 'guess'])):
"""Enum covering the values for the default workflow option.
guess - Try to classify compile targets based on whether they are Scala/Java or test targets.
zinc-only - Always use zinc."""


class _JvmCompileWorkflowType(enum(['zinc-only', 'rsc-then-zinc'])):
"""Target classifications used to correctly schedule Zinc and Rsc jobs.
@@ -149,16 +156,33 @@ def ensure_output_dirs_exist(self):
class RscCompile(ZincCompile):
"""Compile Scala and Java code to classfiles using Rsc."""

_name = 'rsc' # noqa
_name = 'mixed' # noqa
compiler_name = 'rsc'

@classmethod
def implementation_version(cls):
return super(RscCompile, cls).implementation_version() + [('RscCompile', 172)]

def __init__(self, *args, **kwargs):
super(RscCompile, self).__init__(*args, **kwargs)

self._rsc_compat_tag = self.get_options().force_compiler_tag_prefix
self._compiler_tags = {'{}:{}'.format(self._rsc_compat_tag, workflow.value): workflow
for workflow in _JvmCompileWorkflowType.all_variants}
self._default_workflow = self.get_options().default_workflow

# If the default workflow is conveyed through a flag, override tag values.
self._ignore_tags_when_calculating_workflow = self.get_options().is_flagged('default_workflow')

@classmethod
def register_options(cls, register):
super(RscCompile, cls).register_options(register)
register('--force-compiler-tag-prefix', default='use-compiler', metavar='<tag>',
help='Always compile targets marked with this tag with rsc, unless the workflow is '
'specified on the cli.')

This comment has been minimized.

Copy link
@cosmicexplorer

cosmicexplorer Mar 12, 2019

Contributor

Post #7315, this looks fine. Figuring out the right options here is hard.

register('--default-workflow', type=_CompileWorkflowChoice,
default=_CompileWorkflowChoice.guess, metavar='<workflow>',
help='Defines how a compile workflow is chosen when a tag is not present.')

cls.register_jvm_tool(
register,
@@ -257,7 +281,19 @@ def select(self, target):
return self._classify_compile_target(target) is not None

def _classify_compile_target(self, target):
return _JvmCompileWorkflowType.classify_target(target)
if not target.has_sources('.java') and not target.has_sources('.scala'):
return None

if not self._ignore_tags_when_calculating_workflow:
for t in target.tags:
flow = self._compiler_tags.get(t)
if flow:
return _JvmCompileWorkflowType(flow)

return self._default_workflow.resolve_for_enum_variant({
'zinc-only': _JvmCompileWorkflowType.zinc_only,
'guess': _JvmCompileWorkflowType.classify_target(target)
})

This comment has been minimized.

Copy link
@cosmicexplorer

cosmicexplorer Mar 15, 2019

Contributor

Thanks, this is much much better than what was there before!


def _key_for_target_as_dep(self, target, workflow):
# used for jobs that are either rsc jobs or zinc jobs run against rsc
@@ -42,6 +42,46 @@ class RscCompileTest(TaskTestBase):
def task_type(cls):
return RscCompile

def test_force_compiler_tags(self):
self.init_dependencies_for_scala_libraries()

java_target = self.make_target(
'java/classpath:java_lib',
target_type=JavaLibrary,
sources=['com/example/Foo.java'],
dependencies=[],
tags={'use-compiler:rsc-then-zinc'}
)
scala_target = self.make_target(
'scala/classpath:scala_lib',
target_type=ScalaLibrary,
sources=['com/example/Foo.scala'],
dependencies=[],
tags={'use-compiler:zinc-only'}
)

with temporary_dir() as tmp_dir:
invalid_targets = [java_target, scala_target]
task = self.create_task_with_target_roots(
target_roots=[java_target]
)

jobs = task._create_compile_jobs(
compile_contexts=self.create_compile_contexts([java_target, scala_target], task, tmp_dir),
invalid_targets=invalid_targets,
invalid_vts=self.wrap_in_vts(invalid_targets),
classpath_product=None)

dependee_graph = self.construct_dependee_graph_str(jobs, task)
print(dependee_graph)
self.assertEqual(dedent("""
rsc(java/classpath:java_lib) -> {
zinc_against_rsc(java/classpath:java_lib)
}
zinc_against_rsc(java/classpath:java_lib) -> {}
zinc(scala/classpath:scala_lib) -> {}""").strip(),
dependee_graph)

This comment has been minimized.

Copy link
@cosmicexplorer

cosmicexplorer Mar 15, 2019

Contributor

This testing will allow us to iterate on execution methods much more reliably, I think, especially for an eventual v2 conversion. Thanks!

def test_no_dependencies_between_scala_and_java_targets(self):
self.init_dependencies_for_scala_libraries()

@@ -239,8 +279,13 @@ def init_dependencies_for_scala_libraries(self):
jars=[JarDependency(org='com.example', name='scala', rev='0.0.0')]
)

def create_task_with_target_roots(self, target_roots):
context = self.context(target_roots=target_roots)
def create_task_with_target_roots(self, target_roots, default_workflow=None):

This comment has been minimized.

Copy link
@cosmicexplorer

cosmicexplorer Mar 15, 2019

Contributor

I might remove the default_workflow=None if it's not currently being used.

This comment has been minimized.

Copy link
@baroquebobcat

baroquebobcat Mar 15, 2019

Author Contributor

Ah. that reveals a test I should write!

if default_workflow:
context = self.context(
target_roots=target_roots,
options={'compile.mixed':{'default_workflow': default_workflow}})
else:
context = self.context(target_roots=target_roots)
self.init_products(context)
task = self.create_task(context)
# tried for options, but couldn't get it to reconfig
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.