Copilot commented on code in PR #12737:
URL: https://github.com/apache/gluten/pull/12737#discussion_r3774110610
##########
dev/check-arrow-c-shading.sh:
##########
@@ -76,12 +81,29 @@ for cls in "${CLASSES[@]}"; do
fi
done
+# Second check: no org.apache.arrow.c.* class may *call* a shaded Arrow class.
+# Signatures alone miss org.apache.arrow.util.Preconditions & friends, which
are
+# invoked from constructors but never appear in a descriptor.
+mkdir -p "$WORKDIR/all"
+unzip -qo "$JAR" 'org/apache/arrow/c/*' -d "$WORKDIR/all" 2>/dev/null || true
+if compgen -G "$WORKDIR/all/org/apache/arrow/c/*.class" > /dev/null; then
+ refs=$(grep -rahoE
"org/apache/gluten/shaded/org/apache/arrow/[a-zA-Z0-9/$]+" \
Review Comment:
The regex used to detect shaded Arrow references can miss valid JVM internal
names because Java identifiers may contain underscores (`_`). This can create
false negatives in the new constant-pool scan. Expand the character class to
include `_` so shaded references like `.../Some_Class` are caught reliably.
##########
gluten-arrow/pom.xml:
##########
@@ -89,13 +89,23 @@
<groupId>org.apache.arrow</groupId>
<artifactId>${arrow-memory.artifact}</artifactId>
<version>${arrow.version}</version>
- <scope>runtime</scope>
+ <scope>${arrow.deps.scope}</scope>
Review Comment:
This changes `${arrow-memory.artifact}` from `runtime` to `compile` whenever
`${arrow.deps.scope}` is `compile` (Spark 3.x), increasing the transitive
*compile* surface area for downstream modules. If the allocator implementation
artifact is only required at runtime (as it was before), consider splitting
scope control (e.g., a dedicated property defaulting to `runtime` and
overridden to `provided` for Spark 4.x) so Spark 3.x keeps the narrower
runtime-only exposure while Spark 4.x still avoids shipping it.
##########
dev/check-arrow-c-shading.sh:
##########
@@ -76,12 +81,29 @@ for cls in "${CLASSES[@]}"; do
fi
done
+# Second check: no org.apache.arrow.c.* class may *call* a shaded Arrow class.
+# Signatures alone miss org.apache.arrow.util.Preconditions & friends, which
are
+# invoked from constructors but never appear in a descriptor.
+mkdir -p "$WORKDIR/all"
+unzip -qo "$JAR" 'org/apache/arrow/c/*' -d "$WORKDIR/all" 2>/dev/null || true
+if compgen -G "$WORKDIR/all/org/apache/arrow/c/*.class" > /dev/null; then
+ refs=$(grep -rahoE
"org/apache/gluten/shaded/org/apache/arrow/[a-zA-Z0-9/$]+" \
Review Comment:
The shaded-prefix (`org/apache/gluten/shaded`) is hard-coded in the
detection pattern. Since the actual shade prefix is driven by Maven
(`${gluten.shade.packageName}` in `package/pom.xml`), consider centralizing
this in the script as a single variable (or allow overriding via env/arg) to
avoid future drift between build config and validation logic.
--
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]
