Upgrade gradle to v8.14.3 #8950
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,8 +11,8 @@ | |
| import java.util.Set; | ||
| import net.bytebuddy.asm.Advice; | ||
| import net.bytebuddy.matcher.ElementMatcher; | ||
| import org.gradle.internal.service.DefaultServiceRegistry; | ||
| import org.gradle.internal.service.ServiceRegistry; | ||
|
|
||
| @AutoService(InstrumenterModule.class) | ||
| public class GradleBuildScopeServices_8_3_Instrumentation extends InstrumenterModule.CiVisibility | ||
|
|
@@ -55,10 +55,15 @@ public void methodAdvice(MethodTransformer transformer) { | |
| public static class Construct { | ||
| @Advice.OnMethodExit(suppress = Throwable.class) | ||
| public static void afterConstructor( | ||
| @Advice.This final DefaultServiceRegistry buildScopeServices, | ||
| @Advice.Argument(0) final ServiceRegistry parentServices) { | ||
| CiVisibilityGradleListenerInjector_8_3.injectCiVisibilityGradleListener( | ||
| buildScopeServices, parentServices); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public String muzzleDirective() { | ||
| return "skipMuzzle"; | ||
| } | ||
| } | ||
|
There was a problem hiding this comment. The muzzle check for this instrumentation fails on a classloader mismatch when running on Gradle 8.14.3, but since this instrumentation targets Gradle 8.3, skip muzzle like what is done for 8_10_Instrumentation. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,3 @@ | ||
| apply from: "$rootDir/gradle/java.gradle" | ||
|
|
||
|
There was a problem hiding this comment. The updates to the latest version of JUnit Jupiter and Platform address the error: Not quite sure why the gradle upgrade initiated this, but others have faced the same problem (unplanned junit issue) and resolved it by updating versions. There was a problem hiding this comment. I'd like to use the oldest possible version here, as we want to ensure JUnit 5 instrumentation is working for older releases. Could you please double-check which artifacts' version changes when Gradle version is bumped? There was a problem hiding this comment. @nikita-tkachenko-datadog Maybe something you can do is to declare them as constraints in this case ? https://docs.gradle.org/current/userguide/dependency_constraints.html There was a problem hiding this comment. Good to know. From the command you shared, it looks like 1.12.0 and 5.12.0 are the oldest versions we can pull in (e.g. |
||
| // JUnit5 5.3.0+ version is needed because of the fix in the TestInheritance test suite names. | ||
|
|
@@ -33,9 +32,9 @@ dependencies { | |
|
|
||
| // versions used below are not the minimum ones that we support, | ||
| // but the tests need to use them in order to be compliant with Spock 2.x | ||
| testImplementation group: 'org.junit.platform', name: 'junit-platform-launcher', version: '1.12.0' | ||
| testImplementation group: 'org.junit.jupiter', name: 'junit-jupiter-engine', version: '5.12.0' | ||
| testImplementation group: 'org.junit.jupiter', name: 'junit-jupiter-params', version: '5.12.0' | ||
|
|
||
| latestDepTestImplementation group: 'org.junit.platform', name: 'junit-platform-launcher', version: '+' | ||
| latestDepTestImplementation group: 'org.junit.jupiter', name: 'junit-jupiter-engine', version: '+' | ||
|
|
@@ -44,8 +43,8 @@ dependencies { | |
|
|
||
| configurations.matching({ it.name.startsWith('test') }).each({ | ||
| it.resolutionStrategy { | ||
| force group: 'org.junit.platform', name: 'junit-platform-launcher', version: '1.12.0' | ||
| force group: 'org.junit.jupiter', name: 'junit-jupiter-engine', version: '5.12.0' | ||
| force group: 'org.junit.jupiter', name: 'junit-jupiter-params', version: '5.12.0' | ||
| } | ||
| }) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| import datadog.trace.agent.test.base.AbstractPromiseTest | ||
| import io.netty.util.concurrent.DefaultEventExecutorGroup | ||
| import io.netty.util.concurrent.GenericProgressiveFutureListener | ||
| import io.netty.util.concurrent.ProgressiveFuture | ||
| import io.netty.util.concurrent.ProgressivePromise | ||
| import spock.lang.Shared | ||
|
|
||
|
|
@@ -61,15 +61,12 @@ class NettyProgressivePromiseTest extends AbstractPromiseTest<ProgressivePromise | |
| when: | ||
| runUnderTrace("parent") { | ||
| def listeners = iterations.collect { int i -> | ||
| return new GenericProgressiveFutureListener<ProgressiveFuture<?>>() { | ||
| void operationComplete(ProgressiveFuture<?> future) throws Exception { | ||
| runUnderTrace("listen$i") {} | ||
| } | ||
|
|
||
| void operationProgressed(ProgressiveFuture<?> future, long progress, long total) throws Exception { | ||
| runUnderTrace("progress$i") {} | ||
| } | ||
| } | ||
|
There was a problem hiding this comment. GenericProgressiveFutureListener argument extends ProgressiveFuture (ref), and with the Gradle upgrade, I get the error: There was a problem hiding this comment. For reference, there is a warning on master on this, suggesting to use |
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,7 +27,7 @@ final testTasks = scalaVersions.collect { scalaLibrary -> | |
| def (major, minor) = version.split('_').collect(Integer.&valueOf) | ||
|
There was a problem hiding this comment. Changes in this file address aesthetic comments in the previous Gradle upgrade #8886 |
||
| final javaConcatenation = major > 2 || minor > 11 // after 2.11 scala uses java.lang.StringBuilder to perform concatenation | ||
|
|
||
| final implementationConfiguration = configurations.create("${version}Implementation") { | ||
| canBeConsumed = false | ||
| canBeResolved = false | ||
| canBeDeclared = true | ||
|
|
@@ -36,14 +36,14 @@ final testTasks = scalaVersions.collect { scalaLibrary -> | |
| canBeConsumed = false | ||
| canBeResolved = true | ||
| canBeDeclared = false | ||
| extendsFrom(implementationConfiguration) | ||
| } | ||
|
|
||
| dependencies { handler -> | ||
| handler.add(implementationConfiguration.name, scalaLibrary) | ||
| handler.add(implementationConfiguration.name, libs.slf4j) | ||
| if (javaConcatenation) { | ||
| handler.add(implementationConfiguration.name, project(':dd-java-agent:instrumentation:java-lang')) | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -59,7 +59,7 @@ final testTasks = scalaVersions.collect { scalaLibrary -> | |
| .filter { !it.toString().contains('scala-library') } // exclude default scala-library | ||
| .minus(files(sourceSets.test.scala.classesDirectory)) // exclude default /build/classes/scala/test folder | ||
| .plus(customSourceSet.output.classesDirs) // add /build/classes/scala/${version} folder | ||
| .plus(classPathConfiguration) // add new scala-library configuration | ||
| systemProperty('uses.java.concat', javaConcatenation) | ||
| dependsOn(tasks.named("compile${version.capitalize()}Scala")) | ||
| group = 'verification' | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -60,7 +60,7 @@ class GradleLauncherSmokeTest extends AbstractGradleTest { | |
| ]) | ||
| String[] command = ["./gradlew", "--no-daemon", "--info"] | ||
| if (gradleDaemonCmdLineParams) { | ||
| command += "-Dorg.gradle.jvmargs=$gradleDaemonCmdLineParams".toString() | ||
| } | ||
|
There was a problem hiding this comment. This resolves errors I was getting with assigning GStringImpl to String[] (others' experiences: apache issue, stackoverflow issue). Seems like this should be resolved with groovy 3.0.7+, but I guess not… |
||
| return shellCommandExecutor.executeCommand(IOUtils::readFully, command) | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| import org.gradle.api.internal.provider.PropertyFactory | ||
| import org.gradle.jvm.toolchain.internal.SpecificInstallationToolchainSpec | ||
|
|
||
| apply plugin: 'java-library' | ||
|
|
@@ -175,7 +176,7 @@ project.afterEvaluate { | |
| def testJvmHomePath = getJavaHomePath(testJvmHome) | ||
| // Only change test JVM if it's not the one we are running the gradle build with | ||
| if (currentJavaHomePath != testJvmHomePath) { | ||
| def jvmSpec = new SpecificInstallationToolchainSpec(project.services.get(PropertyFactory), file(testJvmHomePath)) | ||
| // The provider always says that a value is present so we need to wrap it for proper error messages | ||
|
There was a problem hiding this comment. This change is needed due to the introduced error There was a problem hiding this comment. This internal Gradle type has changed twice its constructor signature since 8.5
|
||
| Provider<JavaLauncher> launcher = providers.provider { | ||
| try { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,7 +7,7 @@ okhttp-legacy = "[3.0,3.12.12]" # 3.12.x is last version to support Java7 | |
| okio = "1.17.6" # Datadog fork | ||
|
|
||
| spock = "2.3-groovy-3.0" | ||
| groovy = "3.0.24" | ||
| junit5 = "5.9.2" | ||
|
There was a problem hiding this comment. When upgrading from Gradle <= 8.13, Groovy needs to be upgraded to 3.0.24: https://docs.gradle.org/current/userguide/upgrading_version_8.html#upgrade_to_groovy_3_0_24 |
||
| logback = "1.2.3" | ||
| bytebuddy = "1.17.5" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| import org.gradle.api.plugins.jvm.JvmTestSuite | ||
|
|
||
| ext.addTestSuiteExtendingForDir = { testSuiteName, parentSuiteName, dirName -> | ||
| testing { | ||
| suites { | ||
| create(testSuiteName, JvmTestSuite, { | ||
|
|
@@ -27,16 +27,12 @@ ext.addTestSuiteExtendingForDir = (String testSuiteName, String parentSuiteName, | |
| } | ||
| } | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| configurations { | ||
| def extendConf = { suffix -> | ||
| def config = named("${testSuiteName}${suffix}") | ||
| def parentConfig = named("${parentSuiteName}${suffix}") | ||
| if (parentConfig.present) { | ||
|
|
@@ -65,12 +61,19 @@ ext.addTestSuiteExtendingForDir = (String testSuiteName, String parentSuiteName, | |
| from(sourceSets.named(testSuiteName).get().output) | ||
| archiveClassifier = testSuiteName | ||
| } | ||
|
|
||
| // The project dependency definition cannot sit inside previous blocks. As of Gradle 8.8, configurations cannot be mutated after they have been used as part of the dependency graph. | ||
| // See: https://docs.gradle.org/current/userguide/upgrading_version_8.html#mutate_configuration_after_locking | ||
| // And related issue: https://github.com/gradle/gradle/issues/28867 | ||
| project.dependencies { | ||
| "${testSuiteName}Implementation"(project(project.path)) | ||
| } | ||
| } | ||
|
There was a problem hiding this comment. Project dependencies was separated out due to the error There was a problem hiding this comment. Indeed this looks like it : gradle/gradle#28867 Maybe drop a comment there, to explain why the documented method did not work. I wasn't able to reliably reproduce this issue. So there might be something odd happending. |
||
|
|
||
| ext.addTestSuiteForDir = { testSuiteName, dirName -> | ||
| ext.addTestSuiteExtendingForDir(testSuiteName, 'test', dirName) | ||
| } | ||
|
|
||
| ext.addTestSuite = { testSuiteName -> | ||
| ext.addTestSuiteForDir(testSuiteName, testSuiteName) | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When trying to
./gradlew clean assemble, I ran across the errorincompatible types: org.gradle.internal.service.scopes.BuildScopeServices cannot be converted to org.gradle.internal.service.DefaultServiceRegistry. Explicitly setting thebuildScopeServicestype toDefaultServiceRegistry(which is what injectCiVisibilityGradleListener takes) resolves this.I'm thinking that the Gradle upgrade has led to stricter type resolution as seen from other changes needed in this PR 🤔