Copilot commented on code in PR #53621:
URL: https://github.com/apache/spark/pull/53621#discussion_r2648093988
##########
project/SparkBuild.scala:
##########
@@ -1247,6 +1263,11 @@ object ExcludeShims {
classPathFilter.value((Test / internalDependencyClasspath).value),
Test / internalDependencyAsJars :=
classPathFilter.value((Test / internalDependencyAsJars).value),
+ // Filter bloop's internal classpath for correct IDE integration
+ Compile / BloopKeys.bloopInternalClasspath :=
+ bloopClasspathFilter.value((Compile /
BloopKeys.bloopInternalClasspath).value),
Review Comment:
The bloop classpath filtering is missing for the Runtime configuration. The
existing code filters Runtime / internalDependencyClasspath and Runtime /
internalDependencyAsJars (lines 1258-1261), but there's no corresponding
Runtime / BloopKeys.bloopInternalClasspath filtering. This inconsistency means
the Runtime configuration won't have the same shims exclusion in bloop as it
does in the regular classpath, which could cause IDE integration issues.
Consider adding Runtime / BloopKeys.bloopInternalClasspath filtering to
maintain consistency.
```suggestion
bloopClasspathFilter.value((Compile /
BloopKeys.bloopInternalClasspath).value),
Runtime / BloopKeys.bloopInternalClasspath :=
bloopClasspathFilter.value((Runtime /
BloopKeys.bloopInternalClasspath).value),
```
##########
project/SparkBuild.scala:
##########
@@ -1235,6 +1242,15 @@ object ExcludeShims {
identity _
}
},
+ bloopClasspathFilter := {
+ if (!shimmedProjects(moduleName.value)) {
+ cp => cp.filterNot { case (f1, f2) =>
+ f1.getPath.contains("connect-shims") ||
f2.getPath.contains("connect-shims")
Review Comment:
The filtering logic for bloop classpath is inconsistent with the existing
classPathFilter. The classPathFilter checks if the file name contains
"spark-connect-shims" (line 1240), but the bloopClasspathFilter checks if the
path contains "connect-shims" (line 1248). This inconsistency could lead to
different filtering behavior between regular classpath and bloop classpath.
Consider using "spark-connect-shims" in the bloopClasspathFilter to match the
existing logic.
```suggestion
f1.getPath.contains("spark-connect-shims") ||
f2.getPath.contains("spark-connect-shims")
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
