sunchao commented on PR #5976: URL: https://github.com/apache/datafusion-comet/pull/5976#issuecomment-5718728247
Following up on [your latest review](https://github.com/apache/datafusion-comet/pull/5976#pullrequestreview-5239088335). Thanks, Andy. I addressed the two connected concerns in [80296cb7b](https://github.com/apache/datafusion-comet/pull/5976/commits/80296cb7bcddcd539fbf80e6cdeae67ba64ab877) and revised the description around the post-#5973 baseline. Both CI and debug caches now contain only `native/target`; I removed the registry/git paths and the unused `cargo-home` output. The effective Cargo home still participates in the environment fingerprint. I checked the newer [#5991 native job](https://github.com/apache/datafusion-comet/actions/runs/35248078842/job/105294466457): it restored the target archive, fetched registry/git sources, and rebuilt only six Comet workspace crates. A separate disposable Cargo build with registry and Git dependencies also kept all compiled dependencies fresh after their source directories were deleted and fetched again. The description now uses that job's **3m51s** and the [nightly Linux job's **4m13s**](https://github.com/apache/datafusion-comet/actions/runs/35189906693/job/105101195047), including **43–47s restoring the target cache** and **1m57s–2m22s in Cargo**. Both were fallback restores with workspace recompilation, so I explicitly distinguish them from the exact-library-hit path. The historical 23-minute cold build is no longer presented as the expected saving. Actual library-hit and end-to-end savings remain to be measured after main publishes an entry. For migration, I agree with deliberate deletion: the description calls for inventorying and deleting the superseded legacy Cargo cache IDs on `refs/heads/main` at merge time, preserving the new namespaces and unrelated caches. The first main build will populate the new caches from cold. This avoids keeping both large generations resident, with the explicit tradeoff that PRs using old keys lose their warm cache and need to rebase. No live caches were deleted as part of this update. For #5841, this PR reuses a library across runs; #5841 shares the producer within a run. Together, one producer restores or builds and then distributes the library. There is no functional ordering requirement, but the planned integration is to land this PR first, then rebase #5841 and use this composite action in its shared producer, preserving main-only writes. Ordinary unlabeled PRs already have one covered producer, so #5841 mainly removes duplicates in queue/nightly/manual runs. The six fingerprint tests, action-flow tests, CI configuration checks, actionlint, Markdown formatting and whitespace checks pass locally. Hosted CI for this new head is pending; the description keeps that separate from the earlier green head and the still-unmeasured library-hit path. -- 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]
