Way to find root cause of a Gradle deprecation warning is arcane knowledge

not-a-bug

(Jörn Huxhorn) #1

It’s possible to get a stacktrace pointing to the source of a Gradle deprecation warning by using -Dorg.gradle.deprecation.trace=true. This is extremely helpful but isn’t mentioned in the guide or the output of Gradle. I only found out about it in this forum.

I’d suggest to change both of that.

Mention it in the guide and tell the users about this option if a deprecation warning has been printed by a Gradle build, similarly to the --debug/--info/--stacktrace message.

Ideally, Gradle should point to the source of the deprecation warning directly instead of making me search a stacktrace for .gradle: but that is definitely a much bigger change.

Gradle Version: 3.0-rc-1


(Stefan Wolf) #2

Hi Jörn,

I agree that this would be a great feature! We had some internal discussion and we agreed that adding the line and file for each deprecation warning would be great.

Do you think you would have time to provide a pull request for that feature? We should already have all the information in LoggingDeprecatedFeatureHandler.

Cheers,
Stefan


(Jörn Huxhorn) #3

I’ll give it a look over the weekend if nothing interferes.


(Stefan Wolf) #4

Awesome! Thanks for working on this!


(Jörn Huxhorn) #5

I gave this a shot but it escalated a bit… (note that the escalated part is entirely optional)

At that point I realized that org.gradle.util.DeprecationLogger

  • exists
  • is part of the public API
  • isn’t just logging deprecation warnings

and that org.gradle.util.SingleMessageLogger

  • exists
  • is part of the public API
  • isn’t responsible for only logging only a single message. That’s the responsibility of LoggingDeprecatedFeatureHandler.

Because of the, I converted the deprecation warning methods from static methods to
instance methods, i.e. org.gradle.util.UserNagger interface and
org.gradle.util.DefaultUserNagger implementation.

SingleMessageLogger, DeprecationLogger and DefaultUserNagger.getInstance() are now each using their own specific instance of DefaultUserNagger using their own class as calledFrom.

Let me know what you think. I could understand if you considered my changes too extensive…

I’d otherwise change all internal SingleMessageLogger/DeprecationLogger calls to DefaultUserNagger.getInstance() calls and would prepare a pull request.

nag messages will now always print the root element of the stack trace and any stack trace element originating from Gradle script files. Using the arcane property prints the full stack trace as before.

Edit: I just force-pushed the original branch and separated the sane, minimal changes into branch https://github.com/huxi/gradle/tree/deprecations while the architectural changes mentioned above are now located in branch https://github.com/huxi/gradle/tree/deprecations-user-nagger that’s building upon https://github.com/huxi/gradle/tree/deprecations .

./gradlew quickCheck :logging:check still works for both branches.


(Stefan Oehme) #7

I haven’t looked at the changes, just wanted to let you know that org.gradle.util.* is not part of the public API. It is undocumented and not intended to be used outside the Gradle codebase itself. I did a quick search on GitHub and didn’t find usages of the DeprecationLogger outside our code base. So it seems safe to do a more straightforward, breaking change.


(Jörn Huxhorn) #8

Oh! Good to know!
I thought we’d potentially end up in a situation were the DeprecationLogger would warn about itself.

Let me know when you (or someone else) had the time to give the deprecations-user-nagger branch a closer look. I’m not happy with the names UserNagger/DefaultUserNagger naming but the switch from static methods to instance methods (the main point of the change) makes sense for me for several reasons.

The changes of the deprecations branch should be a pretty safe bet and I’d PR the branch if you give the OK.

The only change in behavior (besides the original task of printing nag stack trace elements originating from Gradle script files) is documented here:


(Stefan Wolf) #9

Hi Jörn,

I had a look at the deprecations branch. Given the following build script

task binZip(type: Zip) {
    outputs.file('build/some').upToDateWhen { false }
    into('some')
}

When running gradle build with your changes I get the following output:

The chaining of the upToDateWhen(Closure) method has been deprecated and is scheduled to be removed in Gradle 4.0. Use 'upToDateWhen(Closure)' on TaskOutputs directly instead.
        at org.gradle.api.internal.tasks.DefaultTaskOutputs$BasePropertySpec.getTaskOutputs(DefaultTaskOutputs.java:283)
        at build_alawflfnvbheflz1u3pnprkyz$_run_closure1.doCall(/Users/wolfs/projects/gradle/test-tryout/dep-warnings/build.gradle:2)
        at build_alawflfnvbheflz1u3pnprkyz.run(/Users/wolfs/projects/gradle/test-tryout/dep-warnings/build.gradle:1)

