use aspects for scrooge / thrift support for scala.

[johnynek]

we currently implement scrooge by adding a generic mechanism to see all dependencies in with a field called "extra_information" tacked onto the scala rules. By using that, we can see the full transitive dependencies of any target.

https://github.com/bazelbuild/rules_scala/blob/master/twitter_scrooge/twitter_scrooge.bzl#L76

We use this to make sure scrooge is called on a full covering of the thrift graph.

An alternative could be to use aspects to generate the scrooge:
https://docs.bazel.build/versions/master/skylark/aspects.html#advanced-example

in this picture, aspects help us generate 1 scrooge target for each thrift target. This way, we have a 1:1 mapping. This fixes the need to verify that we cover all the thrifts with a scrooge target, and potentially improves performance (but since scrooge parses so slowly, and we may need to parse many times now it could actually slow down performance).

It would be really nice for old code to continue to build, it may not be possible and may need a migration. In any case, using aspects seems like the right direction since this is what they were designed for.

[ittaiz]

Looks good!

…

On Wed, 30 May 2018 at 6:19 P. Oscar Boykin ***@***.***> wrote: we currently implement scrooge by adding a generic mechanism to see all dependencies in with a field called "extra_information" tacked onto the scala rules. By using that, we can see the full transitive dependencies of any target. https://github.com/bazelbuild/rules_scala/blob/master/twitter_scrooge/twitter_scrooge.bzl#L76 We use this to make sure scrooge is called on a full covering of the thrift graph. An alternative could be to use aspects to generate the scrooge: https://docs.bazel.build/versions/master/skylark/aspects.html#advanced-example in this picture, aspects help us generate 1 scrooge target for each thrift target. This way, we have a 1:1 mapping. This fixes the need to verify that we cover all the thrifts with a scrooge target, and potentially improves performance (but since scrooge parses so slowly, and we may need to parse many times now it could actually slow down performance). It would be really nice for old code to continue to build, it may not be possible and may need a migration. In any case, using aspects seems like the right direction since this is what they were designed for. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#510>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABUIFxZfeFPTUstHXizwG4OgKO2hedvvks5t3g-6gaJpZM4USiOe> .