ci: reuse Linux native libraries across workflow runs

[sunchao]

Which issue does this PR close?

Part of #5830. Rebased onto main after #5973 merged, incorporating its main-only cache writes to reduce eviction pressure from PR and merge-queue runs. Complements #5841, which shares native builds within one workflow run.

Rationale for this change

Creating or updating a PR can send CI through an expensive native build even when the change only affects Spark-side code. The JVM tests need Comet's Rust library, libcomet.so, but the same native inputs may already have been compiled on main. Repeating that work delays the tests that depend on the library and consumes shared ASF runner capacity.

For example, suppose a PR changes a Scala planner rule, then updates a Scala test after review. Both runs need to compile and test the changed JVM code against the same native engine. With a matching library available from main, each run can download that library and proceed to testing.

In one sampled CI run, the native build step took about 23 minutes. This PR makes that step avoidable when the build inputs match an existing cache entry. The eventual improvement in total PR turnaround will also depend on queue time and the remaining builds and tests; it needs to be measured after main populates the cache.

What changes are included in this PR?

The Linux, Spark SQL, Iceberg, and manual writer workflows share a build-or-restore step for the finished native library. Before invoking Cargo, it computes a fingerprint of the native sources, protobufs, dependencies, build configuration, and observed toolchain environment. An exact cache match restores libcomet.so and skips native compilation. A miss restores available intermediate Cargo files and runs cargo build --locked --profile ci. The --locked flag deliberately fails if a manifest change requires updating native/Cargo.lock: contributors must include that lockfile update in the PR. Restoring intermediate files alone always leaves Cargo responsible for checking and rebuilding them.

Main produces the reusable libraries, and PRs consume them. Only pushes to main save these native-library and Cargo caches. Main's native job still invokes Cargo so that it also maintains the intermediate build files used when a PR needs to compile changed native code. Changes to shared native inputs trigger main's cache-building jobs. The fingerprint and main's routing share the same input lists and glob matcher, keeping the producer aligned with the cache key.

This makes the reuse boundary explicit. A Scala-only change can reuse main's library when the native inputs and environment match. A change to library Rust code or protobuf inputs needs a new build. If that Rust-changing PR later receives a Scala-only update, it still needs a native build: its unmerged Rust changes remain part of the fingerprint, and PR runs do not publish their own cache entries. The fingerprint describes the current checkout, including changes from earlier commits in the PR.

The fingerprint follows what the native build actually consumes. It includes the observed Rust, system-package, and JDK versions, plus the build environment: Cargo/Rust settings, C/C++ compiler and flag overrides (including target-specific variants), and the HDFS library overrides used by the default dependencies. The shared build and setup actions remain in the key; the four caller workflows do not. For example, moving a Spark suite between shards no longer discards a reusable native library. The compiler and JDK selected by those callers are observed directly instead.

This contract is scoped to the existing official Linux builder. An environment variable that points to an arbitrary external tool or file does not fingerprint that file's contents; introducing such inputs requires updating the contract. Documentation, generated outputs, and benchmarks are excluded from the library key. Disabled contrib crates contribute their manifests, which Cargo still resolves, but their Rust sources and standalone lockfiles do not invalidate this default-feature build. Rust checks and tests continue to run with a separate debug cache that includes benchmarks; downstream JVM tests use the restored or rebuilt library through the existing artifact flow.

The incremental fallback retains the build-environment identity too. Default HDFS support compiles C against JNI headers and links libjvm; Cargo does not fully track external compiler/header changes. Keeping that boundary can miss after unrelated package updates, but avoids carrying old native objects into a library published under a new environment key. The fallback path also uses the effective CARGO_HOME, fixing the mismatch between the old ~/.cargo cache paths and the container's /usr/local/cargo. This increases the incremental entry's size: it now contains the registry and Git checkouts as well as native/target. That growth is separate from adding the finished-library cache; the main-only write policy from #5973 is included in this PR's base. JVM workers that only consume the compiled library no longer restore an unused Cargo cache.

Precedents for this approach

ClickHouse uses a closely related approach: its CI combines build inputs and container/configuration digests and reuses artifacts from an earlier matching run. Microsoft's vcpkg binary cache similarly identifies reusable native packages from their build inputs, compilers, dependencies, and configuration. Bazel remote caching applies the same principle to individual build steps. This PR applies it to Comet's finished native library, with a fingerprint maintained alongside the existing Cargo workflows.

