[WIP] Add detection for unused classpath entries as part of strict mode #438
Conversation
|
This should be it's own flag. I bundled it with |
|
Wanted to provide an update here: we're still working on this and should hopefully have something within the next couple weeks. We found an issue with macros and I like their suggestion of adding a whitelist for deps that are actually used, but the plugin marks as unused. That way We're moving into using our internal version of this to remove unused deps from our build files in combination with ibazel. If that goes well, we will polish it up and update this PR. |
|
Sounds good!
Sorry I haven’t had the chance to go over the code.
How do you think of handling the whitelist?
…On Fri, 23 Mar 2018 at 23:16 James Judd ***@***.***> wrote:
Wanted to provide an update here: we're still working on this and should
hopefully have something within the next couple weeks.
We found an issue with macros and classpath-shrinker, which I filed an
issue about: scalacenter/classpath-shrinker#4
<scalacenter/classpath-shrinker#4>
I like their suggestion of adding a whitelist for deps that are actually
used, but the plugin marks as unused. That way unused_deps doesn't become
totally blocked on bugs in the classpath shrinking mechanism.
We're moving into using our internal version of this to remove unused deps
from our build files in combination with ibazel. If that goes well, we will
polish it up and update this PR.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#438 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABUIFwki1nrPN2See0E1hQVi38ZYOd6eks5thVgCgaJpZM4Sf7CG>
.
|
|
really excited about this! Thank you! |
|
I was planning on adding an attribute to the scala rules for the whitelist. The attribute lists the deps that should not be marked as unused. I'm open to other ways of solving this. Ideally I'd be able to tag deps in the actual deps list, but I don't think Bazel supports that. |
|
Does the bug mean you remove “macro code”?
Meaning if you were to automatically whitelist code that was tagged as
having macro would that solve the problem?
Because we have scala_macro_library (or similar name) and scala_import will
soon also have this distinction (since it will use ijars for non macro code)
…On Sat, 24 Mar 2018 at 0:38 James Judd ***@***.***> wrote:
I was planning on adding an attribute to the scala rules for the
whitelist. The attribute lists the deps that should not be marked as unused.
I'm open to other ways of solving this. Ideally I'd be able to tag deps in
the actual deps list, but I'm I don't think Bazel supports that.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#438 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABUIF8xgxph0EWpNIIlYi6RZ4HsFh_ztks5thWthgaJpZM4Sf7CG>
.
|
|
I'm not totally certain if that macro classpath bug happens for all macros or just some. I was able to repro with those two jars, but there are others that I can't repro the problem with. Automatically whitelisting scala_macro_library and scala_import would likely fix the issue. It may whitelist too many things, though. As I work on optimizing our internal build files over the next few weeks, I'll see what happens and keep people updated. |
|
Sounds good.
To be clear- I don’t think we need to whitelist all of scala_import but we
want to turn on ijars for scala_import by default and have the ability to
turn them off. For those where we turn them off we can take that as a cue
for macro existence and whitelist them
…On Sat, 24 Mar 2018 at 20:16 James Judd ***@***.***> wrote:
I'm not totally certain if that macro classpath bug happens for all macros
or just some. I was able to repro with those two jars, but there are others
that I can't repro the problem with.
Automatically whitelisting scala_macro_library and scala_import would
likely fix the issue. It may whitelist too many things, though.
As I work on optimizing our internal build files over the next few weeks,
I'll see what happens and keep people updated.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#438 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABUIF1pIWtcQuDF-Z6fpSGYrKDXTNioAks5thn9qgaJpZM4Sf7CG>
.
|
|
Closing this since we have unused deps. Thanks. |
Based on what I read in #235, I hacked together a quick
unused_depsimplementation that is currently bundled together withstrict_java_deps.This is a work in progress. I would like to get people's feedback on this before continuing.