rules_scala
rules_scala copied to clipboard
bazelbuild
Stricter unused dependency checking for plus one deps mode
When using plus one deps mode, many unused deps do get marked, but some deps can be left out because they are already the dep of another dep, and it also makes sense to leave them out because they never appear in the source code of the package being compiled, and the only reason the dep is needed is to make scalac happy. Would it be possible to have a unused deps mode for plus one deps mode where a dep is marked as an unused, unless it is explicitly referenced in the source code? (Not sure if this would end up with a number of false positives - not that familiar with scalac's needs)
This is now in progress, the below summarizes the status
- Anyone who wants to test this on their projects should do so
Known potential issues
- external deps such as https://github.com/anchlovi/unused-deps-specs2 - when +1 can't grab the deps of deps, deps and/or exports should be used on the external dependency to expose what's needed for its ijar
- Since +1 is a heuristic, not everything will work fine and unused deps may report issues. In our experience this has not happened but there are examples of problems, such as A->B->C->D subclassing and A's deps is only listed as B. There are ideas to fix this (and the previous issue as well) described below
Things to do
- Handle incoming issues discovered in the AST tracker
- Decide how/if to handle the above issues. https://github.com/bazelbuild/rules_scala/issues/867#issuecomment-588516930 discusses some solutions. It is unclear how often these issues happen if we export iface deps of external deps properly. Our (not speaking for rules_scala) plan is to use unused_dependency_checker_ignored_targets for such issues, as they are so rare we did not need any unused_dependency_checker_ignored_targets when enabling the new mode.
- Once the new AST mode is sufficiently validated [definition is 1 month since no more true errors [the last known one requiring a fix being https://github.com/bazelbuild/rules_scala/pull/1008 ], 3 months after April 30 [which would be July 30], and also @ittaiz approves] do the following steps sequentially, with enough time in between them to allow people to migrate
- Remove dependency_tracking_method on toolchain and ast becomes the method for transitive/+1, while high-level becomes the method for direct a. At this point take another look at https://github.com/bazelbuild/rules_scala/pull/1034 and see if it is landable
- Change default for strict_deps to
erroron toolchain, andstrict_deps=defaultis deprecated - Remove
strict_deps=defaultas an option
Thanks for the issue! TLDR: Unfortunately not.
Longer: The current unused deps mode uses a heuristic which works sometimes (in Wix’s experience less, in Stripe’s experience more). It’s not source based but rather uses some internal accounting scalac has (which has both false positives and false negatives).
Can something be done?
-
Develop a different unused deps mechanism which is source based as part of the build- I’m not sure how complicated this is to be honest. Java Bazel people have something in the Bazel repo which we looked at in the past but haven’t been able to get anywhere with. My initial plan with the +1 was to have something like this (as a separate action which will fail the build but doesn’t block the compilation graph).
-
Have an IntelliJ based unused deps source based mechanism- since IntelliJ does many of the heavy lifting AST wise then it seems easier to have some sort of IJ Inspection that looks at the entire target and the sources and removed deps. I think this won’t be cheap perf wise but is an interesting approach.
-
Drop and then recreate pattern- with buildozer you can drop all deps in one command and if you then have a tool which automatically adds dependencies you can use this pattern every X times to clean up. We (Wix) have one such tool which we plan on open sourcing (tentatively Q1) and are working on another one inside of our IJ plugin.
Would love to hear your thoughts
On Fri, 1 Nov 2019 at 21:36 Jamie5 [email protected] wrote:
When using plus one deps mode, many unused deps do get marked, but some deps can be left out because they are already the dep of another dep, and it also makes sense to leave them out because they never appear in the source code of the package being compiled, and the only reason the dep is needed is to make scalac happy. Would it be possible to have a unused deps mode for plus one deps mode where a dep is marked as an unused, unless it is explicitly referenced in the source code? (Not sure if this would end up with a number of false positives - not that familiar with scalac's needs)
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/bazelbuild/rules_scala/issues/867?email_source=notifications&email_token=AAKQQFY7W6JE5NVLAYRSOKDQRSAKHA5CNFSM4JH7D3J2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HWGHTRA, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKQQF53MIXENSTDWEJSLRDQRSAKHANCNFSM4JH7D3JQ .
Re 1, I don't think walking over the tree and finding used types would be very hard - the unused deps mechanism already does something similar (though they walk over different things). The tricky part may be to understand, when you see a type, should it be a direct dep? Though that might not be tricky at all and actually be really straightforward.
For 3, that seems useful though 1 feels more preferable. Also wouldn't this potentially remove direct dependencies that you also happen to depend on in a +1 situation, which would violate the desire of strict-deps?
Re 3- you nailed it on both problems this brings. Our existing solution is indeed not a very good one but better than letting build files rot. The new mechanism in the plugin won’t solve the need for user activation but will be source based so should solve the +1 issue.
Re 1- how do you suggest to do this? Our main goal was to have a small to zero overhead for unused deps mechanism. Performing this iteration on the scalac action (as a plugin for example) was deemed costly by people more familiar with scalac than me. Performing this in a separate action has a big cost resources wise even though you might not increase the effective build time. This is the main reason we went with the current heuristic which capitalized on rough information scalac already collects. Another thought was to work with the zinc people to extract their analysis module to be more decoupled and then depend on that but it required bandwidth we didn’t have.
On Sat, 2 Nov 2019 at 21:23 Jamie5 [email protected] wrote:
Re 1, I don't think walking over the tree and finding used types would be very hard - the unused deps mechanism already does something similar (though they walk over different things). The tricky part may be to understand, when you see a type, should it be a direct dep? Though that might not be tricky at all and actually be really straightforward.
For 3, that seems useful though 1 feels more preferable. Also wouldn't this potentially remove direct dependencies that you also happen to depend on in a +1 situation, which would violate the desire of strict-deps?
— You are receiving this because you commented.
Reply to this email directly, view it on GitHub https://github.com/bazelbuild/rules_scala/issues/867?email_source=notifications&email_token=AAKQQFYY6QZ635EWXBJOAS3QRXHRHA5CNFSM4JH7D3J2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEC5DCCQ#issuecomment-549073162, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKQQF6362WZSHZWZBIU74DQRXHRHANCNFSM4JH7D3JQ .
Yes, agreed that 3 is better than nothing, even with the problem of non-strict deps.
For 1, IME iterating over the tree as a plugin was not overly expensive (though non-zero). But certainly I'm not that experienced with scalac, nor the cost of the specific operations needed for this. From my understanding the existing strict-deps mechanism already iterates over the full AST and pays some reasonable amount of cost for it.
Actually is there a reason the existing strict-deps mechanism can't give the list of directly-referenced deps (from my understanding, it should be able to do that) which we can use to find the unneeded ones?
Have you read the code of the existing strict deps mechanism? It doesn’t do any iteration over the AST but rather just take the list of jars scalac needed to load.
If you can do it via a plugin then maybe do it externally and measure the cost?
On Sat, 2 Nov 2019 at 22:38 Jamie5 [email protected] wrote:
Yes, agreed that 3 is better than nothing, even with the problem of non-strict deps.
For 1, IME iterating over the tree as a plugin was not overly expensive (though non-zero). But certainly I'm not that experienced with scalac, nor the cost of the specific operations needed for this. From my understanding the existing strict-deps mechanism already iterates over the full AST and pays some reasonable amount of cost for it.
Actually is there a reason the existing strict-deps mechanism can't give the list of directly-referenced deps (from my understanding, it should be able to do that) which we can use to find the unneeded ones?
— You are receiving this because you commented.
Reply to this email directly, view it on GitHub https://github.com/bazelbuild/rules_scala/issues/867?email_source=notifications&email_token=AAKQQF7VM2I24LXEPQS4MKDQRXQKXA5CNFSM4JH7D3J2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEC5EQ3Y#issuecomment-549079151, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKQQF4KQK3RYL2ETKVGRPLQRXQKXANCNFSM4JH7D3JQ .
Ah I see, I misread the code. It does seem to use the native java strict_deps but only for java files. Which does appear to iterate over the AST. Now I see that the strict_deps for scala files does not do that.
Just to be sure, is https://github.com/bazelbuild/rules_scala/blob/master/third_party/dependency_analyzer/src/main/io/bazel/rulesscala/dependencyanalyzer/DependencyAnalyzer.scala what handles strict deps or am I misreading again? That one does appear to do very limited iteration over the AST in a way I don't fully follow.
Hmm maybe will try that out and see, it would hopefully answer the question easily enough. Is there a particular big bazel-ified codebase you would recommend?
None of the needed combo (OSS+Scala+Bazel). Do you work for a company that uses Bazel and Scala? Maybe you can time it on an internal codebase. If diff is small enough we can continue the discussion (we can time it on our codebase as well)
On Sat, 2 Nov 2019 at 23:19 Jamie5 [email protected] wrote:
Ah I see, I misread the code. It does seem to use the native java strict_deps but only for java files. Which does appear to iterate over the AST. Now I see that the strict_deps for scala files does not do that.
Just to be sure, is https://github.com/bazelbuild/rules_scala/blob/master/third_party/dependency_analyzer/src/main/io/bazel/rulesscala/dependencyanalyzer/DependencyAnalyzer.scala what handles strict deps or am I misreading again? That one does appear to do very limited iteration over the AST in a way I don't fully follow.
Hmm maybe will try that out and see, it would hopefully answer the question easily enough. Is there a particular big bazel-ified codebase you would recommend?
— You are receiving this because you commented.
Reply to this email directly, view it on GitHub https://github.com/bazelbuild/rules_scala/issues/867?email_source=notifications&email_token=AAKQQF5CFZAACN6MLU4VCODQRXVGHA5CNFSM4JH7D3J2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEC5FIPY#issuecomment-549082175, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKQQF27DN2A3SVVH5F4DLTQRXVGHANCNFSM4JH7D3JQ .
Ok https://github.com/Jamie5/rules_scala/commit/cbba543c1e4a6c1174043d9dd5b4cd952bdc03b4 has a diff which is rather hacky (at least in terms of some plumbing) but does appear to do what we want.
Run on a 2.12.8 codebase, some of the rules testing old vs new unused dependency checker were as follows. Note that while testing methodology was probably fairly reasonable, it would be far from airtight. The results suggest that timing is not significantly different, but if you have good infra for timing it would probably have more reliable results.
Rule 1 New: 169.07, 166.10, 165.01 => 166.73 Old: 168.59, 160.43, 164.30 => 164.44
Rule 2 New: 22.06, 22.18, 25.65, 25.45 => 23.84 Old: 22.39, 21.79, 25.50, 25.84 => 23.88
Rule 3 New: 19.56, 19.56, 19.81, 19.28 => 19.55 Old: 19.80, 19.21, 19.85, 20.29 => 19.78
Some notes
- Due to constant folding, this has false positives. It appears to have a potential solution as noted in the code for certain versions of 2.12.x and up. In the test codebase, 3 ignores were needed to prevent this.
- There is some strange issue probably around exports where certain rules not listed in deps were still reported as unused. This might be due to how the list of direct jars is produced, but did not fully dig into it yet
- This might have additional false positives
No I'm sorry. This is really interesting to me but I'm a bit under capacity trying to wrap my head around the refactor PR. I'll do my best to get to it in the next few days, ok?
On Wed, Nov 13, 2019 at 8:18 PM Jamie5 [email protected] wrote:
@ittaiz https://github.com/ittaiz did you get a chance to look at this?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bazelbuild/rules_scala/issues/867?email_source=notifications&email_token=AAKQQF4GWJ3YM7L4BUXMEWLQTRAG5A5CNFSM4JH7D3J2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOED7EFZA#issuecomment-553534180, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKQQF7325EZLJRHJ52OGC3QTRAG5ANCNFSM4JH7D3JQ .
Sounds good, no worries I just never know if something is in someone's queue or got lost in the notification void.
Fair enough, unfortunately we have so many notifications it does happen sometimes. Please feel free to ping me again mid of next week if I don't respond.
On Wed, Nov 13, 2019 at 11:27 PM Jamie5 [email protected] wrote:
Sounds good, no worries I just never know if something is in someone's queue or got lost in the notification void.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bazelbuild/rules_scala/issues/867?email_source=notifications&email_token=AAKQQFZ73SKZHMUAZHMFT7TQTRWNZA5CNFSM4JH7D3J2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOED7XBCA#issuecomment-553611400, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKQQFZTCE66EGMA54GTPFLQTRWNZANCNFSM4JH7D3JQ .
One other random thought, if this works could a similar mechanism be used to not do plus one deps but examine only the ijar and determine which deps are actually needed to compile the ijar, and propogate only those ones? (IIRC I read somewhere java rules did something similar like that but not sure about it).
And maybe strict deps can stem from this as well, the mechanisms seem similar at least.
Potentially. I think that there are more nuances in this space so work will need to be iterative but a strong enough tool can definitely help us improve things. I encourage you to dive a bit more into how java rules work since they are very advanced in these areas.
I've re-read the thread and I think I originally replied to what I read and not what you wrote :)
To align our discussion let's agree on the issues:
- Performance- different kinds of classpath have different performance characteristics both with respect to the runfiles, upload of inputs (remote execution), size of classpath for scalac to work with but probably most importantly cache churn impact when you have many inputs which calculate into the cache key.
- scalac transitivity- scalac often needs transitive dependencies even though the source file doesn't explicitly need them. see scala-center proposal and discussion. This can have a huge impact for a large codebase using source dependencies since some forms of changing implementations for root targets (while adding dependencies) will actually break users since scalac for their target will fail. For us (Wix) this was a blocker at scale.
- Hygiene- You'd like build files to be clean for performance and so that roots can change deps without leaves breaking (since they were dependent on roots bringing some deps they also use).
To mitigate these issues rules_scala currently has 3 strategies which impact the classpath:
- Direct only- 1.1. Pros- Performance. 1.2. Cons- 1.2.1. scalac issues mentioned above. 1.2.2. scalac error messages are often less than helpful and definitely don't allow fluent work since devs need to translate them to labels.
- Plus one (direct deps and their direct deps)- 2.1. Pros- 2.1.1. Performance (smaller classpath than 3) 2.1.2. Less false positives than 1 2.2. Cons- 2.2.1. Targets can use the +1 deps as direct deps without noticing 2.2.2. Targets can have unused deps which are used as +1 deps and not get notifications 2.2.3. Heuristic based: +1 catches many cases of 2.2.4. scalac error messages for missing deps
- Strict deps (all deps appear on classpath)- 3.1. Pros- nice label based error messages 3.2 Cons- 3.2.1. Performance. 3.2.2. False positives (related to the strict-deps plugin which aims to have small overhead).
Note all of the above tackle the hygiene issue even if in different ways and tradeoffs.
Do I understand correct that you want to solve 2.2.1. and maybe 2.2.2. while still having nice label based errors?
I think you hinted towards maybe trying to tackle 2.2.3. later on which is also great.
If so I'm indeed very much in favor.
I took a look at the code and it's nice! If we'll decide to merge it in we'll of course need to clean it up a bit like you mentioned but my two main concerns are still performance and false positives. I'm trying to think how to validate them. One option might be to run this against Wix's codebase but that will take me some time since we're currently using +1 and some errors will be correct and some (hopefully few to none) will be false. For performance it might be easier but still require some time.
Can you see these +1 tests still pass with the flag turned on?
To summarize (and assuming I understand correctly what you're trying to solve):
- How urgent is this for you? I'm very excited about the potential but due to impact would like more time to evaluate it.
- Do you want to add strict-deps based on it? Seems like it's a very small addition to the above commit
- Do you have any thoughts/suggestions on how we can validate it apart from Wix's codebase? (It's just very large, many different patterns of scalac and deps). I'd probably still like to run it on Wix's codebase but if we get good strong results from other places maybe I can run on a few select repos internally.
Thank you for your efforts here! I think that if we'll be able to polish it product, performance and false positives wise this will be a very big jump forward!
Note- Direct only does solve the above problems while introducing others mentioned above.
I guess SCP-009 has not made much progress since? Because if it did and they can backport it to 2.12.x or even just 2.13.x then that would be great and this becomes much less important IIUC.
This diff would tackle 2.2.2, and if strict-deps was added (which I will try out) then it will also tackle 2.2.1. I don't fully follow what 2.2.3 and 2.2.4 are.
If 2.2.3 means that some unneeded deps are captured by the +1 then in theory this might be able to help but it might be easier to do it by examining the ijar directly after it is produced, or something like that. Because otherwise we need to make assumptions about what exactly the ijar keeps and doesn't keep (which I guess might not be that complicated). But this would definitely be very up in the air and with very unclear feasibility/correctness/timelines.
If I understand correctly, to run the tests you specified one only needs to do ./test_rules_scala.sh. Having done that, it looks like everything says successful.
Regarding your last questions
- Currently we are using +1 deps, and I have used this to manually clean up unused deps, and that is not the worst world to be in (especially if it can also have strict deps working). That being said, this is a rather manual mode of operation so would perfer not to be in it for overly long. But definitely it is worthwhile to make sure that we can get things pretty right.
- Will try it and see.
- Not really sure, any public big codebase that uses scala and bazel I guess but don't know of any. As far as false positives, we might just need to do a whack-a-mole thing (luckily, usually the issues are easy to simplify and repro and then the solution is clear, except for the literal issue mentioned). One issue is validating on all the supported versions of scala, which hopefully we can automate easily enough.
SCP-009 made some progress and this is how we built the strict deps mechanism. We copied this work into rules_scala and adapted it into bazel. It’s too simplistic however...
Never mind 2.2.3/2.2.4 for now
Re the tests- the support you added works only if the user turns on both unused deps and plus 1, no? Because the tests I linked to only turn on plus 1 (you can modify them to only turn on unused deps and see)
Ok, https://github.com/Jamie5/rules_scala/commit/8724a32b45a7be02ec66aa64bb71fdd3d573fe9f#diff-3830e6e26d863974d38e511b04761916 has code for unused_deps as well.
Notes
- Strict deps check seems to work well generally speaking
- With the test changes of adding error on unused deps, running
./test_rules_scala.shstill passes. Though running it again without abazel cleancausestest_scala_import_expect_failure_on_missing_direct_deps_warn_modeto fail (probably because it doesn't get recompiled). But this happens on master as well so probably doesn't matter. - There is no way in this diff to disable strict deps checking - if you have unused deps check on (in plus one mode), then strict deps checking will be too
- This includes all transitive dependencies to populate indirect_deps - we shouldn't need to do that (and can just go +1) but it was easiest to copy the non-plus-one strict_deps logic.
- Sometimes an undesirable label suggestion is picked to add as deps, but the logic does seem to make sense - just not what we would want in the sense of our codebase.
- There is an issue, where if A calls a method which has a default parameter of type B, but A doesn't provide a value for B (and lets the default stand), the strict deps checker still claims that A depends on B. Arguably this can be the correct behavior, but it also seems strange that we would report such. But it was tricky to find out a way to ignore this case (there were some potential possibilities but with unclear consequences) so for now it is left as claiming that we need the dep directly.
- The position specified on unused_deps is sometimes a bit off and sometimes ends up as NoPosition. This can likely be improved but for now it generally does point to the reason that we need a given direct dependency.
- Did not check the performance impact of this new change, but likely it will not be too bad given that the transitive deps we provide are no worse than non-plus-one strict-deps mode, and the additional plugin code does not seem like it would be too heavy.
As for potential small steps while validating the overall things
- It seems strange that we have both dependency_analyzer and unused_dependency_checker projects, which seem to be pretty similar and I also ended up copying lots of code from the former to the latter. So if there is no strong reason to have them separate it might make sense to unify them?
- Then, possibly the creation of the files listing the direct deps/indirect deps/etc can be unified a bit from its current state.
Thanks! A few thoughts just from reading your message (haven't looked at the code yet):
There is no way in this diff to disable strict deps checking - if you have unused deps check on (in plus one mode), then strict deps checking will be too
Completely fine for POC. When we'll want to merge it we'll need to consider if it's ok or not.
This includes all transitive dependencies to populate indirect_deps - we shouldn't need to do that (and can just go +1) but it was easiest to copy the non-plus-one strict_deps logic.
Again for POC fine, for merge we'll need the +1.
There is an issue, where if A calls a method which has a default parameter of type B, but A doesn't provide a value for B (and lets the default stand), the strict deps checker still claims that A depends on B. Arguably this can be the correct behavior, but it also seems strange that we would report such. But it was tricky to find out a way to ignore this case (there were some potential possibilities but with unclear consequences) so for now it is left as claiming that we need the dep directly.
From our experience working with the existing strict-deps for a long time (6 months? 1 year?) on a very large codebase and many developers is that outputting unclear errors (to the developer) is super harmful. People started saying they need to "please the bazel beast". This was one of the main reasons why we moved to +1.
I'd like to error to the side of not reporting transitively used than reporting transitive deps which aren't used. Maybe this needs to be configured on level of strictness (if not too complicated API wise).
This is also why I think it's really important to run this on a large codebase.
I'll try to see if I can get two people inside of Wix to spend a few days just analyzing the results of running this internally and analyzing the errors. The false ratio needs to be close to zero IMHO.
Re code duplication- I agree. I'd probably prefer this be in separate commits to ease review.
Completely fine for POC. When we'll want to merge it we'll need to consider if it's ok or not. Again for POC fine, for merge we'll need the +1.
Agreed on both counts,
I'd like to error to the side of not reporting transitively used than reporting transitive deps which aren't used. Maybe this needs to be configured on level of strictness (if not too complicated API wise). This is also why I think it's really important to run this on a large codebase.
Fine with me, we can hammer the issues as we discover them. Would want to look at potential approaches for this, have some ideas but maybe some compiler expert knows the actual correct way to do things.
One thing is that as we do more of this then the risk of breaking on different scala versions matters more and it would be useful to run the unit tests against all supported scala versions, not sure if there is already some mechanism to do that. (We already have this issue with final vals which are another false positive as mentioned above)
Re code duplication- I agree. I'd probably prefer this be in separate commits to ease review.
Agreed, would prefer to merge in small chunks that don't break things. I can look at the initial steps here if we gain confidence on the overall idea.
@ittiaz just to make sure, you are not waiting for anything from me in order to test right? (want to make sure we are not both thinking we are waiting on something from the other and hence nothing ever happens)
Indeed. Sorry for the silence, was sick and in BazelCon. Yes, ball is in my court and I'm trying to find someone in Wix that will run with your diff and analyze impact (functionally wise).
The amazing @anchlovi is taking this week to run it on some large codebases internally
Just started my tests and there is an issue with external source repos. I'm not sure that the unused deps tool should test external source repos targets. Also buildozer (at least the version I'm running - 0.29.0) can not handle external source repos
I think this is a bazel wide issue and not this tool specifically. This is because Bazel treats everything as a mono repo (once fetch finishes). The pattern I think we'd need to use it to run with "warn" mode for alignment and then switch to error. WDYT?
On Sun, Dec 22, 2019 at 10:52 AM Shachar Anchelovich < [email protected]> wrote:
Just started my tests and there is an issue with external source repos. I'm not sure that the unused deps tool should test external source repos targets. Also buildozer (at least the version I'm running - 0.29.0) can not handle external source repos
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bazelbuild/rules_scala/issues/867?email_source=notifications&email_token=AAKQQF7IRR2CK5QNVNXCBFLQZ4TC7A5CNFSM4JH7D3J2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHPLKBY#issuecomment-568243463, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKQQF3TIL2YTEAPQVXIGRDQZ4TC7ANCNFSM4JH7D3JQ .
--
Ittai Zeidman
Cell: 054-6735021
40 Hanamal street, Tel Aviv, Israel
Don't you think it can work for all use-cases? Or do you mean if you use something you don't control like rules_scala? Because that is indeed a known issue in the bazel ecosystem
On Sun, Dec 22, 2019 at 10:59 AM Shachar Anchelovich < [email protected]> wrote:
It will work for our use case
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bazelbuild/rules_scala/issues/867?email_source=notifications&email_token=AAKQQFZP5WR3RQD6HDM7YA3QZ4T7BA5CNFSM4JH7D3J2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHPLNQQ#issuecomment-568243906, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKQQF6PAYATJE554BPGUT3QZ4T7BANCNFSM4JH7D3JQ .
--
Ittai Zeidman
40 Hanamal street, Tel Aviv, Israel
Exactly, your suggestion to start with warn and then to move to error will work as long as you control all external repos
👍 let's continue then. Mainly since in Bazel it's very easy to control external source repos if you must (fork them)