sunchao commented on PR #5976: URL: https://github.com/apache/datafusion-comet/pull/5976#issuecomment-5701822218
Thanks, Andy. Addressed this pass in 347fb073e and updated the description and workflow documentation. **Caller workflows and environment.** The four caller files are out of the fingerprint. The shared setup/build actions remain included, and the observed environment now captures Cargo/Rust controls, compiler/linker/protobuf overrides, target-qualified compiler variables, and the HDFS controls used by our existing dependencies. For example, a shard edit preserves the key, while `CARGO_PROFILE_CI_OPT_LEVEL`, `CC_x86_64_unknown_linux_gnu`, or `HDFS_LIB_DIR` changes invalidate it. The documentation keeps the scope explicit: this describes our official builder; recording a path to an arbitrary external tool or library does not identify its contents. One small clarification on the earlier behavior: a caller edit invalidated the finished-library key, but it preserved the incremental restore prefix when dependencies and environment were unchanged, so compilation was required without necessarily being cold. **Flags and routing.** `RUSTFLAGS` is now defined once through `GITHUB_ENV`, before fingerprinting and compilation. The test file is removed from the explicit consumer loop. It still runs in Preflight and retains existing Linux routing, but test-only edits no longer select the extra Spark/Iceberg suites on the merge queue or nightly tier. The actual cache recipes remain in that loop. **Cache size and sequencing.** The description now explicitly says that fixing `CARGO_HOME` grows the incremental entry by adding registry/Git contents, separately from the new finished-library entry. The first-main validation calls for reporting both compressed cache sizes from the save logs or cache API, then observing an exact library hit and passing downstream tests. Those measurements are still pending main publication. #5973 remains an explicit prerequisite, followed by rebasing this PR before merge. I would keep this PR together for now: the extraction and reuse behavior share one recipe, and splitting it would add another transition without removing the cache-retention prerequisite. **Smaller cleanup.** Both files now use the same `importlib` loading idiom, the test reuses the already-loaded matcher, and the two short helper docstrings are removed. The longer contract explanations remain. Validation: the existing six cache-key tests pass with expanded caller/environment/routing coverage; all 15 Iceberg shard tests, CI configuration and suite checks, benchmark-runner checks, actionlint, Markdown formatting, and whitespace checks pass. Independent review found no further issues. Hosted CI for this new commit is pending; the previous head's selected checks all passed. -- 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]
