Rsc compile task fixes

[cosmicexplorer]

Problem

The RscCompile task is supposed to start running the zinc compile of a target once the rsc compiles of of all the target's dependencies are complete -- this is what allows for the massive parallelism from rsc in the first place. However, currently, the execution graph has the zinc compile of a target depending on the zinc compiles of all its dependencies, failing to use the rsc output at all.

This issue covered up a separate issue, because javac still can't actually use rsc output yet, and that should have caused failures, but since the zinc compiles were all scheduled after other zinc compiles, the rsc output was never being used.

These issues were fixed in #7227, but the implementation there was deemed too complex and parts were reverted without enough discussion when the "mixed compile" changes landed in master.

Solution

• Fix execution graph dependency breakages.
• Rename rsc-then-zinc to rsc-and-zinc, since they're supposed to happen in parallel.
• Default the --workflow option to rsc-and-zinc.
• Change the display of each job in the debug output displaying the dynamic execution graph (e.g. zinc[zinc-only], zinc[zinc-java], and zinc[rsc-and-zinc]).

Result

Some bugs are fixed!

[stuhood]

Assuming I'm misreading the below, #shipit!

It looks like there is a legit unit test failure though.

[stuhood]

Does this block:

rsc(scala/classpath:scala_dep) -> {
  rsc(scala/classpath:scala_lib),
  zinc[rsc-and-zinc](scala/classpath:scala_lib)
}

...mean that using rsc for a target depends on using both rsc and zinc for a scala-only dep, or am I misreading? That would be surprising.

[cosmicexplorer]

Replied out of line: #7742 (comment) -- the arrow might be considered to be pointing the wrong direction.

[stuhood]

Heh, yes: definitely. Thanks!

[stuhood]

(I've targeted this at 1.16.x as a post 1.16.0 item: we shouldn't cherry-pick it until after 1.16.0 is out.)

[cosmicexplorer]

This should be read as “the rsc and zinc compiles for scala/classpath:scala_lib both declare a dependency on the rsc compile for scala/classpath:scala_dep”, which I believe is correct. Win and I have gone back and forth over alternative graph drawings which may be less counterintuitive, but haven’t settled on one yet. This can absolutely be improved, I believe.

…

On Fri, May 24, 2019 at 19:55 Stu Hood ***@***.***> wrote: ***@***.**** approved this pull request. Assuming I'm misreading the below, #shipit! ------------------------------ In tests/python/pants_test/backend/jvm/tasks/jvm_compile/rsc/test_rsc_compile.py <#7742 (comment)>: > } + zinc[rsc-and-zinc](scala/classpath:scala_lib) -> {} rsc(scala/classpath:scala_dep) -> { Does this block: rsc(scala/classpath:scala_dep) -> { rsc(scala/classpath:scala_lib), zinc[rsc-and-zinc](scala/classpath:scala_lib) } ...mean that using rsc for a target depends on using both rsc and zinc for a scala-only dep, or am I misreading? That would be surprising. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#7742?email_source=notifications&email_token=AAJ6UTZ5Y7HF763FP6ZGLLDPXB56HA5CNFSM4HNPF3EKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOBZWESKQ#pullrequestreview-241977642>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAJ6UT472X5LB4KB2VV3OBDPXB56HANCNFSM4HNPF3EA> .

[stuhood]

I think that this is green with the exception of a shard that lost a race with another change landing in master? Might be good to rebase before landing.

[cosmicexplorer]

Rebased!