Skip to content

Commit

Permalink
Upgrade Gradle to 8.1.1 (#4854)
Browse files Browse the repository at this point in the history 
Motivation:

[Gradle 8](https://docs.gradle.org/8.0/release-notes.html) has been
released.

Modifications:

- In Gradle 8, dependencies can't be declared in a resolvable
configuration (`canBeResolved=true`). `gradle-scripts` used resolvable
configurations such as `compileClasspath`, `runtimeClasspath` to inject
platform dependencies.
https://github.com/line/armeria/blob/20969832640bcc0fa66edc68c531891ab35cea25/gradle/scripts/lib/common-dependencies.gradle#L180-L183
  ```
  * Where: Script '.../gradle/scripts/lib/common-dependencies.gradle'

  * What went wrong: A problem occurred configuring project ':foobar'.
> Dependencies can not be declared against the `compileClasspath`
configuration.
  ```

  `platform` dependencies should be added to non resolvable and non
  configurations. There is no workaround to set dependencies to
  resolvable configurations. So I changed `common-dependencies.gradle`
  to set platform dependencies to the non resolvable and non
  configurations. 

  As a workaround `shadedTestImplementation` is newly added to declare
dependencies while configuring `shadedTest` task. The original
`shadedTestRuntime`
extends `shadedTestImplementation` and is only used to resolve its
dependencies.

Related discussion:
https://discuss.gradle.org/t/problem-with-compileclasspath-after-gradle-8-update/44940

- Gradle 8 also disallows implicit dependencies between two tasks. The
solution was quite simple. I added an explicit dependency for the
implicit dependency with `.dependsOn(...)` whenever encountering the
error.

Result:

You can now use Gradle 8 with `gradle-scripts`.
  • Loading branch information
@ikhoon
ikhoon authored May 15, 2023
1 parent 92e7017 commit e15c7b7
Show file tree
Hide file tree
Showing 29 changed files with 349 additions and 162 deletions.
5 changes: 4 additions & 1 deletion .github/workflows/actions_build.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,10 @@ jobs:

- name: Run flaky tests
run: |
./gradlew --no-daemon --stacktrace --max-workers=2 --parallel check -PnoLint -PflakyTests=true
./gradlew --no-daemon --stacktrace --max-workers=2 --parallel check -PnoLint -PflakyTests=true \
-PbuildJdkVersion=${{ env.BUILD_JDK_VERSION }} \
-PtestJavaVersion=${{ env.BUILD_JDK_VERSION }} \
-Porg.gradle.java.installations.paths=${{ steps.setup-build-jdk.outputs.path }}
- name: Summarize the failed tests
if: failure()
Expand Down
2 changes: 2 additions & 0 deletions benchmarks/jmh/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ dependencies {
implementation project(':testing-internal')
}

tasks.sourcesJar.dependsOn(tasks.compileJmhThrift)

jmh {
duplicateClassesStrategy = DuplicatesStrategy.EXCLUDE
jmhVersion = libs.versions.jmh.core.get()
Expand Down
4 changes: 2 additions & 2 deletions build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ allprojects {
tasks.withType(JavaForkOptions) {
maxHeapSize = '768m'

if (rootProject.ext.testJavaVersion >= 9) {
if (project.ext.testJavaVersion >= 9) {
jvmArgs '--add-exports=java.base/jdk.internal.misc=ALL-UNNAMED'
}

Expand All @@ -74,7 +74,7 @@ allprojects {
// `-Pjava.security.manager` was added in Java 17 and the default value was changed to `disallow` in Java 18.
// - https://openjdk.org/jeps/411
// - https://bugs.openjdk.org/browse/JDK-8270380
if (rootProject.ext.testJavaVersion >= 17) {
if (project.ext.testJavaVersion >= 17) {
systemProperty "java.security.manager", "allow"
}

Expand Down
90 changes: 54 additions & 36 deletions core/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ mrJarVersions.each { version->
classpath = sourceSets."java${version}Test".runtimeClasspath

enabled = Integer.parseInt(JavaVersion.current().getMajorVersion()) >= version &&
rootProject.ext.testJavaVersion >= version
project.ext.testJavaVersion >= version
}

configurations."java${version}Implementation".extendsFrom configurations.implementation
Expand Down Expand Up @@ -173,45 +173,63 @@ if (!rootProject.hasProperty('noWeb')) {
// Run HttpServerStreamingTest separately with memory constraint.
tasks.test.exclude '**/HttpServerStreamingTest**'
tasks.shadedTest.exclude '**/HttpServerStreamingTest**'
task testStreaming(type: Test,
group: 'Verification',
description: 'Runs the streaming tests.',
dependsOn: tasks.shadedTestClasses) {
// Use small heap for streaming tests to quickly ensure we can stream the content larger than heap.
maxHeapSize = '64m'

include '**/HttpServerStreamingTest**'
testClassesDirs = tasks.shadedTest.testClassesDirs
classpath = testClassesDirs

// Set the class path as late as possible so that the 'shadedTest' task has the correct classpath.
doFirst {
classpath += project.files(configurations.shadedTestRuntime.resolve())
testing {
suites {
testStreaming(JvmTestSuite) {

group = 'Verification'
description = 'Runs the streaming tests.'

targets {
all {
testTask.configure {
dependsOn(tasks.shadedTestClasses)
// Use small heap for streaming tests to quickly ensure we can stream the content larger than heap.
maxHeapSize = '64m'

include '**/HttpServerStreamingTest**'
testClassesDirs = tasks.shadedTest.testClassesDirs
classpath = testClassesDirs

// Set the class path as late as possible so that the 'shadedTest' task has the correct classpath.
doFirst {
classpath += project.files(configurations.shadedTestRuntime.resolve())
}
}
}
}
}
}
}
tasks.shadedTest.finalizedBy tasks.testStreaming
tasks.check.dependsOn tasks.testStreaming

if (rootProject.findProperty('FLAKY_TESTS') != 'true') {
// Run the test cases based on reactive-streams-tck only with non-flaky mode
task testNg(type: Test,
group: 'Verification',
description: 'Runs the TestNG unit tests.',
dependsOn: tasks.shadedTestClasses) {
useTestNG()
include '/com/linecorp/armeria/common/**'

scanForTestClasses = false
testClassesDirs = tasks.shadedTest.testClassesDirs
classpath = testClassesDirs

// Set the class path as late as possible so that the 'shadedTest' task has the correct classpath.
doFirst {
classpath += project.files(configurations.shadedTestRuntime.resolve())
tasks.shadedTest.finalizedBy testing.suites.testStreaming
tasks.check.dependsOn testing.suites.testStreaming

// Run the test cases based on reactive-streams-tck only with non-flaky mode
if (rootProject.findProperty('flakyTests') != 'true') {
testing {
suites {
testNg(JvmTestSuite) {
useTestNG()

targets {
all {
testTask.configure {
include '/com/linecorp/armeria/**/common/**'
dependsOn tasks.shadedTestClasses
scanForTestClasses = false
testClassesDirs = tasks.shadedTest.testClassesDirs
classpath = testClassesDirs
doFirst {
classpath += project.files(configurations.shadedTestRuntime.resolve())
}
}
}
}
}
}
}
tasks.shadedTest.finalizedBy tasks.testNg
tasks.check.dependsOn tasks.testNg
tasks.shadedTest.finalizedBy testing.suites.testNg
tasks.check.dependsOn testing.suites.testNg
}

if (tasks.findByName('trimShadedJar')) {
Expand Down
5 changes: 1 addition & 4 deletions core/src/test/java/com/linecorp/armeria/common/stream/StreamMessageVerification.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,9 +151,6 @@ public void required_completionFutureMustCompleteOnCancellation() throws Throwab
@Test
public void required_subscribeOnAbortedStreamMustFail() throws Throwable {
final StreamMessage<T> pub = createAbortedPublisher(0);
if (pub == null) {
notVerified();
}
assumeAbortedPublisherAvailable(pub);
assertThatThrownBy(() -> pub.whenComplete().join())
.isInstanceOf(CompletionException.class)
Expand Down Expand Up @@ -234,7 +231,7 @@ public void required_abortMustNotifySubscriber() throws Throwable {
.hasCauseInstanceOf(AbortedStreamException.class);
}

private void assumeAbortedPublisherAvailable(@Nullable Publisher<T> pub) {
protected void assumeAbortedPublisherAvailable(@Nullable Publisher<T> pub) {
if (pub == null) {
throw new SkipException("Skipping because no aborted StreamMessage provided.");
}
Expand Down
53 changes: 53 additions & 0 deletions ...test/java/com/linecorp/armeria/internal/common/stream/FixedStreamMessageVerification.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,19 @@

package com.linecorp.armeria.internal.common.stream;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

import java.util.concurrent.CompletionException;
import java.util.concurrent.atomic.AtomicReference;
import java.util.stream.LongStream;

import org.reactivestreams.Subscription;
import org.reactivestreams.tck.TestEnvironment.Latch;
import org.reactivestreams.tck.TestEnvironment.TestSubscriber;
import org.testng.annotations.Test;

import com.linecorp.armeria.common.stream.AbortedStreamException;
import com.linecorp.armeria.common.stream.StreamMessage;
import com.linecorp.armeria.common.stream.StreamMessageVerification;

Expand Down Expand Up @@ -58,4 +67,48 @@ public void required_spec317_mustNotSignalOnErrorWhenPendingAboveLongMaxValue()
// are all pre-allocated.
notVerified();
}

/**
* Modified from {@link #optional_spec104_mustSignalOnErrorWhenFails()}.
*/
@Test
public void required_subscribeOnAbortedStreamMustFail() throws Throwable {
// EmptyFixedStreamMessage performs nothing on abortion.
final StreamMessage<Long> pub = createAbortedPublisher(1);
if (pub == null || pub instanceof EmptyFixedStreamMessage) {
notVerified();
}
assumeAbortedPublisherAvailable(pub);
assertThatThrownBy(() -> pub.whenComplete().join())
.isInstanceOf(CompletionException.class)
.hasCauseInstanceOf(AbortedStreamException.class);
assertThat(pub.isOpen()).isFalse();

final AtomicReference<Throwable> capturedCause = new AtomicReference<>();
final Latch onErrorLatch = new Latch(env());
final Latch onSubscribeLatch = new Latch(env());
pub.subscribe(new TestSubscriber<Long>(env()) {
@Override
public void onSubscribe(Subscription subs) {
onSubscribeLatch.assertOpen("Only one onSubscribe call expected");
onSubscribeLatch.close();
}

@Override
public void onError(Throwable cause) {
onSubscribeLatch.assertClosed("onSubscribe should be called prior to onError always");
onErrorLatch.assertOpen(String.format(
"Error-state Publisher %s called `onError` twice on new Subscriber", pub));
capturedCause.set(cause);
onErrorLatch.close();
}
});

onSubscribeLatch.expectClose("Should have received onSubscribe");
onErrorLatch.expectClose(String.format(
"Error-state Publisher %s did not call `onError` on new Subscriber", pub));

env().verifyNoAsyncErrors();
assertThat(capturedCause.get()).isInstanceOf(AbortedStreamException.class);
}
}
4 changes: 2 additions & 2 deletions gradle/scripts/.gitrepo
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
[subrepo]
remote = https://github.com/line/gradle-scripts
branch = main
commit = a706e2826247c958aed17a1d7ec1168bb1a9c7e1
parent = 506e1b3c1236579d7344e3f10f3de13f811af5b1
commit = c20bc5b416c7fe19475dd1a2ca658d9177d23f35
parent = d48d5fa5c0e30f34deac6df1ada7e41b58ae444c
method = merge
cmdver = 0.4.5
15 changes: 1 addition & 14 deletions gradle/scripts/lib/common-dependencies.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ buildscript {
}
}

def managedDependencyOverrides = [] as Set

allprojects { p ->
ext {
// Add managedVersions() for backward compatibility with dependencies.yml
Expand Down Expand Up @@ -178,19 +176,8 @@ configure(projectsWithFlags('java')) {
dependencies {
configurations.configureEach { configuration ->
// Add to resolvable configurations
if (configuration.canBeResolved && !configuration.canBeConsumed) {
add(configuration.name, platform(dependencyManagementProject))
}

// Find version overrides in dependency declaration configurations
if (!configuration.canBeResolved && !configuration.canBeConsumed) {
configuration.dependencies.configureEach { dep ->
if (dep instanceof org.gradle.api.artifacts.ExternalDependency) {
if (dep.version != null) {
managedDependencyOverrides.add(String.valueOf("${dep.module}:${dep.version}"))
}
}
}
add(configuration.name, platform(dependencyManagementProject))
}
}
}
Expand Down
9 changes: 7 additions & 2 deletions gradle/scripts/lib/common-git.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,12 +101,17 @@ private static String executeCommand(...command) {
def stdout = new StringWriter()
def stderr = new StringWriter()
proc.waitForProcessOutput(stdout, stderr)
def outMsg = stdout.toString().trim()

if (proc.exitValue() != 0) {
var errMsg = stderr.toString().trim()
if (errMsg.isEmpty()) {
errMsg = outMsg
}
throw new IOException(
"'${command}' exited with a non-zero exit code: ${proc.exitValue()}:" +
"${System.lineSeparator()}${stderr.toString().trim()}")
"${System.lineSeparator()}$errMsg")
}

return stdout.toString().trim()
return outMsg
}
15 changes: 10 additions & 5 deletions gradle/scripts/lib/java-alpn.gradle
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
// JDK 9 or above does not require Jetty ALPN agent at all.
if (JavaVersion.current() >= JavaVersion.VERSION_1_9 && rootProject.ext.testJavaVersion != 8) {
return
}

configure(projectsWithFlags('java')) {
// JDK 9 or above does not require Jetty ALPN agent at all.
if (JavaVersion.current() >= JavaVersion.VERSION_1_9 && project.ext.testJavaVersion != 8) {
return
}

// Use Jetty ALPN agent if dependencyManagement mentions it.
if (managedVersions.containsKey('org.mortbay.jetty.alpn:jetty-alpn-agent')) {
configurations {
Expand All @@ -24,6 +24,11 @@ configure(projectsWithFlags('java')) {
fileName.replaceFirst("-[0-9]+\\.[0-9]+\\.[0-9]+(?:\\.[^.]+)?\\.jar", ".jar")
}
}
tasks.compileJava.dependsOn(tasks.copyAlpnAgent)
tasks.processResources.dependsOn(tasks.copyAlpnAgent)
tasks.sourcesJar.dependsOn(tasks.copyAlpnAgent)
tasks.javadoc.dependsOn(tasks.copyAlpnAgent)
project.ext.getLintTask().dependsOn(tasks.copyAlpnAgent)

tasks.withType(JavaForkOptions) {
dependsOn tasks.copyAlpnAgent
Expand Down
10 changes: 5 additions & 5 deletions gradle/scripts/lib/java-coverage.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,11 @@ configure(rootProject) {
task jacocoTestReport(type: JacocoReport) {
def reportTask = delegate
reports {
csv.enabled = false
xml.enabled = true
xml.destination = file("${rootProject.buildDir}/report/jacoco/jacocoTestReport.xml")
html.enabled = true
html.destination = file("${rootProject.buildDir}/report/jacoco/html")
csv.required = false
xml.required = true
xml.outputLocation.set(file("${rootProject.buildDir}/report/jacoco/jacocoTestReport.xml"))
html.required = true
html.outputLocation.set(file("${rootProject.buildDir}/report/jacoco/html"))
}

jacocoClasspath = configurations.jacocoAnt
Expand Down
8 changes: 8 additions & 0 deletions gradle/scripts/lib/java-rpc-proto.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,14 @@ configure(projectsWithFlags('java')) {
// declaring an explicit or implicit dependency.
sourcesJarTask.dependsOn(task)
}

def copyAlpnAgentTask= tasks.findByName('copyAlpnAgent')
if (copyAlpnAgentTask) {
// A workaround for ':generateProto' uses this output of task ':copyAlpnAgent' without
// declaring an explicit or implicit dependency.
task.dependsOn(copyAlpnAgentTask)
}

project.ext.getGenerateSourcesTask().dependsOn(task)
}
}
Expand Down
7 changes: 7 additions & 0 deletions gradle/scripts/lib/java-rpc-thrift.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,13 @@ configure(projectsWithFlags('java')) {
compileTask.dependsOn(task)
}

def copyAlpnAgentTask= tasks.findByName('copyAlpnAgent')
if (copyAlpnAgentTask) {
// A workaround for ':generateProto' uses this output of task ':copyAlpnAgent' without
// declaring an explicit or implicit dependency.
task.dependsOn(copyAlpnAgentTask)
}

project.ext.getGenerateSourcesTask().dependsOn(task)
}
}
Expand Down
Loading

0 comments on commit e15c7b7

Please sign in to comment.