dwsmith1983 commented on PR #5365: URL: https://github.com/apache/datafusion-comet/pull/5365#issuecomment-5618418055
> point layer 1 at `cargo tree -p datafusion-comet` with no flag, and reword the header and the `.github/workflows/delta_build_gate.yml` comment to the invariant that actually holds Done. Layer 1 now runs `cargo tree -p datafusion-comet` as the primary check and `--no-default-features` as a second case through the same helper, each with the anti-vacuous root-crate guard. The script header, the section headers and the workflow comment now state the invariant the checks prove: the kernel-backed `comet-contrib-delta` crate stays out of every shipped build, while the default-on `delta` feature is deliberately in. > Pinning the default-on surface near the 82 KB you measured would make that layer say something the grep cannot drift away from. The symbol layer now asserts three things on the default library: zero contrib/kernel symbols, at least one symbol from the `delta` feature's own modules (`delta_dv`, `delta_scan`, `delta_spark_scan`), and, where `nm -S` reports sizes, their total below a cap. In a Linux debug build that footprint is 84 KB across 372 symbols, 63 KB of it the decoder in `delta_dv` and the rest the planner arm, so the cap is 512 KB: room for toolchain drift, none for a kernel. Mach-O `nm` reports every size as zero, so on macOS the layer checks presence and skips the cap; CI runs on Linux where it applies. A default build that dropped the feature fails the presence check, and one that pulled something kernel-sized in through the feature fails the cap, which is what the old grep could not say. The size comparison between the default and contrib libraries is gone from the same section. On an unstripped debug library the contrib code is a few hundred kilobytes against 1.4 GB, so layout noise decided that assertion, and it has failed on changes that never touched the contrib crate. The sizes are still printed, and `nm` is now required rather than silently skipped, since the symbol table is the only evidence the section relies on. > it should go in the `Cargo.toml` comment next to the #5411 pointer Added to the `delta` feature comment: `roaring` and `crc32fast` are already in the default tree through iceberg and the shuffle crate, so the feature adds no crate and only promotes two transitive dependencies to direct ones. `dev/verify-contrib-delta-gate.sh` passes end to end on this head on macOS, and the symbol functions run against a Linux debug library report 0 contrib symbols and the 84 KB footprint under the cap. -- 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]
