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

ConfigureUtil deprecation warning #152

Closed
ben-manes opened this issue Nov 3, 2022 · 33 comments · Fixed by #172
Closed

ConfigureUtil deprecation warning #152

ben-manes opened this issue Nov 3, 2022 · 33 comments · Fixed by #172
Milestone

Comments

@ben-manes
Copy link

fyi I noticed this in a build scan,

The org.gradle.util.ConfigureUtil type has been deprecated.
This is scheduled to be removed in Gradle 9.0.
Documentation

@erichaagdev
Copy link

I've also started seeing this after upgrading to Gradle 7.6.

Build Scan: https://scans.gradle.com/s/ongbkxs4wvvba/deprecations?expanded=WyIzIl0&expanded-stacktrace=WyI1Il0&focused-stack-frame=0-0-2

@remal
Copy link

remal commented Nov 30, 2022

Looks like Project.configure() method should be used instead of ConfigureUtil.configureUsing()

@dili91
Copy link

dili91 commented Dec 13, 2022

I've also noticed the same since I bumped the wrapper to 7.6 inside one of my company's project 👀

Deprecated Gradle features were used in this build, making it incompatible with Gradle 8.0.

If useful, here the build.gradle file

@nedtwigg
Copy link

nedtwigg commented Jan 9, 2023

8.0-rc-1 is out now. This issue will become a blocker pretty soon.

@TWiStErRob
Copy link
Collaborator

TWiStErRob commented Feb 2, 2023

There's another callsite in the same file (curtesy of Mockito using this plugin):

The org.gradle.util.ConfigureUtil type has been deprecated. This is scheduled to be removed in Gradle 9.0. Consult the upgrading guide for further information: https://docs.gradle.org/7.6/userguide/upgrading_version_7.html#org_gradle_util_reports_deprecations
        at org.gradle.util.ConfigureUtil.logDeprecation(ConfigureUtil.java:216)
        at org.gradle.util.ConfigureUtil.configureSelf(ConfigureUtil.java:192)
        at io.github.gradlenexus.publishplugin.DefaultNexusRepositoryContainer.configure(DefaultNexusRepositoryContainer.kt:42)
        at io.github.gradlenexus.publishplugin.DefaultNexusRepositoryContainer.configure(DefaultNexusRepositoryContainer.kt:27)
        at org.gradle.util.internal.ConfigureUtil.configure(ConfigureUtil.java:104)
        at org.gradle.util.internal.ConfigureUtil$WrappedConfigureAction.execute(ConfigureUtil.java:167)
        at io.github.gradlenexus.publishplugin.NexusPublishExtension.repositories(NexusPublishExtension.kt:57)
        at shipkit_91h1hqg1wr0a5t608js79f84t$_run_closure3.doCall(P:\projects\contrib\github-mockito\gradle\shipkit.gradle:22)
        at shipkit_91h1hqg1wr0a5t608js79f84t.run(P:\projects\contrib\github-mockito\gradle\shipkit.gradle:21)
        at build_e6jjcbuu9ezmj4rzb0cym1acv.run(P:\projects\contrib\github-mockito\build.gradle:35)

@TWiStErRob
Copy link
Collaborator

Note: Gradle recommends to work around the need for ConfigureUtil by following this approach: https://docs.gradle.org/7.6/javadoc/org/gradle/util/ConfigureUtil.html

@szpak
Copy link
Contributor

szpak commented Feb 12, 2023

8.0-rc-1 is out now. This issue will become a blocker pretty soon.

Luckily, they seem to postpone the removal to 9.0 ;-).

The org.gradle.util.ConfigureUtil type has been deprecated. This is scheduled to be removed in Gradle 9.0.

Anyway (from #157, part full-faced by me):

... because #152 is important and easy to fix and is remaining unfixed.

@nedtwigg If you provide a simple and elegant solution, preferably keeping Gradle 6+ compatibility, I will do my best to review your PR in a timely manner :-).

Key places in code:

val repositories: NexusRepositoryContainer = DefaultNexusRepositoryContainer(

and:

internal class DefaultNexusRepositoryContainer(delegate: NamedDomainObjectContainer<NexusRepository>) : NexusRepositoryContainer, NamedDomainObjectContainer<NexusRepository> by delegate {
override fun sonatype(): NexusRepository = sonatype {}
override fun sonatype(closure: Closure<*>): NexusRepository {
return sonatype(ConfigureUtil.configureUsing(closure))
}
override fun sonatype(action: Action<in NexusRepository>): NexusRepository = create("sonatype") {
nexusUrl.set(URI.create("https://oss.sonatype.org/service/local/"))
snapshotRepositoryUrl.set(URI.create("https://oss.sonatype.org/content/repositories/snapshots/"))
action.execute(this)
}
override fun configure(configureClosure: Closure<*>): NamedDomainObjectContainer<NexusRepository> =
ConfigureUtil.configureSelf(configureClosure, this, NamedDomainObjectContainerConfigureDelegate(configureClosure, this))
}

@TWiStErRob
Copy link
Collaborator

Luckily, they seem to postpone the removal to 9.0 ;-).

Do you know why? Because the plugin authors are not moving and Gradle don't want to break/overload end users. 😉

I also did a background investigation for a similar deprecated API which was quite a rollercoaster of commits. 😅

@nedtwigg
Copy link

@szpak Two options provided above! I would follow Gradle's recs and just remove the Closure interfaces (#161) but if you want to keep them I provided an option for that too in #160.

@szpak
Copy link
Contributor

szpak commented Feb 13, 2023

Thanks @nedtwigg. To be honest, before I wrote my comment, I've just removed the methods with Closure, but it broke functional tests (Gradle 6, AFAIR also 7).

As it is mentioned in the linked documentation:

Gradle automatically generates a Closure-taking method at runtime for each method with an Action as a single argument as long as the object is created with ObjectFactory.newInstance(Class, Object...).

which is nice, however, we create DefaultNexusRepositoryContainer manually. The ObjectFactory is there (project.objects), however, I am not familiar with the "recent" changes in the API (and my quick & dirty attempts failed), so maybe you could try to follow the recommended path to achieve that?

As you see in the CI build reports, both builds fail on CI :-/. To test it locally with functional tests, you can call ./gradlew compatTest with JDK 11 to cover lower Gradle version (btw, it is advised edit .shutter/java11.lock to keep there only 6.9.1 (or 6.0 if possible) and 7.6 to speed up testing - we can easily drop Gradle 5 support).

WDYT?

@ben-manes
Copy link
Author

maybe you can use Groovy closures from Kotlin?

@vlsi
Copy link
Contributor

vlsi commented Feb 13, 2023

maybe you can use Groovy closures from Kotlin?

There's no point in doing that.

Action<in T> should be enough for all the cases:

  • It plays nicely in Kotlin
  • It does specify the type of the receiver
  • It works with Groovy as well (Gradle adds the needed overloads)

TWiStErRob added a commit to TWiStErRob/nexus-publish-gradle-plugin that referenced this issue Feb 13, 2023
… internal APIs.

This method is implemented from NamedDomainObjectContainer, which we implement by delegation, so we should keep configuring the original collection. This container only exists to slap some more methods onto the original class.
@TWiStErRob
Copy link
Collaborator

TWiStErRob commented Feb 13, 2023

@nedtwigg @szpak using objects.newInstance is hard requirement for this to work "out of the box".

There are two issues here (two distinct usages of ConfigureUtil):

  • Fixing configureUsing seems trivial, by way of newInstance. Raised Fix #152: remove usage of ConfigureUtil.configureUsing #162 to specifically fix this, this goes halfway in fixing this issue. Locally all tests passed for me on this with gradlew build -x spotlessKotlinCheck --console=verbose --continue.
  • Fixing configureSelf seems like a bigger challenge. We essentially need to mimic how RepositoryHandler (Project.repositories), ArtifactHandler (Project.artifacts), ConfigurationContainer (Project.configurations) are created, but they're all using internal APIs. I have a feeling Fix deprecation warnings #160 (i.e. copy-paste) might be the only way. So far I've only faced failure trying to do this.

@TWiStErRob
Copy link
Collaborator

Raised gradle/gradle#23874 to maybe get help with removing configureSelf.

@szpak szpak closed this as completed in 7da6a7d Feb 13, 2023
@szpak
Copy link
Contributor

szpak commented Feb 13, 2023

Thanks guys. I've merged #162 and will wait a while for any response in gradle/gradle#23874 before "copy-pasting" ConfigureUtil into our codebase (#160). Gradle 8.0 is out, but luckily, we have still some time for 9.0 ;-).

@szpak szpak reopened this Feb 13, 2023
TWiStErRob added a commit to TWiStErRob/nexus-publish-gradle-plugin that referenced this issue Feb 16, 2023
…le 7.6. Building with Gradle 8.0 will obviously also warn. Let's suppress it for now, this will be dealt with in gradle-nexus#152
@TWiStErRob
Copy link
Collaborator

