Modernize build #678

ZacSweers · 2023-01-13T20:56:16Z

Highlights

Gradle 7.6
Switch to using version catalogs
Modernize gradle build bits using things like pluginManagement, dependencyResolutionManagement, etc.
Use Java 11 for integration and samples
- Necessary because of Dagger using the Java 11+ Generated annotation even though the release target is 8. Figure this is more or less ok but lmk if you want to do something different.
Compile/target SDK 33
Use plugins {} syntax in gradle
Update Actions CI for new override pattern. Overrides are set from env vars in settings.gradle files.
Update maven publishing plugin
Testing previews requires the anvil.allowSnapshots gradle property.

build.gradle

gradle/wrapper/gradle-wrapper.properties

.github/workflows/ci.yml

gradle/libs.versions.toml

JoelWilcox · 2023-01-17T18:18:55Z

gradle-plugin/settings.gradle

+  versionCatalogs {
+    libs {
+      // TODO remove this once the dirs aren't symlinked anymore


Does this mean remove the commented out from(files line, the if (System.getenv("DEP_OVERRIDES") == "true") block, or something else? Could use a little more explicitness on the comment.

Also, are we not able to remove the symlinks now?

ah no it means uncomment it and use that instead of symlinks. Wanted to reduce churn in this PR though, but I can give it a try if you want

Why do the symlinks need to be removed? I don't want to manage the gradle wrapper twice.

With version catalogs you could actually start using Renovate and let it handle updates like that for you. Gradle in general really doesn't like symlinks. I can leave it in place and remove the comment if you want though

build.gradle

settings.gradle

integration-tests/library/build.gradle

gradle-plugin/build.gradle

JoelWilcox · 2023-01-17T18:38:29Z

gradle-plugin/src/main/java/com/squareup/anvil/plugin/AnvilPlugin.kt

@@ -136,7 +136,6 @@ internal open class AnvilPlugin : KotlinCompilerPluginSupportPlugin {
    if (variant.variantFilter.syncGeneratedSources) {
      val isIdeSyncProvider = project.providers
        .systemProperty("idea.sync.active")
-        .forUseAtConfigurationTime()


Does this not need to be replaced by a new api call?

Nope, this API was deprecated and there is no replacement necessary. Gradle implemented a different solution for tracking these because frankly this API sucked

This change has a side-effect. You now require all consuming projects to use the latest Gradle version. That's why it stayed there for so long.

It was deprecated in Gradle 7.4 though, which is quite old now. Does that seem ok?

If it's deprecated and there's not equivalent replacement or workaround from Gradle then removing it seems reasonable. But we should verify the new behavior and add a note in the changelog if that new restriction exists now.

There's no replacement necessary. Little bit of background - to support configuration cache, Gradle wanted to capture used system properties as a part of the cache key. Initially, they made this work by requiring any properties read at configuration-time to use forUseAtConfigurationTime() to make that explicit to gradle. This was super awkward and tedious for users though, so gradle deprecated this API and instead now tracks this automatically via internal mechanism rather than needing explicit opt-in like that API.

vRallev

This PR is much bigger than I anticipated. I'm really not a fan of mixing the migration to a .toml file with upgrading AGP and Gradle. This should have been a separate PR for each. Now, if all these commits are squashed it's not possible to revert one or the other anymore. Would it be possible to separate these things? I'm almost afraid to say that almost every bullet point from the PR message should have been a separate PR. At the moment I can't tell if something will break, there are just too many changes at once.

Other than it's unclear to me how the overrides now work. I asked this with a comment inline as well.

build.gradle

vRallev · 2023-01-17T20:41:44Z

build.gradle

-        //
-        // Caused by: groovy.lang.MissingPropertyException: Could not set unknown property 'jvmTarget' for object of type org.jetbrains.kotlin.gradle.tasks.internal.KotlinMultiplatformCommonOptionsCompat.
-        jvmTarget = JavaVersion.VERSION_1_8
+  tasks.withType(KotlinCompile).configureEach {


I hate imports in build.gradle files. Often enough when the import is not needed anymore it stays around. I liked the old way better.

wait does spotless not clear unused imports in your build files?

I guess this project doesn't have spotless enabled for groovy files

build.gradle

gradle/libs.versions.toml

vRallev · 2023-01-17T21:01:22Z

integration-tests/mpp/android-module/gradle.properties

@@ -0,0 +1 @@
+kotlin.mpp.androidSourceSetLayoutVersion=2


gradle/libs.versions.toml

.github/workflows/ci.yml

Co-authored-by: Ralf Wondratschek <r.wondr@gmail.com>

buildSrc/settings.gradle

ZacSweers · 2023-01-31T22:04:11Z

Updated per our chat in kotlin-lang, now it uses system props and you can override by using the -Doverride_{toml version}=<version> syntax

./gradlew tasks -Doverride_kotlin=1.7.22

This reverts commit 5c2aff3.

JoelWilcox · 2023-02-03T22:22:38Z

gradle/libs.versions.toml

+# false, then we run each test with one annotation instead of all options. We also skip tests
+# that run the Dagger annotation processor (KAPT is slow).
+config-fullTestRun = "true"
+config-generateDaggerFactoriesWithAnvil = "true"


It looks like there are two more references to the old property for this that need to be updated, in compare_size.sh

JoelWilcox

:bless:

vRallev · 2023-02-04T01:54:45Z

CHANGELOG.md

@@ -6,6 +6,9 @@

 ### Changed

+- Raise minimum AGP version to 7.1.0.
+- The Kotlin Gradle Plugin (both the core plugin and the API artifact) are no longer a dependency of the Anvil Gradle Plugin. Instead, it's now a `compileOnly` dependency, allowing the plugin to defer to whatever version the user already has. If you were accidentally depending on KGP through Anvil, you'll need to explicitly add the plugin yourself now.
+


vRallev · 2023-02-04T01:59:49Z

buildSrc/src/main/kotlin/com/squareup/anvil/benchmark/CreateBenchmarkProjectTask.kt

-      |  api deps.dagger2.dagger
-      |  kapt deps.dagger2.compiler
+      |  api libs.dagger2
+      |  kapt libs.dagger2.compiler


I'd still like to change this.

vRallev · 2023-02-04T02:00:51Z

compiler-utils/build.gradle

+plugins {
+  alias(libs.plugins.kotlin.jvm)
+  alias(libs.plugins.mavenPublish)
+  id 'java-test-fixtures'


In general with Gradle, this is painful to read 😅

vRallev · 2023-02-04T02:02:40Z

compiler/generate_build_properties.gradle

+
+// So Gradle stops complaining about the missing dependency
+tasks.named("runKtlintCheckOverTestSourceSet").configure {
+  it.dependsOn generateBuildProperties


I believe a mustRunAfter is good enough here. The benefit is that it doesn't make the other task run unnecessarily, but still teaches Gradle about the ordering.

ZacSweers added 30 commits January 10, 2023 16:15

Update AGP to 7.0

323a2b7

Use lazy configureEach

979eb0a

Update from deprecated with to using

59b874c

Gradle 7.6

c8740d4

Add version catalog

127b511

Use version catalogs in buildSrc

6d523cf

Use version catalogs in gradle-plugin

6d7c581

Use version catalogs in root build files

4a8feec

Use version catalogs in generate_build_properties.gradle

3bbf7d5

Modernize remaining subprojects

2728b55

Align multiplatform source sets with new kotlin 1.8.0 structure

8cf69b0

Merge branch 'main' into z/modernizeBuild

36145bb

Update sample target/compile SDKs

33487b1

Fix source set name

e8f45a6

Don't impose any specific gradle plugins

868d161

Modernize another

0e1859f

Bump AGP to 7.1.0

27c6702

Add missing decl

358dbca

Update a couple deprecations

68a4e65

Add missing exported=true

942a491

Avoid a gradle warning about missing task deps

e584697

Put more in ext for prev scripts

2d36a04

Update espresso to avoid missing exported activities issue

32334cb

Make mpp play nice with jvm targets

c9ea7fd

Remove dependencies.gradle

448bf1d

Update CI

a155931

Update github actions

1b006d3

Fix env location

4cd0813

Missing configs

d485efc

Update benchmark gen

4ca160f

ZacSweers added 3 commits January 13, 2023 16:13

Update maven publishing

f0390c5

Use imports

9c8d2b0

Add missing group + remove redundant repository

0092d58

ZacSweers changed the title ~~Modernize build (WIP)~~ Modernize build Jan 13, 2023

ZacSweers marked this pull request as ready for review January 13, 2023 22:22

JoelWilcox reviewed Jan 17, 2023

View reviewed changes

vRallev reviewed Jan 17, 2023

View reviewed changes

ZacSweers and others added 6 commits January 25, 2023 21:09

Restore fullTestRun doc

9000a06

Restore allWarningsAsErrors TODOs

d1cb249

Add KGP dep change to changelog

655a45e

startsWith

aa8f871

Co-authored-by: Ralf Wondratschek <r.wondr@gmail.com>

Use import

cb2bb29

Add min AGP version bump to changelog

310f051

efemoney reviewed Jan 31, 2023

View reviewed changes

buildSrc/settings.gradle Outdated Show resolved Hide resolved

Switch to using just system props

2a353f2

ZacSweers added 4 commits February 3, 2023 17:04

Remove anvil from toml

d4a3ea3

Gradle 7.5.1

5c2aff3

Add override example

e3091bc

Revert "Gradle 7.5.1"

045f870

This reverts commit 5c2aff3.

JoelWilcox reviewed Feb 3, 2023

View reviewed changes

ZacSweers added 3 commits February 3, 2023 17:24

Update compare_size.sh

ddb4e36

Merge branch 'main' into z/modernizeBuild

d24fdf5

Fix CI

1a00582

JoelWilcox approved these changes Feb 3, 2023

View reviewed changes

vRallev approved these changes Feb 4, 2023

View reviewed changes

vRallev merged commit 66e3155 into square:main Feb 4, 2023

JoelWilcox mentioned this pull request Feb 8, 2023

Modernize AGP integration #674

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modernize build #678

Modernize build #678

ZacSweers commented Jan 13, 2023 •

edited

JoelWilcox Jan 17, 2023

ZacSweers Jan 17, 2023

vRallev Jan 17, 2023

ZacSweers Jan 26, 2023

JoelWilcox Jan 17, 2023

ZacSweers Jan 17, 2023

vRallev Jan 17, 2023

ZacSweers Jan 26, 2023

JoelWilcox Jan 30, 2023

ZacSweers Jan 30, 2023

vRallev left a comment

vRallev Jan 17, 2023

ZacSweers Jan 26, 2023

ZacSweers Jan 26, 2023

vRallev Jan 17, 2023

ZacSweers commented Jan 31, 2023

JoelWilcox Feb 3, 2023

JoelWilcox left a comment

vRallev Feb 4, 2023

vRallev Feb 4, 2023

vRallev Feb 4, 2023

vRallev Feb 4, 2023

		@@ -0,0 +1 @@
		kotlin.mpp.androidSourceSetLayoutVersion=2

Modernize build #678

Modernize build #678

Conversation

ZacSweers commented Jan 13, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vRallev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ZacSweers commented Jan 31, 2023

Choose a reason for hiding this comment

JoelWilcox left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ZacSweers commented Jan 13, 2023 •

edited