I would currently only expect to be warned about line 2 of build.gradle. What do you think? I would print the whole stacktrace when running gradle -s.

I would also be happy to continue this discussion on a PR, so feel free to open one.

I did not yet have time to look at deprecations-user-nagger branch, but I will have a look later that week.

Thank you very much for looking into this!

Regards,
Stefan


(Jörn Huxhorn) #10

I opened the pull request Deprecation warning should print location in build file.

I changed the implementation to only print the first Gradle script location as you suggested.

I agree with you that --stacktrace should cause the full stack trace of the warning to be printed but I see no easy way of obtaining that info from inside LoggingDeprecatedFeatureHandler. Please tell me if you know of one.

So I’d suggest to postpone this to the potential deprecation logger redesign and keep the current behavior as it is for now, i.e. -Dorg.gradle.deprecation.trace=true causes a full stack trace.

I’d like to change some more stuff regarding deprecation warnings in the long run.

Most importantly, I’d like to not prevent duplicate deprecation warnings while still preventing duplicate incubation warnings.

I was getting a bit erratic while fixing the deprecation warnings in our 490 module build, being forced to fix one after the other, restarting the build after every single fix… :stuck_out_tongue:

There is one other thing:
I added your build file example above to the integration tests but I was unable to make the integration test work like a “normal” Gradle would. It would always print the full stack trace of the deprecation warning, regardless of anything I tried (like setting the system property). Any idea why this is happening?


(Stefan Wolf) #11

Regarding the integration tests always printing the full stacktrace: I think this is because of https://github.com/gradle/gradle/blob/c8281087bcb1eaa0fe97f5942563d05347beccaa/subprojects/internal-integ-testing/src/main/groovy/org/gradle/integtests/fixtures/executer/AbstractGradleExecuter.java#L708-L708


(Jörn Huxhorn) #12

Yep, I found it. Will extend AbstractGradleExecuter accordingly. Thanks!


(Chris Doré) #13

I cannot recall if plugins even cause deprecation warnings or not, but have you considered or tested how your changes behave in that scenario?
I’m interested in what the stack trace looks like.


(Jörn Huxhorn) #14

I deleted deprecations-user-nagger so you don’t waste your time taking a look at it. I’m working on something way better, ETA after the weekend.
That was the complex changes branch on top of the simple changes I opened a PR for. Don’t worry, the PR is still there. :wink:


(Jörn Huxhorn) #15

I’m not 100% sure I understand your question correctly…

The build file above would now print this:

The chaining of the upToDateWhen(Closure) method has been deprecated and is scheduled to be removed in Gradle 4.0. Use 'upToDateWhen(Closure)' on TaskOutputs directly instead.
    at build_donqvs54ajsw5gylb6jrb3ze$_run_closure1.doCall([..]/gradle/subprojects/logging/build/tmp/test files/DeprecationHandlingIntegrationTest/upToDateWhen_deprec...acktrace_/wex5b/build.gradle:3)

I just pushed a new commit adding an integration test using the deprecated Jetty plugin. The message in that case would look like this:

The Jetty plugin has been deprecated and is scheduled to be removed in Gradle 4.0. Consider using the Gretty (https://github.com/akhikhl/gretty) plugin instead.
	at [..]/gradle/subprojects/logging/build/tmp/test files/DeprecationHandlingIntegrationTest/jetty_deprecation_i...acktrace_/7dg3o/build.gradle:2)

where line 2 is apply plugin: 'jetty'.

Does that answer your question?


(Jörn Huxhorn) #16

I just created another PR with the API changes I’d suggest:

Sorry about the delay.

Ping @Stefan_Wolf


(Chris Doré) #17

Sorry, I meant to ask what happens if plugin code calls something deprecated, not what happens if you apply a deprecated plugin. Does that make sense?


(Jörn Huxhorn) #18

The stack trace of the deprecation is created at the point where the nagUser method is called (i.e. the “something” that is deprecated is called) and the deepest stack trace element that has a Gradle build fileName is printed, if any.

It’s not important which kind of deprecation it is, they all work the same.

So if you apply a plugin which adds a task to your project, two things can happen:

  1. applying the plugin generates the warning
  2. executing a task of the plugin generates the warning

The first case would print the location of the apply plugin in your Gradle file.

The second case would only print a location if such a location in a Gradle file exists. Otherwise you’d have to use the -s/-S switch to get the full stack trace and search the cause of the warning manually because there is no good heuristic to find the “correct” element.

See DeprecationHandlingIntegrationTest for some examples.