TWiStErRob commented Feb 16, 2023

A quick update on this (and guidance for @szpak to navigate my contributions):

@szpak, also while doing the above I found a few other opportunities for reducing tech debt:

@nedtwigg
Copy link

In my view, the best thing is to remove Closure from the API, and I think the fix from @vlsi in #165 is the best fit. If I was Gradle, I would triage gradle/gradle#23874 to a very low priority - they have been pretty clear that plugin authors should just move to Action.

If keeping Closure for pre-7.0 Groovy users is important to you, I think the fix in #160 is the best way. But definitely bumping the minimum to Gradle 7 and removing the Closure from API should happen someday.

@szpak
Copy link
Contributor

szpak commented Feb 16, 2023

I would prefer to release one more version supporting Gradle 6+. To do so, I would sway for #160 (copy paste ConfigurationUtil to our code and remove the deprecation warning), replaced (after the release) with TWiStErRob#5 with bumping the minimal required Gradle version to 7.1 (and removing copied ConfigurationUtil).
I like the hacky #165 by @vlsi and it could be next, but it is a non-backward compatible change requiring probably version bump to 2.0.0 (unless we are able to find a way to use sonatype() in the Groovy DSL), so I still hope to have some feedback from Gradle in gradle/gradle#23874 before making a decision.

WDYT about this plan?

@nedtwigg
Copy link

Works for me!

@TWiStErRob
Copy link
Collaborator

My latest invention is backwards and forwards compatible on all supported versions. It doesn't matter what Gradle the build is using, only the jar it produces. So using the internal API temporarily is a good stopgap to support any version range. So there's no need to copy-paste and then delete it. Note: I can make a quick fix out of this without needing Gradle 7/8 upgrade too, because the class/interface is well-known, so reflection is fine.

@TWiStErRob
Copy link
Collaborator

By the time this becomes an issue again, hopefully we'll get some response from Gradle, but we can always fall back to copy-paste.

@vlsi
Copy link
Contributor

vlsi commented Feb 16, 2023

I would triage gradle/gradle#23874 to a very low priority

I would disagree here.
As @TWiStErRob says, the current API does not allow clients create type-safe entities that extend NamedDomain...
Even you one uses Action, there's just no way to do that, especially in Groovy DSL.

I incline that solution I propose in gradle/gradle#23874 (comment) is a low-hanging fruit. At the end of the day, they could expose getTarget() and use it. It sounds like a trivial change that helps the ones who use domain collections.

So using the internal API temporarily is a good stopgap to support any version range

+1
There's no need to copy the code, as we'll detect it quite fast as they happen to remove the method/class.

@szpak
Copy link
Contributor

szpak commented Feb 16, 2023

My latest invention is backwards and forwards compatible on all supported versions. It doesn't matter what Gradle the build is using, only the jar it produces.

Oh, I missed that fact that it requires 7.1, but should work also with 6.x. Nice.

I locally cherry-picked that change to master (and Gradle migration to 7.1) and from the regression tests, it works with Gradle 6.8+ [1], which is enough. However, I'm more a Groovy guy than a Kotlin one. Do you think, Gradle 7 and its migration to Kotlin 1.4 could affect 6.x users? Or we should rather use a reflection and still build that next release with Gradle 6.9?


[1] - FYI. For lower Gradle versions (<6.8) I had:

Execution failed for task ':closeSonatypeStagingRepository'.
'void kotlin.jvm.internal.PropertyReference1Impl.(java.lang.Class, java.lang.String, java.lang.String, int)'

and:

Execution failed for task ':closeSonatypeStagingRepository'.
Could not initialize class io.github.gradlenexus.publishplugin.AbstractTransitionNexusStagingRepositoryTask$transitionStagingRepo$retrier$1$1

@remal
Copy link

remal commented Feb 16, 2023

Sorry if I got smth wrong, but what prevents you from using project.configure()? Doesn't this use ConfigureUtil internally?

@TWiStErRob
Copy link
Collaborator

TWiStErRob commented Feb 16, 2023

I locally cherry-picked that change to master (and Gradle migration to 7.1)

Gradle migration to 7.1 is also non-trivial, you'll still need most of the changes in the Gradle 8 PR. Let's just merge that one and get through, @szpak.

FYI. For lower Gradle versions (<6.8) I had:

The PR (TWiStErRob#5) is against the Gradle 8 branch for a reason. The errors you've seen I went through and fixed each in the Gradle 8 PR.

Do you think, Gradle 7 and its migration to Kotlin 1.4 could affect 6.x users? Or we should rather use a reflection and still build that next release with Gradle 6.9?

If you merge the Gradle 8 PR, this project will build with Gradle 8, but it'll be compatible with all the Gradle 5-8 versions with all their Kotlin 1.3-1.8 range (it's similar to building with Java 17, but also targetCompatibility = 1.8). There's no need to drop support for anything to get it in. See the CI and manual QA on that PR, it's all good. @vlsi could you also have a look?

but what prevents you from using project.configure()?

Re @remal, actually I'll have to recheck, because I might've looked on Gradle 6.9 APIs, which could be missing an overload we need (that was added in Gradle 7.1), but Gradle 8 PR will still be necessary to be able to at least conditionally call the replacement API, if it exists.

@TWiStErRob
Copy link
Collaborator

@szpak re errors, have a read of the individual commit messages in the Gradle 8 PR, I tried to list all the problems and solutions I encountered.

@remal
Copy link

remal commented Feb 17, 2023

@TWiStErRob, as I can see, Project.configure(Object, Closure) has been there "forever". This method exists even in Gradle 1.0 (proof).

I've written a bunch of Gradle plugins but never used Kotlin's by delegate. Probably, because of that I'm missing something. I haven't used this method because Gradle API is evolving, and I don't want to depend on it by delegating in compile-time. For such cases, I have reflection-based proxies that do such delegation.

@TWiStErRob
Copy link
Collaborator

TWiStErRob commented Feb 17, 2023

@remal I've tried to use Project.configure, you can see here: https://github.com/TWiStErRob/nexus-publish-gradle-plugin/commits/fix152_configureUsing_configureSelf, but now I realize I was calling the wrong overload, there's Project.configure(Iterable, Closure) which wins over (Object, Closure), because this thing is a NamedDomainObjectCollection. Now, I tried the "right" overload too, but it's a StackOverflowError, which makes sense, because this magical configure function we're overriding is called by the configuration mechanism. The problem is that NamedDomainObjectContainer implements Configurable, and that exception exists inside ConfigureUtil.configure (not configureSelf) called by Project.configure.

Do you have an example where you've written a repositories-like block as described in gradle/gradle#23874 ?

TWiStErRob added a commit to TWiStErRob/nexus-publish-gradle-plugin that referenced this issue Feb 17, 2023
…le 7.6. Building with Gradle 8.0 will obviously also warn. Let's suppress it for now, this will be dealt with in gradle-nexus#152
@vlsi
Copy link
Contributor

vlsi commented Feb 17, 2023

@remal , project.configure does not work since it is implemented as follows:
https://github.com/gradle/gradle/blob/master/subprojects/model-core/src/main/java/org/gradle/util/ConfigureUtil.java#L147-L154

In other words, if the target object "implements Configurable", then Gradle uses the implemented configure method.
Apparently NamedDomainObjectContainer implements Configurable interface since it wants exposing custom behavior like "converting abc to create("abc")".
Unfortunately, NamedDomainObjectContainer.configure is hard-coded, and it does not allow plugin authors augmenting the behavior (see gradle/gradle#23874 (comment) )

szpak pushed a commit that referenced this issue Feb 17, 2023
* Bump Gradle Wrapper to Gradle 8.0 (from 6.9)

* Remove outdated DSL: kotlinDslPluginOptions.experimentalWarning

> Configure project :buildSrc
e: P:\projects\contrib\github-nexus-publish-plugin\buildSrc\build.gradle.kts:14:5: Unresolved reference: experimentalWarning

> Configure project :buildSrc
The KotlinDslPluginOptions.experimentalWarning property has been deprecated. This is scheduled to be removed in Gradle 8.0. Flag has no effect since `kotlin-dsl` no longer relies on experimental features.
        at Build_gradle$3.execute(build.gradle.kts:14)
        (Run with --stacktrace to get the full stack trace of this deprecation warning.)

* Update Shadow plugin, because 6.1.0 is not compatible with Gradle 7.x and Gradle 8.x

* What went wrong:
org/gradle/api/plugins/MavenPlugin

* Try:
> Run with --info or --debug option to get more log output.

* Exception is:
java.lang.NoClassDefFoundError: org/gradle/api/plugins/MavenPlugin
        at com.github.jengelman.gradle.plugins.shadow.ShadowJavaPlugin$1.execute(ShadowJavaPlugin.groovy:101)

* Regenerate Gradle Wrapper with Gradle 8.0

* Bump Versions plugin, Gradle 7.x support is full-blown from 0.43.0

Gradle 7.4 fixes:
https://github.com/ben-manes/gradle-versions-plugin/releases/tag/v0.42.0

Gradle 7 required:
https://github.com/ben-manes/gradle-versions-plugin/releases/tag/v0.43.0

* Bump plugin-publish plugin to 1.x

pluginBundle was deprecated in Gradle 7.6 and removed in Gradle 8.0:
https://plugins.gradle.org/plugin/com.gradle.plugin-publish/1.0.0

maven-publish is auto-applied

* Lock in Java compilation version to the same as Kotlin, otherwise things can go wrong.

> Task :compileKotlin FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':compileKotlin'.
> 'compileJava' task (current target is 11) and 'compileKotlin' task (current target is 1.8) jvm target compatibility should be set to the same Java version.
  Consider using JVM toolchain: https://kotl.in/gradle/jvm/toolchain

* ConfigureUtil is deprecated in Gradle 7.1 and started nagging in Gradle 7.6. Building with Gradle 8.0 will obviously also warn. Let's suppress it for now, this will be dealt with in #152

* Add support for Spotless on Windows

Also in #167, but it's annoying not being able to get a green build locally.

* Move sourceCompatibility to the existing java block

* Keep building Kotlin with 1.3 compatibility, otherwise there are failures on older Gradle versions.

For example: NexusPublishPluginTests.should close staging repository
```
> Task :closeSonatypeStagingRepository FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':closeSonatypeStagingRepository'.
> kotlin.jvm.internal.PropertyReference1Impl.<init>(Ljava/lang/Class;Ljava/lang/String;Ljava/lang/String;I)V
```

* Silence unnecessary build warnings.

> Task :compileKotlin
w: API version 1.3 is deprecated and its support will be removed in a future version of Kotlin

> Task :compileTestKotlin
w: API version 1.3 is deprecated and its support will be removed in a future version of Kotlin

> Task :compileCompatTestKotlin
w: API version 1.3 is deprecated and its support will be removed in a future version of Kotlin

* Remove duplicate calls to withSourcesJar

It's done by the plugin-publish plugin too.
In the Gradle Metadata (.module) it duplicates artifacts if called multiple times.

* Remove unnecessary shadowed configuration

There is built-in support for standard configuration in plugin-publish and shadow plugins.

* Remove unidentified workaround

It doesn't change anything in the output publishToMavenLocal, it was probably for an older combination of Gradle / plugin-publish / shadow plugins.

* Work around unnecessary ClassPath in MANIFEST.MF

* Downgrade language version from Kotlin 1.8 to Kotlin 1.3 to generate compatible @kotlin.Metadata in the .class files.

* Remove unrelated change.

* Diagnose #168 (comment)

* Revert "Diagnose #168 (comment)"

This reverts commit c14acc5.

* Mention Kotlin 1.3 #168 (comment)

* Reduce CI load, re #168 (comment)

* Diagnose #168 (comment)

* Revert "Diagnose #168 (comment)"

This reverts commit 8488741.

* Add sha256 to wrapper
@szpak
Copy link
Contributor

szpak commented Feb 17, 2023

@TWiStErRob Could you rebase TWiStErRob#5 and create it as a regular PR (in this repo)? WE could merge it next (as #168 is done).

@TWiStErRob
Copy link
Collaborator

TWiStErRob commented Feb 17, 2023

Raised #172 as rebase of TWiStErRob#5 + fix for #164's ignore. (no testing locally, I'll let the CI do the honors)

Edit: too confident 😅, investigating.

@TWiStErRob
Copy link
Collaborator

Fixed, how good that we merged #164, huh? 😁

szpak pushed a commit that referenced this issue Feb 19, 2023
* Workaround to "remove" the last usage of ConfigureUtil.

* Remove condition so that strict checking stays the default.
@szpak
Copy link
Contributor

szpak commented Feb 19, 2023

Hopefully, the deprecation warning is addressed for the time being. I still hope to get some response from the Gradle team about gradle/gradle#23874, with #165 as a non backward compatible idea as a fallback.

Big thanks to @TWiStErRob @vlsi @nedtwigg for theirs PRs and the others for suggestions and nagging ;).

Btw, I enabled Discussions for the project which would a probably a better place to discuss different things, also related with the incoming release of 1.2.0.

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