How are these changes tested?

Six focused regression tests pass. They check that native, dependency, toolchain, and configuration changes invalidate the appropriate keys; unrelated changes and disabled contrib sources preserve reuse; CI and debug caches remain separate; container checkout ownership is handled; and every tracked library-key input selects main's cache-building job. This includes contrib-manifest and nested-path regressions, build-environment overrides, and stability across caller-workflow edits. CI configuration checks, actionlint, Markdown formatting, and whitespace checks also pass.

The earlier key-helper revision was exercised in the public Rust container with a real JDK and a checkout owned by another user: repeated runs and generated/Spark-only changes preserved the key, while a protobuf edit changed it. The five original tests passed there; the current revision's six tests, including the expanded contrib and shared-matcher regressions, passed locally.

All selected hosted checks passed for the pre-rebase head 347fb073e. The rebased head passes the local checks above, including the cache-save scope check added by #5973; hosted CI for the rebased head is pending. On the first main push that populates the new namespaces, record the compressed sizes in bytes of both the finished-library and incremental Cargo entries from the cache-save logs or Actions cache API. Then observe an exact library hit that skips Cargo, confirm that downstream tests pass, and measure the effect on PR turnaround. Cache retention and actual reuse remain unverified until those main-produced entries exist.

[andygrove]

Thanks for this. Collapsing four copy-pasted cargo cache blocks into one composite action is a clear win on its own, and the security posture is right: only push-to-main saves and pull requests only consume, so a fork PR cannot plant a libcomet.so that another PR then executes. That is the property that matters most in a scheme like this and it is handled correctly. The lookup-only flag on main so the producer does not download a library it will not run is a nice touch, and fixing the ~/.cargo versus /usr/local/cargo CARGO_HOME mismatch is a real latent bug fix. The test file using throwaway git repositories rather than mocks is also good to see.

A few things I would like to work through before this lands.

Sequencing against #5973

I think this needs #5973 in front of it, and I would rather not merge it first.

I queried the cache API while reading this. The repository reports 9.06 GB in use, and refs/heads/main holds nothing at all. Everything live is on refs/pull/5420/merge, refs/pull/5615/merge, refs/pull/5565/merge and one gh-readonly-queue/main/pr-5934-* branch, and there is no Linux-cargo-ci-* or Linux-cargo-debug-* entry left anywhere. That independently reproduces what you documented on #5973 from 2026-09-15.

The concern is that this PR's whole premise is that main publishes the library and pull requests restore it, but a run can only restore from its own ref plus the default branch. With main holding nothing and 2.1 to 2.3 GB Maven entries still being written from PR refs, I would expect the new library entry to be evicted before any pull request gets to read it. This PR is well behaved on its own writes, so it cannot fix that from here. It also adds two fresh namespaces, Linux-cargo-ci-v3- and Linux-native-ci-v2-, which consume budget while delivering nothing until the eviction pressure is gone.

Would you be up for landing #5973 first and then rebasing this onto it? That would also give you a real hit rate to put in the description instead of the current 28m45s cold build.

The incremental restore prefix gets weaker than what it replaces

In cache_keys, the prefix is Linux-cargo-{profile}-v3-{digest([environment, dependencies])}-, and environment carries the full dpkg-query -W package list along with rustc -vV, java_release, java_home and cargo_home.

setup-builder runs apt-get update && apt-get install -y protobuf-compiler clang against amd64/rust, which is an unpinned rolling tag. So package versions can drift between main's producer run and a pull request run hours later with no repository change at all. When that happens we miss the binary key and the incremental prefix together and get a fully cold build. Today the fallback is just the Cargo.lock and Cargo.toml hash, so it would still restore.

Could the prefix stay coarse and keep packages in the binary key only? That keeps the exact-match safety where it matters without giving up the incremental fallback.

