This is an automated email from the ASF dual-hosted git repository.
sunchao pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/datafusion-comet.git
The following commit(s) were added to refs/heads/main by this push:
new 392da2ca7a ci: gate the Delta contrib build on symbols, not on
libcomet size (#5827)
392da2ca7a is described below
commit 392da2ca7a99fd223644eff650406f61580eb46b
Author: Andy Grove <[email protected]>
AuthorDate: Wed Sep 9 21:05:19 2026 -0600
ci: gate the Delta contrib build on symbols, not on libcomet size (#5827)
The build gate asserted that the `--features contrib-delta` libcomet is
strictly larger than the default one, as a proxy for "contrib did not get
linked into the default build". The proxy has no signal:
`comet-contrib-delta` is a 75-line stub, and it is being weighed against a
~1.5 GB unstripped debug cdylib whose byte count moves by about a megabyte
for source changes that have nothing to do with Delta, because rustc
re-emits DWARF per codegen unit and a small edit repartitions them. The two
builds also move independently, since the second `cargo build` only
recompiles `datafusion-comet` and relinks.
On #5810 the entire native diff against its base commit is a 20-line
`sort_unstable_by` in the Iceberg writer. The default lib grew 875 KB and
the contrib-enabled lib shrank 396 KB, inverting a +1.2 MB gap and failing
the gate. Three runs across two unrelated branches have hit it, and the
result is deterministic per commit, so re-running does not clear it.
Report the sizes instead of asserting an ordering. The invariant is already
measured directly a few lines away -- the default libcomet must carry zero
Delta symbols, and the contrib-enabled one at least one, which is what keeps
the first check from going vacuous if mangling drifts -- so no coverage is
lost.
Also make a missing `nm` fail rather than silently skip. Both symbol checks
were wrapped in `if command -v nm`, which left the size comparison as the
only enforcement on an image without it, and would have left nothing at all
once that comparison went.
Closes #5826
Co-authored-by: test <[email protected]>
---
dev/verify-contrib-delta-gate.sh | 73 +++++++++++++++++++++++-----------------
1 file changed, 42 insertions(+), 31 deletions(-)
diff --git a/dev/verify-contrib-delta-gate.sh b/dev/verify-contrib-delta-gate.sh
index 3ce3a63ae0..62b0fe4a26 100755
--- a/dev/verify-contrib-delta-gate.sh
+++ b/dev/verify-contrib-delta-gate.sh
@@ -24,9 +24,9 @@
# doesn't pull `delta_kernel` into the dependency tree.
# 2. Maven: default `mvn ... package` doesn't compile any
# `org/apache/comet/contrib/` classes and doesn't pull `io.delta:*` deps.
-# 3. Symbol/size: the resulting `libcomet` (`.so` on Linux, `.dylib` on
macOS) from the default build is
-# meaningfully smaller than the contrib-enabled build, and carries no
-# `comet_contrib_delta`/`delta_kernel`/etc. external symbols.
+# 3. Symbols: the resulting `libcomet` (`.so` on Linux, `.dylib` on macOS)
from the default
+# build carries no `comet_contrib_delta`/`delta_kernel`/etc. symbols, and
the
+# contrib-enabled build carries some (so the pattern is known to still
match).
#
# Exit non-zero on the first failure. Designed to be wired into CI so a future
# change that leaks Delta into core gets caught immediately.
@@ -225,9 +225,9 @@ if [[ -n "$SERVICE_LEAKS" ]]; then
fi
green "OK: default build registers no contrib services (empty ServiceLoader
registries at runtime)"
-# ---- libcomet symbol/size gate -------------------------------------------
+# ---- libcomet symbol gate -------------------------------------------------
-hdr "libcomet: default build is smaller and has no Delta symbols"
+hdr "libcomet: default build has no Delta symbols"
cd "$NATIVE_DIR"
# The cdylib extension is platform-specific: `libcomet.so` on Linux (CI),
`libcomet.dylib` on
# macOS. Find whichever the build produced; `stat`/`nm` flags also differ
across the two.
@@ -251,6 +251,17 @@ delta_syms() {
nm "$1" 2>/dev/null | grep -ciE
'comet_contrib_delta|delta_kernel|deltadvfilter|deltasynthetic' || true
}
+# `nm` is the only direct measurement this section makes, so a missing `nm`
has to fail rather
+# than silently skip -- same anti-vacuous discipline as the cargo-tree and
effective-pom guards
+# above. Every image the gate runs on ships one (CI's `amd64/rust` has
binutils; macOS has
+# Xcode's). Previously a missing `nm` degraded both symbol checks to no-ops
and left the size
+# comparison below as the only enforcement, which is backwards: the size
number is the weak
+# signal and the symbols are the strong one.
+if ! command -v nm >/dev/null 2>&1; then
+ red "FAIL: 'nm' not found on PATH; refusing to conclude 'no Delta symbols'
vacuously"
+ exit 1
+fi
+
cargo clean -p comet-contrib-delta -p datafusion-comet >/dev/null 2>&1 || true
cargo build -j 4 -p datafusion-comet >/dev/null 2>&1
LIB_DEFAULT="$(comet_lib)"
@@ -259,11 +270,7 @@ if [[ -z "$LIB_DEFAULT" ]]; then
exit 1
fi
SIZE_DEFAULT="$(lib_size "$LIB_DEFAULT")"
-if command -v nm >/dev/null 2>&1; then
- EXT_SYMS="$(delta_syms "$LIB_DEFAULT")"
-else
- EXT_SYMS=0
-fi
+EXT_SYMS="$(delta_syms "$LIB_DEFAULT")"
if [[ "$EXT_SYMS" -ne 0 ]]; then
red "FAIL: default libcomet contains $EXT_SYMS Delta-related symbols"
exit 1
@@ -273,34 +280,38 @@ green "OK: default libcomet has 0 Delta symbols
(size=$SIZE_DEFAULT bytes)"
cargo build -j 4 -p datafusion-comet --features contrib-delta >/dev/null 2>&1
LIB_CONTRIB="$(comet_lib)"
SIZE_CONTRIB="$(lib_size "$LIB_CONTRIB")"
-if [[ "$SIZE_CONTRIB" -le "$SIZE_DEFAULT" ]]; then
- red "FAIL: contrib-enabled libcomet (size=$SIZE_CONTRIB) is not larger than
default (size=$SIZE_DEFAULT)"
- red " (would indicate contrib was being linked into default build too)"
+# The contrib-enabled libcomet MUST contain Delta-related symbols. Without
this, a future Rust
+# toolchain that mangles symbols differently (so our grep pattern stops
matching) would silently
+# make the default-build check a no-op while still passing -- the gate would
lie about being
+# enforced. Asserting both "default has 0" AND "contrib has >0" catches
grep-pattern drift.
+CONTRIB_SYMS="$(delta_syms "$LIB_CONTRIB")"
+if [[ "$CONTRIB_SYMS" -lt 1 ]]; then
+ red "FAIL: contrib-enabled libcomet has 0 Delta-related symbols matching our
grep pattern."
+ red " This means the symbol-name pattern in this script has drifted
from what"
+ red " Rust currently emits, and the default-build check above is now a
no-op."
+ red " Inspect the dylib's exports and update the grep pattern."
exit 1
fi
-# Sanity check: the contrib-enabled libcomet MUST contain Delta-related
symbols.
-# Without this, a future Rust toolchain that mangles symbols differently (so
our
-# grep pattern stops matching) would silently make the default-build check a
no-op
-# while still passing -- the gate would lie about being enforced. Asserting
both
-# "default has 0" AND "contrib has >0" catches grep-pattern drift.
-if command -v nm >/dev/null 2>&1; then
- CONTRIB_SYMS="$(delta_syms "$LIB_CONTRIB")"
- if [[ "$CONTRIB_SYMS" -lt 1 ]]; then
- red "FAIL: contrib-enabled libcomet has 0 Delta-related symbols matching
our grep pattern."
- red " This means the symbol-name pattern in this script has drifted
from what"
- red " Rust currently emits, and the default-build check above is now
a no-op."
- red " Inspect the dylib's exports and update the grep pattern."
- exit 1
- fi
-fi
-DIFF_MB=$(( (SIZE_CONTRIB - SIZE_DEFAULT) / 1024 / 1024 ))
-green "OK: contrib-enabled libcomet is ${DIFF_MB} MB larger than default
(size=$SIZE_CONTRIB bytes)"
+# Sizes are REPORTED, not asserted. This gate used to require the
contrib-enabled lib to be
+# strictly larger than the default one, on the theory that a default build
which had quietly
+# linked contrib would show no size gap. That does not survive contact with a
1.5 GB unstripped
+# debug cdylib: `comet-contrib-delta` is a ~75-line stub, so what it actually
adds is swamped by
+# how rustc happens to partition the crate into codegen units -- DWARF is
re-emitted per unit, so
+# the total moves by ~1 MB in response to source changes that have nothing to
do with Delta, and
+# the two builds move independently because only `datafusion-comet` is
recompiled for the second
+# one. Measured on apache/datafusion-comet#5810, whose entire native diff is a
20-line
+# `sort_unstable_by` in the Iceberg writer: against its base commit the
default lib grew 875 KB
+# while the contrib-enabled lib shrank 396 KB, inverting a gap that had been
+1.2 MB one commit
+# earlier and failing the gate. The two symbol checks are the direct
measurement of the property
+# we care about -- "the default build links zero Delta symbols" is asserted,
not inferred from a
+# byte count -- so nothing is lost by printing the sizes and moving on.
+green "OK: contrib-enabled libcomet has $CONTRIB_SYMS Delta symbols
(size=$SIZE_CONTRIB bytes, $((SIZE_CONTRIB - SIZE_DEFAULT)) bytes vs default)"
# ---- Summary --------------------------------------------------------------
hdr "All gate checks passed"
echo " default cargo: no comet-contrib-delta, no delta_kernel"
echo " default mvn: no io.delta:*, no contrib/delta classes"
-echo " default dylib: ${DIFF_MB} MB smaller than contrib build, 0 Delta
symbols"
+echo " default dylib: 0 Delta symbols (contrib build has $CONTRIB_SYMS)"
echo
echo "Run with: dev/verify-contrib-delta-gate.sh"
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
