Add 7.4.2 gradle version in tests

[rougsig]

Background:
To add support for version catalogs, build caches, incremental build and other new gradle features - project need a newer version of the gradle.

Changes:
Run tests also with 7.4.2 gradle version and 7.2.1 agp version. To be sure that project works as expected on the 7.4.2 gradle version of gradle

Test plan:
Green pipelines, production code was not changed.

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[ejona86]

The s/compile/implementation/ changes look really easy to accept. The AGP version bump is going to be rockier; it may make sense to separate into separate PRs so the easy thing isn't blocked on the hard thing.

[ejona86]

I expect the AGP upgrade is failing because of #540

[rougsig]

Extracted the compile/implementation migration to #567.

[rougsig]

JDK 11 is required for AGP 7.0

Is it possible to run all tests with java 11? Or is it necessary to run old tests with java 8, and for AGP 7.0 with java 11?

[ejona86]

Hmm... Java 8 no longer has Premier Support from Oracle. But new Gradle versions still support it. We could try to see whether animalsniffer can work for groovy. If it does, then we could use Java 11 JDK to build and test while also being reasonably confident things still work with Java 8.

[ejona86]

I just tried, and the animalsniffer plugin isn't triggering, even though it claims to support Groovy. Unclear why.

I expected this to fail:

diff --git a/build.gradle b/build.gradle
index 6c16702..86cab8f 100644
--- a/build.gradle
+++ b/build.gradle
@@ -7,6 +7,7 @@ buildscript {
         classpath 'com.jfrog.bintray.gradle:gradle-bintray-plugin:1.6'
         classpath "com.gradle.publish:plugin-publish-plugin:0.11.0"
         classpath "com.github.ben-manes:gradle-versions-plugin:0.12.0"
+        classpath "ru.vyarus:gradle-animalsniffer-plugin:1.5.4"
     }
 }
 
@@ -33,6 +34,7 @@ apply plugin: 'signing'
 apply plugin: 'com.jfrog.bintray'
 apply plugin: 'com.gradle.plugin-publish'
 apply plugin: 'com.github.ben-manes.versions'
+apply plugin: 'ru.vyarus.animalsniffer'
 
 group = 'com.google.protobuf'
 version = '0.8.19-SNAPSHOT'
@@ -65,6 +67,8 @@ dependencies {
     exclude module: 'groovy-all'
   }
   testImplementation 'commons-io:commons-io:2.5'
+
+  signature "org.codehaus.mojo.signature:java17:1.0@signature"
 }
 
 test.inputs.files fileTree("$projectDir/testProject")
diff --git a/src/main/groovy/com/google/protobuf/gradle/GenerateProtoTask.groovy b/src/main/groovy/com/google/protobuf/gradle/GenerateProtoTask.groovy
index cbf51b5..27afc44 100644
--- a/src/main/groovy/com/google/protobuf/gradle/GenerateProtoTask.groovy
+++ b/src/main/groovy/com/google/protobuf/gradle/GenerateProtoTask.groovy
@@ -111,6 +111,7 @@ public abstract class GenerateProtoTask extends DefaultTask {
     if (Utils.isAndroidProject(project)) {
       return isTestVariant
     }
+    System.out.println("" + java.util.Collections.emptyNavigableMap());
     return Utils.isTest(sourceSet.name)
   }
 

[rougsig]

Interestingly, I would try to build the project and run the test with JDK 11 in a separate PR.

[rougsig]

@ejona86 I took some time to understand how animalsniffer do checks. It simply checks the compiled class bytecode for undefined references in older versions of Java.

In our case, the checks don't work, thanks to the @CompileDynamic annotation. @CompileDynamic does not save strict types, it replaces all strict types with groovy magic at runtime.

Details under the spoiler.

Details

Test project sources

I compiled the same class three times:

1. with @CompileDynamic
2. with @CompileStatic
3. without any annotation

@CompileDynamic behaves the same as without annotations.

Compiled class sources:

class Default {
  def foo() {
    // From Path.of JavaDoc
    // @since 11
    System.out.println(Path.of("Hello World"));
  }
}

Bytecode compiled from a class with @CompileDynamic (the only interesting part), we can't find invokestatic on Path.

