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

FR: harmonize GradleException content between List and Tree #79

Open
SimonMarquis opened this issue Feb 14, 2023 · 6 comments
Open

FR: harmonize GradleException content between List and Tree #79

SimonMarquis opened this issue Feb 14, 2023 · 6 comments

Comments

@SimonMarquis
Copy link
Contributor

SimonMarquis commented Feb 14, 2023

List and Tree modes throw different exception content.

Tree

***** DEPENDENCY CHANGE DETECTED *****
-\--- androidx.activity:activity:1.4.0
-     +--- androidx.annotation:annotation:1.1.0 -> 1.2.0
-     +--- androidx.core:core:1.7.0
-     |    +--- androidx.annotation:annotation:1.2.0
-     |    +--- androidx.annotation:annotation-experimental:1.1.0
-     |    +--- androidx.lifecycle:lifecycle-runtime:2.3.1
-     |    |    +--- androidx.arch.core:core-runtime:2.1.0
-     |    |    |    +--- androidx.annotation:annotation:1.1.0 -> 1.2.0
-     |    |    |    \--- androidx.arch.core:core-common:2.1.0
-     |    |    |         \--- androidx.annotation:annotation:1.1.0 -> 1.2.0
-     |    |    +--- androidx.lifecycle:lifecycle-common:2.3.1
-     |    |    |    \--- androidx.annotation:annotation:1.1.0 -> 1.2.0
-     |    |    +--- androidx.arch.core:core-common:2.1.0 (*)
-     |    |    \--- androidx.annotation:annotation:1.1.0 -> 1.2.0
-     |    +--- androidx.versionedparcelable:versionedparcelable:1.1.1
-     |    |    +--- androidx.annotation:annotation:1.1.0 -> 1.2.0
-     |    |    \--- androidx.collection:collection:1.0.0
-     |    |         \--- androidx.annotation:annotation:1.0.0 -> 1.2.0
-     |    +--- androidx.collection:collection:1.0.0 (*)
-     |    \--- androidx.concurrent:concurrent-futures:1.0.0
-     |         +--- com.google.guava:listenablefuture:1.0
-     |         \--- androidx.annotation:annotation:1.1.0 -> 1.2.0
-     +--- androidx.lifecycle:lifecycle-runtime:2.3.1 (*)
-     +--- androidx.lifecycle:lifecycle-viewmodel:2.3.1
-     |    \--- androidx.annotation:annotation:1.1.0 -> 1.2.0
-     +--- androidx.savedstate:savedstate:1.1.0
-     |    +--- androidx.annotation:annotation:1.1.0 -> 1.2.0
-     |    +--- androidx.arch.core:core-common:2.0.1 -> 2.1.0 (*)
-     |    \--- androidx.lifecycle:lifecycle-common:2.0.0 -> 2.3.1 (*)
-     +--- androidx.lifecycle:lifecycle-viewmodel-savedstate:2.3.1
-     |    +--- androidx.annotation:annotation:1.0.0 -> 1.2.0
-     |    +--- androidx.savedstate:savedstate:1.1.0 (*)
-     |    +--- androidx.lifecycle:lifecycle-livedata-core:2.3.1
-     |    |    +--- androidx.arch.core:core-common:2.1.0 (*)
-     |    |    +--- androidx.arch.core:core-runtime:2.1.0 (*)
-     |    |    \--- androidx.lifecycle:lifecycle-common:2.3.1 (*)
-     |    \--- androidx.lifecycle:lifecycle-viewmodel:2.3.1 (*)
-     +--- org.jetbrains.kotlin:kotlin-stdlib:1.5.31 -> 1.6.10 (*)
-     +--- androidx.collection:collection:1.0.0 (*)
-     \--- androidx.tracing:tracing:1.0.0
-          \--- androidx.annotation:annotation:1.1.0 -> 1.2.0
+\--- androidx.activity:activity:1.4.1 FAILED


FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':sample:app:dependencyTreeDiffReleaseRuntimeClasspath'.
> ***** DEPENDENCY CHANGE DETECTED *****
  Dependency Tree comparison to baseline does not match.
  
  If this is intentional, re-baseline using ./gradlew :sample:app:dependencyGuardBaseline
  Or use ./gradlew dependencyGuardBaseline to re-baseline dependencies in entire project.


* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights.

* Get more help at https://help.gradle.org

BUILD FAILED in 9s
9:07:35 pm: Execution finished ':sample:app:dependencyGuard --quiet'.

List

Dependencies Changed in :sample:app for configuration releaseRuntimeClasspath
- androidx.activity:activity:1.4.0
- androidx.annotation:annotation-experimental:1.1.0
- androidx.annotation:annotation:1.2.0
- androidx.arch.core:core-common:2.1.0
- androidx.arch.core:core-runtime:2.1.0
- androidx.collection:collection:1.0.0
- androidx.concurrent:concurrent-futures:1.0.0
- androidx.core:core:1.7.0
- androidx.lifecycle:lifecycle-common:2.3.1
- androidx.lifecycle:lifecycle-livedata-core:2.3.1
- androidx.lifecycle:lifecycle-runtime:2.3.1
- androidx.lifecycle:lifecycle-viewmodel-savedstate:2.3.1
- androidx.lifecycle:lifecycle-viewmodel:2.3.1
- androidx.savedstate:savedstate:1.1.0
- androidx.tracing:tracing:1.0.0
- androidx.versionedparcelable:versionedparcelable:1.1.1
- com.google.guava:listenablefuture:1.0


