Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistent log level of eviction warning summary #287

Closed
bigwheel opened this issue Jan 9, 2019 · 1 comment
Closed

Inconsistent log level of eviction warning summary #287

bigwheel opened this issue Jan 9, 2019 · 1 comment

Comments

@bigwheel
Copy link
Contributor

bigwheel commented Jan 9, 2019

Summary

Eviction warning summary shows in [warn] log level even with binary compatible conflicts.

Detail

Eviction warning summary is:

[warn] There may be incompatibilities among your library dependencies; run 'evicted' to see detailed eviction warnings.

This is introduced in sbt 1.2.0:

Displays only the eviction warning summary by default, and make it configurable using ThisBuild / evictionWarningOptions. lm211 and #3947 by @exoego

My test

I tested update and evicted command in sbt 1.1.6 and 1.2.8.

source code to reproduce: https://github.com/bigwheel/sbt-warn-eviction-summary-inconsistency-behavior

Result:

1.1.6 with b-compatible 1.1.6 with b-incompatible 1.2.8 with b-compatible 1.2.8 with b-incompatible
Eviction warning summary
shows in update command
no no yes yes
Log level of evicted command info warn info warn

Problem

I understand evicted command prints binary compatible conflict as [info] level and binary incompatible conflict as [warn] because binary compatible conflict is no problem in many case.
However, if there are only binary compatible conflicts Eviction warning summary shows in [warn] level in contrast to evicted command prints only in [info] level.

Solution

I figured out some solutions.

1. Eviction warning sumary shows only there are binary incompatible conflicts

https://github.com/sbt/librarymanagement/blob/96a3293c/core/src/main/scala/sbt/librarymanagement/EvictionWarning.scala#L317

if ((a.options.warnEvictionSummary || a.reportedEvictions.nonEmpty) && a.allEvictions.nonEmpty) {

to

if (a.options.warnEvictionSummary && a.reportedEvictions.nonEmpty) {

I think this is best fix.

2. Eviction warning sumary with info level

This is not easy fix because it seems like we cannot select log level in there.

3. Change default EvictionWarningOptions from summary to empty

Easy fix.

Conclusion

Please tell me how to fix this.
After that, I will create Pull Request for it.

Related to

scala - addSbtPlugin(...) cause eviction warning but 'evicted' command shows nothing - Stack Overflow

@eed3si9n
Copy link
Member

eed3si9n commented Jan 9, 2019

Thanks @bigwheel for the research!
Your first option seems reasonable. Let's try that.

bigwheel added a commit to bigwheel/librarymanagement that referenced this issue Jan 10, 2019
bigwheel added a commit to bigwheel/librarymanagement that referenced this issue Jan 10, 2019
@eed3si9n eed3si9n added the ready label Jan 10, 2019
bigwheel added a commit to bigwheel/librarymanagement that referenced this issue Jan 14, 2019
eed3si9n added a commit that referenced this issue Jan 23, 2019
[Fix #287] Eviction warning summary shows only binary incompatible
@eed3si9n eed3si9n removed the ready label Jan 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants