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

Conversation

Projects
None yet
3 participants
@baroquebobcat
Copy link
Contributor

commented Mar 11, 2019

In order to be able to incrementally enable rsc-then-zinc, we need to be able to opt targets into rsc, and to be able to opt them out if there are issues. This adds tags for opting targets into either rsc or zinc.

It also causes setting the default workflow on the CLI to override target tag values. This allows for testing workflows on targets that are tagged with the other workflow.

Depends on #7324. The interesting bits are in
9d1787b .

baroquebobcat added some commits Mar 7, 2019

[rsc-compile] define workflow in context instead of on fly; use workf…
…low in traversal

This pulls the workflow up into the context objects and draws them through to the places we classify targets. This way we're centralizing how we do classification and only doing it once per target.

It also fixes an issue with how we were handling the invalid dep traversal where we were depending on the sources of targets in the predicate, instead of the workflow
Show resolved Hide resolved src/python/pants/backend/jvm/tasks/jvm_compile/rsc/rsc_compile.py Outdated
Show resolved Hide resolved src/python/pants/backend/jvm/tasks/jvm_compile/rsc/rsc_compile.py Outdated
Show resolved Hide resolved src/python/pants/backend/jvm/tasks/jvm_compile/rsc/rsc_compile.py Outdated
@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.

baroquebobcat added some commits Mar 13, 2019

[rsc-compile] support explicit tagging of targets in rsc compile
In order to be able to incrementally enable rsc-then-zinc, we need to be able to opt targets into rsc, and to be able to opt them out if there are issues. This adds tags for opting targets into either rsc or zinc.

It also causes setting the default workflow on the CLI to override target tag values. This allows for testing workflows on targets that are tagged with the other workflow

@baroquebobcat baroquebobcat force-pushed the twitter:nhoward/mixed_rsc_compile branch from 9d1787b to e009bc0 Mar 14, 2019

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!

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!

@@ -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!

@stuhood stuhood added this to the 1.15.x milestone Mar 15, 2019

@baroquebobcat baroquebobcat merged commit 16cefff into pantsbuild:master Mar 16, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.