public java.lang.Object foo();
    descriptor: ()Ljava/lang/Object;
    flags: (0x0001) ACC_PUBLIC
    Code:
      stack=5, locals=2, args_size=1
         0: nop
         1: invokestatic  #20                 // Method $getCallSiteArray:()[Lorg/codehaus/groovy/runtime/callsite/CallSite;
         4: astore_1
         5: aload_1
         6: ldc           #33                 // int 0
         8: aaload
         9: aload_1
        10: ldc           #34                 // int 1
        12: aaload
        13: ldc           #36                 // class java/lang/System
        15: invokeinterface #42,  2           // InterfaceMethod org/codehaus/groovy/runtime/callsite/CallSite.callGetProperty:(Ljava/lang/Object;)Ljava/lang/Object;
        20: aload_1
        21: ldc           #43                 // int 2
        23: aaload
        24: ldc           #45                 // class java/nio/file/Path
        26: ldc           #47                 // String Hello World
        28: invokeinterface #51,  3           // InterfaceMethod org/codehaus/groovy/runtime/callsite/CallSite.call:(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;
        33: invokeinterface #51,  3           // InterfaceMethod org/codehaus/groovy/runtime/callsite/CallSite.call:(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;
        38: areturn

Bytecode compiled from class with @CompileStatic (only interesting part), we can find invokestatic on Path (9#44).

public java.lang.Object foo();
    descriptor: ()Ljava/lang/Object;
    flags: (0x0001) ACC_PUBLIC
    Code:
      stack=3, locals=1, args_size=1
         0: getstatic     #34                 // Field java/lang/System.out:Ljava/io/PrintStream;
         3: ldc           #36                 // String Hello World
         5: iconst_0
         6: anewarray     #38                 // class java/lang/String
         9: invokestatic  #44                 // InterfaceMethod java/nio/file/Path.of:(Ljava/lang/String;[Ljava/lang/String;)Ljava/nio/file/Path;
        12: invokestatic  #50                 // Method org/codehaus/groovy/runtime/DefaultGroovyMethods.println:(Ljava/io/PrintStream;Ljava/lang/Object;)V
        15: aconst_null
        16: areturn

As result animalsniffer throw only one exception for class compiled with @CompileStatic.

com.example.Static:11  Undefined reference: java.nio.file.Path java.nio.file.Path.of(String, String[])

---

About compiling important things with java 8. Since all important classes have the @CompileDynamic annotation, we can compile any code in any version of Java. No exceptions in compilation phase, all exceptions will be in runtime.

Some ways can help us to be confident in Java 8 support.

a. We can run test two times:

1. all tests which can be launched on java 8
2. only java 11 tests

b. Mark all groovy files with @CompileStatic.
c. Rewrite project to java or kotlin.

[rougsig]

I think the best solution is mark all groovy files with @CompileStatic. It's will add extra compiler type checks, and allow animalsniffer do work.

[ejona86]

I thought I had started a comment, but I can't find it now. CompileStatic sounds fair. Java rewrite wouldn't be unacceptable, but obviously is a much larger change.

I started poking at this and most classes look easy. Although the interesting classes are a bit harder. I have a feeling we are probably using CompileDynamic some places to avoid depending on Android Gradle Plugin. We'll probably keep the specific methods doing something like that as dynamic.

[rougsig]

Locally ProtobufKotlinDslPluginTest runs in 15 minutes. Have any ideas why the pipeline might get stuck?

[ejona86]

Locally ProtobufKotlinDslPluginTest runs in 15 minutes. Do you know why the pipeline might get stuck?

No, I saw it got stuck yesterday so restarted it. An hour later it was still running which surprised me. That's when I noticed the first run had been cancelled after 6 hours!

Best hope I'd have is some memory limit needs to be increased. But that's really just shooting in the dark.

[rougsig]

Best hope I'd have is some memory limit needs to be increased. But that's really just shooting in the dark.

You're right.

Daemon will be stopped at the end of the build after running out of JVM memory

[rougsig]

Best hope I'd have is some memory limit needs to be increased. But that's really just shooting in the dark.

I turn on some logs and the pipeline stuck in the dex task, and only for new agp version.
The new version of agp does not support setting dexOptions. It ignores this parameter.
https://scans.gradle.com/s/kpiwwpehwgjpy/performance/build#memory-g-1-old-gen

DSL element 'dexOptions' is obsolete and should be removed.
It will be removed in version 8.0 of the Android Gradle plugin.
Using it has no effect, and the AndroidGradle plugin optimizes dexing automatically.

Set memory limit for whole android gradle runner. Its working.

---

I think we need to improve the test project configuration. Add customization options for each version of gradle/agp/kotlin. Like, use dexOptions only for agp < 7. In agp 8 dexOptions will be removed, but agp 5 still need it.

[ejona86]

In agp 8 dexOptions will be removed, but agp 5 still need it.

Let's just ignore the issue for AGP 7 for now.

I hoped we might end up dropping support for some AGP versions. For example, if a version doesn't support new enough API levels for the Google Play store that's a trivial thing to drop. But it looks like 3.x is still usable today. Maybe once we deal with AGP 8 we can drop at least one version.

[rougsig]

Its not final, do not merge.
Unnecessary commits have been squashed.

[ejona86]

I'm working on a patch release now. After that this could be merged, since this will be a new minor release.