{"action":"created","issue":{"url":"https://api.github.com/repos/bazelbuild/rules_scala/issues/235","repository_url":"https://api.github.com/repos/bazelbuild/rules_scala","labels_url":"https://api.github.com/repos/bazelbuild/rules_scala/issues/235/labels{/name}","comments_url":"https://api.github.com/repos/bazelbuild/rules_scala/issues/235/comments","events_url":"https://api.github.com/repos/bazelbuild/rules_scala/issues/235/events","html_url":"https://github.com/bazelbuild/rules_scala/issues/235","id":239069599,"node_id":"MDU6SXNzdWUyMzkwNjk1OTk=","number":235,"title":"Strict scala deps and unused_deps","user":{"login":"ittaiz","id":1378327,"node_id":"MDQ6VXNlcjEzNzgzMjc=","avatar_url":"https://avatars2.githubusercontent.com/u/1378327?v=4","gravatar_id":"","url":"https://api.github.com/users/ittaiz","html_url":"https://github.com/ittaiz","followers_url":"https://api.github.com/users/ittaiz/followers","following_url":"https://api.github.com/users/ittaiz/following{/other_user}","gists_url":"https://api.github.com/users/ittaiz/gists{/gist_id}","starred_url":"https://api.github.com/users/ittaiz/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ittaiz/subscriptions","organizations_url":"https://api.github.com/users/ittaiz/orgs","repos_url":"https://api.github.com/users/ittaiz/repos","events_url":"https://api.github.com/users/ittaiz/events{/privacy}","received_events_url":"https://api.github.com/users/ittaiz/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":25,"created_at":"2017-06-28T06:57:21Z","updated_at":"2018-10-03T19:51:55Z","closed_at":null,"author_association":"MEMBER","body":"So,\r\nI met with @cgrushko a few days ago and apparently the way we (rules_scala) currently do strict deps (allowing a target to only use what it directly depends on) is very different from the way java rules do it and IMHO not as good.\r\n\r\n**Problem:**\r\nWe currently pass to scalac only the direct jars and let scalac emit an error to the user.\r\nThese errors are quite often [cryptic](https://github.com/scalacenter/advisoryboard/pull/12) and unuseful for developers to be able to easily fix the problem.\r\nMoreover because of how the scala compiler works there are many cases where it needs transitive dependencies even if they aren't mentioned in the source code of the unit we're compiling[1]. This leads to having to add many transitive targets without any reasoning just to appease the compiler or  worse to start using `exports` loosely around the codebase. \r\n\r\nWe suffered from the above transitive dependencies so much that we're just adding the whole transitive closure to the `deps` of a target. This is in the onboarding of a project so we're ok with it but this solution can help greatly minimize it.\r\n\r\n**Proposed solution:**\r\nPass direct and indirect jars to scalac's classpath (like java compilation does).  \r\nAdd a scala compiler plugin which essentially will walk the AST after the type annotation phase, will stop at each 'type expression' and check that the jar from which the type was loaded is a direct_dependency. If not, it is an --indirect_dependency, and the plugin generates an error message. The message includes an 'add_dep' command to add the missing direct dep.\r\n\r\nWe will also support a lenient mode where this error is outputted as a warning.\r\nadd_dep is a command which [buildozer](https://github.com/bazelbuild/buildtools/tree/master/buildozer) can take and apply it to your BUILD file.\r\nI think (not sure) that [ibazel](https://github.com/bazelbuild/bazel-watcher) can pick up these warnings and automatically apply them to your BUILD file. Probably [rebazel](https://github.com/meetup/rebazel) will be able to do this as well if they'll want to.\r\n\r\n**Value:**\r\nSeparation of concerns (scalac won't enforce graph concerns)\r\nAllows higher level messages and so call to actions\r\nMight allow finer grain modeling of when indirect dependencies are ok (for an example where scalac \"messes\" up but we might be able to improve see this [repo](https://github.com/ittaiz/scalac-troubleshooting))  \r\nBetter cycle for developers since they can run with lenient mode and iterate quickly and fix the deps before pushing (or use ibazel/rebazel). The caveat is obviously the fact people can have a local green build which fails on CI.\r\n\r\n**Open questions:**\r\nIt's unclear how smart the compiler plugin can be given scalac's limitations and so unclear on how much noise will still be added to a target's `deps`.\r\n\r\n**Small implementation detail:**\r\nTo do this we'll need to pass to the compiler plugin a list of direct dependencies and a list of indirect dependencies where at least the latter needs to have both jars and the targets so that the error/warning message can be useful (add `//src/main/foo:baz` to your `deps`)\r\n\r\n[1][Improve user experience for builds that use only direct dependencies](https://github.com/scalacenter/advisoryboard/pull/12)"},"comment":{"url":"https://api.github.com/repos/bazelbuild/rules_scala/issues/comments/426776581","html_url":"https://github.com/bazelbuild/rules_scala/issues/235#issuecomment-426776581","issue_url":"https://api.github.com/repos/bazelbuild/rules_scala/issues/235","id":426776581,"node_id":"MDEyOklzc3VlQ29tbWVudDQyNjc3NjU4MQ==","user":{"login":"johnynek","id":67958,"node_id":"MDQ6VXNlcjY3OTU4","avatar_url":"https://avatars0.githubusercontent.com/u/67958?v=4","gravatar_id":"","url":"https://api.github.com/users/johnynek","html_url":"https://github.com/johnynek","followers_url":"https://api.github.com/users/johnynek/followers","following_url":"https://api.github.com/users/johnynek/following{/other_user}","gists_url":"https://api.github.com/users/johnynek/gists{/gist_id}","starred_url":"https://api.github.com/users/johnynek/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/johnynek/subscriptions","organizations_url":"https://api.github.com/users/johnynek/orgs","repos_url":"https://api.github.com/users/johnynek/repos","events_url":"https://api.github.com/users/johnynek/events{/privacy}","received_events_url":"https://api.github.com/users/johnynek/received_events","type":"User","site_admin":false},"created_at":"2018-10-03T19:51:55Z","updated_at":"2018-10-03T19:51:55Z","author_association":"MEMBER","body":"It would be nice... that said, they work across purposes a bit.\r\n\r\nAt Stripe, we minimize the compiler classpath to reduce cache invalidation. So, we don't use strict_deps (which actually puts all your transitive deps on the classpath, but then errors if you use them). Instead, we just don't have them, and we get errors (although harder to diagnose) if you use them.\r\n\r\nReally, I start to wonder if compiler plugins are the right path here. As long as scalac is not trying to do separate compilation, the experience will always be a bit second rate. I'd like to see separate compilation with minimal classpaths be a supported feature of scalac, but right now it is kind of tolerated, but not really strongly supported."}}
{"actor":{"display_login":"johnynek"}}