contrib/*/native/** in the binary key

native/Cargo.toml carries exclude = ["../contrib"] and pulls the contrib crates in only as optional path dependencies behind their features, and the CI build is cargo build --locked --profile ci with no contrib feature enabled.

Does a contrib/*/native/** change actually affect libcomet.so? If it cannot, having it in INPUT_PATTERNS means a Delta-only change invalidates the shared library key for every consumer.

The JDK entries reverse a documented invariant

This drops # Note: Java version intentionally excluded - Rust target is JDK-independent from the debug key, and environment_inputs now feeds java_home and java_release into both keys.

Was that comment wrong? If the JNI headers and libjvm genuinely are build inputs it would help to say so where the old note used to be, since this directly contradicts it. If the target really is JDK independent, leaving the JDK out would avoid invalidating everything on a toolcache patch bump. Worth noting java_home is a path carrying the exact version, so it moves on patch bumps too. Every caller passes java: 17 today, so nothing is fragmented across jobs right now, but that is the part I would not want to rely on silently.

Two glob dialects for one set of paths

native-cache-key.py matches with fnmatch.fnmatchcase, where * crosses / and ** carries no special meaning. compute-changes.py matches with glob_to_regex, where * becomes [^/]* and ** is recursive. The same path strings now appear in three places: INPUT_PATTERNS, the new inline list inside compute(), and the FILTERS loop.

Concretely, contrib/a/b/native/x.rs enters the binary key under the first dialect but does not warm main under the second. test_every_binary_key_input_has_a_main_cache_warmer only walks files present in the tree today plus three hardcoded names, so that kind of drift would not be caught.

Could the helper import the matcher from compute-changes.py so there is one dialect and ideally one list?

--locked is a behaviour change worth calling out

The build step goes from cargo build --profile ci to cargo build --locked --profile ci. A pull request that edits Cargo.toml without refreshing Cargo.lock now fails the build rather than updating the lock. That seems like the right call for something we are going to cache and reuse, but it is not mentioned in the description and it will surprise someone. Worth confirming it is deliberate and noting it there.

[sunchao]

Thanks, @andygrove, for the review. I pushed e3b9e8542 and updated the description. Going through the six points:

1. Sequencing: agreed. The description now explicitly depends on ci: write large actions/cache entries only on push to main #5973 landing first and calls for rebasing onto it before merging. ci: write large actions/cache entries only on push to main #5973 is still open, so that rebase and verification of main's cache retention remain pending. The description continues to distinguish the sampled cold-build cost from savings that still need measurement.

2. Incremental fallback: I kept the package/JDK compatibility boundary for the compiled target cache after checking the native dependencies. There is a concrete correctness issue with relying on Cargo alone here: in a small offline build using Comet's exact locked cc 1.4.5, changing an external C header from a value of 1 to 2 left the ordinary rebuild returning 1; cleaning the target produced 2. Replacing a compiler at the same path similarly left its old output cached until cleaning. The default hdfs-sys dependency compiles C against JDK headers, and those external inputs are not fully tracked by its build scripts. A coarse target restore could therefore publish old native objects under the new library fingerprint. I documented why the environment stays in the fallback prefix and extended the existing test to cover that boundary. Unrelated package updates can still cause misses; narrowing that identity needs evidence about the actual native toolchain inputs, or a pinned builder.

3. Contrib inputs: narrowed to contrib/*/native/Cargo.toml. Changes to disabled contrib Rust sources and their standalone lockfiles now preserve the key and no longer select the shared Linux cache warmer. The manifests stay included because Cargo resolves optional dependencies when validating the native workspace lockfile, even when those features are disabled.

4. JDK identity: the old JDK-independent comment was wrong for the default HDFS build. It uses JNI headers and links libjvm; core/build.rs already documents a stale cached JDK-path failure. I added that explanation beside the debug fingerprint and retained the JDK identity.

5. Glob dialects: the key helper now imports the existing matcher and shared native-input lists from compute-changes.py. Main's warmer uses that same library-input list. The matcher module itself is included in the fingerprint, and the existing tests cover nested contrib paths as well as the supported one-level manifests.

6. --locked: deliberate, and now explicit in both the description and workflow documentation. A manifest edit that requires a new native/Cargo.lock fails CI until the lockfile update is included.

The six focused tests, CI configuration checks, actionlint, Markdown formatting, and whitespace checks pass. The expanded contrib/matcher regressions failed before these changes and pass afterward. Hosted CI for this new head is pending; actual cross-run library reuse still needs verification after main populates the cache.

[andygrove]

Thanks for the revision. I re-read the whole thing at e3b9e8542 and checked out the branch to verify the answers rather than just reading the diff. Four of the six are clean as far as I can tell:

The glob dialects really are unified now. contrib/a/b/native/Cargo.toml gets the same answer from the fingerprint and from main's routing, and the tests pin both directions. The contrib narrowing is right, and the rationale holds up: native/Cargo.toml has exclude = ["../contrib"], native/core/Cargo.toml has default = ["hdfs-opendal"] with no contrib feature, and both comet-contrib-delta and comet-contrib-lance do appear in native/Cargo.lock, so --locked genuinely depends on those manifests. The JDK point is settled: hdfs-opendal is a default feature, it pulls hdfs-sys, and core/build.rs reads JAVA_HOME, so the old comment was simply wrong. And --locked is now called out in both places.

On the incremental prefix I was wrong and you were right. The stale C object argument is the correct one and I should have tested cc's header tracking before asserting the old fallback was strictly better. The paragraph you added explaining it lands exactly where I wanted it.

I also want to flag something structural that I did not say clearly enough the first time, because it is the lens for most of what follows. An exact binary key hit is the first cache in Comet's CI that skips compilation outright. Every cargo cache we have had until now was an incremental aid where Cargo still re-validated everything, so an incomplete key cost time and nothing else. Here an incomplete key produces a wrong library that the JVM suites then test against. That raises the bar on key completeness a long way, and it is worth the two of us being paranoid about it.

Five things from this pass.

The four caller workflows in the library key are the biggest source of churn

NATIVE_BUILD_INPUTS hashes pr_build_linux.yml, spark_sql_test_reusable.yml, iceberg_spark_test_reusable.yml and spark_sql_writer_tests.yml into the library key. I counted commits touching each library key input on main over the last 90 days:

input	commits
.github/workflows/pr_build_linux.yml	57
native/Cargo.lock	40
dev/ci/compute-changes.py	17
.github/workflows/spark_sql_test_reusable.yml	9
.github/workflows/iceberg_spark_test_reusable.yml	7
.github/workflows/spark_sql_writer_tests.yml	4
.github/actions/setup-builder/action.yaml	0

pr_build_linux.yml is the highest churn input in the entire key, ahead of Cargo.lock, at roughly one edit every 1.6 days. Each of those is a cold native build for the PR that makes the edit.

After this PR I do not think those files carry anything the key needs. The build recipe moved into .github/actions/build-native-ci, which is already in the key. The only native relevant content left in the callers is the container image, RUST_VERSION, the JDK version and RUSTFLAGS, and environment_inputs already observes all four directly through dpkg-query, rustc -vV, $JAVA_HOME/release and the step env. The observational probe is the stronger guard anyway, because it also catches a base image change that no file in the repository records.

spark_sql_writer_tests.yml is the clearest case. It is workflow_dispatch only, so a manual workflow that never runs on a PR currently invalidates the shared library for everybody.

Could NATIVE_BUILD_INPUTS keep .github/actions/setup-builder/** and NATIVE_CACHE_RECIPES and drop the four workflow paths?

environment_inputs allowlists three variables

It reads JAVA_HOME, CARGO_HOME and RUSTFLAGS. Everything else that reaches the compiler passes through unrecorded: CC, CXX, CFLAGS, PROTOC, RUSTC_WRAPPER, CARGO_BUILD_*, CARGO_PROFILE_CI_*.

I checked all four callers and none of them sets anything beyond RUST_VERSION, RUST_BACKTRACE and RUSTFLAGS, so there is no bug in this revision. What bothers me is that nothing in the new tests or in check-ci-config.py would notice a fourth variable appearing, and per the point above this is the one place where being wrong produces a stale library rather than a slow build.

Would a prefix sweep be safer than an allowlist? Something like

"env": {k: v for k, v in sorted(env.items())
        if k.startswith(("CARGO_", "RUST"))
        or k in {"CC", "CXX", "CFLAGS", "CXXFLAGS", "LDFLAGS", "AR", "PROTOC", "JAVA_HOME"}},

It picks up RUSTFLAGS, CARGO_HOME and JAVA_HOME for free, stays deterministic on the fixed builder, and means a future env: addition invalidates the key by default rather than by someone remembering to update this function. It matters more if you take the previous point, since dropping the caller workflows from the file list leaves this probe as the only guard.

RUSTFLAGS is written out twice in the composite

Lines 29 and 58 of .github/actions/build-native-ci/action.yaml carry the same literal, once as the env the key is computed under and once as the env the build runs under. Those two have to agree or the key describes a build that did not happen, and nothing fails if they drift.

Composite actions do not take a runs: level env:, but a leading step would do it:

- name: Pin native build flags
  shell: bash
  run: echo 'RUSTFLAGS=-Ctarget-cpu=x86-64-v3 -Clink-arg=-fuse-ld=bfd' >> "$GITHUB_ENV"

and then both step level env: blocks can go. That also makes the flags visible to the sweep above, if you take it.

The test file is routed to the merge queue suites

The _native_consumer loop appends dev/ci/test-native-cache-key.py alongside the recipes. I diffed the routing before and after: on merge_group, editing only that file now selects spark_4_1, spark_4_1_hive and iceberg_1_11, which are the three heaviest suites left on that tier after #5963.

The test cannot affect the library, the routing or the recipe, and Preflight already runs it on every event through the new "Check native cache keys" step, so those three suites are not checking anything. The comment above NATIVE_CACHE_RECIPES says tests "are routed separately below", but separately turns out to be the same nine jobs.

Dropping it from the loop leaves it covered by Preflight plus the existing dev/ci/** route into build_linux. I would keep dev/ci/compute-changes.py in the loop, since that one really is in the fingerprint.

The CARGO_HOME fix makes the entry bigger, not the same size

Worth saying explicitly in the description. ~/.cargo/registry does not exist in the amd64/rust container, so today's Linux-cargo-ci-* entry only ever held native/target. Pointed at /usr/local/cargo it will now also carry the registry for a 675 crate lockfile and the iceberg-rust git checkout.

I re-queried the cache API while reading this revision. The repository is now at 17.34 GB across 22 entries, up from the 9.06 GB I reported yesterday, refs/heads/main still holds nothing, and there is still no Linux-cargo-ci-* or Linux-cargo-debug-* entry anywhere. Everything live is on a PR merge ref or the merge queue, dominated by Linux-java-maven-* entries at 0.9 to 1.9 GB each. That is not an objection to the design, it just confirms the sequencing you already agreed to, and it means the new namespaces will be landing into a tighter budget than the description assumes.

Could the first main push after this lands report the measured size of both new entries?

Two smaller things

native-cache-key.py reaches for runpy.run_path and test-native-cache-key.py reaches for importlib.util.spec_from_file_location, for the same "this filename has a hyphen" problem, and then line 119 of the test goes back to runpy. Worth picking one so the next person copying either file gets a consistent answer.

digest and command have docstrings about as long as their bodies. The longer ones further down earn their keep, particularly the note on why the environment stays in the fallback prefix.

One question about shape

Since #5973 is still open and this is waiting on it, is there a case for landing the composite on its own first? The extraction, the CARGO_HOME fix and deleting the read-write Linux-cargo-registry-* cache from the linux-test matrix all stand alone, and that last one only ever ran in a job with skip-native-build: true that never calls cargo, so it is pure budget back at a moment when we are short of budget. The composite could carry the existing hashFiles key at first and take the fingerprint in a follow-up once main is actually retaining entries.

Happy to be told the double churn on the composite's key is not worth it.

---

For what it is worth, on the things I could check locally the fingerprint looks complete. The six new tests pass, check-ci-config.py and the Iceberg shard tests still pass, and on the real tree 297 of the 399 tracked files under native/ land in the CI key with the other 102 being exactly the 6 markdown files and the 96 under benches/. There is no include_str! or include_bytes! anywhere in native/, so excluding markdown is safe, and there is no [patch] section or path dependency outside native/ and contrib/. I walked the six step conditions in the composite by hand, including the case where steps.cargo-cache is skipped and cache-hit evaluates to the empty string, and did not find a hole.

[sunchao]

Thanks, Andy. Addressed this pass in 347fb07 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.