Gradle: eliminate some more doFirst/doLast script references (#6516)

Gradle: eliminate some more `doFist` + `doLast` script references

buildSrc/src/main/kotlin/Testing.kt

@@ -81,12 +81,9 @@ class NessieTestingPlugin : Plugin<Project> {
       }
       if (plugins.hasPlugin("io.quarkus")) {
         // This directory somehow disappears... Maybe some weird Quarkus code.
-        tasks.named("quarkusGenerateCodeTests") {
-          doFirst { buildDir.resolve("resources/testFixtures").mkdirs() }
-        }
-        tasks.withType<Test>().configureEach {
-          doFirst { buildDir.resolve("resources/testFixtures").mkdirs() }
-        }
+        val testFixturesDir = buildDir.resolve("resources/testFixtures")
+        tasks.named("quarkusGenerateCodeTests") { doFirst { testFixturesDir.mkdirs() } }
+        tasks.withType<Test>().configureEach { doFirst { testFixturesDir.mkdirs() } }
       }
 
       @Suppress("UnstableApiUsage") apply<JvmTestSuitePlugin>()

buildSrc/src/main/kotlin/Utilities.kt

@@ -19,13 +19,16 @@ import java.io.File
 import java.io.FileInputStream
 import java.lang.IllegalStateException
 import java.util.Properties
+import org.gradle.api.Action
 import org.gradle.api.DefaultTask
 import org.gradle.api.JavaVersion
 import org.gradle.api.Project
+import org.gradle.api.Task
 import org.gradle.api.artifacts.Dependency
 import org.gradle.api.artifacts.ExternalModuleDependency
 import org.gradle.api.artifacts.ModuleDependency
 import org.gradle.api.artifacts.VersionCatalogsExtension
+import org.gradle.api.file.FileTree
 import org.gradle.api.file.RegularFileProperty
 import org.gradle.api.plugins.JavaPluginExtension
 import org.gradle.api.tasks.InputFile
@@ -296,3 +299,24 @@ abstract class UnixExecutableTask : DefaultTask() {
     exec.setExecutable(true)
   }
 }
+
+class ReplaceInFiles(val files: FileTree, val replacements: Map<String, String>) : Action<Task> {
+  override fun execute(task: Task) {
+    files.forEach { f ->
+      val src = f.readText()
+      var txt = src
+      for (e in replacements.entries) {
+        txt = txt.replace(e.key, e.value)
+      }
+      if (txt != src) {
+        f.writeText(txt)
+      }
+    }
+  }
+}
+
+class WriteFile(val file: File, val text: String) : Action<Task> {
+  override fun execute(task: Task) {
+    file.writeText(text)
+  }
+}

gc/gc-iceberg-files/build.gradle.kts

@@ -78,9 +78,9 @@ dependencies {
 
 tasks.withType(Test::class.java).configureEach {
   systemProperty("aws.region", "us-east-1")
+  val tmpdir = project.buildDir.resolve("tmpdir")
   jvmArgumentProviders.add(
     CommandLineArgumentProvider {
-      val tmpdir = project.buildDir.resolve("tmpdir")
       tmpdir.mkdirs()
       listOf("-Djava.io.tmpdir=$tmpdir")
     }

gc/gc-iceberg-inttest/build.gradle.kts

@@ -116,9 +116,9 @@ forceJava11ForTests()
 
 tasks.withType(Test::class.java).configureEach {
   systemProperty("aws.region", "us-east-1")
+  val tmpdir = project.buildDir.resolve("tmpdir")
   jvmArgumentProviders.add(
     CommandLineArgumentProvider {
-      val tmpdir = project.buildDir.resolve("tmpdir")
       tmpdir.mkdirs()
       listOf("-Djava.io.tmpdir=$tmpdir")
     }

integrations/spark-antlr-grammar/build.gradle.kts

@@ -37,21 +37,17 @@ configurations.api.get().setExtendsFrom(listOf())
 
 tasks.named<AntlrTask>("generateGrammarSource") {
   arguments.add("-visitor")
-  doLast {
-    val dir = project.buildDir.resolve("generated-src/antlr/main")
-    fileTree(dir)
-      .matching { include("**/*.java") }
-      .forEach { f ->
-        f.writeText(
-          "package org.apache.spark.sql.catalyst.parser.extensions;\n" +
-            f.readText()
-              .replace(
-                "import org.antlr.v4.runtime.",
-                "import org.projectnessie.shaded.org.antlr.v4.runtime."
-              )
-        )
-      }
-  }
+  doLast(
+    ReplaceInFiles(
+      fileTree(project.buildDir.resolve("generated-src/antlr/main")).matching {
+        include("**/*.java")
+      },
+      mapOf(
+        "import org.antlr.v4.runtime." to "import org.projectnessie.shaded.org.antlr.v4.runtime.",
+        "// PACKAGE_PLACEHOLDER" to "package org.apache.spark.sql.catalyst.parser.extensions;"
+      )
+    )
+  )
 }
 
 tasks.withType<Checkstyle>().configureEach {

integrations/spark-antlr-grammar/src/main/antlr4/org/apache/spark/sql/catalyst/parser/extensions/NessieSqlExtensions.g4

@@ -15,6 +15,10 @@
  */
 grammar NessieSqlExtensions;
 
+@header {
+// PACKAGE_PLACEHOLDER
+}
+
 @lexer::members {
   /**
    * Verify whether current token is a valid decimal token (which contains dot).

servers/store-proto/build.gradle.kts

@@ -42,13 +42,12 @@ extensions.configure<ProtobufExtension> {
 }
 
 tasks.named<GenerateProtoTask>("generateProto") {
-  doLast {
-    fileTree("$buildDir/generated/source/proto/main").forEach {
-      it.writeText(
-        it.readText().replace("com.google.protobuf", "org.projectnessie.nessie.relocated.protobuf")
-      )
-    }
-  }
+  doLast(
+    ReplaceInFiles(
+      fileTree(project.buildDir.resolve("generated/source/proto/main")),
+      mapOf("com.google.protobuf" to "org.projectnessie.nessie.relocated.protobuf")
+    )
+  )
 }
 
 reflectionConfig {

ui/build.gradle.kts

@@ -15,11 +15,11 @@
  */
 
 import com.diffplug.gradle.spotless.SpotlessTask
-import com.github.gradle.node.npm.task.NpmInstallTask
 import com.github.gradle.node.npm.task.NpmSetupTask
 import com.github.gradle.node.npm.task.NpmTask
 import com.github.gradle.node.task.NodeSetupTask
 import org.apache.tools.ant.taskdefs.condition.Os
+import org.gradle.api.internal.file.FileOperations
 
 plugins {
   // The ':nessie-ui' module is technically not a Java library, but declaring it as such provides
@@ -53,14 +53,20 @@ val testCoverageDir = project.projectDir.resolve("coverage")
 val nodeModulesDir = project.projectDir.resolve("node_modules")
 val shadowPackageJson = project.buildDir.resolve("packageJson")
 
+val fs = properties.get("fileOperations") as FileOperations
+
+class DeleteFiles(private val fs: FileOperations, private val files: Any) : Action<Task> {
+  override fun execute(task: Task) {
+    fs.delete(files)
+  }
+}
+
 val clean =
   tasks.named("clean") {
-    doFirst {
-      delete(nodeModulesDir)
-      delete(project.projectDir.resolve("tsconfig.tsbuildinfo"))
-      delete(testCoverageDir)
-      delete(generatedOpenApiCode)
-    }
+    doFirst(DeleteFiles(fs, nodeModulesDir))
+    doFirst(DeleteFiles(fs, project.projectDir.resolve("tsconfig.tsbuildinfo")))
+    doFirst(DeleteFiles(fs, testCoverageDir))
+    doFirst(DeleteFiles(fs, generatedOpenApiCode))
   }
 
 val nodeSetup =
@@ -77,30 +83,43 @@ val npmSetup =
     logging.captureStandardError(LogLevel.LIFECYCLE)
   }
 
-val npmInstallReal =
-  tasks.named<NpmInstallTask>("npmInstall") {
-    doFirst {
-      // Delete the node_modules dir so it stays consistent
-      delete(nodeModulesDir)
-    }
+class NpmInstallCacheIf(val dotGradleDir: File, val npmVersion: Property<String>) : Spec<Task> {
+  override fun isSatisfiedBy(element: Task?): Boolean {
+    // Do not let the UI module assume that npm is already setup (#4461)
+    //
+    // ... which can happen when using multiple Git worktrees, when one of those
+    // did the NPM-setup dance, letting the "other" Git worktree "think", that
+    // it did already the NPM-setup dance, too.
+    return dotGradleDir
+      .resolve("npm/npm-v${npmVersion}/lib/node_modules/npm/node_modules")
+      .isDirectory
+  }
+}
+
+class NpmInstallActionSync(private val target: File) : Action<SyncSpec> {
+  override fun execute(sync: SyncSpec) {
+    sync.from(".")
+    sync.include("package*.json")
+    sync.into(target)
+  }
+}
+
+class NpmInstallSync(private val fileOperations: FileOperations, private val target: File) :
+  Action<Task> {
+  override fun execute(t: Task) {
+    fileOperations.sync(NpmInstallActionSync(target))
   }
+}
 
 // The node_modules directory is huge (> 500M), so checking the contents takes a couple of seconds.
 // To mitigate the effect, this script uses a trick by using the package*.json files and only
 // execute
 // the "real" npmInstall, when those are our of sync.
 val npmInstall =
-  tasks.register("npmInstallFacade") {
+  tasks.named("npmInstall") {
     mustRunAfter(clean)
     dependsOn(npmSetup)
-    outputs.cacheIf {
-      // Do not let the UI module assume that npm is already setup (#4461)
-      //
-      // ... which can happen when using multiple Git worktrees, when one of those
-      // did the NPM-setup dance, letting the "other" Git worktree "think", that
-      // it did already the NPM-setup dance, too.
-      dotGradle.resolve("npm/npm-v${node.npmVersion}/lib/node_modules/npm/node_modules").isDirectory
-    }
+    outputs.cacheIf(NpmInstallCacheIf(dotGradle, node.npmVersion))
     // Need to add the fully qualified path here, so a "cached" npmInstallFacade run from another
     // directory with Nessie doesn't let "this" code-tree "think" that it has one.
     inputs.property("node.modules.dir", project.projectDir)
@@ -110,14 +129,9 @@ val npmInstall =
     outputs.dir(shadowPackageJson)
     logging.captureStandardOutput(LogLevel.INFO)
     logging.captureStandardError(LogLevel.LIFECYCLE)
-    doFirst {
-      sync {
-        from(".")
-        include("package*.json")
-        into(shadowPackageJson)
-      }
-      npmInstallReal.get().exec()
-    }
+    // Delete the node_modules dir so it stays consistent
+    doFirst(NpmInstallSync(fs, shadowPackageJson))
+    doFirst(DeleteFiles(fs, nodeModulesDir))
   }
 
 val openapiGenerator by configurations.creating
@@ -144,19 +158,18 @@ val npmGenerateAPI =
       "src/generated/utils/api",
       "--additional-properties=supportsES6=true"
     )
-    doFirst {
-      // Remove previously generated code to have generated files consistent with the OpenAPI spec
-      delete(generatedOpenApiCode)
-    }
-    doLast {
-      // openapi-generator produces Line 264 in runtime.ts as
-      //    export type FetchAPI = GlobalFetch['fetch'];
-      // but must be
-      //    export type FetchAPI = WindowOrWorkerGlobalScope['fetch'];
-      val f = generatedOpenApiCode.resolve("utils/api/runtime.ts")
-      val src = f.readText()
-      f.writeText(src.replace("GlobalFetch", "WindowOrWorkerGlobalScope"))
-    }
+    // Remove previously generated code to have generated files consistent with the OpenAPI spec
+    doFirst(DeleteFiles(fs, generatedOpenApiCode))
+    // openapi-generator produces Line 264 in runtime.ts as
+    //    export type FetchAPI = GlobalFetch['fetch'];
+    // but must be
+    //    export type FetchAPI = WindowOrWorkerGlobalScope['fetch'];
+    doLast(
+      ReplaceInFiles(
+        fileTree(generatedOpenApiCode.resolve("utils/api/runtime.ts")),
+        mapOf("GlobalFetch" to "WindowOrWorkerGlobalScope")
+      )
+    )
   }
 
 /*
@@ -183,10 +196,8 @@ val npmBuild =
       .files(fileTree(".") { include("*.json", "*.js") })
       .withPathSensitivity(PathSensitivity.RELATIVE)
     outputs.dir(npmBuildDir)
-    doFirst {
-      // Remove all previously generated output
-      delete(npmBuildTarget)
-    }
+    // Remove all previously generated output
+    doFirst(DeleteFiles(fs, npmBuildTarget))
     args.set(listOf("run", "build"))
     environment.put("GENERATE_SOURCEMAP", "false")
     environment.put("DISABLE_ESLINT_PLUGIN", "false")
@@ -205,7 +216,7 @@ val npmTest =
     inputs.dir(generatedOpenApiCode).withPathSensitivity(PathSensitivity.RELATIVE)
     inputs.dir(npmBuildDir).withPathSensitivity(PathSensitivity.RELATIVE)
     outputs.dir(testCoverageDir)
-    doFirst { delete(testCoverageDir) }
+    doFirst(DeleteFiles(fs, testCoverageDir))
     npmCommand.add("test")
     args.addAll("--", "--coverage")
     usesService(
@@ -224,8 +235,8 @@ val npmLint =
       .withPathSensitivity(PathSensitivity.RELATIVE)
     val lintedMarker = project.buildDir.resolve("npmLintRun")
     outputs.file(lintedMarker)
-    doFirst { delete(lintedMarker) }
-    doLast { lintedMarker.writeText("linted") }
+    doFirst(DeleteFiles(fs, lintedMarker))
+    doLast(WriteFile(lintedMarker, "linted"))
     args.set(listOf("run", "lint"))
   }
 

versioned/persist/serialize-proto/build.gradle.kts

@@ -42,13 +42,12 @@ extensions.configure<ProtobufExtension> {
 }
 
 tasks.named<GenerateProtoTask>("generateProto") {
-  doLast {
-    fileTree("$buildDir/generated/source/proto/main").forEach {
-      it.writeText(
-        it.readText().replace("com.google.protobuf", "org.projectnessie.nessie.relocated.protobuf")
-      )
-    }
-  }
+  doLast(
+    ReplaceInFiles(
+      fileTree(project.buildDir.resolve("generated/source/proto/main")),
+      mapOf("com.google.protobuf" to "org.projectnessie.nessie.relocated.protobuf")
+    )
+  )
 }
 
 reflectionConfig {

versioned/storage/common-proto/build.gradle.kts

@@ -44,13 +44,12 @@ extensions.configure<ProtobufExtension> {
 }
 
 tasks.named<GenerateProtoTask>("generateProto") {
-  doLast {
-    fileTree("$buildDir/generated/source/proto/main").forEach {
-      it.writeText(
-        it.readText().replace("com.google.protobuf", "org.projectnessie.nessie.relocated.protobuf")
-      )
-    }
-  }
+  doLast(
+    ReplaceInFiles(
+      fileTree(project.buildDir.resolve("generated/source/proto/main")),
+      mapOf("com.google.protobuf" to "org.projectnessie.nessie.relocated.protobuf")
+    )
+  )
 }
 
 reflectionConfig {

versioned/transfer-proto/build.gradle.kts

@@ -16,7 +16,6 @@
 
 import com.google.protobuf.gradle.GenerateProtoTask
 import com.google.protobuf.gradle.ProtobufExtension
-import com.google.protobuf.gradle.ProtobufExtract
 
 plugins {
   `java-library`
@@ -40,13 +39,12 @@ extensions.configure<ProtobufExtension> {
 }
 
 tasks.named<GenerateProtoTask>("generateProto") {
-  doLast {
-    fileTree("$buildDir/generated/source/proto/main").forEach {
-      it.writeText(
-        it.readText().replace("com.google.protobuf", "org.projectnessie.nessie.relocated.protobuf")
-      )
-    }
-  }
+  doLast(
+    ReplaceInFiles(
+      fileTree(project.buildDir.resolve("generated/source/proto/main")),
+      mapOf("com.google.protobuf" to "org.projectnessie.nessie.relocated.protobuf")
+    )
+  )
 }
 
 reflectionConfig {