If this is intentional, re-baseline using ./gradlew :sample:app:dependencyGuardBaseline
Or use ./gradlew dependencyGuardBaseline to re-baseline dependencies in entire project.


FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':sample:app:dependencyGuard'.
> Dependencies Changed in :sample:app for configuration releaseRuntimeClasspath
  - androidx.activity:activity:1.4.0
  - androidx.annotation:annotation-experimental:1.1.0
  - androidx.annotation:annotation:1.2.0
  - androidx.arch.core:core-common:2.1.0
  - androidx.arch.core:core-runtime:2.1.0
  - androidx.collection:collection:1.0.0
  - androidx.concurrent:concurrent-futures:1.0.0
  - androidx.core:core:1.7.0
  - androidx.lifecycle:lifecycle-common:2.3.1
  - androidx.lifecycle:lifecycle-livedata-core:2.3.1
  - androidx.lifecycle:lifecycle-runtime:2.3.1
  - androidx.lifecycle:lifecycle-viewmodel-savedstate:2.3.1
  - androidx.lifecycle:lifecycle-viewmodel:2.3.1
  - androidx.savedstate:savedstate:1.1.0
  - androidx.tracing:tracing:1.0.0
  - androidx.versionedparcelable:versionedparcelable:1.1.1
  - com.google.guava:listenablefuture:1.0
  
  If this is intentional, re-baseline using ./gradlew :sample:app:dependencyGuardBaseline
  Or use ./gradlew dependencyGuardBaseline to re-baseline dependencies in entire project.
  


* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights.

* Get more help at https://help.gradle.org

BUILD FAILED in 2s

As you can see, the List output contains the diff twice. Moreover, the colored diff output is generally hidden, and only the last bit appear, which is the GradleException message. (and the first line is different as well)
Do you think we could harmonize this behavior?

  • Tree (ad-hoc message)

throw GradleException(
StringBuilder().apply {
appendLine(Messaging.dependencyChangeDetected)
appendLine("Dependency Tree comparison to baseline does not match.")
appendLine()
appendLine(Messaging.rebaselineMessage(projectPath))
}.toString()

  • List (reuses the exception message, leading to duplicated output)

@handstandsam
Copy link
Collaborator

🤔 . The reason why I printed it and then it's part of the exception is that I didn't know how to add color to the exception message, and I loved the color. Any tips here?

@SimonMarquis
Copy link
Contributor Author

SimonMarquis commented Mar 13, 2023

I do too 🌈 But what I don't understand is why we need the same message in both stdout and as Gradle error. In the "tree" mode we only have it on the stdout (not in the Gradle error).

Also, it seems you disabled to colors explicitely here:

// Add to exception message without color
exceptionMessage.appendLine(diffResult.createDiffMessage(withColor = false))

Without this, colors are visible in the Gradle exception.
image

@SimonMarquis
Copy link
Contributor Author

SimonMarquis commented Mar 13, 2023

Though it does not play nice with is some places where rich output is unsupported like here for instance:
image

We might be able to ask Gradle what the current behavior is with this: https://docs.gradle.org/current/userguide/command_line_interface.html#sec:command_line_customizing_log_format or use Gradle's own internal StyledTextOutput.

@handstandsam
Copy link
Collaborator

Happy to accept a PR regarding this. I personally only use tree format locally when trying to track down where a transitive dependency came from. That being said, I don't use it much, and don't have much of an opinion about the messaging.

If you would like to update/improve the messaging for tree, I'm open to it!

--

RE: Disable coloring

// Add to exception message without color
exceptionMessage.appendLine(diffResult.createDiffMessage(withColor = false))

Because it's a plaintext string and not console output, it just looks like some crazy encoded characters if you don't do it that way unfortunately.

@handstandsam
Copy link
Collaborator

handstandsam commented Mar 14, 2023

StyledTextOutput sounds nice. I'd love to use something built in.

Additionally I dislike the double messages, but now I remember why I did it:
In our CI system, it only bubbles up the exception to users when looking at a PR. If I did not add it to the exception, then users had to dig really deep, so for me at this time, it's a requirement. I'm open to having a flag for this if it's undesirable for others.


Also, if you want to just chat about any of this, feel free to message me on Slack (Kotlin, Gradle, or ASG).

@SimonMarquis
Copy link
Contributor Author

Ok, this makes more sense now. Then I guess there is no one solution fits all.
And adding even more settings to configure the desired output feels like the wrong approach for this kind of plugin.

Gradle's StyledTextOutput has been moved to their internal package, so I would not rely on this to work across many Gradle versions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants