Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Apr 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 3 additions & 6 deletions buildSrc/src/main/kotlin/Testing.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Such changes elimniate the use of script references, which do not work with Gradle's configuration cache (not serializable)

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>()
Expand Down
24 changes: 24 additions & 0 deletions buildSrc/src/main/kotlin/Utilities.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
}
2 changes: 1 addition & 1 deletion gc/gc-iceberg-files/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,9 @@ dependencies {

tasks.withType(Test::class.java).configureEach {
systemProperty("aws.region", "us-east-1")
val tmpdir = project.buildDir.resolve("tmpdir")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moves the reference to project out of task execution, which is mandatory to enable Gradle's configuration cache.

jvmArgumentProviders.add(
CommandLineArgumentProvider {
val tmpdir = project.buildDir.resolve("tmpdir")
tmpdir.mkdirs()
listOf("-Djava.io.tmpdir=$tmpdir")
}
Expand Down
2 changes: 1 addition & 1 deletion gc/gc-iceberg-inttest/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
26 changes: 11 additions & 15 deletions integrations/spark-antlr-grammar/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@
*/
grammar NessieSqlExtensions;

@header {
// PACKAGE_PLACEHOLDER
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes the ReplaceInFiles idempotent

}

@lexer::members {
/**
* Verify whether current token is a valid decimal token (which contains dot).
Expand Down
13 changes: 6 additions & 7 deletions servers/store-proto/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
111 changes: 61 additions & 50 deletions ui/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 =
Expand All @@ -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)
Expand All @@ -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
Expand All @@ -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")
)
)
}

/*
Expand All @@ -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")
Expand All @@ -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(
Expand All @@ -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"))
}

Expand Down
13 changes: 6 additions & 7 deletions versioned/persist/serialize-proto/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
13 changes: 6 additions & 7 deletions versioned/storage/common-proto/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
14 changes: 6 additions & 8 deletions versioned/transfer-proto/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand All @@ -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 {
Expand Down