gemini-code-assist[bot] commented on code in PR #38233:
URL: https://github.com/apache/beam/pull/38233#discussion_r3101928956
##########
runners/spark/spark_runner.gradle:
##########
@@ -89,38 +89,115 @@ def hadoopVersions = [
hadoopVersions.each { kv -> configurations.create("hadoopVersion$kv.key") }
-def sourceBase = "${project.projectDir}/../src"
-def sourceBaseCopy = "${project.buildDir}/sourcebase/src"
-
-def useCopiedSourceSet = { scope, type, trigger ->
- def taskName = "copy${scope.capitalize()}${type.capitalize()}"
- trigger.dependsOn tasks.register(taskName, Copy) {
- from "$sourceBase/$scope/$type"
- into "$sourceBaseCopy/$scope/$type"
- duplicatesStrategy DuplicatesStrategy.INCLUDE
+/*
+ * Per-version source overrides (mirrors runners/flink/flink_runner.gradle).
+ *
+ * Layout:
+ * runners/spark/src/ -- shared base (lowest supported
version uses these directly)
+ * runners/spark/<major>/src/ -- version-specific overrides (later
overrides win)
+ *
+ * The lowest supported `spark_major` builds straight from the shared base.
+ * Higher versions copy <shared> + <previous majors> + <current> into a single
+ * source-overrides directory using DuplicatesStrategy.INCLUDE so the current
+ * version's files override earlier ones.
+ */
+def base_path = ".."
+
+def overrides = { versions, type, group = 'java' ->
+ // order matters: later entries override earlier ones during the Copy
+ ["${base_path}/src/${type}/${group}"] +
+ versions.collect { "${base_path}/${it}/src/${type}/${group}" } +
+ ["./src/${type}/${group}"]
+}
+
+def all_versions = spark_versions.split(",").collect { it.trim() }
+// Determine version order by list position rather than string comparison so
two-digit
+// majors (e.g. "10") still sort after single-digit ones.
+def spark_major_index = all_versions.indexOf(spark_major)
+if (spark_major_index < 0) {
+ throw new GradleException(
+ "spark_major='${spark_major}' is not listed in
spark_versions='${spark_versions}' " +
+ "(see root gradle.properties).")
+}
+def previous_versions = spark_major_index > 0 ? all_versions.subList(0,
spark_major_index) : []
+
+def main_source_overrides = overrides(previous_versions, "main")
+def test_source_overrides = overrides(previous_versions, "test")
+def main_resources_overrides = overrides(previous_versions, "main",
"resources")
+def test_resources_overrides = overrides(previous_versions, "test",
"resources")
+
+def sourceOverridesBase =
project.layout.buildDirectory.dir('source-overrides/src').get()
+
+def copySourceOverrides = tasks.register('copySourceOverrides', Copy) {
copyTask ->
+ copyTask.from main_source_overrides
+ copyTask.into "${sourceOverridesBase}/main/java"
+ copyTask.duplicatesStrategy DuplicatesStrategy.INCLUDE
+ if (project.ext.has('excluded_files') &&
project.ext.excluded_files.containsKey('main')) {
+ project.ext.excluded_files.main.each { f -> copyTask.exclude "**/${f}" }
+ }
+}
+
+def copyResourcesOverrides = tasks.register('copyResourcesOverrides', Copy) {
+ it.from main_resources_overrides
+ it.into "${sourceOverridesBase}/main/resources"
+ it.duplicatesStrategy DuplicatesStrategy.INCLUDE
+}
+
+def copyTestSourceOverrides = tasks.register('copyTestSourceOverrides', Copy)
{ copyTask ->
+ copyTask.from test_source_overrides
+ copyTask.into "${sourceOverridesBase}/test/java"
+ copyTask.duplicatesStrategy DuplicatesStrategy.INCLUDE
+ if (project.ext.has('excluded_files') &&
project.ext.excluded_files.containsKey('test')) {
+ project.ext.excluded_files.test.each { f -> copyTask.exclude "**/${f}" }
}
- // append copied sources to srcDirs
- sourceSets."$scope"."$type".srcDirs "$sourceBaseCopy/$scope/$type"
}
-if (copySourceBase) {
- // Copy source base into build directory.
- // While this is not necessary, having multiple source sets referencing the
same shared base will typically confuse an IDE and harm developer experience.
- // The copySourceBase flag can be swapped without any implications and
allows to pick a main version that is actively worked on.
- useCopiedSourceSet("main", "java", compileJava)
- useCopiedSourceSet("main", "resources", processResources)
- useCopiedSourceSet("test", "java", compileTestJava)
- useCopiedSourceSet("test", "resources", processTestResources)
+def copyTestResourcesOverrides = tasks.register('copyTestResourcesOverrides',
Copy) {
+ it.from test_resources_overrides
+ it.into "${sourceOverridesBase}/test/resources"
+ it.duplicatesStrategy DuplicatesStrategy.INCLUDE
+}
+
+def use_override = (spark_major_index > 0)
+def sourceBase = "${project.projectDir}/../src"
+
+if (use_override) {
+ compileJava.dependsOn copySourceOverrides
+ processResources.dependsOn copyResourcesOverrides
+ compileTestJava.dependsOn copyTestSourceOverrides
+ processTestResources.dependsOn copyTestResourcesOverrides
+
+ def sourcesJar = project.tasks.findByName('sourcesJar')
+ if (sourcesJar != null) {
+ sourcesJar.dependsOn copySourceOverrides
+ sourcesJar.dependsOn copyResourcesOverrides
+ }
+ def testSourcesJar = project.tasks.findByName('testSourcesJar')
+ if (testSourcesJar != null) {
+ testSourcesJar.dependsOn copyTestSourceOverrides
+ testSourcesJar.dependsOn copyTestResourcesOverrides
+ }
+ // Pin srcDirs explicitly so each higher version sees only its merged
overrides tree.
+ sourceSets {
+ main {
+ java { srcDirs = ["${sourceOverridesBase}/main/java"] }
+ resources { srcDirs = ["${sourceOverridesBase}/main/resources"] }
+ }
+ test {
+ java { srcDirs = ["${sourceOverridesBase}/test/java"] }
+ resources { srcDirs = ["${sourceOverridesBase}/test/resources"] }
+ }
+ }
Review Comment:
Instead of manually adding `dependsOn` to a specific list of tasks (which
can be incomplete, e.g., missing `javadoc`), you can use the `TaskProvider`
directly in the `srcDirs` configuration. Gradle will automatically establish
the necessary task dependencies for any task that consumes these source sets
(including compilation, javadoc, and jar tasks). This is more idiomatic and
robust.
```
// Pin srcDirs explicitly so each higher version sees only its merged
overrides tree.
sourceSets {
main {
java { srcDirs = [copySourceOverrides] }
resources { srcDirs = [copyResourcesOverrides] }
}
test {
java { srcDirs = [copyTestSourceOverrides] }
resources { srcDirs = [copyTestResourcesOverrides] }
}
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
