From 245eb564506fb9dfcafcb9d4fb6fdb30de95d702 Mon Sep 17 00:00:00 2001 From: Chao Sun Date: Wed, 16 Sep 2026 04:10:36 +0000 Subject: [PATCH 1/8] ci: reuse Linux native libraries by validated build inputs --- .github/actions/build-native-ci/action.yaml | 109 ++++++ .github/workflows/README.md | 50 +++ .github/workflows/ci.yml | 6 + .../workflows/iceberg_spark_test_reusable.yml | 29 +- .github/workflows/pr_build_linux.yml | 69 +--- .github/workflows/spark_sql_test_reusable.yml | 29 +- .github/workflows/spark_sql_writer_tests.yml | 19 +- dev/ci/check-ci-config.py | 8 +- dev/ci/compute-changes.py | 15 + dev/ci/native-cache-key.py | 314 ++++++++++++++++ dev/ci/native-library-cache.py | 176 +++++++++ dev/ci/test-native-cache-key.py | 337 ++++++++++++++++++ dev/ci/test-native-cache-workflow.py | 222 ++++++++++++ dev/ci/test-native-library-cache.py | 218 +++++++++++ 14 files changed, 1476 insertions(+), 125 deletions(-) create mode 100644 .github/actions/build-native-ci/action.yaml create mode 100644 dev/ci/native-cache-key.py create mode 100644 dev/ci/native-library-cache.py create mode 100644 dev/ci/test-native-cache-key.py create mode 100644 dev/ci/test-native-cache-workflow.py create mode 100644 dev/ci/test-native-library-cache.py diff --git a/.github/actions/build-native-ci/action.yaml b/.github/actions/build-native-ci/action.yaml new file mode 100644 index 00000000000..ff4d4369f0e --- /dev/null +++ b/.github/actions/build-native-ci/action.yaml @@ -0,0 +1,109 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +name: Build or restore the Linux CI native library +description: 'Reuse an exact-input native library, otherwise build it with the CI profile' +outputs: + cache-hit: + description: 'Whether a validated cached library replaced compilation' + value: ${{ steps.library.outputs.hit == 'true' && 'true' || 'false' }} +runs: + using: composite + steps: + # Call after checkout and setup-builder. Compute once, before Cargo writes + # generated Rust files, and use the same keys for both restore and save. + - name: Fingerprint native build inputs + id: key + shell: bash + env: + RUSTFLAGS: '-Ctarget-cpu=x86-64-v3 -Clink-arg=-fuse-ld=bfd' + run: python3 dev/ci/native-cache-key.py --profile ci --github-output "$GITHUB_OUTPUT" + + - name: Restore native library cache + id: binary-cache + uses: actions/cache/restore@v6 + with: + path: ${{ runner.temp }}/comet-native-library + key: ${{ steps.key.outputs.binary-key }} + # Main still builds to keep its incremental Cargo cache warm. Lookup + # only avoids downloading a library that this run will not execute. + lookup-only: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }} + + - name: Validate cached native library + id: library + if: steps.binary-cache.outputs.cache-hit == 'true' && !(github.event_name == 'push' && github.ref == 'refs/heads/main') + shell: bash + env: + NATIVE_CACHE_KEY: ${{ steps.key.outputs.binary-key }} + run: | + python3 dev/ci/native-library-cache.py restore \ + --key "$NATIVE_CACHE_KEY" \ + --cache-dir "$RUNNER_TEMP/comet-native-library" \ + --library native/target/ci/libcomet.so \ + --github-output "$GITHUB_OUTPUT" + + - name: Restore incremental Cargo cache + id: cargo-cache + if: steps.library.outputs.hit != 'true' + uses: actions/cache/restore@v6 + with: + path: | + ${{ steps.key.outputs.cargo-home }}/registry + ${{ steps.key.outputs.cargo-home }}/git + native/target + key: ${{ steps.key.outputs.source-key }} + restore-keys: ${{ steps.key.outputs.restore-prefix }} + + - name: Build native library (CI profile) + if: steps.library.outputs.hit != 'true' + shell: bash + env: + RUSTFLAGS: '-Ctarget-cpu=x86-64-v3 -Clink-arg=-fuse-ld=bfd' + run: | + cd native + # A target-directory cache is only an incremental build aid. Cargo + # must run even on an exact target-cache hit; only the validated, + # separately keyed binary cache can replace compilation. + cargo build --locked --profile ci + + - name: Prepare native library cache + if: github.event_name == 'push' && github.ref == 'refs/heads/main' && steps.binary-cache.outputs.cache-hit != 'true' + shell: bash + env: + NATIVE_CACHE_KEY: ${{ steps.key.outputs.binary-key }} + run: | + python3 dev/ci/native-library-cache.py prepare \ + --key "$NATIVE_CACHE_KEY" \ + --cache-dir "$RUNNER_TEMP/comet-native-library" \ + --library native/target/ci/libcomet.so + + - name: Save native library cache + if: github.event_name == 'push' && github.ref == 'refs/heads/main' && steps.binary-cache.outputs.cache-hit != 'true' + uses: actions/cache/save@v6 + with: + path: ${{ runner.temp }}/comet-native-library + key: ${{ steps.key.outputs.binary-key }} + + - name: Save incremental Cargo cache + if: github.event_name == 'push' && github.ref == 'refs/heads/main' && steps.cargo-cache.outputs.cache-hit != 'true' + uses: actions/cache/save@v6 + with: + path: | + ${{ steps.key.outputs.cargo-home }}/registry + ${{ steps.key.outputs.cargo-home }}/git + native/target + key: ${{ steps.key.outputs.source-key }} diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 3b8f9c56a65..7f2baeb66fd 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -404,6 +404,56 @@ entry through `restore-keys` and downloads whatever else it needs, which is what a cold pull request already did. See the push-tier discussion above for which jobs do run on main and therefore do write. +## Reusing Linux native builds + +The Linux, Spark SQL, Iceberg and manual Spark writer workflows use +`.github/actions/build-native-ci` after checkout and toolchain setup. It keeps +two separate caches: + +- A compact `libcomet.so` cache avoids compilation when every native build + input matches. It uses an exact key, with no fallback prefix. A manifest + records the input key and the library's SHA-256; a missing, partial or + invalid entry falls back to compilation. The library is staged at the same + `native/target/ci/libcomet.so` path as a fresh build, then uploaded under the + caller's existing artifact name. All downstream tests still run. +- The larger Cargo cache contains the actual `CARGO_HOME` registry/git + directories and `native/target`. A matching dependency prefix can seed a + build after native sources change. Restoring this cache always runs + `cargo build --locked --profile ci`; a target-directory cache hit alone + never authorizes reusing a binary without compiling. + +`dev/ci/native-cache-key.py` computes the keys once, before Cargo generates +Rust source files. It hashes tracked native and contrib files, shared JVM +inputs, build configuration and CI definitions, together with the resolved +Rust/C/C++/protobuf tools, JDK, installed system packages, architecture and +build environment. Generated files and untracked build output do not change +the save key. The CPU target remains explicitly `x86-64-v3`; binaries built +with `target-cpu=native` must not enter this cache. Unsupported external tool +or library overrides fail key generation rather than create an incomplete +identity. Ordinary changes confined to Spark sources can retain the same +native key; changing protobuf, toolchains or build flags cannot. + +Only pushes to `main` save these caches. PR, queue, nightly and manual runs +consume them without writing new entries. A main push still invokes Cargo, +even when the compact entry exists, to keep the larger compiler cache warm. +It skips re-saving an exact cache entry. The Rust test job uses the same key +snapshot and Cargo-home resolution with a separate debug profile, and still +runs every Rust check and test. + +The first main push after adoption populates the new cache namespace. Until +then, or after eviction, runs build normally. Cache reuse is scoped by GitHub's +cache access rules; it does not fetch a binary from an arbitrary PR or use the +latest main binary when inputs differ. The manifest detects corruption, while +the main-only write policy determines which builds can populate the cache. + +Preflight runs the native-key, compact-library and workflow-flow regression +tests. To verify a hosted hit, compare the native input key between a main +push and a later run with unchanged inputs: `Validate cached native library` +must report `hit=true`, the Cargo restore/build steps must skip, and the +normal library upload and downstream tests must succeed. A native or protobuf +edit must instead invoke Cargo. These timings depend on cache availability; +the change does not promise a fixed build-time reduction. + ## Retrying flaky network operations **Maven.** `.mvn/maven.config` tunes the Maven Resolver HTTP transport: six diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d5bdde5aa68..a6ea5f7a90d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -134,6 +134,12 @@ jobs: - name: Check Iceberg shard inventory validation run: python3 dev/ci/test-iceberg-shards.py + - name: Check native cache identity and reuse + run: | + python3 dev/ci/test-native-cache-key.py + python3 dev/ci/test-native-library-cache.py + python3 dev/ci/test-native-cache-workflow.py + - name: Check CI config invariants run: python3 dev/ci/check-ci-config.py diff --git a/.github/workflows/iceberg_spark_test_reusable.yml b/.github/workflows/iceberg_spark_test_reusable.yml index ae20b6d7683..829202b2b4b 100644 --- a/.github/workflows/iceberg_spark_test_reusable.yml +++ b/.github/workflows/iceberg_spark_test_reusable.yml @@ -81,33 +81,8 @@ jobs: id: shards run: python3 dev/ci/check-iceberg-shards.py --github-output "$GITHUB_OUTPUT" - - name: Restore Cargo cache - uses: actions/cache/restore@v6 - with: - path: | - ~/.cargo/registry - ~/.cargo/git - native/target - key: ${{ runner.os }}-cargo-ci-${{ hashFiles('native/**/Cargo.lock', 'native/**/Cargo.toml') }}-${{ hashFiles('native/**/*.rs') }} - restore-keys: | - ${{ runner.os }}-cargo-ci-${{ hashFiles('native/**/Cargo.lock', 'native/**/Cargo.toml') }}- - - - name: Build native library - # Use CI profile for faster builds (no LTO) and to share cache with pr_build_linux.yml. - run: | - cd native && cargo build --profile ci - env: - RUSTFLAGS: "-Ctarget-cpu=x86-64-v3 -Clink-arg=-fuse-ld=bfd" - - - name: Save Cargo cache - uses: actions/cache/save@v6 - if: github.ref == 'refs/heads/main' - with: - path: | - ~/.cargo/registry - ~/.cargo/git - native/target - key: ${{ runner.os }}-cargo-ci-${{ hashFiles('native/**/Cargo.lock', 'native/**/Cargo.toml') }}-${{ hashFiles('native/**/*.rs') }} + - name: Build or restore native library + uses: ./.github/actions/build-native-ci - name: Upload native library uses: ./.github/actions/upload-artifact-retry diff --git a/.github/workflows/pr_build_linux.yml b/.github/workflows/pr_build_linux.yml index fd1b0a6c0a7..dc798fbb8c9 100644 --- a/.github/workflows/pr_build_linux.yml +++ b/.github/workflows/pr_build_linux.yml @@ -369,27 +369,10 @@ jobs: uses: ./.github/actions/setup-builder with: rust-version: ${{ env.RUST_VERSION }} - jdk-version: 17 # JDK only needed for JVM module proto generation + jdk-version: 17 # JNI headers and libjvm are native build inputs. - - name: Restore Cargo cache - uses: actions/cache/restore@v6 - with: - path: | - ~/.cargo/registry - ~/.cargo/git - native/target - key: ${{ runner.os }}-cargo-ci-${{ hashFiles('native/**/Cargo.lock', 'native/**/Cargo.toml') }}-${{ hashFiles('native/**/*.rs') }} - restore-keys: | - ${{ runner.os }}-cargo-ci-${{ hashFiles('native/**/Cargo.lock', 'native/**/Cargo.toml') }}- - - - name: Build native library (CI profile) - run: | - cd native - # CI profile: same overflow behavior as release, but faster compilation - # (no LTO, parallel codegen) - cargo build --profile ci - env: - RUSTFLAGS: "-Ctarget-cpu=x86-64-v3 -Clink-arg=-fuse-ld=bfd" + - name: Build or restore native library + uses: ./.github/actions/build-native-ci - name: Upload native library uses: ./.github/actions/upload-artifact-retry @@ -398,18 +381,6 @@ jobs: path: native/target/ci/libcomet.so retention-days: 1 - - name: Save Cargo cache - uses: actions/cache/save@v6 - # The push run is the cache warmer (see the header); a scheduled run - # at the same sha would only re-archive an entry that already exists. - if: github.event_name == 'push' && github.ref == 'refs/heads/main' - with: - path: | - ~/.cargo/registry - ~/.cargo/git - native/target - key: ${{ runner.os }}-cargo-ci-${{ hashFiles('native/**/Cargo.lock', 'native/**/Cargo.toml') }}-${{ hashFiles('native/**/*.rs') }} - # Run Rust tests (runs in parallel with build-native, uses debug builds). # Owns main's `cargo-debug` cache entry, so it runs in cache-refresh-only mode # as well. The tests themselves are a small slice of its runtime -- almost all @@ -432,17 +403,20 @@ jobs: rust-version: ${{ env.RUST_VERSION }} jdk-version: 17 + - name: Fingerprint Rust test build inputs + id: cargo-key + run: python3 dev/ci/native-cache-key.py --profile debug --github-output "$GITHUB_OUTPUT" + - name: Restore Cargo cache + id: cargo-cache uses: actions/cache/restore@v6 with: path: | - ~/.cargo/registry - ~/.cargo/git + ${{ steps.cargo-key.outputs.cargo-home }}/registry + ${{ steps.cargo-key.outputs.cargo-home }}/git native/target - # Note: Java version intentionally excluded - Rust target is JDK-independent - key: ${{ runner.os }}-cargo-debug-${{ hashFiles('native/**/Cargo.lock', 'native/**/Cargo.toml') }}-${{ hashFiles('native/**/*.rs') }} - restore-keys: | - ${{ runner.os }}-cargo-debug-${{ hashFiles('native/**/Cargo.lock', 'native/**/Cargo.toml') }}- + key: ${{ steps.cargo-key.outputs.source-key }} + restore-keys: ${{ steps.cargo-key.outputs.restore-prefix }} - name: Rust test steps uses: ./.github/actions/rust-test @@ -451,13 +425,13 @@ jobs: uses: actions/cache/save@v6 # The push run is the cache warmer (see the header); a scheduled run # at the same sha would only re-archive an entry that already exists. - if: github.event_name == 'push' && github.ref == 'refs/heads/main' + if: github.event_name == 'push' && github.ref == 'refs/heads/main' && steps.cargo-cache.outputs.cache-hit != 'true' with: path: | - ~/.cargo/registry - ~/.cargo/git + ${{ steps.cargo-key.outputs.cargo-home }}/registry + ${{ steps.cargo-key.outputs.cargo-home }}/git native/target - key: ${{ runner.os }}-cargo-debug-${{ hashFiles('native/**/Cargo.lock', 'native/**/Cargo.toml') }}-${{ hashFiles('native/**/*.rs') }} + key: ${{ steps.cargo-key.outputs.source-key }} linux-test: # `lint` is already upstream via build-native; it is listed here so this @@ -643,17 +617,6 @@ jobs: # Download to release/ since Maven's -Prelease expects libcomet.so there path: native/target/release/ - # Restore cargo registry cache (for any cargo commands that might run) - - name: Restore Cargo registry - uses: actions/cache/restore@v6 - with: - path: | - ~/.cargo/registry - ~/.cargo/git - key: ${{ runner.os }}-cargo-registry-${{ hashFiles('native/**/Cargo.lock') }} - restore-keys: | - ${{ runner.os }}-cargo-registry- - - name: Java test steps uses: ./.github/actions/java-test with: diff --git a/.github/workflows/spark_sql_test_reusable.yml b/.github/workflows/spark_sql_test_reusable.yml index 9fa254f9b60..c5805a11973 100644 --- a/.github/workflows/spark_sql_test_reusable.yml +++ b/.github/workflows/spark_sql_test_reusable.yml @@ -97,33 +97,8 @@ jobs: rust-version: ${{ env.RUST_VERSION }} jdk-version: ${{ inputs.java }} - - name: Restore Cargo cache - uses: actions/cache/restore@v6 - with: - path: | - ~/.cargo/registry - ~/.cargo/git - native/target - key: ${{ runner.os }}-cargo-ci-${{ hashFiles('native/**/Cargo.lock', 'native/**/Cargo.toml') }}-${{ hashFiles('native/**/*.rs') }} - restore-keys: | - ${{ runner.os }}-cargo-ci-${{ hashFiles('native/**/Cargo.lock', 'native/**/Cargo.toml') }}- - - - name: Build native library (CI profile) - run: | - cd native - cargo build --profile ci - env: - RUSTFLAGS: "-Ctarget-cpu=x86-64-v3 -Clink-arg=-fuse-ld=bfd" - - - name: Save Cargo cache - uses: actions/cache/save@v6 - if: github.ref == 'refs/heads/main' - with: - path: | - ~/.cargo/registry - ~/.cargo/git - native/target - key: ${{ runner.os }}-cargo-ci-${{ hashFiles('native/**/Cargo.lock', 'native/**/Cargo.toml') }}-${{ hashFiles('native/**/*.rs') }} + - name: Build or restore native library + uses: ./.github/actions/build-native-ci - name: Upload native library uses: ./.github/actions/upload-artifact-retry diff --git a/.github/workflows/spark_sql_writer_tests.yml b/.github/workflows/spark_sql_writer_tests.yml index c813a8b39e1..1ceb73b16d0 100644 --- a/.github/workflows/spark_sql_writer_tests.yml +++ b/.github/workflows/spark_sql_writer_tests.yml @@ -83,23 +83,8 @@ jobs: rust-version: ${{ env.RUST_VERSION }} jdk-version: ${{ steps.resolve.outputs.java }} - - name: Restore Cargo cache - uses: actions/cache/restore@v6 - with: - path: | - ~/.cargo/registry - ~/.cargo/git - native/target - key: ${{ runner.os }}-cargo-ci-${{ hashFiles('native/**/Cargo.lock', 'native/**/Cargo.toml') }}-${{ hashFiles('native/**/*.rs') }} - restore-keys: | - ${{ runner.os }}-cargo-ci-${{ hashFiles('native/**/Cargo.lock', 'native/**/Cargo.toml') }}- - - - name: Build native library (CI profile) - run: | - cd native - cargo build --profile ci - env: - RUSTFLAGS: "-Ctarget-cpu=x86-64-v3 -Clink-arg=-fuse-ld=bfd" + - name: Build or restore native library + uses: ./.github/actions/build-native-ci - name: Stage native library at release path run: | diff --git a/dev/ci/check-ci-config.py b/dev/ci/check-ci-config.py index abb6b1f2d4b..fb1dd2f7b40 100644 --- a/dev/ci/check-ci-config.py +++ b/dev/ci/check-ci-config.py @@ -129,6 +129,12 @@ [".github/actions/maven-bootstrap/action.yaml"], {"build_linux", "build_linux_full", "build_linux_all_profiles"}, ), + # Linux's compact native cache is shared by every Spark/Iceberg producer. + # Helper edits also match macOS's broad dev/ci filter, as before. + ([".github/actions/build-native-ci/action.yaml"], BUILD_JOBS - {"build_macos"}), + (["dev/ci/native-cache-key.py"], BUILD_JOBS), + (["dev/ci/native-library-cache.py"], BUILD_JOBS), + (["dev/ci/test-native-cache-workflow.py"], BUILD_JOBS), # Spot checks that the additions above did not widen unrelated routes. (["docs/source/user-guide/overview.md"], {"docs"}), (["native/core/benches/parquet_read.rs"], {"benchmark"}), @@ -399,7 +405,7 @@ CACHE_REFRESH_WORKFLOW = WORKFLOWS / "pr_build_linux.yml" CACHE_REFRESH_JOBS = { "lint": "gates build-native and linux-test-rust, and costs 40 seconds", - "build-native": "writes the cargo-ci cache (native/target, CI profile)", + "build-native": "writes the compact native library and cargo-ci caches", "linux-test-rust": "writes the cargo-debug cache (native/target, debug)", "verify-benchmark-results-tpch": "writes the TPC-H SF=1 dataset and java-maven caches", "verify-benchmark-results-tpcds": "writes the TPC-DS SF=1 dataset and java-maven caches", diff --git a/dev/ci/compute-changes.py b/dev/ci/compute-changes.py index 799b791624a..6df22af9782 100644 --- a/dev/ci/compute-changes.py +++ b/dev/ci/compute-changes.py @@ -388,6 +388,21 @@ "mvnw", ], } +# These inputs are shared by the Linux native producers. Keep the routes in +# one place so an action-only cache change exercises each applicable consumer. +for _native_consumer in ( + "build_linux", "spark_3_4", "spark_3_5", "spark_4_0", "spark_4_1", + "iceberg_1_8", "iceberg_1_9", "iceberg_1_10", "iceberg_1_11", +): + FILTERS[_native_consumer].extend([ + ".github/actions/build-native-ci/**", + "dev/ci/native-cache-key.py", + "dev/ci/native-library-cache.py", + "dev/ci/test-native-cache-key.py", + "dev/ci/test-native-library-cache.py", + "dev/ci/test-native-cache-workflow.py", + ]) + FILTERS["spark_4_1_hive"] = FILTERS["spark_4_1"] FILTERS["build_linux_full"] = FILTERS["build_linux"] FILTERS["build_linux_all_profiles"] = FILTERS["build_linux"] diff --git a/dev/ci/native-cache-key.py b/dev/ci/native-cache-key.py new file mode 100644 index 00000000000..82c32f55ec1 --- /dev/null +++ b/dev/ci/native-cache-key.py @@ -0,0 +1,314 @@ +#!/usr/bin/env python3 +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Snapshot Linux CI cache keys before Cargo creates generated sources. + +The dependency prefix permits Cargo to rebuild changed source incrementally. +The compact library key is exact-only: a hit permits skipping Cargo altogether. +Neither key contains the commit SHA, so unrelated Spark edits can reuse native +outputs. Unknown build environments fail before any outputs are written. +""" + +import argparse +import hashlib +import json +import os +from pathlib import Path +import shlex +import shutil +import stat +import subprocess +import sys +import tomllib + + +SOURCE_PREFIXES = ("native/", "contrib/", "common/", ".cargo/", ".mvn/", + ".github/actions/", ".github/workflows/", "dev/ci/") +SOURCE_FILES = {"Makefile", "pom.xml", "mvnw", "rust-toolchain", "rust-toolchain.toml"} +# These can select arbitrary executable/source files outside the tracked input +# set. Supporting one requires adding its transitive inputs to the identity. +UNSUPPORTED_ENV = { + "RUSTC", "RUSTDOC", "RUSTC_WRAPPER", "RUSTC_WORKSPACE_WRAPPER", + "CARGO_ENCODED_RUSTFLAGS", "CARGO_TARGET_DIR", "CARGO_BUILD_TARGET", + "CARGO_BUILD_RUSTC", "CARGO_BUILD_RUSTDOC", "CARGO_BUILD_RUSTC_WRAPPER", + "CARGO_BUILD_RUSTC_WORKSPACE_WRAPPER", "CARGO_BUILD_TARGET_DIR", + "CC", "CXX", "AR", "LD", "PROTOC", "PROTOC_INCLUDE", "LIBCLANG_PATH", + "DOCS_RS", "HDFS_LIB_DIR", "HADOOP_HOME", "HDFS_STATIC", + "CFLAGS", "CXXFLAGS", "CPPFLAGS", "LDFLAGS", "LIBRARY_PATH", "CPATH", + "C_INCLUDE_PATH", "CPLUS_INCLUDE_PATH", "OBJC_INCLUDE_PATH", "LD_PRELOAD", + "CRATE_CC_NO_DEFAULTS", "CMAKE", "MAKE", "MAKEFLAGS", +} +BUILD_ENV_PREFIXES = ("CARGO_", "RUST", "CC_", "CXX_", "AR_", "CFLAGS", "CXXFLAGS", + "CPPFLAGS", "LDFLAGS", "BINDGEN_", "PKG_CONFIG", "OPENSSL_", + "ZSTD_", "LZ4_", "SNAPPY_", "HDFS_", "COMET_") +BUILD_ENV_NAMES = {"PATH", "JAVA_HOME", "CARGO_HOME", "HOME", "LIBRARY_PATH", + "LD_LIBRARY_PATH", "CPATH", "C_INCLUDE_PATH", "CPLUS_INCLUDE_PATH", + "SOURCE_DATE_EPOCH"} +TOOLS = { + "rustc": ("-vV",), "cargo": ("--version",), "rustfmt": ("--version",), + "protoc": ("--version",), + "cc": ("--version",), "c++": ("--version",), "clang": ("--version",), + "ld.bfd": ("--version",), "ar": ("--version",), "pkg-config": ("--version",), +} + + +def digest(value): + """Return SHA-256 of a JSON-compatible value with stable map ordering. + + Values stay in memory; callers publish only the digest, never raw build + environment values, which may contain credentials in Cargo settings. + """ + return hashlib.sha256(json.dumps(value, sort_keys=True, separators=(",", ":")) + .encode("utf-8")).hexdigest() + + +def command(args, cwd, env): + """Return nonempty command stdout as bytes using an explicit cwd/environment. + + Nonzero status, missing tools, or empty output raises ValueError without + echoing stdout/stderr or environment values. No shell interpolation occurs. + """ + try: + result = subprocess.run(args, cwd=cwd, env=env, check=True, + stdout=subprocess.PIPE, stderr=subprocess.PIPE) + except (OSError, subprocess.CalledProcessError) as exc: + raise ValueError(f"cannot fingerprint required command {args[0]}") from exc + if not result.stdout.strip(): + raise ValueError(f"empty fingerprint from required command {args[0]}") + return result.stdout + + +def file_identity(path): + """Return mode and content digest for one required regular input file. + + Symlinks and missing files fail closed: hashing a link alone would omit + mutable external inputs. Files are read without modifying their contents. + """ + mode = path.lstat().st_mode + if not stat.S_ISREG(mode): + raise ValueError(f"unsupported non-regular input: {path}") + return [stat.S_IMODE(mode), hashlib.sha256(path.read_bytes()).hexdigest()] + + +def directory_identity(path): + """Return every regular file's relative name and digest below a required tree. + + Directory symlinks are rejected as well as file symlinks, so an external JNI + include tree cannot silently escape the snapshot. The tree is read only. + """ + if path.is_symlink() or not path.is_dir(): + raise ValueError(f"unsupported input directory: {path}") + result = {} + for child in sorted(path.rglob("*")): + if child.is_symlink() or not child.is_dir(): + result[str(child.relative_to(path))] = file_identity(child) + return result + + +def tracked_inputs(root, env): + """Return dependency and source snapshots from Git's tracked file inventory. + + Read worktree bytes and modes, so staged or unstaged edits invalidate keys. + The inventory excludes generated/untracked files and target directories. + Missing tracked inputs, conflicts, symlinks, or submodules fail closed. + """ + inventory = command(["git", "ls-files", "--stage", "-z"], root, env) + sources = {} + dependencies = {} + for record in inventory.split(b"\0"): + if not record: + continue + metadata, raw_path = record.split(b"\t", 1) + mode, _, stage = metadata.decode("ascii").split() + name = os.fsdecode(raw_path) + if name not in SOURCE_FILES and not name.startswith(SOURCE_PREFIXES): + continue + if stage != "0" or mode not in {"100644", "100755"}: + raise ValueError(f"unsupported tracked input: {name}") + identity = file_identity(root / name) + sources[name] = identity + if Path(name).name in {"Cargo.toml", "Cargo.lock"}: + dependencies[name] = identity + for name in ("native/Cargo.toml", "native/Cargo.lock"): + if name not in dependencies: + raise ValueError(f"missing tracked build input: {name}") + return dependencies, sources + + +def cargo_config_directories(root, cargo_home): + """Return Cargo's ordered search locations without reading or writing them. + + Cargo starts at the native workspace and visits ancestor .cargo directories, + plus CARGO_HOME. Returning a set in stable order avoids duplicate reads. + """ + directories = {cargo_home} + directories.update(path / ".cargo" for path in (root / "native", root, *root.parents)) + return sorted(directories) + + +def cargo_configs(root, cargo_home): + """Return content identities for Cargo configs in its search locations. + + Include both supported filenames from native/ through filesystem root and + CARGO_HOME, including untracked external configs. Reject config features + that refer to extra executable/source files outside this input snapshot; + adding support for them requires extending this fingerprint first. + """ + result = {} + for directory in cargo_config_directories(root, cargo_home): + for name in ("config", "config.toml"): + path = directory / name + if not path.exists(): + continue + content = tomllib.loads(path.read_text(encoding="utf-8")) + # Registry/transport settings do not select source files outside + # Cargo.lock. Build flags are content-addressed; wrappers, custom + # targets/linkers, source replacement, and config includes are not. + if set(content) - {"build", "net", "http", "registries", "registry"}: + raise ValueError(f"unsupported Cargo config section: {path}") + if set(content.get("build", {})) - {"rustflags", "rustdocflags", "jobs", "incremental"}: + raise ValueError(f"unsupported Cargo build configuration: {path}") + if "target-cpu=native" in path.read_text(encoding="utf-8"): + raise ValueError(f"host-specific CPU flags are not reusable: {path}") + result[str(path)] = file_identity(path) + return result + + +def environment_identity(root, profile, env): + """Return the required Linux toolchain, platform, JDK and build-env snapshot. + + The supplied mapping is the caller's effective environment before building. + This supports the repository's fixed Linux CI commands, not arbitrary local + tool overrides. Missing fingerprints raise before a reusable key can exist. + Package versions cover linker/compiler libraries in the mutable CI image. + Compiler/include/library overrides are rejected because hashing a path or + flag such as `-include /tmp/header.h` does not fingerprint the file it reads. + The only extra library search path supported is the fingerprinted JDK's + server directory. Rustup proxies are resolved to their actual tool binaries. + """ + for name in UNSUPPORTED_ENV: + if env.get(name): + raise ValueError(f"unsupported build override: {name}") + for name in env: + if name.startswith(("CARGO_TARGET_", "CARGO_PROFILE_", "CC_", "CXX_", "AR_", + "CFLAGS_", "CXXFLAGS_", "CPPFLAGS_", "LDFLAGS_", "HDFS_", + "HOST_CC", "HOST_CXX", "HOST_AR", "HOST_CFLAGS", "HOST_CXXFLAGS", + "TARGET_CC", "TARGET_CXX", "TARGET_AR", "TARGET_CFLAGS", "TARGET_CXXFLAGS", + "CMAKE_", "HOST_CMAKE", "TARGET_CMAKE", + "OPENSSL_", "PKG_CONFIG", "BINDGEN_", "ZSTD_", "LZ4_", "SNAPPY_")): + raise ValueError(f"unsupported build override: {name}") + if name.startswith("CARGO_BUILD_") and name not in {"CARGO_BUILD_JOBS", "CARGO_BUILD_INCREMENTAL"}: + raise ValueError(f"unsupported build override: {name}") + flags = shlex.split(env.get("RUSTFLAGS", "")) + if profile == "ci" and flags != ["-Ctarget-cpu=x86-64-v3", "-Clink-arg=-fuse-ld=bfd"]: + raise ValueError("CI library reuse requires the fixed x86-64-v3/bfd RUSTFLAGS") + if profile == "debug" and flags != ["-Clink-arg=-fuse-ld=bfd"]: + raise ValueError("debug cache reuse requires the fixed bfd RUSTFLAGS") + if not env.get("JAVA_HOME"): + raise ValueError("JAVA_HOME is required") + java_home = Path(env["JAVA_HOME"]).resolve(strict=True) + library_path = env.get("LD_LIBRARY_PATH", "") + if library_path and library_path not in {str(java_home / "lib/server"), + str(Path(env["JAVA_HOME"]) / "lib/server")}: + raise ValueError("unsupported build override: LD_LIBRARY_PATH") + if not (java_home / "include/jni.h").is_file(): + raise ValueError("JAVA_HOME must contain JNI headers") + cargo_home = Path(env.get("CARGO_HOME") or str(Path(env["HOME"]) / ".cargo")).resolve() + if "\n" in str(cargo_home) or "\r" in str(cargo_home): + raise ValueError("CARGO_HOME must fit one GitHub output line") + versions = {} + for tool, args in TOOLS.items(): + executable = shutil.which(tool, path=env.get("PATH")) + if not executable: + raise ValueError(f"missing required tool: {tool}") + launcher = Path(executable).resolve(strict=True) + resolved = launcher + if tool in {"rustc", "cargo", "rustfmt"}: + resolved = Path(command(["rustup", "which", tool], root / "native", env).decode().strip()) + if not resolved.is_absolute(): + raise ValueError(f"rustup returned a non-absolute path for {tool}") + resolved = resolved.resolve(strict=True) + versions[tool] = { + "launcher_path": str(launcher), "launcher": file_identity(launcher), + "path": str(resolved), "binary": file_identity(resolved), + "version": command([tool, *args], root / "native", env).decode("utf-8"), + } + system = command(["uname", "-s"], root, env).decode().strip() + architecture = command(["uname", "-m"], root, env).decode().strip() + if system != "Linux" or architecture != "x86_64": + raise ValueError("native cache identity supports Linux x86_64 only") + packages = command(["dpkg-query", "-W", "-f=${binary:Package}\t${Version}\t${Architecture}\n"], + root, env).decode("utf-8").splitlines() + return cargo_home, { + "profile": profile, "root": str(root), "system": system, + "architecture": architecture, "packages": sorted(packages), "tools": versions, + "java_home": str(java_home), "java_release": file_identity(java_home / "release"), + "libjvm": file_identity(java_home / "lib/server/libjvm.so"), + "jni_headers": directory_identity(java_home / "include"), + "cargo_configs": cargo_configs(root, cargo_home), + "environment": {key: value for key, value in env.items() + if key in BUILD_ENV_NAMES or key.startswith(BUILD_ENV_PREFIXES)}, + } + + +def cache_keys(profile, dependencies, sources, environment, cargo_home): + """Return immutable GitHub output strings for a complete input snapshot. + + dependency-key includes environment and manifests; source-key additionally + includes tracked source/build files. Only ci has a usable exact binary-key. + The restore-prefix intentionally excludes source so Cargo can rebuild it. + """ + dependency_key = f"Linux-cargo-{profile}-v2-{digest([environment, dependencies])}" + source_key = f"{dependency_key}-{digest(sources)}" + return { + "cargo-home": str(cargo_home), "dependency-key": dependency_key, + "source-key": source_key, "restore-prefix": f"{dependency_key}-", + "binary-key": f"Linux-native-ci-v1-{digest([environment, sources])}" if profile == "ci" else "", + } + + +def main(): + """Snapshot keys at repository root and publish them only after full success. + + --github-output optionally appends the same key=value records printed on + stdout. Failure returns status 1 with a concise error and writes no outputs. + """ + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--profile", required=True, choices=("ci", "debug")) + parser.add_argument("--github-output", type=Path) + args = parser.parse_args() + try: + env = dict(os.environ) + root = Path(command(["git", "rev-parse", "--show-toplevel"], Path.cwd(), env) + .decode().strip()).resolve() + dependencies, sources = tracked_inputs(root, env) + cargo_home, environment = environment_identity(root, args.profile, env) + keys = cache_keys(args.profile, dependencies, sources, environment, cargo_home) + output = "".join(f"{key}={value}\n" for key, value in keys.items()) + if args.github_output: + with args.github_output.open("a", encoding="utf-8") as stream: + stream.write(output) + print(output, end="") + except (OSError, ValueError, KeyError) as exc: + print(f"Native cache identity unavailable: {exc}", file=sys.stderr) + return 1 + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/dev/ci/native-library-cache.py b/dev/ci/native-library-cache.py new file mode 100644 index 00000000000..e6c20fc4f2a --- /dev/null +++ b/dev/ci/native-library-cache.py @@ -0,0 +1,176 @@ +#!/usr/bin/env python3 +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Prepare or restore one Linux libcomet.so cache entry without executing it. + +The workflow supplies the complete native-input key and controls which runs may +save caches. A checksum detects incomplete or corrupted entries; it is not a +substitute for restricting cache writers to trusted builds. Restore treats bad +cache contents as a miss, removes any previous destination, and publishes +hit=true only after installing all verified bytes. Destination I/O failures are +fatal so permission or disk failures cannot masquerade as cache misses. +""" + +import argparse +import hashlib +import json +import os +from pathlib import Path +import stat +import sys +import tempfile + + +def open_regular_file(path): + """Return an owned binary stream for a Linux regular, non-symlink file. + + The caller closes the stream. O_NOFOLLOW rejects symlinks, and O_NONBLOCK + prevents a malformed cache FIFO from hanging before fstat can reject it. + Missing/unreadable paths raise OSError; other file types raise ValueError. + """ + descriptor = os.open(path, os.O_RDONLY | os.O_NOFOLLOW | os.O_NONBLOCK) + try: + if not stat.S_ISREG(os.fstat(descriptor).st_mode): + raise ValueError(f"Not a regular file: {path}") + return os.fdopen(descriptor, "rb") + except BaseException: + os.close(descriptor) + raise + + +def copy_library(source, destination, expected_sha256=None): + """Atomically copy an open binary stream to destination and return its SHA256. + + Reads start at the stream's current offset; ownership stays with the caller. + If supplied, expected_sha256 must match the copied bytes before replacement. + Bad source reads/checksums raise ValueError; destination I/O errors propagate. + Temporary files are always removed and an incomplete copy is never installed. + """ + destination.parent.mkdir(parents=True, exist_ok=True) + temporary = None + try: + with tempfile.NamedTemporaryFile(dir=destination.parent, delete=False) as output: + temporary = Path(output.name) + digest = hashlib.sha256() + while True: + try: + chunk = source.read(1024 * 1024) + except OSError as error: + raise ValueError("Cannot read native library") from error + if not chunk: + break + digest.update(chunk) + output.write(chunk) + checksum = digest.hexdigest() + if expected_sha256 is not None and checksum != expected_sha256: + raise ValueError("Native library checksum mismatch") + temporary.replace(destination) + return checksum + finally: + if temporary is not None: + temporary.unlink(missing_ok=True) + + +def prepare(key, cache_dir, library): + """Write libcomet.so and its key/checksum manifest into cache_dir. + + library is a complete build output; all paths are pathlib Paths. Both files + are installed atomically, with the manifest last. A prior manifest is removed + first so an interrupted refresh cannot advertise an old successful entry. + Returns nothing; invalid source files and all write failures propagate. + """ + cache_dir.mkdir(parents=True, exist_ok=True) + manifest = cache_dir / "manifest.json" + manifest.unlink(missing_ok=True) + with open_regular_file(library) as source: + checksum = copy_library(source, cache_dir / "libcomet.so") + temporary = None + try: + with tempfile.NamedTemporaryFile( + mode="w", encoding="utf-8", dir=cache_dir, delete=False) as output: + temporary = Path(output.name) + json.dump({"key": key, "sha256": checksum}, output, sort_keys=True) + output.write("\n") + temporary.replace(manifest) + finally: + if temporary is not None: + temporary.unlink(missing_ok=True) + + +def restore(key, cache_dir, library): + """Install a matching cached library and return True, or return False on a miss. + + key is the exact native-input fingerprint; paths are pathlib Paths. Removes + the previous library before checking the cache, including dangling symlinks. + Missing, unreadable, malformed, symlinked or mismatched cache data is a miss. + Directory creation, destination removal and write failures propagate so the + workflow fails instead of using stale output. No cached code is executed. + """ + library.unlink(missing_ok=True) + try: + with open_regular_file(cache_dir / "manifest.json") as source: + manifest = json.load(source) + if not isinstance(manifest, dict) or manifest.get("key") != key: + return False + checksum = manifest.get("sha256") + if not isinstance(checksum, str) or len(checksum) != 64: + return False + source = open_regular_file(cache_dir / "libcomet.so") + except (OSError, ValueError): + return False + with source: + try: + copy_library(source, library, checksum) + except ValueError: + return False + return True + + +def main(argv): + """Run prepare/restore using CLI arguments and return a process exit status. + + Restore appends hit=true/false to --github-output and prints the same value. + Invalid cache data is a successful miss; operational failures return 1. + Output-file write failures also fail the command rather than report a hit. + """ + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("operation", choices=("prepare", "restore")) + parser.add_argument("--key", required=True) + parser.add_argument("--cache-dir", required=True, type=Path) + parser.add_argument("--library", required=True, type=Path) + parser.add_argument("--github-output", type=Path) + args = parser.parse_args(argv) + try: + if args.operation == "prepare": + prepare(args.key, args.cache_dir, args.library) + else: + hit = restore(args.key, args.cache_dir, args.library) + result = f"hit={str(hit).lower()}\n" + if args.github_output: + with args.github_output.open("a", encoding="utf-8") as output: + output.write(result) + print(result, end="") + except (OSError, ValueError) as error: + print(f"error: {error}", file=sys.stderr) + return 1 + return 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:])) diff --git a/dev/ci/test-native-cache-key.py b/dev/ci/test-native-cache-key.py new file mode 100644 index 00000000000..6686aca77cb --- /dev/null +++ b/dev/ci/test-native-cache-key.py @@ -0,0 +1,337 @@ +#!/usr/bin/env python3 +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Check cache invalidation boundaries without installing native build tools.""" + +import importlib.util +import io +import os +from pathlib import Path +import subprocess +import shutil +import tempfile +import unittest +from unittest.mock import patch + + +SPEC = importlib.util.spec_from_file_location("native_cache_key", Path(__file__).with_name("native-cache-key.py")) +CACHE = importlib.util.module_from_spec(SPEC) +SPEC.loader.exec_module(CACHE) + + +class NativeCacheKeyTests(unittest.TestCase): + """Exercise tracked inputs and mocked toolchains in disposable Git repos.""" + + def setUp(self): + """Create tracked build inputs and fake tools/JDK owned by this test. + + Git inventory is real; tool version queries alone are mocked. Cleanup + removes the entire temporary tree even when an assertion fails. + """ + temporary = tempfile.TemporaryDirectory() + self.addCleanup(temporary.cleanup) + self.directory = Path(temporary.name) + self.root = self.directory / "repo" + self.root.mkdir() + self.git("init", "--quiet") + self.inputs = { + "native/Cargo.toml": "[workspace]\nmembers = []\n", + "native/Cargo.lock": "version = 4\n", + "native/core/src/lib.rs": "pub fn value() -> i32 { 1 }\n", + "native/proto/src/proto/expr.proto": 'syntax = "proto3";\n', + "native/proto/build.rs": "fn main() {}\n", + "contrib/lance/native/Cargo.toml": "[package]\nname = 'lance'\n", + "common/src/main/java/Native.java": "class Native {}\n", + "pom.xml": "\n", + "rust-toolchain.toml": "[toolchain]\nchannel = 'stable'\n", + ".github/actions/build-native-ci/action.yaml": "runs: {}\n", + "dev/ci/native-cache-key.py": "# identity implementation\n", + "spark/src/main/scala/Plan.scala": "object Plan {}\n", + "README.md": "# Comet\n", + } + for name, content in self.inputs.items(): + self.write(self.root / name, content) + self.git("add", ".") + self.java_home = self.directory / "jdk" + self.write(self.java_home / "release", 'JAVA_VERSION="17.0.1"\n') + self.write(self.java_home / "lib/server/libjvm.so", "fake JVM library") + self.write(self.java_home / "include/jni.h", "fake JNI headers") + self.cargo_home = self.directory / "cargo" + self.cargo_home.mkdir() + locations = CACHE.cargo_config_directories + config_patch = patch.object(CACHE, "cargo_config_directories", + side_effect=lambda root, home: [path for path in locations(root, home) + if path.is_relative_to(self.directory)]) + config_patch.start() + self.addCleanup(config_patch.stop) + self.tool_path = self.directory / "tools" + self.rust_tool_path = self.directory / "toolchain/bin" + for tool in CACHE.TOOLS: + self.write(self.tool_path / tool, "fake tool binary " + tool) + for tool in ("rustc", "cargo", "rustfmt"): + self.write(self.rust_tool_path / tool, "fake resolved tool binary " + tool) + self.env = { + "HOME": str(self.directory / "home"), "PATH": os.environ["PATH"], + "JAVA_HOME": str(self.java_home), "CARGO_HOME": str(self.cargo_home), + "RUSTFLAGS": "-Ctarget-cpu=x86-64-v3 -Clink-arg=-fuse-ld=bfd", + } + self.versions = {name: f"{name} version 1\n".encode() for name in CACHE.TOOLS} + self.versions.update({"uname -s": b"Linux\n", "uname -m": b"x86_64\n", + "dpkg-query": b"libc6\t1.0\tamd64\n"}) + + def git(self, *args): + """Run Git against the test repository; setup errors fail the test.""" + return subprocess.run(["git", *args], cwd=self.root, check=True, + stdout=subprocess.PIPE, stderr=subprocess.PIPE) + + def write(self, path, content): + """Write fixture text, creating parents; mutation stays inside the temp tree.""" + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(content, encoding="utf-8") + + def fake_command(self, args, cwd, env): + """Return fixture output for a required tool or fail on an unexpected query.""" + if args[:2] == ["rustup", "which"]: + return str(self.rust_tool_path / args[2]).encode() + name = " ".join(args) if args[0] == "uname" else args[0] + return self.versions[name] + + def keys(self, profile="ci"): + """Snapshot real files with a fake Linux toolchain and return output keys.""" + dependencies, sources = CACHE.tracked_inputs(self.root, dict(os.environ)) + with patch.object(CACHE, "command", side_effect=self.fake_command), \ + patch.object(CACHE.shutil, "which", side_effect=lambda name, path: str(self.tool_path / name)): + cargo_home, environment = CACHE.environment_identity(self.root, profile, self.env) + return CACHE.cache_keys(profile, dependencies, sources, environment, cargo_home) + + def test_native_and_build_inputs_invalidate_exact_keys(self): + """Rust/proto/JNI/build edits invalidate binaries while preserving dependency reuse.""" + before = self.keys() + for name in ("native/core/src/lib.rs", "native/proto/src/proto/expr.proto", + "native/proto/build.rs", "common/src/main/java/Native.java", "pom.xml", + "rust-toolchain.toml", ".github/actions/build-native-ci/action.yaml", + "dev/ci/native-cache-key.py"): + with self.subTest(name=name): + self.write(self.root / name, self.inputs[name] + "\n# changed\n") + after = self.keys() + self.assertNotEqual(before["source-key"], after["source-key"]) + self.assertNotEqual(before["binary-key"], after["binary-key"]) + self.assertEqual(before["restore-prefix"], after["restore-prefix"]) + self.write(self.root / name, self.inputs[name]) + + def test_dependency_edits_invalidate_incremental_prefix(self): + """Manifest/lockfile changes isolate both source and dependency caches.""" + before = self.keys() + for name in ("native/Cargo.toml", "native/Cargo.lock", "contrib/lance/native/Cargo.toml"): + with self.subTest(name=name): + self.write(self.root / name, self.inputs[name] + "\n# changed\n") + after = self.keys() + self.assertNotEqual(before["restore-prefix"], after["restore-prefix"]) + self.assertNotEqual(before["binary-key"], after["binary-key"]) + self.write(self.root / name, self.inputs[name]) + + def test_unrelated_jvm_and_untracked_generated_files_do_not_invalidate(self): + """Spark/docs edits and generated protobuf/target files preserve reuse.""" + before = self.keys() + self.write(self.root / "spark/src/main/scala/Plan.scala", "object NewPlan {}\n") + self.write(self.root / "README.md", "new docs\n") + self.write(self.root / "native/proto/src/generated/expr.rs", "generated Rust") + self.write(self.root / "native/target/ci/libcomet.so", "built artifact") + self.assertEqual(before, self.keys()) + + def test_tracked_addition_deletion_and_executable_mode(self): + """New/deleted native files and mode changes cannot keep an exact hit.""" + before = self.keys() + new = self.root / "native/core/src/new.rs" + self.write(new, "new tracked source") + self.git("add", "native/core/src/new.rs") + self.assertNotEqual(before["binary-key"], self.keys()["binary-key"]) + self.git("rm", "--cached", "native/core/src/new.rs") + source = self.root / "native/core/src/lib.rs" + source.chmod(0o755) + self.assertNotEqual(before["binary-key"], self.keys()["binary-key"]) + source.chmod(0o644) + source.unlink() + with self.assertRaises(FileNotFoundError): + self.keys() + self.git("rm", "--cached", "native/core/src/lib.rs") + self.assertNotEqual(before["binary-key"], self.keys()["binary-key"]) + + def test_tool_jdk_and_platform_changes_invalidate_all_caches(self): + """Tool/package versions, JVM bytes and JVM paths enter the environment key.""" + before = self.keys() + for tool in self.versions: + if tool.startswith("uname"): + continue + with self.subTest(tool=tool): + old = self.versions[tool] + self.versions[tool] += b"changed version\n" + after = self.keys() + self.assertNotEqual(before["restore-prefix"], after["restore-prefix"]) + self.assertNotEqual(before["binary-key"], after["binary-key"]) + self.versions[tool] = old + for name in ("release", "lib/server/libjvm.so", "include/jni.h"): + with self.subTest(jdk_input=name): + path = self.java_home / name + old = path.read_text() + path.write_text(old + "changed\n") + self.assertNotEqual(before["binary-key"], self.keys()["binary-key"]) + path.write_text(old) + new_java_home = self.directory / "other-jdk" + shutil.copytree(self.java_home, new_java_home) + self.env["JAVA_HOME"] = str(new_java_home) + self.assertNotEqual(before["binary-key"], self.keys()["binary-key"]) + self.env["JAVA_HOME"] = str(self.java_home) + (self.tool_path / "cc").write_text("same version, different executable") + self.assertNotEqual(before["binary-key"], self.keys()["binary-key"]) + self.versions["uname -m"] = b"aarch64\n" + with self.assertRaisesRegex(ValueError, "Linux x86_64 only"): + self.keys() + + def test_external_and_ancestor_cargo_configs_invalidate(self): + """Cargo home and ancestor configs matter even though Git cannot list them.""" + before = self.keys() + for directory in (self.cargo_home, self.directory / ".cargo", self.root / ".cargo"): + with self.subTest(directory=directory): + config = directory / "config.toml" + self.write(config, "[build]\nincremental = false\n") + after = self.keys() + self.assertNotEqual(before["restore-prefix"], after["restore-prefix"]) + self.assertNotEqual(before["binary-key"], after["binary-key"]) + config.unlink() + + def test_unsupported_config_and_tool_overrides_fail_closed(self): + """Unknown external build inputs cannot produce an apparently safe cache key.""" + for variable in ("RUSTC_WRAPPER", "CC", "PROTOC", "HDFS_LIB_DIR", "DOCS_RS", + "OPENSSL_LIB_DIR", "CARGO_BUILD_RUSTC_WRAPPER", + "CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_LINKER"): + with self.subTest(variable=variable): + self.env[variable] = "/untracked/tool" + with self.assertRaisesRegex(ValueError, "unsupported build override"): + self.keys() + del self.env[variable] + config = self.cargo_home / "config.toml" + self.write(config, "[build]\nrustc-wrapper = '/untracked/tool'\n") + with self.assertRaisesRegex(ValueError, "unsupported Cargo build"): + self.keys() + self.write(config, "include = ['extra.toml']\n") + with self.assertRaisesRegex(ValueError, "unsupported Cargo config"): + self.keys() + + def test_flags_are_pinned_and_other_build_environment_is_hashed(self): + """Only fixed compiler flags are reusable; safe Cargo build settings are hashed.""" + before = self.keys() + self.env["CARGO_BUILD_JOBS"] = "2" + self.assertNotEqual(before["binary-key"], self.keys()["binary-key"]) + self.env["RUSTFLAGS"] = "-Ctarget-cpu=native -Clink-arg=-fuse-ld=bfd" + with self.assertRaisesRegex(ValueError, "fixed x86-64-v3"): + self.keys() + self.env["RUSTFLAGS"] = "-Clink-arg=-fuse-ld=bfd" + debug = self.keys("debug") + self.assertEqual(debug["binary-key"], "") + self.assertNotEqual(before["source-key"], debug["source-key"]) + self.env["RUSTFLAGS"] += " -Clinker=/untracked/linker" + with self.assertRaisesRegex(ValueError, "fixed bfd"): + self.keys("debug") + + def test_external_compiler_and_library_inputs_are_rejected(self): + """Untracked header/library changes cannot hide behind unchanged override strings.""" + for name, value in { + "CFLAGS": "-include /tmp/header.h", "CXXFLAGS": "-I/tmp/include", + "CPPFLAGS": "-I/tmp/include", "LDFLAGS": "-L/tmp/lib", + "LIBRARY_PATH": "/tmp/lib", "CPATH": "/tmp/include", + "C_INCLUDE_PATH": "/tmp/include", "CPLUS_INCLUDE_PATH": "/tmp/include", + "LD_LIBRARY_PATH": "/tmp/lib", "LD_PRELOAD": "/tmp/lib/injected.so", + "CARGO_BUILD_RUSTC": "/tmp/rustc", "CARGO_BUILD_RUSTFLAGS": "-Clinker=/tmp/ld", + "CARGO_BUILD_TARGET": "/tmp/target.json", "CARGO_BUILD_FUTURE_OVERRIDE": "anything", + "TARGET_CC": "/tmp/compiler", "HOST_CFLAGS": "-include /tmp/header.h", + "CMAKE_TOOLCHAIN_FILE": "/tmp/toolchain.cmake", + }.items(): + with self.subTest(name=name): + self.env[name] = value + with self.assertRaisesRegex(ValueError, "unsupported build override"): + self.keys() + del self.env[name] + self.env["LD_LIBRARY_PATH"] = str(self.java_home / "lib/server") + before = self.keys() + (self.java_home / "lib/server/libjvm.so").write_text("changed linked JVM") + self.assertNotEqual(before["binary-key"], self.keys()["binary-key"]) + + def test_rustup_proxies_hash_the_resolved_compiler(self): + """A changed real Rust tool invalidates the key even with unchanged proxy/version.""" + before = self.keys() + for tool in ("rustc", "cargo", "rustfmt"): + with self.subTest(tool=tool): + binary = self.rust_tool_path / tool + old = binary.read_text() + binary.write_text("changed underlying tool with the same version") + self.assertNotEqual(before["binary-key"], self.keys()["binary-key"]) + binary.write_text(old) + (self.rust_tool_path / "rustc").unlink() + with self.assertRaises(FileNotFoundError): + self.keys() + + def test_cargo_home_fallback_and_output_values(self): + """Use effective CARGO_HOME, or HOME/.cargo, and publish bounded opaque keys.""" + outputs = self.keys() + self.assertEqual(outputs["cargo-home"], str(self.cargo_home)) + self.assertTrue(outputs["source-key"].startswith(outputs["restore-prefix"])) + del self.env["CARGO_HOME"] + fallback = self.keys() + self.assertEqual(fallback["cargo-home"], str(Path(self.env["HOME"]) / ".cargo")) + for name, value in fallback.items(): + self.assertNotIn("\n", value) + self.assertLess(len(value), 512) + if name != "cargo-home": + self.assertNotIn("JAVA_HOME", value) + + def test_cli_publishes_outputs_only_after_complete_snapshot(self): + """Failed fingerprinting preserves GitHub outputs; success emits opaque keys.""" + output_file = self.directory / "github-output" + output_file.write_text("previous=value\n") + arguments = ["native-cache-key.py", "--profile", "ci", "--github-output", str(output_file)] + with patch.object(CACHE.sys, "argv", arguments), \ + patch.object(CACHE, "command", return_value=str(self.root).encode()), \ + patch.object(CACHE, "tracked_inputs", return_value=({}, {})), \ + patch.object(CACHE, "environment_identity", side_effect=ValueError("missing tool")), \ + patch.object(CACHE.sys, "stdout", new_callable=io.StringIO) as stdout, \ + patch.object(CACHE.sys, "stderr", new_callable=io.StringIO): + self.assertEqual(CACHE.main(), 1) + self.assertEqual(stdout.getvalue(), "") + self.assertEqual(output_file.read_text(), "previous=value\n") + with patch.object(CACHE.sys, "argv", arguments), \ + patch.object(CACHE, "command", return_value=str(self.root).encode()), \ + patch.object(CACHE, "tracked_inputs", return_value=({}, {})), \ + patch.object(CACHE, "environment_identity", return_value=(self.cargo_home, {})), \ + patch.object(CACHE.sys, "stdout", new_callable=io.StringIO) as stdout: + self.assertEqual(CACHE.main(), 0) + self.assertEqual(output_file.read_text(), "previous=value\n" + stdout.getvalue()) + self.assertIn("binary-key=Linux-native-ci-v1-", stdout.getvalue()) + + def test_missing_tool_and_jvm_fail_closed(self): + """Essential fingerprint failures never fall back to a partial identity.""" + with patch.object(CACHE.shutil, "which", return_value=None): + with self.assertRaisesRegex(ValueError, "missing required tool"): + CACHE.environment_identity(self.root, "ci", self.env) + (self.java_home / "lib/server/libjvm.so").unlink() + with self.assertRaises(FileNotFoundError): + self.keys() + + +if __name__ == "__main__": + unittest.main() diff --git a/dev/ci/test-native-cache-workflow.py b/dev/ci/test-native-cache-workflow.py new file mode 100644 index 00000000000..ef42da15543 --- /dev/null +++ b/dev/ci/test-native-cache-workflow.py @@ -0,0 +1,222 @@ +#!/usr/bin/env python3 +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Exercise the real native-cache action guards and shell steps without building Rust. + +Only remote cache operations and Cargo compilation are simulated. The real +manifest helper, shell commands, step order, and guards run from the checkout. +The extractor deliberately supports this action's formatting, not general YAML. +""" + +import ast +import os +from pathlib import Path +import re +import shutil +import subprocess +import tempfile +import unittest + + +ROOT = Path(__file__).resolve().parents[2] +ACTION = ROOT / ".github/actions/build-native-ci/action.yaml" +STEPS = dict(part.split("\n", 1) for part in + re.split(r"(?m)^ - name: ", ACTION.read_text())[1:]) +VALIDATE = "Validate cached native library" +BUILD = "Build native library (CI profile)" +PREPARE = "Prepare native library cache" +RESTORE_TARGET = "Restore incremental Cargo cache" +SAVE_BINARY = "Save native library cache" +SAVE_TARGET = "Save incremental Cargo cache" + + +def field(block, name, indent=6): + """Return one scalar field from a step block, or an empty string if absent. + + The supplied indentation distinguishes step fields from nested inputs. + This read-only extractor handles this action's single-line fields only. + """ + match = re.search(rf"(?m)^{' ' * indent}{re.escape(name)}: (.+)$", block) + return match.group(1) if match else "" + + +def condition(expression, context): + """Evaluate the action's comparisons/boolean operators against string outputs. + + Missing outputs become empty strings as in Actions. Unsupported syntax + raises ValueError instead of silently inventing new GitHub semantics. + Only literal comparisons and boolean operators are accepted; no calls run. + """ + expression = expression.removeprefix("${{ ").removesuffix(" }}") + expression = re.sub(r"\b(?:github\.[\w-]+|steps\.[\w-]+\.outputs\.[\w-]+)\b", + lambda match: repr(context.get(match.group(), "")), expression) + expression = expression.replace("&&", " and ").replace("||", " or ") + expression = re.sub(r"!(?!=)", " not ", expression).strip() + tree = ast.parse(expression or "True", mode="eval") + allowed = (ast.Expression, ast.BoolOp, ast.UnaryOp, ast.Compare, ast.Constant, + ast.And, ast.Or, ast.Not, ast.Eq, ast.NotEq) + if any(not isinstance(node, allowed) for node in ast.walk(tree)): + raise ValueError(f"Unsupported action guard: {expression}") + return eval(compile(tree, str(ACTION), "eval"), {"__builtins__": {}}, {}) + + +def shell_step(name, workspace, environment): + """Execute the named action's literal shell block in an isolated workspace. + + Apply its RUSTFLAGS override and return a captured subprocess result. + Commands use bash's Actions-style fail-fast flags; no real Cargo runs. + A missing multiline block raises ValueError before starting a process. + """ + block = STEPS[name] + match = re.search(r"(?m)^ run: \|\n((?: .*\n|\n)*)", block + "\n") + if not match: + raise ValueError(f"Missing literal shell block: {name}") + script = "\n".join(line[8:] for line in match.group(1).splitlines()) + environment = environment.copy() + flags = field(block, "RUSTFLAGS", 8) + if flags: + environment["RUSTFLAGS"] = flags.strip("'\"") + return subprocess.run(["bash", "--noprofile", "--norc", "-eo", "pipefail", "-c", script], + cwd=workspace, env=environment, capture_output=True, text=True) + + +def run_scenario(cache_hit="false", payload="missing", event="pull_request", + ref="refs/pull/123/merge", fail_cargo=False, target_hit="false"): + """Return observed operations/output for one simulated remote-cache scenario. + + A temporary workspace holds real manifest-helper inputs and a fake Cargo + executable. Remote actions record their invocation and supply cache outputs; + all other relevant steps execute their actual shell bodies. Failed commands + suppress subsequent steps, matching Actions' implicit success() guard. + The workspace and payloads are deleted before returning copied observations. + """ + with tempfile.TemporaryDirectory(prefix="comet-cache-workflow-") as temporary: + workspace = Path(temporary) + (workspace / "dev/ci").mkdir(parents=True) + shutil.copyfile(ROOT / "dev/ci/native-library-cache.py", + workspace / "dev/ci/native-library-cache.py") + library = workspace / "native/target/ci/libcomet.so" + library.parent.mkdir(parents=True) + executable = workspace / "bin/cargo" + executable.parent.mkdir() + executable.write_text("#!/bin/bash\nset -eu\n" + 'printf "%s|%s|%s\\n" "$PWD" "$*" "$RUSTFLAGS" >> "$CARGO_LOG"\n' + '[ "$FAIL_CARGO" = 0 ] || exit 23\n' + 'mkdir -p target/ci\nprintf built > target/ci/libcomet.so\n') + executable.chmod(0o755) + output = workspace / "step-output" + environment = dict(os.environ, PATH=f"{executable.parent}:{os.environ['PATH']}", + RUNNER_TEMP=str(workspace), NATIVE_CACHE_KEY="fixture-native-key", + GITHUB_OUTPUT=str(output), CARGO_LOG=str(workspace / "cargo-log"), + FAIL_CARGO=str(int(fail_cargo))) + if payload != "missing": + library.write_bytes(b"cached") + prepared = shell_step(PREPARE, workspace, environment) + if prepared.returncode: + raise AssertionError(prepared.stderr) + library.unlink() + if payload == "corrupt": + (workspace / "comet-native-library/libcomet.so").write_bytes(b"damaged") + elif payload == "wrong-key": + environment["NATIVE_CACHE_KEY"] = "different-native-key" + context = {"github.event_name": event, "github.ref": ref} + operations, successful, lookup_only = [], True, False + for name, block in STEPS.items(): + if name == "Fingerprint native build inputs": + continue # The dedicated key tests exercise tool/source fingerprinting. + if not successful or not condition(field(block, "if"), context): + continue + operations.append(name) + if field(block, "uses"): + if name == "Restore native library cache": + lookup_only = condition(field(block, "lookup-only", 8), context) + context["steps.binary-cache.outputs.cache-hit"] = cache_hit + elif name == RESTORE_TARGET: + context["steps.cargo-cache.outputs.cache-hit"] = target_hit + continue + output.write_text("") + result = shell_step(name, workspace, environment) + successful = result.returncode == 0 + for line in output.read_text().splitlines(): + key, value = line.split("=", 1) + context[f"steps.{field(block, 'id')}.outputs.{key}"] = value + log = workspace / "cargo-log" + return dict(operations=operations, successful=successful, lookup_only=lookup_only, + cargo=log.read_text() if log.exists() else "", + library=library.read_bytes() if library.exists() else None) + + +class NativeCacheWorkflowTest(unittest.TestCase): + """Check native reuse, compile fallback, and trusted cache ownership end to end.""" + + def test_exact_valid_hit_skips_compilation_and_target_archive(self): + """A verified exact hit installs cached bytes and avoids expensive target I/O.""" + result = run_scenario("true", "good") + self.assertTrue(result["successful"]) + self.assertEqual(result["library"], b"cached") + self.assertEqual(result["cargo"], "") + for step in (BUILD, RESTORE_TARGET, SAVE_TARGET, SAVE_BINARY, PREPARE): + self.assertNotIn(step, result["operations"]) + + def test_all_misses_compile_even_with_an_exact_target_cache(self): + """Missing, partial, damaged, and mismatched binary entries all run locked Cargo.""" + for hit, payload in (("", "missing"), ("false", "good"), ("true", "missing"), + ("true", "corrupt"), ("true", "wrong-key")): + with self.subTest(hit=hit, payload=payload): + result = run_scenario(hit, payload, target_hit="true") + self.assertTrue(result["successful"]) + self.assertEqual(result["library"], b"built") + self.assertIn(RESTORE_TARGET, result["operations"]) + self.assertRegex(result["cargo"], r"/native\|build --locked --profile ci\|") + self.assertIn("-Ctarget-cpu=x86-64-v3 -Clink-arg=-fuse-ld=bfd", result["cargo"]) + + def test_only_main_push_saves_caches(self): + """Equivalent cold builds save only for push-to-main, including manual/main cases.""" + for event, ref in (("pull_request", "refs/pull/123/merge"), + ("merge_group", "refs/heads/gh-readonly-queue/main/test"), + ("schedule", "refs/heads/main"), ("workflow_dispatch", "refs/heads/main"), + ("push", "refs/heads/feature"), ("push", "refs/heads/main")): + with self.subTest(event=event, ref=ref): + result = run_scenario(event=event, ref=ref) + writes = event == "push" and ref == "refs/heads/main" + self.assertTrue(result["successful"]) + for step in (PREPARE, SAVE_BINARY, SAVE_TARGET): + self.assertEqual(step in result["operations"], writes) + + def test_main_push_warms_target_even_when_binary_exists(self): + """Main does lookup-only for an existing binary, but still compiles and saves target.""" + result = run_scenario("true", "good", "push", "refs/heads/main") + self.assertTrue(result["lookup_only"]) + self.assertEqual(result["library"], b"built") + self.assertIn(BUILD, result["operations"]) + self.assertIn(SAVE_TARGET, result["operations"]) + for step in (VALIDATE, PREPARE, SAVE_BINARY): + self.assertNotIn(step, result["operations"]) + + def test_failed_cargo_cannot_prepare_or_save(self): + """A failed main build stops before publishing either incomplete cache entry.""" + result = run_scenario(event="push", ref="refs/heads/main", fail_cargo=True) + self.assertFalse(result["successful"]) + self.assertIsNone(result["library"]) + for step in (PREPARE, SAVE_BINARY, SAVE_TARGET): + self.assertNotIn(step, result["operations"]) + + +if __name__ == "__main__": + unittest.main() diff --git a/dev/ci/test-native-library-cache.py b/dev/ci/test-native-library-cache.py new file mode 100644 index 00000000000..05dc7496fdc --- /dev/null +++ b/dev/ci/test-native-library-cache.py @@ -0,0 +1,218 @@ +#!/usr/bin/env python3 +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Exercise native cache validation and failure handling without running a library.""" + +from contextlib import redirect_stderr, redirect_stdout +import hashlib +import importlib.util +import io +import json +import os +from pathlib import Path +import tempfile +import unittest +from unittest.mock import Mock, patch + + +SPEC = importlib.util.spec_from_file_location( + "native_library_cache", Path(__file__).with_name("native-library-cache.py")) +CACHE = importlib.util.module_from_spec(SPEC) +SPEC.loader.exec_module(CACHE) + + +class NativeLibraryCacheTest(unittest.TestCase): + """Use isolated, temporary cache/build paths; test bytes are never executable.""" + + def setUp(self): + """Create a multi-chunk source fixture; unittest owns directory cleanup.""" + temporary = tempfile.TemporaryDirectory(prefix="comet-native-cache-test-") + self.addCleanup(temporary.cleanup) + self.root = Path(temporary.name) + self.cache = self.root / "cache" + self.source = self.root / "built.so" + self.destination = self.root / "native" / "target" / "ci" / "libcomet.so" + self.output = self.root / "github-output" + self.key = "comet-native-v1-expected-input-fingerprint" + self.contents = b"native-library-test-bytes\x00" * 100000 + self.source.write_bytes(self.contents) + + def prepare(self): + """Populate this fixture's cache from its source, propagating failures.""" + CACHE.prepare(self.key, self.cache, self.source) + + def assert_miss(self, key=None): + """Require a successful CLI miss to delete an existing stale destination.""" + self.destination.parent.mkdir(parents=True, exist_ok=True) + self.destination.write_bytes(b"stale build must not be accepted") + self.output.write_text("existing=value\n", encoding="utf-8") + with redirect_stdout(io.StringIO()) as stdout: + result = CACHE.main([ + "restore", "--key", key or self.key, "--cache-dir", str(self.cache), + "--library", str(self.destination), "--github-output", str(self.output), + ]) + self.assertEqual(result, 0) + self.assertEqual(stdout.getvalue(), "hit=false\n") + self.assertEqual(self.output.read_text(), "existing=value\nhit=false\n") + self.assertFalse(self.destination.exists()) + self.assertEqual(list(self.destination.parent.iterdir()), []) + + def test_prepare_and_restore_exact_bytes(self): + """The CLI preserves bytes, records their SHA256, and appends a hit output.""" + self.assertEqual(CACHE.main([ + "prepare", "--key", self.key, "--cache-dir", str(self.cache), + "--library", str(self.source), + ]), 0) + manifest = json.loads((self.cache / "manifest.json").read_text()) + self.assertEqual(manifest, { + "key": self.key, "sha256": hashlib.sha256(self.contents).hexdigest(), + }) + self.assertEqual(sorted(path.name for path in self.cache.iterdir()), + ["libcomet.so", "manifest.json"]) + self.destination.parent.mkdir(parents=True) + self.destination.write_bytes(b"stale") + self.output.write_text("existing=value\n", encoding="utf-8") + with redirect_stdout(io.StringIO()) as stdout: + result = CACHE.main([ + "restore", "--key", self.key, "--cache-dir", str(self.cache), + "--library", str(self.destination), "--github-output", str(self.output), + ]) + self.assertEqual(result, 0) + self.assertEqual(stdout.getvalue(), "hit=true\n") + self.assertEqual(self.output.read_text(), "existing=value\nhit=true\n") + self.assertEqual(self.destination.read_bytes(), self.contents) + self.assertEqual(list(self.destination.parent.iterdir()), [self.destination]) + + def test_wrong_key_is_miss(self): + """A valid binary from different native inputs cannot be reused.""" + self.prepare() + self.assert_miss("comet-native-v1-different-input-fingerprint") + + def test_missing_cache_is_miss(self): + """A cold cache removes stale output and falls through to a fresh build.""" + self.assert_miss() + + def test_missing_entry_file_is_miss(self): + """Either absent cache file invalidates an otherwise complete entry.""" + for name in ("manifest.json", "libcomet.so"): + with self.subTest(name=name): + self.prepare() + (self.cache / name).unlink() + self.assert_miss() + + def test_corrupt_manifest_is_miss(self): + """Malformed JSON/UTF8, invalid shapes and missing fields are misses.""" + for contents in (b"{", b"\xff", b"null", b"[]", b"{}", + json.dumps({"key": self.key}).encode(), + json.dumps({"key": self.key, "sha256": 123}).encode()): + with self.subTest(contents=contents): + self.prepare() + (self.cache / "manifest.json").write_bytes(contents) + self.assert_miss() + + def test_corrupt_or_truncated_library_is_miss(self): + """The checksum rejects changed or incomplete bytes before installation.""" + for contents in (b"changed", b"", self.contents[:-1]): + with self.subTest(length=len(contents)): + self.prepare() + (self.cache / "libcomet.so").write_bytes(contents) + self.assert_miss() + + def test_symlinked_cache_files_are_misses(self): + """Even symlinks to matching bytes are excluded from the cache contract.""" + for name in ("manifest.json", "libcomet.so"): + with self.subTest(name=name): + self.prepare() + cached = self.cache / name + target = self.root / f"linked-{name}" + cached.replace(target) + cached.symlink_to(target) + self.assert_miss() + + def test_non_regular_cache_files_are_misses(self): + """Reject directories and FIFOs without blocking or accepting stale output.""" + for name in ("manifest.json", "libcomet.so"): + for kind in ("directory", "fifo"): + with self.subTest(name=name, kind=kind): + self.prepare() + cached = self.cache / name + cached.unlink() + if kind == "directory": + cached.mkdir() + else: + os.mkfifo(cached) + self.assert_miss() + if kind == "directory": + cached.rmdir() + else: + cached.unlink() + + def test_unreadable_cache_is_miss(self): + """A failed cache read is a miss even under a privileged test account.""" + self.prepare() + with patch.object(CACHE, "open_regular_file", side_effect=PermissionError("unreadable")): + self.assert_miss() + + def test_stale_destination_symlink_is_removed(self): + """A miss unlinks the old output without touching its symlink target.""" + self.destination.parent.mkdir(parents=True) + self.destination.symlink_to(self.source) + self.assertFalse(CACHE.restore(self.key, self.cache, self.destination)) + self.assertFalse(self.destination.is_symlink()) + self.assertEqual(self.source.read_bytes(), self.contents) + + def test_destination_write_failure_is_fatal(self): + """A verified cache cannot mask destination failures or leave partial output.""" + self.prepare() + self.destination.parent.mkdir(parents=True) + self.destination.write_bytes(b"stale") + with patch.object(Path, "replace", side_effect=OSError("disk failure")): + with redirect_stderr(io.StringIO()) as stderr: + result = CACHE.main([ + "restore", "--key", self.key, "--cache-dir", str(self.cache), + "--library", str(self.destination), "--github-output", str(self.output), + ]) + self.assertEqual(result, 1) + self.assertIn("disk failure", stderr.getvalue()) + self.assertFalse(self.output.exists()) + self.assertEqual(list(self.destination.parent.iterdir()), []) + + def test_failed_prepare_invalidates_previous_manifest(self): + """An interrupted refresh cannot retain a manifest advertising success.""" + self.prepare() + self.source.unlink() + with self.assertRaises(FileNotFoundError): + self.prepare() + self.assertFalse((self.cache / "manifest.json").exists()) + self.assert_miss() + + def test_read_failure_discards_temporary_copy(self): + """A mid-read failure preserves an old copy and cleans its temporary file.""" + self.destination.parent.mkdir(parents=True) + self.destination.write_bytes(b"old complete copy") + source = Mock() + source.read.side_effect = [b"partial", OSError("read failure")] + with self.assertRaises(ValueError): + CACHE.copy_library(source, self.destination) + self.assertEqual(self.destination.read_bytes(), b"old complete copy") + self.assertEqual(list(self.destination.parent.iterdir()), [self.destination]) + + +if __name__ == "__main__": + unittest.main() From 9d7cbeddcb03eec584aeaf56ade7cdbc47b467a2 Mon Sep 17 00:00:00 2001 From: Chao Sun Date: Wed, 16 Sep 2026 04:22:14 +0000 Subject: [PATCH 2/8] ci: trust the checked-out repository when fingerprinting native inputs --- dev/ci/native-cache-key.py | 9 +++++++-- dev/ci/test-native-cache-key.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/dev/ci/native-cache-key.py b/dev/ci/native-cache-key.py index 82c32f55ec1..fe847b3df43 100644 --- a/dev/ci/native-cache-key.py +++ b/dev/ci/native-cache-key.py @@ -126,8 +126,10 @@ def tracked_inputs(root, env): Read worktree bytes and modes, so staged or unstaged edits invalidate keys. The inventory excludes generated/untracked files and target directories. Missing tracked inputs, conflicts, symlinks, or submodules fail closed. + Trust this checkout for this command: container CI can run under a different + owner than checkout, whose temporary global Git configuration is not kept. """ - inventory = command(["git", "ls-files", "--stage", "-z"], root, env) + inventory = command(["git", "-c", f"safe.directory={root}", "ls-files", "--stage", "-z"], root, env) sources = {} dependencies = {} for record in inventory.split(b"\0"): @@ -287,6 +289,8 @@ def main(): --github-output optionally appends the same key=value records printed on stdout. Failure returns status 1 with a concise error and writes no outputs. + Invoke from the checkout root; Git trusts only that directory for these + reads without changing global configuration or trusting other checkouts. """ parser = argparse.ArgumentParser(description=__doc__) parser.add_argument("--profile", required=True, choices=("ci", "debug")) @@ -294,7 +298,8 @@ def main(): args = parser.parse_args() try: env = dict(os.environ) - root = Path(command(["git", "rev-parse", "--show-toplevel"], Path.cwd(), env) + cwd = Path.cwd().resolve() + root = Path(command(["git", "-c", f"safe.directory={cwd}", "rev-parse", "--show-toplevel"], cwd, env) .decode().strip()).resolve() dependencies, sources = tracked_inputs(root, env) cargo_home, environment = environment_identity(root, args.profile, env) diff --git a/dev/ci/test-native-cache-key.py b/dev/ci/test-native-cache-key.py index 6686aca77cb..28124cdcfac 100644 --- a/dev/ci/test-native-cache-key.py +++ b/dev/ci/test-native-cache-key.py @@ -300,6 +300,36 @@ def test_cargo_home_fallback_and_output_values(self): if name != "cargo-home": self.assertNotIn("JAVA_HOME", value) + def test_container_checkout_ownership_does_not_block_git_inventory(self): + """Read a foreign-owned checkout without changing persistent Git trust. + + Git's test switch reproduces the runner/container ownership mismatch + without requiring root. Real Git must reject the unconfigured checkout, + then the CLI must discover its root and tracked files successfully. + Only native tool discovery is mocked; global Git config stays untouched. + """ + global_config = self.directory / "global.gitconfig" + global_config.write_text("") + env = dict(os.environ, GIT_TEST_ASSUME_DIFFERENT_OWNER="1", + GIT_CONFIG_GLOBAL=str(global_config), GIT_CONFIG_NOSYSTEM="1") + untrusted = subprocess.run(["git", "rev-parse", "--show-toplevel"], + cwd=self.root, env=env, capture_output=True) + self.assertEqual(untrusted.returncode, 128) + self.assertIn(b"dubious ownership", untrusted.stderr) + arguments = ["native-cache-key.py", "--profile", "ci"] + with patch.dict(CACHE.os.environ, env, clear=True), \ + patch.object(CACHE.Path, "cwd", return_value=self.root), \ + patch.object(CACHE.sys, "argv", arguments), \ + patch.object(CACHE, "environment_identity", return_value=(self.cargo_home, {})), \ + patch.object(CACHE.sys, "stdout", new_callable=io.StringIO) as stdout, \ + patch.object(CACHE.sys, "stderr", new_callable=io.StringIO) as stderr: + self.assertEqual(CACHE.main(), 0, stderr.getvalue()) + dependencies, sources = CACHE.tracked_inputs(self.root, dict(os.environ)) + expected = CACHE.cache_keys("ci", dependencies, sources, {}, self.cargo_home) + actual = dict(line.split("=", 1) for line in stdout.getvalue().splitlines()) + self.assertEqual(actual, expected) + self.assertEqual(global_config.read_text(), "") + def test_cli_publishes_outputs_only_after_complete_snapshot(self): """Failed fingerprinting preserves GitHub outputs; success emits opaque keys.""" output_file = self.directory / "github-output" From 5099e8cc081812cba5a0519f3ee4a821c9acd10f Mon Sep 17 00:00:00 2001 From: Chao Sun Date: Wed, 16 Sep 2026 04:39:53 +0000 Subject: [PATCH 3/8] ci: simplify native caching around the fixed CI toolchain --- .github/actions/build-native-ci/action.yaml | 40 +- .github/workflows/README.md | 74 ++-- .github/workflows/ci.yml | 7 +- dev/ci/check-ci-config.py | 2 - dev/ci/compute-changes.py | 3 - dev/ci/native-cache-key.py | 318 ++++------------ dev/ci/native-library-cache.py | 176 --------- dev/ci/test-native-cache-key.py | 389 +++++--------------- dev/ci/test-native-cache-workflow.py | 222 ----------- dev/ci/test-native-library-cache.py | 218 ----------- 10 files changed, 188 insertions(+), 1261 deletions(-) delete mode 100644 dev/ci/native-library-cache.py delete mode 100644 dev/ci/test-native-cache-workflow.py delete mode 100644 dev/ci/test-native-library-cache.py diff --git a/.github/actions/build-native-ci/action.yaml b/.github/actions/build-native-ci/action.yaml index ff4d4369f0e..630e8e57bed 100644 --- a/.github/actions/build-native-ci/action.yaml +++ b/.github/actions/build-native-ci/action.yaml @@ -17,10 +17,6 @@ name: Build or restore the Linux CI native library description: 'Reuse an exact-input native library, otherwise build it with the CI profile' -outputs: - cache-hit: - description: 'Whether a validated cached library replaced compilation' - value: ${{ steps.library.outputs.hit == 'true' && 'true' || 'false' }} runs: using: composite steps: @@ -37,28 +33,15 @@ runs: id: binary-cache uses: actions/cache/restore@v6 with: - path: ${{ runner.temp }}/comet-native-library + path: native/target/ci/libcomet.so key: ${{ steps.key.outputs.binary-key }} # Main still builds to keep its incremental Cargo cache warm. Lookup # only avoids downloading a library that this run will not execute. lookup-only: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }} - - name: Validate cached native library - id: library - if: steps.binary-cache.outputs.cache-hit == 'true' && !(github.event_name == 'push' && github.ref == 'refs/heads/main') - shell: bash - env: - NATIVE_CACHE_KEY: ${{ steps.key.outputs.binary-key }} - run: | - python3 dev/ci/native-library-cache.py restore \ - --key "$NATIVE_CACHE_KEY" \ - --cache-dir "$RUNNER_TEMP/comet-native-library" \ - --library native/target/ci/libcomet.so \ - --github-output "$GITHUB_OUTPUT" - - name: Restore incremental Cargo cache id: cargo-cache - if: steps.library.outputs.hit != 'true' + if: steps.binary-cache.outputs.cache-hit != 'true' || (github.event_name == 'push' && github.ref == 'refs/heads/main') uses: actions/cache/restore@v6 with: path: | @@ -69,33 +52,22 @@ runs: restore-keys: ${{ steps.key.outputs.restore-prefix }} - name: Build native library (CI profile) - if: steps.library.outputs.hit != 'true' + if: steps.binary-cache.outputs.cache-hit != 'true' || (github.event_name == 'push' && github.ref == 'refs/heads/main') shell: bash env: RUSTFLAGS: '-Ctarget-cpu=x86-64-v3 -Clink-arg=-fuse-ld=bfd' run: | cd native # A target-directory cache is only an incremental build aid. Cargo - # must run even on an exact target-cache hit; only the validated, - # separately keyed binary cache can replace compilation. + # must run even on an exact target-cache hit; only the separately + # keyed library cache can replace compilation. cargo build --locked --profile ci - - name: Prepare native library cache - if: github.event_name == 'push' && github.ref == 'refs/heads/main' && steps.binary-cache.outputs.cache-hit != 'true' - shell: bash - env: - NATIVE_CACHE_KEY: ${{ steps.key.outputs.binary-key }} - run: | - python3 dev/ci/native-library-cache.py prepare \ - --key "$NATIVE_CACHE_KEY" \ - --cache-dir "$RUNNER_TEMP/comet-native-library" \ - --library native/target/ci/libcomet.so - - name: Save native library cache if: github.event_name == 'push' && github.ref == 'refs/heads/main' && steps.binary-cache.outputs.cache-hit != 'true' uses: actions/cache/save@v6 with: - path: ${{ runner.temp }}/comet-native-library + path: native/target/ci/libcomet.so key: ${{ steps.key.outputs.binary-key }} - name: Save incremental Cargo cache diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 7f2baeb66fd..55f4790ffff 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -406,53 +406,33 @@ which jobs do run on main and therefore do write. ## Reusing Linux native builds -The Linux, Spark SQL, Iceberg and manual Spark writer workflows use -`.github/actions/build-native-ci` after checkout and toolchain setup. It keeps -two separate caches: - -- A compact `libcomet.so` cache avoids compilation when every native build - input matches. It uses an exact key, with no fallback prefix. A manifest - records the input key and the library's SHA-256; a missing, partial or - invalid entry falls back to compilation. The library is staged at the same - `native/target/ci/libcomet.so` path as a fresh build, then uploaded under the - caller's existing artifact name. All downstream tests still run. -- The larger Cargo cache contains the actual `CARGO_HOME` registry/git - directories and `native/target`. A matching dependency prefix can seed a - build after native sources change. Restoring this cache always runs - `cargo build --locked --profile ci`; a target-directory cache hit alone - never authorizes reusing a binary without compiling. - -`dev/ci/native-cache-key.py` computes the keys once, before Cargo generates -Rust source files. It hashes tracked native and contrib files, shared JVM -inputs, build configuration and CI definitions, together with the resolved -Rust/C/C++/protobuf tools, JDK, installed system packages, architecture and -build environment. Generated files and untracked build output do not change -the save key. The CPU target remains explicitly `x86-64-v3`; binaries built -with `target-cpu=native` must not enter this cache. Unsupported external tool -or library overrides fail key generation rather than create an incomplete -identity. Ordinary changes confined to Spark sources can retain the same -native key; changing protobuf, toolchains or build flags cannot. - -Only pushes to `main` save these caches. PR, queue, nightly and manual runs -consume them without writing new entries. A main push still invokes Cargo, -even when the compact entry exists, to keep the larger compiler cache warm. -It skips re-saving an exact cache entry. The Rust test job uses the same key -snapshot and Cargo-home resolution with a separate debug profile, and still -runs every Rust check and test. - -The first main push after adoption populates the new cache namespace. Until -then, or after eviction, runs build normally. Cache reuse is scoped by GitHub's -cache access rules; it does not fetch a binary from an arbitrary PR or use the -latest main binary when inputs differ. The manifest detects corruption, while -the main-only write policy determines which builds can populate the cache. - -Preflight runs the native-key, compact-library and workflow-flow regression -tests. To verify a hosted hit, compare the native input key between a main -push and a later run with unchanged inputs: `Validate cached native library` -must report `hit=true`, the Cargo restore/build steps must skip, and the -normal library upload and downstream tests must succeed. A native or protobuf -edit must instead invoke Cargo. These timings depend on cache availability; -the change does not promise a fixed build-time reduction. +The Linux, Spark SQL, Iceberg and manual writer workflows call +`.github/actions/build-native-ci` after checkout and `setup-builder`. An exact +cache hit restores `native/target/ci/libcomet.so` and skips Cargo. A miss restores +an incremental cache and runs `cargo build --locked --profile ci`. Artifacts and +downstream tests use the same paths in either case. + +`dev/ci/native-cache-key.py` snapshots tracked native/protobuf/dependency files, +shared JVM inputs, build configuration and CI definitions before Cargo generates +source files. The key also includes Rust versions, installed system package +versions, architecture, JDK release/path and compiler flags. The helper targets +our official Rust container and `setup-builder`, not arbitrary local toolchains. +Spark-only edits and generated files preserve the key; native/protobuf changes +invalidate it. The shared action uses portable `x86-64-v3` code generation. + +The incremental cache contains the effective `CARGO_HOME` registry/git directories +and `native/target`. Its dependency prefix permits reuse after source changes, +but every restore still invokes Cargo. The Rust test job uses a separate debug +key and continues to run all checks and tests. + +Only pushes to `main` save either cache. Main always compiles to keep the +incremental cache warm. Other runs consume matching entries; a cold or evicted +cache builds normally. GitHub Actions handles cache storage and restoration. + +Preflight tests key invalidation, generated-file stability and container checkout +ownership. After main populates the new namespace, verify a hosted library hit +by checking that `Restore native library cache` reports an exact hit and the +Cargo steps skip, while the normal artifact upload and downstream tests pass. ## Retrying flaky network operations diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a6ea5f7a90d..d97e7e55aff 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -134,11 +134,8 @@ jobs: - name: Check Iceberg shard inventory validation run: python3 dev/ci/test-iceberg-shards.py - - name: Check native cache identity and reuse - run: | - python3 dev/ci/test-native-cache-key.py - python3 dev/ci/test-native-library-cache.py - python3 dev/ci/test-native-cache-workflow.py + - name: Check native cache keys + run: python3 dev/ci/test-native-cache-key.py - name: Check CI config invariants run: python3 dev/ci/check-ci-config.py diff --git a/dev/ci/check-ci-config.py b/dev/ci/check-ci-config.py index fb1dd2f7b40..0e0f9e5a1e0 100644 --- a/dev/ci/check-ci-config.py +++ b/dev/ci/check-ci-config.py @@ -133,8 +133,6 @@ # Helper edits also match macOS's broad dev/ci filter, as before. ([".github/actions/build-native-ci/action.yaml"], BUILD_JOBS - {"build_macos"}), (["dev/ci/native-cache-key.py"], BUILD_JOBS), - (["dev/ci/native-library-cache.py"], BUILD_JOBS), - (["dev/ci/test-native-cache-workflow.py"], BUILD_JOBS), # Spot checks that the additions above did not widen unrelated routes. (["docs/source/user-guide/overview.md"], {"docs"}), (["native/core/benches/parquet_read.rs"], {"benchmark"}), diff --git a/dev/ci/compute-changes.py b/dev/ci/compute-changes.py index 6df22af9782..89247955930 100644 --- a/dev/ci/compute-changes.py +++ b/dev/ci/compute-changes.py @@ -397,10 +397,7 @@ FILTERS[_native_consumer].extend([ ".github/actions/build-native-ci/**", "dev/ci/native-cache-key.py", - "dev/ci/native-library-cache.py", "dev/ci/test-native-cache-key.py", - "dev/ci/test-native-library-cache.py", - "dev/ci/test-native-cache-workflow.py", ]) FILTERS["spark_4_1_hive"] = FILTERS["spark_4_1"] diff --git a/dev/ci/native-cache-key.py b/dev/ci/native-cache-key.py index fe847b3df43..763471aae5d 100644 --- a/dev/ci/native-cache-key.py +++ b/dev/ci/native-cache-key.py @@ -16,12 +16,11 @@ # specific language governing permissions and limitations # under the License. -"""Snapshot Linux CI cache keys before Cargo creates generated sources. +"""Fingerprint the clean Linux checkout and toolchain used by Comet CI. -The dependency prefix permits Cargo to rebuild changed source incrementally. -The compact library key is exact-only: a hit permits skipping Cargo altogether. -Neither key contains the commit SHA, so unrelated Spark edits can reuse native -outputs. Unknown build environments fail before any outputs are written. +Run after setup-builder and before Cargo generates source files. This helper +supports the official Rust container, setup-builder's JDK/packages, and the +build commands in our workflows; it is not a general local-build cache. """ import argparse @@ -29,291 +28,108 @@ import json import os from pathlib import Path -import shlex -import shutil -import stat import subprocess -import sys -import tomllib SOURCE_PREFIXES = ("native/", "contrib/", "common/", ".cargo/", ".mvn/", ".github/actions/", ".github/workflows/", "dev/ci/") SOURCE_FILES = {"Makefile", "pom.xml", "mvnw", "rust-toolchain", "rust-toolchain.toml"} -# These can select arbitrary executable/source files outside the tracked input -# set. Supporting one requires adding its transitive inputs to the identity. -UNSUPPORTED_ENV = { - "RUSTC", "RUSTDOC", "RUSTC_WRAPPER", "RUSTC_WORKSPACE_WRAPPER", - "CARGO_ENCODED_RUSTFLAGS", "CARGO_TARGET_DIR", "CARGO_BUILD_TARGET", - "CARGO_BUILD_RUSTC", "CARGO_BUILD_RUSTDOC", "CARGO_BUILD_RUSTC_WRAPPER", - "CARGO_BUILD_RUSTC_WORKSPACE_WRAPPER", "CARGO_BUILD_TARGET_DIR", - "CC", "CXX", "AR", "LD", "PROTOC", "PROTOC_INCLUDE", "LIBCLANG_PATH", - "DOCS_RS", "HDFS_LIB_DIR", "HADOOP_HOME", "HDFS_STATIC", - "CFLAGS", "CXXFLAGS", "CPPFLAGS", "LDFLAGS", "LIBRARY_PATH", "CPATH", - "C_INCLUDE_PATH", "CPLUS_INCLUDE_PATH", "OBJC_INCLUDE_PATH", "LD_PRELOAD", - "CRATE_CC_NO_DEFAULTS", "CMAKE", "MAKE", "MAKEFLAGS", -} -BUILD_ENV_PREFIXES = ("CARGO_", "RUST", "CC_", "CXX_", "AR_", "CFLAGS", "CXXFLAGS", - "CPPFLAGS", "LDFLAGS", "BINDGEN_", "PKG_CONFIG", "OPENSSL_", - "ZSTD_", "LZ4_", "SNAPPY_", "HDFS_", "COMET_") -BUILD_ENV_NAMES = {"PATH", "JAVA_HOME", "CARGO_HOME", "HOME", "LIBRARY_PATH", - "LD_LIBRARY_PATH", "CPATH", "C_INCLUDE_PATH", "CPLUS_INCLUDE_PATH", - "SOURCE_DATE_EPOCH"} -TOOLS = { - "rustc": ("-vV",), "cargo": ("--version",), "rustfmt": ("--version",), - "protoc": ("--version",), - "cc": ("--version",), "c++": ("--version",), "clang": ("--version",), - "ld.bfd": ("--version",), "ar": ("--version",), "pkg-config": ("--version",), -} def digest(value): - """Return SHA-256 of a JSON-compatible value with stable map ordering. + """Return a stable SHA-256 for JSON-compatible build inputs.""" + return hashlib.sha256(json.dumps(value, sort_keys=True).encode()).hexdigest() - Values stay in memory; callers publish only the digest, never raw build - environment values, which may contain credentials in Cargo settings. - """ - return hashlib.sha256(json.dumps(value, sort_keys=True, separators=(",", ":")) - .encode("utf-8")).hexdigest() - - -def command(args, cwd, env): - """Return nonempty command stdout as bytes using an explicit cwd/environment. - - Nonzero status, missing tools, or empty output raises ValueError without - echoing stdout/stderr or environment values. No shell interpolation occurs. - """ - try: - result = subprocess.run(args, cwd=cwd, env=env, check=True, - stdout=subprocess.PIPE, stderr=subprocess.PIPE) - except (OSError, subprocess.CalledProcessError) as exc: - raise ValueError(f"cannot fingerprint required command {args[0]}") from exc - if not result.stdout.strip(): - raise ValueError(f"empty fingerprint from required command {args[0]}") - return result.stdout - - -def file_identity(path): - """Return mode and content digest for one required regular input file. - - Symlinks and missing files fail closed: hashing a link alone would omit - mutable external inputs. Files are read without modifying their contents. - """ - mode = path.lstat().st_mode - if not stat.S_ISREG(mode): - raise ValueError(f"unsupported non-regular input: {path}") - return [stat.S_IMODE(mode), hashlib.sha256(path.read_bytes()).hexdigest()] - - -def directory_identity(path): - """Return every regular file's relative name and digest below a required tree. - Directory symlinks are rejected as well as file symlinks, so an external JNI - include tree cannot silently escape the snapshot. The tree is read only. - """ - if path.is_symlink() or not path.is_dir(): - raise ValueError(f"unsupported input directory: {path}") - result = {} - for child in sorted(path.rglob("*")): - if child.is_symlink() or not child.is_dir(): - result[str(child.relative_to(path))] = file_identity(child) - return result +def command(args, cwd): + """Read command stdout in cwd; missing tools or unsuccessful commands fail CI.""" + return subprocess.check_output(args, cwd=cwd, text=True).strip() -def tracked_inputs(root, env): - """Return dependency and source snapshots from Git's tracked file inventory. +def source_inputs(root): + """Read tracked build files and return dependency and complete input maps. - Read worktree bytes and modes, so staged or unstaged edits invalidate keys. - The inventory excludes generated/untracked files and target directories. - Missing tracked inputs, conflicts, symlinks, or submodules fail closed. - Trust this checkout for this command: container CI can run under a different - owner than checkout, whose temporary global Git configuration is not kept. + Each map contains relative names, Git modes and content digests. Untracked + generated Rust and target files are excluded. Trust only this checkout for + the Git read: container steps can run as a different owner than checkout. """ - inventory = command(["git", "-c", f"safe.directory={root}", "ls-files", "--stage", "-z"], root, env) + inventory = command(["git", "-c", f"safe.directory={root}", + "ls-files", "--stage", "-z"], root) sources = {} - dependencies = {} - for record in inventory.split(b"\0"): + for record in inventory.split("\0"): if not record: continue - metadata, raw_path = record.split(b"\t", 1) - mode, _, stage = metadata.decode("ascii").split() - name = os.fsdecode(raw_path) - if name not in SOURCE_FILES and not name.startswith(SOURCE_PREFIXES): - continue - if stage != "0" or mode not in {"100644", "100755"}: - raise ValueError(f"unsupported tracked input: {name}") - identity = file_identity(root / name) - sources[name] = identity - if Path(name).name in {"Cargo.toml", "Cargo.lock"}: - dependencies[name] = identity - for name in ("native/Cargo.toml", "native/Cargo.lock"): - if name not in dependencies: - raise ValueError(f"missing tracked build input: {name}") + metadata, name = record.split("\t", 1) + if name in SOURCE_FILES or name.startswith(SOURCE_PREFIXES): + sources[name] = [metadata.split()[0], + hashlib.sha256((root / name).read_bytes()).hexdigest()] + dependencies = {name: value for name, value in sources.items() + if Path(name).name in {"Cargo.toml", "Cargo.lock"}} return dependencies, sources -def cargo_config_directories(root, cargo_home): - """Return Cargo's ordered search locations without reading or writing them. - - Cargo starts at the native workspace and visits ancestor .cargo directories, - plus CARGO_HOME. Returning a set in stable order avoids duplicate reads. - """ - directories = {cargo_home} - directories.update(path / ".cargo" for path in (root / "native", root, *root.parents)) - return sorted(directories) - - -def cargo_configs(root, cargo_home): - """Return content identities for Cargo configs in its search locations. +def environment_inputs(root, env): + """Identify the official tools installed by setup-builder without modifying them. - Include both supported filenames from native/ through filesystem root and - CARGO_HOME, including untracked external configs. Reject config features - that refer to extra executable/source files outside this input snapshot; - adding support for them requires extending this fingerprint first. + Rust's versions include the compiler commit; dpkg identifies the installed + C/C++/protobuf tools and system libraries. The JDK release file identifies + the vendor/build supplying JNI headers and libjvm. Paths and RUSTFLAGS are + included because linking can embed them. Workflow/action files in the source + map cover changes to how these tools are installed and invoked. """ - result = {} - for directory in cargo_config_directories(root, cargo_home): - for name in ("config", "config.toml"): - path = directory / name - if not path.exists(): - continue - content = tomllib.loads(path.read_text(encoding="utf-8")) - # Registry/transport settings do not select source files outside - # Cargo.lock. Build flags are content-addressed; wrappers, custom - # targets/linkers, source replacement, and config includes are not. - if set(content) - {"build", "net", "http", "registries", "registry"}: - raise ValueError(f"unsupported Cargo config section: {path}") - if set(content.get("build", {})) - {"rustflags", "rustdocflags", "jobs", "incremental"}: - raise ValueError(f"unsupported Cargo build configuration: {path}") - if "target-cpu=native" in path.read_text(encoding="utf-8"): - raise ValueError(f"host-specific CPU flags are not reusable: {path}") - result[str(path)] = file_identity(path) - return result - - -def environment_identity(root, profile, env): - """Return the required Linux toolchain, platform, JDK and build-env snapshot. - - The supplied mapping is the caller's effective environment before building. - This supports the repository's fixed Linux CI commands, not arbitrary local - tool overrides. Missing fingerprints raise before a reusable key can exist. - Package versions cover linker/compiler libraries in the mutable CI image. - Compiler/include/library overrides are rejected because hashing a path or - flag such as `-include /tmp/header.h` does not fingerprint the file it reads. - The only extra library search path supported is the fingerprinted JDK's - server directory. Rustup proxies are resolved to their actual tool binaries. - """ - for name in UNSUPPORTED_ENV: - if env.get(name): - raise ValueError(f"unsupported build override: {name}") - for name in env: - if name.startswith(("CARGO_TARGET_", "CARGO_PROFILE_", "CC_", "CXX_", "AR_", - "CFLAGS_", "CXXFLAGS_", "CPPFLAGS_", "LDFLAGS_", "HDFS_", - "HOST_CC", "HOST_CXX", "HOST_AR", "HOST_CFLAGS", "HOST_CXXFLAGS", - "TARGET_CC", "TARGET_CXX", "TARGET_AR", "TARGET_CFLAGS", "TARGET_CXXFLAGS", - "CMAKE_", "HOST_CMAKE", "TARGET_CMAKE", - "OPENSSL_", "PKG_CONFIG", "BINDGEN_", "ZSTD_", "LZ4_", "SNAPPY_")): - raise ValueError(f"unsupported build override: {name}") - if name.startswith("CARGO_BUILD_") and name not in {"CARGO_BUILD_JOBS", "CARGO_BUILD_INCREMENTAL"}: - raise ValueError(f"unsupported build override: {name}") - flags = shlex.split(env.get("RUSTFLAGS", "")) - if profile == "ci" and flags != ["-Ctarget-cpu=x86-64-v3", "-Clink-arg=-fuse-ld=bfd"]: - raise ValueError("CI library reuse requires the fixed x86-64-v3/bfd RUSTFLAGS") - if profile == "debug" and flags != ["-Clink-arg=-fuse-ld=bfd"]: - raise ValueError("debug cache reuse requires the fixed bfd RUSTFLAGS") - if not env.get("JAVA_HOME"): - raise ValueError("JAVA_HOME is required") - java_home = Path(env["JAVA_HOME"]).resolve(strict=True) - library_path = env.get("LD_LIBRARY_PATH", "") - if library_path and library_path not in {str(java_home / "lib/server"), - str(Path(env["JAVA_HOME"]) / "lib/server")}: - raise ValueError("unsupported build override: LD_LIBRARY_PATH") - if not (java_home / "include/jni.h").is_file(): - raise ValueError("JAVA_HOME must contain JNI headers") - cargo_home = Path(env.get("CARGO_HOME") or str(Path(env["HOME"]) / ".cargo")).resolve() - if "\n" in str(cargo_home) or "\r" in str(cargo_home): - raise ValueError("CARGO_HOME must fit one GitHub output line") - versions = {} - for tool, args in TOOLS.items(): - executable = shutil.which(tool, path=env.get("PATH")) - if not executable: - raise ValueError(f"missing required tool: {tool}") - launcher = Path(executable).resolve(strict=True) - resolved = launcher - if tool in {"rustc", "cargo", "rustfmt"}: - resolved = Path(command(["rustup", "which", tool], root / "native", env).decode().strip()) - if not resolved.is_absolute(): - raise ValueError(f"rustup returned a non-absolute path for {tool}") - resolved = resolved.resolve(strict=True) - versions[tool] = { - "launcher_path": str(launcher), "launcher": file_identity(launcher), - "path": str(resolved), "binary": file_identity(resolved), - "version": command([tool, *args], root / "native", env).decode("utf-8"), - } - system = command(["uname", "-s"], root, env).decode().strip() - architecture = command(["uname", "-m"], root, env).decode().strip() - if system != "Linux" or architecture != "x86_64": - raise ValueError("native cache identity supports Linux x86_64 only") - packages = command(["dpkg-query", "-W", "-f=${binary:Package}\t${Version}\t${Architecture}\n"], - root, env).decode("utf-8").splitlines() - return cargo_home, { - "profile": profile, "root": str(root), "system": system, - "architecture": architecture, "packages": sorted(packages), "tools": versions, - "java_home": str(java_home), "java_release": file_identity(java_home / "release"), - "libjvm": file_identity(java_home / "lib/server/libjvm.so"), - "jni_headers": directory_identity(java_home / "include"), - "cargo_configs": cargo_configs(root, cargo_home), - "environment": {key: value for key, value in env.items() - if key in BUILD_ENV_NAMES or key.startswith(BUILD_ENV_PREFIXES)}, + java_home = Path(env["JAVA_HOME"]) + return { + "workspace": str(root), + "architecture": command(["uname", "-m"], root), + "rust": {tool: command([tool, flag], root / "native") + for tool, flag in (("rustc", "-vV"), ("cargo", "--version"), + ("rustfmt", "--version"))}, + "packages": sorted(command(["dpkg-query", "-W", + "-f=${binary:Package}\t${Version}\t${Architecture}\n"], root).splitlines()), + "java_home": str(java_home), + "java_release": (java_home / "release").read_text(), + "cargo_home": env.get("CARGO_HOME", str(Path.home() / ".cargo")), + "rustflags": env["RUSTFLAGS"], } -def cache_keys(profile, dependencies, sources, environment, cargo_home): - """Return immutable GitHub output strings for a complete input snapshot. +def cache_keys(profile, dependencies, sources, environment): + """Return output keys for one pre-build snapshot. - dependency-key includes environment and manifests; source-key additionally - includes tracked source/build files. Only ci has a usable exact binary-key. - The restore-prefix intentionally excludes source so Cargo can rebuild it. + Only the incremental Cargo cache has a source-independent restore prefix. + The library key includes all tracked build inputs and never uses fallback. """ - dependency_key = f"Linux-cargo-{profile}-v2-{digest([environment, dependencies])}" - source_key = f"{dependency_key}-{digest(sources)}" + prefix = f"Linux-cargo-{profile}-v3-{digest([environment, dependencies])}-" return { - "cargo-home": str(cargo_home), "dependency-key": dependency_key, - "source-key": source_key, "restore-prefix": f"{dependency_key}-", - "binary-key": f"Linux-native-ci-v1-{digest([environment, sources])}" if profile == "ci" else "", + "cargo-home": environment["cargo_home"], + "source-key": prefix + digest(sources), + "restore-prefix": prefix, + "binary-key": f"Linux-native-ci-v2-{digest([environment, sources])}" if profile == "ci" else "", } def main(): - """Snapshot keys at repository root and publish them only after full success. + """Snapshot the checkout root and publish keys after all reads succeed. - --github-output optionally appends the same key=value records printed on - stdout. Failure returns status 1 with a concise error and writes no outputs. - Invoke from the checkout root; Git trusts only that directory for these - reads without changing global configuration or trusting other checkouts. + Run from the repository root. --github-output appends the same key=value + records printed to stdout. Git/tool/file failures propagate and fail CI. """ parser = argparse.ArgumentParser(description=__doc__) parser.add_argument("--profile", required=True, choices=("ci", "debug")) parser.add_argument("--github-output", type=Path) args = parser.parse_args() - try: - env = dict(os.environ) - cwd = Path.cwd().resolve() - root = Path(command(["git", "-c", f"safe.directory={cwd}", "rev-parse", "--show-toplevel"], cwd, env) - .decode().strip()).resolve() - dependencies, sources = tracked_inputs(root, env) - cargo_home, environment = environment_identity(root, args.profile, env) - keys = cache_keys(args.profile, dependencies, sources, environment, cargo_home) - output = "".join(f"{key}={value}\n" for key, value in keys.items()) - if args.github_output: - with args.github_output.open("a", encoding="utf-8") as stream: - stream.write(output) - print(output, end="") - except (OSError, ValueError, KeyError) as exc: - print(f"Native cache identity unavailable: {exc}", file=sys.stderr) - return 1 - return 0 + cwd = Path.cwd().resolve() + root = Path(command(["git", "-c", f"safe.directory={cwd}", + "rev-parse", "--show-toplevel"], cwd)) + dependencies, sources = source_inputs(root) + environment = environment_inputs(root, os.environ) + keys = cache_keys(args.profile, dependencies, sources, environment) + output = "".join(f"{key}={value}\n" for key, value in keys.items()) + if args.github_output: + with args.github_output.open("a") as stream: + stream.write(output) + print(output, end="") if __name__ == "__main__": - sys.exit(main()) + main() diff --git a/dev/ci/native-library-cache.py b/dev/ci/native-library-cache.py deleted file mode 100644 index e6c20fc4f2a..00000000000 --- a/dev/ci/native-library-cache.py +++ /dev/null @@ -1,176 +0,0 @@ -#!/usr/bin/env python3 -# -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. - -"""Prepare or restore one Linux libcomet.so cache entry without executing it. - -The workflow supplies the complete native-input key and controls which runs may -save caches. A checksum detects incomplete or corrupted entries; it is not a -substitute for restricting cache writers to trusted builds. Restore treats bad -cache contents as a miss, removes any previous destination, and publishes -hit=true only after installing all verified bytes. Destination I/O failures are -fatal so permission or disk failures cannot masquerade as cache misses. -""" - -import argparse -import hashlib -import json -import os -from pathlib import Path -import stat -import sys -import tempfile - - -def open_regular_file(path): - """Return an owned binary stream for a Linux regular, non-symlink file. - - The caller closes the stream. O_NOFOLLOW rejects symlinks, and O_NONBLOCK - prevents a malformed cache FIFO from hanging before fstat can reject it. - Missing/unreadable paths raise OSError; other file types raise ValueError. - """ - descriptor = os.open(path, os.O_RDONLY | os.O_NOFOLLOW | os.O_NONBLOCK) - try: - if not stat.S_ISREG(os.fstat(descriptor).st_mode): - raise ValueError(f"Not a regular file: {path}") - return os.fdopen(descriptor, "rb") - except BaseException: - os.close(descriptor) - raise - - -def copy_library(source, destination, expected_sha256=None): - """Atomically copy an open binary stream to destination and return its SHA256. - - Reads start at the stream's current offset; ownership stays with the caller. - If supplied, expected_sha256 must match the copied bytes before replacement. - Bad source reads/checksums raise ValueError; destination I/O errors propagate. - Temporary files are always removed and an incomplete copy is never installed. - """ - destination.parent.mkdir(parents=True, exist_ok=True) - temporary = None - try: - with tempfile.NamedTemporaryFile(dir=destination.parent, delete=False) as output: - temporary = Path(output.name) - digest = hashlib.sha256() - while True: - try: - chunk = source.read(1024 * 1024) - except OSError as error: - raise ValueError("Cannot read native library") from error - if not chunk: - break - digest.update(chunk) - output.write(chunk) - checksum = digest.hexdigest() - if expected_sha256 is not None and checksum != expected_sha256: - raise ValueError("Native library checksum mismatch") - temporary.replace(destination) - return checksum - finally: - if temporary is not None: - temporary.unlink(missing_ok=True) - - -def prepare(key, cache_dir, library): - """Write libcomet.so and its key/checksum manifest into cache_dir. - - library is a complete build output; all paths are pathlib Paths. Both files - are installed atomically, with the manifest last. A prior manifest is removed - first so an interrupted refresh cannot advertise an old successful entry. - Returns nothing; invalid source files and all write failures propagate. - """ - cache_dir.mkdir(parents=True, exist_ok=True) - manifest = cache_dir / "manifest.json" - manifest.unlink(missing_ok=True) - with open_regular_file(library) as source: - checksum = copy_library(source, cache_dir / "libcomet.so") - temporary = None - try: - with tempfile.NamedTemporaryFile( - mode="w", encoding="utf-8", dir=cache_dir, delete=False) as output: - temporary = Path(output.name) - json.dump({"key": key, "sha256": checksum}, output, sort_keys=True) - output.write("\n") - temporary.replace(manifest) - finally: - if temporary is not None: - temporary.unlink(missing_ok=True) - - -def restore(key, cache_dir, library): - """Install a matching cached library and return True, or return False on a miss. - - key is the exact native-input fingerprint; paths are pathlib Paths. Removes - the previous library before checking the cache, including dangling symlinks. - Missing, unreadable, malformed, symlinked or mismatched cache data is a miss. - Directory creation, destination removal and write failures propagate so the - workflow fails instead of using stale output. No cached code is executed. - """ - library.unlink(missing_ok=True) - try: - with open_regular_file(cache_dir / "manifest.json") as source: - manifest = json.load(source) - if not isinstance(manifest, dict) or manifest.get("key") != key: - return False - checksum = manifest.get("sha256") - if not isinstance(checksum, str) or len(checksum) != 64: - return False - source = open_regular_file(cache_dir / "libcomet.so") - except (OSError, ValueError): - return False - with source: - try: - copy_library(source, library, checksum) - except ValueError: - return False - return True - - -def main(argv): - """Run prepare/restore using CLI arguments and return a process exit status. - - Restore appends hit=true/false to --github-output and prints the same value. - Invalid cache data is a successful miss; operational failures return 1. - Output-file write failures also fail the command rather than report a hit. - """ - parser = argparse.ArgumentParser(description=__doc__) - parser.add_argument("operation", choices=("prepare", "restore")) - parser.add_argument("--key", required=True) - parser.add_argument("--cache-dir", required=True, type=Path) - parser.add_argument("--library", required=True, type=Path) - parser.add_argument("--github-output", type=Path) - args = parser.parse_args(argv) - try: - if args.operation == "prepare": - prepare(args.key, args.cache_dir, args.library) - else: - hit = restore(args.key, args.cache_dir, args.library) - result = f"hit={str(hit).lower()}\n" - if args.github_output: - with args.github_output.open("a", encoding="utf-8") as output: - output.write(result) - print(result, end="") - except (OSError, ValueError) as error: - print(f"error: {error}", file=sys.stderr) - return 1 - return 0 - - -if __name__ == "__main__": - sys.exit(main(sys.argv[1:])) diff --git a/dev/ci/test-native-cache-key.py b/dev/ci/test-native-cache-key.py index 28124cdcfac..1b5e7413401 100644 --- a/dev/ci/test-native-cache-key.py +++ b/dev/ci/test-native-cache-key.py @@ -16,14 +16,14 @@ # specific language governing permissions and limitations # under the License. -"""Check cache invalidation boundaries without installing native build tools.""" +"""Check native cache boundaries and container checkout ownership with real Git.""" import importlib.util import io import os from pathlib import Path import subprocess -import shutil +import sys import tempfile import unittest from unittest.mock import patch @@ -35,332 +35,115 @@ class NativeCacheKeyTests(unittest.TestCase): - """Exercise tracked inputs and mocked toolchains in disposable Git repos.""" + """Use disposable Git repositories and mock only installed tool versions.""" def setUp(self): - """Create tracked build inputs and fake tools/JDK owned by this test. - - Git inventory is real; tool version queries alone are mocked. Cleanup - removes the entire temporary tree even when an assertion fails. - """ + """Create tracked native/JVM fixtures and JDK metadata; clean up after each test.""" temporary = tempfile.TemporaryDirectory() self.addCleanup(temporary.cleanup) - self.directory = Path(temporary.name) - self.root = self.directory / "repo" - self.root.mkdir() - self.git("init", "--quiet") - self.inputs = { - "native/Cargo.toml": "[workspace]\nmembers = []\n", - "native/Cargo.lock": "version = 4\n", - "native/core/src/lib.rs": "pub fn value() -> i32 { 1 }\n", - "native/proto/src/proto/expr.proto": 'syntax = "proto3";\n', - "native/proto/build.rs": "fn main() {}\n", - "contrib/lance/native/Cargo.toml": "[package]\nname = 'lance'\n", - "common/src/main/java/Native.java": "class Native {}\n", - "pom.xml": "\n", - "rust-toolchain.toml": "[toolchain]\nchannel = 'stable'\n", - ".github/actions/build-native-ci/action.yaml": "runs: {}\n", - "dev/ci/native-cache-key.py": "# identity implementation\n", - "spark/src/main/scala/Plan.scala": "object Plan {}\n", - "README.md": "# Comet\n", - } + self.root = Path(temporary.name) + subprocess.run(["git", "init", "--quiet", str(self.root)], check=True) + self.inputs = {"native/Cargo.toml": "[workspace]\n", "native/Cargo.lock": "version = 4\n", + "native/lib.rs": "fn native() {}\n", "native/proto/expr.proto": "message Expr {}\n", + "spark/Plan.scala": "object Plan {}\n", "README.md": "Comet\n"} for name, content in self.inputs.items(): - self.write(self.root / name, content) - self.git("add", ".") - self.java_home = self.directory / "jdk" - self.write(self.java_home / "release", 'JAVA_VERSION="17.0.1"\n') - self.write(self.java_home / "lib/server/libjvm.so", "fake JVM library") - self.write(self.java_home / "include/jni.h", "fake JNI headers") - self.cargo_home = self.directory / "cargo" - self.cargo_home.mkdir() - locations = CACHE.cargo_config_directories - config_patch = patch.object(CACHE, "cargo_config_directories", - side_effect=lambda root, home: [path for path in locations(root, home) - if path.is_relative_to(self.directory)]) - config_patch.start() - self.addCleanup(config_patch.stop) - self.tool_path = self.directory / "tools" - self.rust_tool_path = self.directory / "toolchain/bin" - for tool in CACHE.TOOLS: - self.write(self.tool_path / tool, "fake tool binary " + tool) - for tool in ("rustc", "cargo", "rustfmt"): - self.write(self.rust_tool_path / tool, "fake resolved tool binary " + tool) - self.env = { - "HOME": str(self.directory / "home"), "PATH": os.environ["PATH"], - "JAVA_HOME": str(self.java_home), "CARGO_HOME": str(self.cargo_home), - "RUSTFLAGS": "-Ctarget-cpu=x86-64-v3 -Clink-arg=-fuse-ld=bfd", - } - self.versions = {name: f"{name} version 1\n".encode() for name in CACHE.TOOLS} - self.versions.update({"uname -s": b"Linux\n", "uname -m": b"x86_64\n", - "dpkg-query": b"libc6\t1.0\tamd64\n"}) - - def git(self, *args): - """Run Git against the test repository; setup errors fail the test.""" - return subprocess.run(["git", *args], cwd=self.root, check=True, - stdout=subprocess.PIPE, stderr=subprocess.PIPE) - - def write(self, path, content): - """Write fixture text, creating parents; mutation stays inside the temp tree.""" + self.write(name, content) + subprocess.run(["git", "add", "."], cwd=self.root, check=True) + self.write("jdk/release", 'JAVA_VERSION="17.0.1"\n') + self.env = {"JAVA_HOME": str(self.root / "jdk"), "CARGO_HOME": str(self.root / "cargo"), + "RUSTFLAGS": "-Ctarget-cpu=x86-64-v3 -Clink-arg=-fuse-ld=bfd"} + self.versions = {"rustc": "rustc 1.90\nhost: x86_64-unknown-linux-gnu\n", + "cargo": "cargo 1.90\n", "rustfmt": "rustfmt 1.8\n", + "dpkg-query": "libc6\t2.40\tamd64\n", "uname": "x86_64\n"} + + def write(self, name, content): + """Write fixture text under the temporary repository, creating its parents.""" + path = self.root / name path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(content, encoding="utf-8") - - def fake_command(self, args, cwd, env): - """Return fixture output for a required tool or fail on an unexpected query.""" - if args[:2] == ["rustup", "which"]: - return str(self.rust_tool_path / args[2]).encode() - name = " ".join(args) if args[0] == "uname" else args[0] - return self.versions[name] + path.write_text(content) def keys(self, profile="ci"): - """Snapshot real files with a fake Linux toolchain and return output keys.""" - dependencies, sources = CACHE.tracked_inputs(self.root, dict(os.environ)) - with patch.object(CACHE, "command", side_effect=self.fake_command), \ - patch.object(CACHE.shutil, "which", side_effect=lambda name, path: str(self.tool_path / name)): - cargo_home, environment = CACHE.environment_identity(self.root, profile, self.env) - return CACHE.cache_keys(profile, dependencies, sources, environment, cargo_home) - - def test_native_and_build_inputs_invalidate_exact_keys(self): - """Rust/proto/JNI/build edits invalidate binaries while preserving dependency reuse.""" + """Return keys from real tracked files and deterministic tool version responses.""" + dependencies, sources = CACHE.source_inputs(self.root) + with patch.object(CACHE, "command", side_effect=lambda args, cwd: self.versions[args[0]]): + environment = CACHE.environment_inputs(self.root, self.env) + return CACHE.cache_keys(profile, dependencies, sources, environment) + + def test_source_and_dependency_changes_invalidate_the_right_keys(self): + """Native/protobuf edits retain the dependency prefix; dependency edits replace it.""" before = self.keys() - for name in ("native/core/src/lib.rs", "native/proto/src/proto/expr.proto", - "native/proto/build.rs", "common/src/main/java/Native.java", "pom.xml", - "rust-toolchain.toml", ".github/actions/build-native-ci/action.yaml", - "dev/ci/native-cache-key.py"): + for name in ("native/lib.rs", "native/proto/expr.proto", "native/Cargo.toml", "native/Cargo.lock"): with self.subTest(name=name): - self.write(self.root / name, self.inputs[name] + "\n# changed\n") + self.write(name, self.inputs[name] + "changed\n") after = self.keys() self.assertNotEqual(before["source-key"], after["source-key"]) self.assertNotEqual(before["binary-key"], after["binary-key"]) - self.assertEqual(before["restore-prefix"], after["restore-prefix"]) - self.write(self.root / name, self.inputs[name]) - - def test_dependency_edits_invalidate_incremental_prefix(self): - """Manifest/lockfile changes isolate both source and dependency caches.""" - before = self.keys() - for name in ("native/Cargo.toml", "native/Cargo.lock", "contrib/lance/native/Cargo.toml"): - with self.subTest(name=name): - self.write(self.root / name, self.inputs[name] + "\n# changed\n") - after = self.keys() - self.assertNotEqual(before["restore-prefix"], after["restore-prefix"]) - self.assertNotEqual(before["binary-key"], after["binary-key"]) - self.write(self.root / name, self.inputs[name]) - - def test_unrelated_jvm_and_untracked_generated_files_do_not_invalidate(self): - """Spark/docs edits and generated protobuf/target files preserve reuse.""" + if name.endswith(("Cargo.toml", "Cargo.lock")): + self.assertNotEqual(before["restore-prefix"], after["restore-prefix"]) + else: + self.assertEqual(before["restore-prefix"], after["restore-prefix"]) + self.write(name, self.inputs[name]) + + def test_generated_files_and_unrelated_jvm_edits_preserve_keys(self): + """Untracked generated Rust/artifacts and unrelated JVM/docs edits permit native reuse.""" before = self.keys() - self.write(self.root / "spark/src/main/scala/Plan.scala", "object NewPlan {}\n") - self.write(self.root / "README.md", "new docs\n") - self.write(self.root / "native/proto/src/generated/expr.rs", "generated Rust") - self.write(self.root / "native/target/ci/libcomet.so", "built artifact") + self.write("native/proto/src/generated/expr.rs", "generated Rust") + self.write("native/target/ci/libcomet.so", "compiled library") + self.write("spark/Plan.scala", "object NewPlan {}") + self.write("README.md", "updated docs") self.assertEqual(before, self.keys()) - def test_tracked_addition_deletion_and_executable_mode(self): - """New/deleted native files and mode changes cannot keep an exact hit.""" - before = self.keys() - new = self.root / "native/core/src/new.rs" - self.write(new, "new tracked source") - self.git("add", "native/core/src/new.rs") - self.assertNotEqual(before["binary-key"], self.keys()["binary-key"]) - self.git("rm", "--cached", "native/core/src/new.rs") - source = self.root / "native/core/src/lib.rs" - source.chmod(0o755) - self.assertNotEqual(before["binary-key"], self.keys()["binary-key"]) - source.chmod(0o644) - source.unlink() - with self.assertRaises(FileNotFoundError): - self.keys() - self.git("rm", "--cached", "native/core/src/lib.rs") - self.assertNotEqual(before["binary-key"], self.keys()["binary-key"]) - - def test_tool_jdk_and_platform_changes_invalidate_all_caches(self): - """Tool/package versions, JVM bytes and JVM paths enter the environment key.""" + def test_tools_jdk_flags_and_tracked_build_configuration_invalidate(self): + """Observed tool/package versions, Java metadata, flags and tracked configs enter keys.""" before = self.keys() for tool in self.versions: - if tool.startswith("uname"): - continue with self.subTest(tool=tool): old = self.versions[tool] - self.versions[tool] += b"changed version\n" - after = self.keys() - self.assertNotEqual(before["restore-prefix"], after["restore-prefix"]) - self.assertNotEqual(before["binary-key"], after["binary-key"]) - self.versions[tool] = old - for name in ("release", "lib/server/libjvm.so", "include/jni.h"): - with self.subTest(jdk_input=name): - path = self.java_home / name - old = path.read_text() - path.write_text(old + "changed\n") + self.versions[tool] += "changed\n" + self.assertNotEqual(before["source-key"], self.keys()["source-key"]) self.assertNotEqual(before["binary-key"], self.keys()["binary-key"]) - path.write_text(old) - new_java_home = self.directory / "other-jdk" - shutil.copytree(self.java_home, new_java_home) - self.env["JAVA_HOME"] = str(new_java_home) + self.versions[tool] = old + self.write("jdk/release", 'JAVA_VERSION="17.0.2"\n') self.assertNotEqual(before["binary-key"], self.keys()["binary-key"]) - self.env["JAVA_HOME"] = str(self.java_home) - (self.tool_path / "cc").write_text("same version, different executable") + self.write("jdk/release", 'JAVA_VERSION="17.0.1"\n') + self.env["RUSTFLAGS"] += " -Copt-level=1" self.assertNotEqual(before["binary-key"], self.keys()["binary-key"]) - self.versions["uname -m"] = b"aarch64\n" - with self.assertRaisesRegex(ValueError, "Linux x86_64 only"): - self.keys() - - def test_external_and_ancestor_cargo_configs_invalidate(self): - """Cargo home and ancestor configs matter even though Git cannot list them.""" - before = self.keys() - for directory in (self.cargo_home, self.directory / ".cargo", self.root / ".cargo"): - with self.subTest(directory=directory): - config = directory / "config.toml" - self.write(config, "[build]\nincremental = false\n") - after = self.keys() - self.assertNotEqual(before["restore-prefix"], after["restore-prefix"]) - self.assertNotEqual(before["binary-key"], after["binary-key"]) - config.unlink() - - def test_unsupported_config_and_tool_overrides_fail_closed(self): - """Unknown external build inputs cannot produce an apparently safe cache key.""" - for variable in ("RUSTC_WRAPPER", "CC", "PROTOC", "HDFS_LIB_DIR", "DOCS_RS", - "OPENSSL_LIB_DIR", "CARGO_BUILD_RUSTC_WRAPPER", - "CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_LINKER"): - with self.subTest(variable=variable): - self.env[variable] = "/untracked/tool" - with self.assertRaisesRegex(ValueError, "unsupported build override"): - self.keys() - del self.env[variable] - config = self.cargo_home / "config.toml" - self.write(config, "[build]\nrustc-wrapper = '/untracked/tool'\n") - with self.assertRaisesRegex(ValueError, "unsupported Cargo build"): - self.keys() - self.write(config, "include = ['extra.toml']\n") - with self.assertRaisesRegex(ValueError, "unsupported Cargo config"): - self.keys() - - def test_flags_are_pinned_and_other_build_environment_is_hashed(self): - """Only fixed compiler flags are reusable; safe Cargo build settings are hashed.""" - before = self.keys() - self.env["CARGO_BUILD_JOBS"] = "2" - self.assertNotEqual(before["binary-key"], self.keys()["binary-key"]) - self.env["RUSTFLAGS"] = "-Ctarget-cpu=native -Clink-arg=-fuse-ld=bfd" - with self.assertRaisesRegex(ValueError, "fixed x86-64-v3"): - self.keys() - self.env["RUSTFLAGS"] = "-Clink-arg=-fuse-ld=bfd" - debug = self.keys("debug") - self.assertEqual(debug["binary-key"], "") - self.assertNotEqual(before["source-key"], debug["source-key"]) - self.env["RUSTFLAGS"] += " -Clinker=/untracked/linker" - with self.assertRaisesRegex(ValueError, "fixed bfd"): - self.keys("debug") - - def test_external_compiler_and_library_inputs_are_rejected(self): - """Untracked header/library changes cannot hide behind unchanged override strings.""" - for name, value in { - "CFLAGS": "-include /tmp/header.h", "CXXFLAGS": "-I/tmp/include", - "CPPFLAGS": "-I/tmp/include", "LDFLAGS": "-L/tmp/lib", - "LIBRARY_PATH": "/tmp/lib", "CPATH": "/tmp/include", - "C_INCLUDE_PATH": "/tmp/include", "CPLUS_INCLUDE_PATH": "/tmp/include", - "LD_LIBRARY_PATH": "/tmp/lib", "LD_PRELOAD": "/tmp/lib/injected.so", - "CARGO_BUILD_RUSTC": "/tmp/rustc", "CARGO_BUILD_RUSTFLAGS": "-Clinker=/tmp/ld", - "CARGO_BUILD_TARGET": "/tmp/target.json", "CARGO_BUILD_FUTURE_OVERRIDE": "anything", - "TARGET_CC": "/tmp/compiler", "HOST_CFLAGS": "-include /tmp/header.h", - "CMAKE_TOOLCHAIN_FILE": "/tmp/toolchain.cmake", - }.items(): - with self.subTest(name=name): - self.env[name] = value - with self.assertRaisesRegex(ValueError, "unsupported build override"): - self.keys() - del self.env[name] - self.env["LD_LIBRARY_PATH"] = str(self.java_home / "lib/server") - before = self.keys() - (self.java_home / "lib/server/libjvm.so").write_text("changed linked JVM") + self.env["RUSTFLAGS"] = "-Ctarget-cpu=x86-64-v3 -Clink-arg=-fuse-ld=bfd" + self.write(".cargo/config.toml", "[build]\nincremental = false\n") + subprocess.run(["git", "add", ".cargo/config.toml"], cwd=self.root, check=True) self.assertNotEqual(before["binary-key"], self.keys()["binary-key"]) - def test_rustup_proxies_hash_the_resolved_compiler(self): - """A changed real Rust tool invalidates the key even with unchanged proxy/version.""" - before = self.keys() - for tool in ("rustc", "cargo", "rustfmt"): - with self.subTest(tool=tool): - binary = self.rust_tool_path / tool - old = binary.read_text() - binary.write_text("changed underlying tool with the same version") - self.assertNotEqual(before["binary-key"], self.keys()["binary-key"]) - binary.write_text(old) - (self.rust_tool_path / "rustc").unlink() - with self.assertRaises(FileNotFoundError): - self.keys() - - def test_cargo_home_fallback_and_output_values(self): - """Use effective CARGO_HOME, or HOME/.cargo, and publish bounded opaque keys.""" - outputs = self.keys() - self.assertEqual(outputs["cargo-home"], str(self.cargo_home)) - self.assertTrue(outputs["source-key"].startswith(outputs["restore-prefix"])) - del self.env["CARGO_HOME"] - fallback = self.keys() - self.assertEqual(fallback["cargo-home"], str(Path(self.env["HOME"]) / ".cargo")) - for name, value in fallback.items(): - self.assertNotIn("\n", value) - self.assertLess(len(value), 512) - if name != "cargo-home": - self.assertNotIn("JAVA_HOME", value) - - def test_container_checkout_ownership_does_not_block_git_inventory(self): - """Read a foreign-owned checkout without changing persistent Git trust. - - Git's test switch reproduces the runner/container ownership mismatch - without requiring root. Real Git must reject the unconfigured checkout, - then the CLI must discover its root and tracked files successfully. - Only native tool discovery is mocked; global Git config stays untouched. - """ - global_config = self.directory / "global.gitconfig" - global_config.write_text("") - env = dict(os.environ, GIT_TEST_ASSUME_DIFFERENT_OWNER="1", - GIT_CONFIG_GLOBAL=str(global_config), GIT_CONFIG_NOSYSTEM="1") - untrusted = subprocess.run(["git", "rev-parse", "--show-toplevel"], - cwd=self.root, env=env, capture_output=True) - self.assertEqual(untrusted.returncode, 128) - self.assertIn(b"dubious ownership", untrusted.stderr) - arguments = ["native-cache-key.py", "--profile", "ci"] - with patch.dict(CACHE.os.environ, env, clear=True), \ - patch.object(CACHE.Path, "cwd", return_value=self.root), \ - patch.object(CACHE.sys, "argv", arguments), \ - patch.object(CACHE, "environment_identity", return_value=(self.cargo_home, {})), \ - patch.object(CACHE.sys, "stdout", new_callable=io.StringIO) as stdout, \ - patch.object(CACHE.sys, "stderr", new_callable=io.StringIO) as stderr: - self.assertEqual(CACHE.main(), 0, stderr.getvalue()) - dependencies, sources = CACHE.tracked_inputs(self.root, dict(os.environ)) - expected = CACHE.cache_keys("ci", dependencies, sources, {}, self.cargo_home) - actual = dict(line.split("=", 1) for line in stdout.getvalue().splitlines()) - self.assertEqual(actual, expected) - self.assertEqual(global_config.read_text(), "") - - def test_cli_publishes_outputs_only_after_complete_snapshot(self): - """Failed fingerprinting preserves GitHub outputs; success emits opaque keys.""" - output_file = self.directory / "github-output" - output_file.write_text("previous=value\n") - arguments = ["native-cache-key.py", "--profile", "ci", "--github-output", str(output_file)] - with patch.object(CACHE.sys, "argv", arguments), \ - patch.object(CACHE, "command", return_value=str(self.root).encode()), \ - patch.object(CACHE, "tracked_inputs", return_value=({}, {})), \ - patch.object(CACHE, "environment_identity", side_effect=ValueError("missing tool")), \ - patch.object(CACHE.sys, "stdout", new_callable=io.StringIO) as stdout, \ - patch.object(CACHE.sys, "stderr", new_callable=io.StringIO): - self.assertEqual(CACHE.main(), 1) - self.assertEqual(stdout.getvalue(), "") - self.assertEqual(output_file.read_text(), "previous=value\n") - with patch.object(CACHE.sys, "argv", arguments), \ - patch.object(CACHE, "command", return_value=str(self.root).encode()), \ - patch.object(CACHE, "tracked_inputs", return_value=({}, {})), \ - patch.object(CACHE, "environment_identity", return_value=(self.cargo_home, {})), \ - patch.object(CACHE.sys, "stdout", new_callable=io.StringIO) as stdout: - self.assertEqual(CACHE.main(), 0) - self.assertEqual(output_file.read_text(), "previous=value\n" + stdout.getvalue()) - self.assertIn("binary-key=Linux-native-ci-v1-", stdout.getvalue()) - - def test_missing_tool_and_jvm_fail_closed(self): - """Essential fingerprint failures never fall back to a partial identity.""" - with patch.object(CACHE.shutil, "which", return_value=None): - with self.assertRaisesRegex(ValueError, "missing required tool"): - CACHE.environment_identity(self.root, "ci", self.env) - (self.java_home / "lib/server/libjvm.so").unlink() - with self.assertRaises(FileNotFoundError): - self.keys() + def test_profiles_have_separate_cargo_caches(self): + """CI/debug keys stay separate and only CI produces a reusable library key.""" + ci, debug = self.keys("ci"), self.keys("debug") + self.assertNotEqual(ci["source-key"], debug["source-key"]) + self.assertNotEqual(ci["restore-prefix"], debug["restore-prefix"]) + self.assertEqual(debug["binary-key"], "") + self.assertTrue(ci["binary-key"].startswith("Linux-native-ci-")) + self.assertTrue(ci["source-key"].startswith(ci["restore-prefix"])) + self.assertEqual(ci["cargo-home"], self.env["CARGO_HOME"]) + + def test_container_ownership_works_without_global_git_config_changes(self): + """A differently owned checkout permits helper root/inventory reads without global trust.""" + self.write("global.gitconfig", "[user]\n\tname = Cache Test\n") + config = self.root / "global.gitconfig" + original = config.read_bytes() + output = self.root / "github-output" + environment = {**self.env, "GIT_TEST_ASSUME_DIFFERENT_OWNER": "1", + "GIT_CONFIG_GLOBAL": str(config), "GIT_CONFIG_NOSYSTEM": "1"} + arguments = ["native-cache-key.py", "--profile", "ci", "--github-output", str(output)] + with patch.dict(os.environ, environment): + ordinary = subprocess.run(["git", "rev-parse", "--show-toplevel"], cwd=self.root, + text=True, capture_output=True) + self.assertNotEqual(ordinary.returncode, 0) + self.assertIn("dubious ownership", ordinary.stderr) + with patch.object(CACHE.Path, "cwd", return_value=self.root), \ + patch.object(sys, "argv", arguments), \ + patch.object(CACHE, "environment_inputs", return_value={"cargo_home": self.env["CARGO_HOME"]}), \ + patch.object(sys, "stdout", new_callable=io.StringIO): + CACHE.main() + self.assertIn("source-key=Linux-cargo-ci-", output.read_text()) + self.assertEqual(config.read_bytes(), original) if __name__ == "__main__": diff --git a/dev/ci/test-native-cache-workflow.py b/dev/ci/test-native-cache-workflow.py deleted file mode 100644 index ef42da15543..00000000000 --- a/dev/ci/test-native-cache-workflow.py +++ /dev/null @@ -1,222 +0,0 @@ -#!/usr/bin/env python3 -# -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. - -"""Exercise the real native-cache action guards and shell steps without building Rust. - -Only remote cache operations and Cargo compilation are simulated. The real -manifest helper, shell commands, step order, and guards run from the checkout. -The extractor deliberately supports this action's formatting, not general YAML. -""" - -import ast -import os -from pathlib import Path -import re -import shutil -import subprocess -import tempfile -import unittest - - -ROOT = Path(__file__).resolve().parents[2] -ACTION = ROOT / ".github/actions/build-native-ci/action.yaml" -STEPS = dict(part.split("\n", 1) for part in - re.split(r"(?m)^ - name: ", ACTION.read_text())[1:]) -VALIDATE = "Validate cached native library" -BUILD = "Build native library (CI profile)" -PREPARE = "Prepare native library cache" -RESTORE_TARGET = "Restore incremental Cargo cache" -SAVE_BINARY = "Save native library cache" -SAVE_TARGET = "Save incremental Cargo cache" - - -def field(block, name, indent=6): - """Return one scalar field from a step block, or an empty string if absent. - - The supplied indentation distinguishes step fields from nested inputs. - This read-only extractor handles this action's single-line fields only. - """ - match = re.search(rf"(?m)^{' ' * indent}{re.escape(name)}: (.+)$", block) - return match.group(1) if match else "" - - -def condition(expression, context): - """Evaluate the action's comparisons/boolean operators against string outputs. - - Missing outputs become empty strings as in Actions. Unsupported syntax - raises ValueError instead of silently inventing new GitHub semantics. - Only literal comparisons and boolean operators are accepted; no calls run. - """ - expression = expression.removeprefix("${{ ").removesuffix(" }}") - expression = re.sub(r"\b(?:github\.[\w-]+|steps\.[\w-]+\.outputs\.[\w-]+)\b", - lambda match: repr(context.get(match.group(), "")), expression) - expression = expression.replace("&&", " and ").replace("||", " or ") - expression = re.sub(r"!(?!=)", " not ", expression).strip() - tree = ast.parse(expression or "True", mode="eval") - allowed = (ast.Expression, ast.BoolOp, ast.UnaryOp, ast.Compare, ast.Constant, - ast.And, ast.Or, ast.Not, ast.Eq, ast.NotEq) - if any(not isinstance(node, allowed) for node in ast.walk(tree)): - raise ValueError(f"Unsupported action guard: {expression}") - return eval(compile(tree, str(ACTION), "eval"), {"__builtins__": {}}, {}) - - -def shell_step(name, workspace, environment): - """Execute the named action's literal shell block in an isolated workspace. - - Apply its RUSTFLAGS override and return a captured subprocess result. - Commands use bash's Actions-style fail-fast flags; no real Cargo runs. - A missing multiline block raises ValueError before starting a process. - """ - block = STEPS[name] - match = re.search(r"(?m)^ run: \|\n((?: .*\n|\n)*)", block + "\n") - if not match: - raise ValueError(f"Missing literal shell block: {name}") - script = "\n".join(line[8:] for line in match.group(1).splitlines()) - environment = environment.copy() - flags = field(block, "RUSTFLAGS", 8) - if flags: - environment["RUSTFLAGS"] = flags.strip("'\"") - return subprocess.run(["bash", "--noprofile", "--norc", "-eo", "pipefail", "-c", script], - cwd=workspace, env=environment, capture_output=True, text=True) - - -def run_scenario(cache_hit="false", payload="missing", event="pull_request", - ref="refs/pull/123/merge", fail_cargo=False, target_hit="false"): - """Return observed operations/output for one simulated remote-cache scenario. - - A temporary workspace holds real manifest-helper inputs and a fake Cargo - executable. Remote actions record their invocation and supply cache outputs; - all other relevant steps execute their actual shell bodies. Failed commands - suppress subsequent steps, matching Actions' implicit success() guard. - The workspace and payloads are deleted before returning copied observations. - """ - with tempfile.TemporaryDirectory(prefix="comet-cache-workflow-") as temporary: - workspace = Path(temporary) - (workspace / "dev/ci").mkdir(parents=True) - shutil.copyfile(ROOT / "dev/ci/native-library-cache.py", - workspace / "dev/ci/native-library-cache.py") - library = workspace / "native/target/ci/libcomet.so" - library.parent.mkdir(parents=True) - executable = workspace / "bin/cargo" - executable.parent.mkdir() - executable.write_text("#!/bin/bash\nset -eu\n" - 'printf "%s|%s|%s\\n" "$PWD" "$*" "$RUSTFLAGS" >> "$CARGO_LOG"\n' - '[ "$FAIL_CARGO" = 0 ] || exit 23\n' - 'mkdir -p target/ci\nprintf built > target/ci/libcomet.so\n') - executable.chmod(0o755) - output = workspace / "step-output" - environment = dict(os.environ, PATH=f"{executable.parent}:{os.environ['PATH']}", - RUNNER_TEMP=str(workspace), NATIVE_CACHE_KEY="fixture-native-key", - GITHUB_OUTPUT=str(output), CARGO_LOG=str(workspace / "cargo-log"), - FAIL_CARGO=str(int(fail_cargo))) - if payload != "missing": - library.write_bytes(b"cached") - prepared = shell_step(PREPARE, workspace, environment) - if prepared.returncode: - raise AssertionError(prepared.stderr) - library.unlink() - if payload == "corrupt": - (workspace / "comet-native-library/libcomet.so").write_bytes(b"damaged") - elif payload == "wrong-key": - environment["NATIVE_CACHE_KEY"] = "different-native-key" - context = {"github.event_name": event, "github.ref": ref} - operations, successful, lookup_only = [], True, False - for name, block in STEPS.items(): - if name == "Fingerprint native build inputs": - continue # The dedicated key tests exercise tool/source fingerprinting. - if not successful or not condition(field(block, "if"), context): - continue - operations.append(name) - if field(block, "uses"): - if name == "Restore native library cache": - lookup_only = condition(field(block, "lookup-only", 8), context) - context["steps.binary-cache.outputs.cache-hit"] = cache_hit - elif name == RESTORE_TARGET: - context["steps.cargo-cache.outputs.cache-hit"] = target_hit - continue - output.write_text("") - result = shell_step(name, workspace, environment) - successful = result.returncode == 0 - for line in output.read_text().splitlines(): - key, value = line.split("=", 1) - context[f"steps.{field(block, 'id')}.outputs.{key}"] = value - log = workspace / "cargo-log" - return dict(operations=operations, successful=successful, lookup_only=lookup_only, - cargo=log.read_text() if log.exists() else "", - library=library.read_bytes() if library.exists() else None) - - -class NativeCacheWorkflowTest(unittest.TestCase): - """Check native reuse, compile fallback, and trusted cache ownership end to end.""" - - def test_exact_valid_hit_skips_compilation_and_target_archive(self): - """A verified exact hit installs cached bytes and avoids expensive target I/O.""" - result = run_scenario("true", "good") - self.assertTrue(result["successful"]) - self.assertEqual(result["library"], b"cached") - self.assertEqual(result["cargo"], "") - for step in (BUILD, RESTORE_TARGET, SAVE_TARGET, SAVE_BINARY, PREPARE): - self.assertNotIn(step, result["operations"]) - - def test_all_misses_compile_even_with_an_exact_target_cache(self): - """Missing, partial, damaged, and mismatched binary entries all run locked Cargo.""" - for hit, payload in (("", "missing"), ("false", "good"), ("true", "missing"), - ("true", "corrupt"), ("true", "wrong-key")): - with self.subTest(hit=hit, payload=payload): - result = run_scenario(hit, payload, target_hit="true") - self.assertTrue(result["successful"]) - self.assertEqual(result["library"], b"built") - self.assertIn(RESTORE_TARGET, result["operations"]) - self.assertRegex(result["cargo"], r"/native\|build --locked --profile ci\|") - self.assertIn("-Ctarget-cpu=x86-64-v3 -Clink-arg=-fuse-ld=bfd", result["cargo"]) - - def test_only_main_push_saves_caches(self): - """Equivalent cold builds save only for push-to-main, including manual/main cases.""" - for event, ref in (("pull_request", "refs/pull/123/merge"), - ("merge_group", "refs/heads/gh-readonly-queue/main/test"), - ("schedule", "refs/heads/main"), ("workflow_dispatch", "refs/heads/main"), - ("push", "refs/heads/feature"), ("push", "refs/heads/main")): - with self.subTest(event=event, ref=ref): - result = run_scenario(event=event, ref=ref) - writes = event == "push" and ref == "refs/heads/main" - self.assertTrue(result["successful"]) - for step in (PREPARE, SAVE_BINARY, SAVE_TARGET): - self.assertEqual(step in result["operations"], writes) - - def test_main_push_warms_target_even_when_binary_exists(self): - """Main does lookup-only for an existing binary, but still compiles and saves target.""" - result = run_scenario("true", "good", "push", "refs/heads/main") - self.assertTrue(result["lookup_only"]) - self.assertEqual(result["library"], b"built") - self.assertIn(BUILD, result["operations"]) - self.assertIn(SAVE_TARGET, result["operations"]) - for step in (VALIDATE, PREPARE, SAVE_BINARY): - self.assertNotIn(step, result["operations"]) - - def test_failed_cargo_cannot_prepare_or_save(self): - """A failed main build stops before publishing either incomplete cache entry.""" - result = run_scenario(event="push", ref="refs/heads/main", fail_cargo=True) - self.assertFalse(result["successful"]) - self.assertIsNone(result["library"]) - for step in (PREPARE, SAVE_BINARY, SAVE_TARGET): - self.assertNotIn(step, result["operations"]) - - -if __name__ == "__main__": - unittest.main() diff --git a/dev/ci/test-native-library-cache.py b/dev/ci/test-native-library-cache.py deleted file mode 100644 index 05dc7496fdc..00000000000 --- a/dev/ci/test-native-library-cache.py +++ /dev/null @@ -1,218 +0,0 @@ -#!/usr/bin/env python3 -# -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. - -"""Exercise native cache validation and failure handling without running a library.""" - -from contextlib import redirect_stderr, redirect_stdout -import hashlib -import importlib.util -import io -import json -import os -from pathlib import Path -import tempfile -import unittest -from unittest.mock import Mock, patch - - -SPEC = importlib.util.spec_from_file_location( - "native_library_cache", Path(__file__).with_name("native-library-cache.py")) -CACHE = importlib.util.module_from_spec(SPEC) -SPEC.loader.exec_module(CACHE) - - -class NativeLibraryCacheTest(unittest.TestCase): - """Use isolated, temporary cache/build paths; test bytes are never executable.""" - - def setUp(self): - """Create a multi-chunk source fixture; unittest owns directory cleanup.""" - temporary = tempfile.TemporaryDirectory(prefix="comet-native-cache-test-") - self.addCleanup(temporary.cleanup) - self.root = Path(temporary.name) - self.cache = self.root / "cache" - self.source = self.root / "built.so" - self.destination = self.root / "native" / "target" / "ci" / "libcomet.so" - self.output = self.root / "github-output" - self.key = "comet-native-v1-expected-input-fingerprint" - self.contents = b"native-library-test-bytes\x00" * 100000 - self.source.write_bytes(self.contents) - - def prepare(self): - """Populate this fixture's cache from its source, propagating failures.""" - CACHE.prepare(self.key, self.cache, self.source) - - def assert_miss(self, key=None): - """Require a successful CLI miss to delete an existing stale destination.""" - self.destination.parent.mkdir(parents=True, exist_ok=True) - self.destination.write_bytes(b"stale build must not be accepted") - self.output.write_text("existing=value\n", encoding="utf-8") - with redirect_stdout(io.StringIO()) as stdout: - result = CACHE.main([ - "restore", "--key", key or self.key, "--cache-dir", str(self.cache), - "--library", str(self.destination), "--github-output", str(self.output), - ]) - self.assertEqual(result, 0) - self.assertEqual(stdout.getvalue(), "hit=false\n") - self.assertEqual(self.output.read_text(), "existing=value\nhit=false\n") - self.assertFalse(self.destination.exists()) - self.assertEqual(list(self.destination.parent.iterdir()), []) - - def test_prepare_and_restore_exact_bytes(self): - """The CLI preserves bytes, records their SHA256, and appends a hit output.""" - self.assertEqual(CACHE.main([ - "prepare", "--key", self.key, "--cache-dir", str(self.cache), - "--library", str(self.source), - ]), 0) - manifest = json.loads((self.cache / "manifest.json").read_text()) - self.assertEqual(manifest, { - "key": self.key, "sha256": hashlib.sha256(self.contents).hexdigest(), - }) - self.assertEqual(sorted(path.name for path in self.cache.iterdir()), - ["libcomet.so", "manifest.json"]) - self.destination.parent.mkdir(parents=True) - self.destination.write_bytes(b"stale") - self.output.write_text("existing=value\n", encoding="utf-8") - with redirect_stdout(io.StringIO()) as stdout: - result = CACHE.main([ - "restore", "--key", self.key, "--cache-dir", str(self.cache), - "--library", str(self.destination), "--github-output", str(self.output), - ]) - self.assertEqual(result, 0) - self.assertEqual(stdout.getvalue(), "hit=true\n") - self.assertEqual(self.output.read_text(), "existing=value\nhit=true\n") - self.assertEqual(self.destination.read_bytes(), self.contents) - self.assertEqual(list(self.destination.parent.iterdir()), [self.destination]) - - def test_wrong_key_is_miss(self): - """A valid binary from different native inputs cannot be reused.""" - self.prepare() - self.assert_miss("comet-native-v1-different-input-fingerprint") - - def test_missing_cache_is_miss(self): - """A cold cache removes stale output and falls through to a fresh build.""" - self.assert_miss() - - def test_missing_entry_file_is_miss(self): - """Either absent cache file invalidates an otherwise complete entry.""" - for name in ("manifest.json", "libcomet.so"): - with self.subTest(name=name): - self.prepare() - (self.cache / name).unlink() - self.assert_miss() - - def test_corrupt_manifest_is_miss(self): - """Malformed JSON/UTF8, invalid shapes and missing fields are misses.""" - for contents in (b"{", b"\xff", b"null", b"[]", b"{}", - json.dumps({"key": self.key}).encode(), - json.dumps({"key": self.key, "sha256": 123}).encode()): - with self.subTest(contents=contents): - self.prepare() - (self.cache / "manifest.json").write_bytes(contents) - self.assert_miss() - - def test_corrupt_or_truncated_library_is_miss(self): - """The checksum rejects changed or incomplete bytes before installation.""" - for contents in (b"changed", b"", self.contents[:-1]): - with self.subTest(length=len(contents)): - self.prepare() - (self.cache / "libcomet.so").write_bytes(contents) - self.assert_miss() - - def test_symlinked_cache_files_are_misses(self): - """Even symlinks to matching bytes are excluded from the cache contract.""" - for name in ("manifest.json", "libcomet.so"): - with self.subTest(name=name): - self.prepare() - cached = self.cache / name - target = self.root / f"linked-{name}" - cached.replace(target) - cached.symlink_to(target) - self.assert_miss() - - def test_non_regular_cache_files_are_misses(self): - """Reject directories and FIFOs without blocking or accepting stale output.""" - for name in ("manifest.json", "libcomet.so"): - for kind in ("directory", "fifo"): - with self.subTest(name=name, kind=kind): - self.prepare() - cached = self.cache / name - cached.unlink() - if kind == "directory": - cached.mkdir() - else: - os.mkfifo(cached) - self.assert_miss() - if kind == "directory": - cached.rmdir() - else: - cached.unlink() - - def test_unreadable_cache_is_miss(self): - """A failed cache read is a miss even under a privileged test account.""" - self.prepare() - with patch.object(CACHE, "open_regular_file", side_effect=PermissionError("unreadable")): - self.assert_miss() - - def test_stale_destination_symlink_is_removed(self): - """A miss unlinks the old output without touching its symlink target.""" - self.destination.parent.mkdir(parents=True) - self.destination.symlink_to(self.source) - self.assertFalse(CACHE.restore(self.key, self.cache, self.destination)) - self.assertFalse(self.destination.is_symlink()) - self.assertEqual(self.source.read_bytes(), self.contents) - - def test_destination_write_failure_is_fatal(self): - """A verified cache cannot mask destination failures or leave partial output.""" - self.prepare() - self.destination.parent.mkdir(parents=True) - self.destination.write_bytes(b"stale") - with patch.object(Path, "replace", side_effect=OSError("disk failure")): - with redirect_stderr(io.StringIO()) as stderr: - result = CACHE.main([ - "restore", "--key", self.key, "--cache-dir", str(self.cache), - "--library", str(self.destination), "--github-output", str(self.output), - ]) - self.assertEqual(result, 1) - self.assertIn("disk failure", stderr.getvalue()) - self.assertFalse(self.output.exists()) - self.assertEqual(list(self.destination.parent.iterdir()), []) - - def test_failed_prepare_invalidates_previous_manifest(self): - """An interrupted refresh cannot retain a manifest advertising success.""" - self.prepare() - self.source.unlink() - with self.assertRaises(FileNotFoundError): - self.prepare() - self.assertFalse((self.cache / "manifest.json").exists()) - self.assert_miss() - - def test_read_failure_discards_temporary_copy(self): - """A mid-read failure preserves an old copy and cleans its temporary file.""" - self.destination.parent.mkdir(parents=True) - self.destination.write_bytes(b"old complete copy") - source = Mock() - source.read.side_effect = [b"partial", OSError("read failure")] - with self.assertRaises(ValueError): - CACHE.copy_library(source, self.destination) - self.assertEqual(self.destination.read_bytes(), b"old complete copy") - self.assertEqual(list(self.destination.parent.iterdir()), [self.destination]) - - -if __name__ == "__main__": - unittest.main() From 9ecf8b0bf12593d9ed6ce12ab50ab07e84896004 Mon Sep 17 00:00:00 2001 From: Chao Sun Date: Wed, 16 Sep 2026 13:30:10 +0000 Subject: [PATCH 4/8] ci: align native cache inputs with main cache warming --- .github/workflows/README.md | 19 ++++++++++++------- dev/ci/compute-changes.py | 15 +++++++++++++-- dev/ci/native-cache-key.py | 32 +++++++++++++++++++++----------- dev/ci/test-native-cache-key.py | 28 ++++++++++++++++++++++++---- 4 files changed, 70 insertions(+), 24 deletions(-) diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 55f4790ffff..8a610b7eb8b 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -413,12 +413,14 @@ an incremental cache and runs `cargo build --locked --profile ci`. Artifacts and downstream tests use the same paths in either case. `dev/ci/native-cache-key.py` snapshots tracked native/protobuf/dependency files, -shared JVM inputs, build configuration and CI definitions before Cargo generates -source files. The key also includes Rust versions, installed system package +Cargo configuration and the native build recipes before Cargo generates source +files. The key also includes Rust versions, installed system package versions, architecture, JDK release/path and compiler flags. The helper targets our official Rust container and `setup-builder`, not arbitrary local toolchains. -Spark-only edits and generated files preserve the key; native/protobuf changes -invalidate it. The shared action uses portable `x86-64-v3` code generation. +Spark-only edits, documentation, unrelated workflows and generated files preserve +the key; native/protobuf changes invalidate it. Benchmarks enter the debug cache +key but not the library key. The shared action uses portable `x86-64-v3` code +generation. The incremental cache contains the effective `CARGO_HOME` registry/git directories and `native/target`. Its dependency prefix permits reuse after source changes, @@ -427,10 +429,13 @@ key and continues to run all checks and tests. Only pushes to `main` save either cache. Main always compiles to keep the incremental cache warm. Other runs consume matching entries; a cold or evicted -cache builds normally. GitHub Actions handles cache storage and restoration. +cache builds normally. Changes to shared native inputs owned by other workflows +also trigger main's cache warmer. GitHub Actions handles cache storage and +restoration. -Preflight tests key invalidation, generated-file stability and container checkout -ownership. After main populates the new namespace, verify a hosted library hit +Preflight tests key invalidation, generated-file stability, container checkout +ownership, and that every binary-key input triggers main's cache warmer. After +main populates the new namespace, verify a hosted library hit by checking that `Restore native library cache` reports an exact hit and the Cargo steps skip, while the normal artifact upload and downstream tests pass. diff --git a/dev/ci/compute-changes.py b/dev/ci/compute-changes.py index 89247955930..cde356795ff 100644 --- a/dev/ci/compute-changes.py +++ b/dev/ci/compute-changes.py @@ -558,11 +558,22 @@ def event_allows(job, event): def compute(files, event): - """Return {job: bool}, folding the path filter and the event policy.""" - return { + """Return job flags, including main's warmer for shared native cache inputs.""" + selected = { name: event_allows(name, event) and matches(patterns, files) for name, patterns in FILTERS.items() } + # These native-key inputs have no ordinary Linux route. Warm main after + # they change, without adding the full Linux pipeline to contrib-only PRs. + if event.get("name") == "push" and matches([ + "contrib/*/native/**", ".cargo/**", "rust-toolchain", + ".github/workflows/spark_sql_test_reusable.yml", + ".github/workflows/iceberg_spark_test_reusable.yml", + ".github/workflows/spark_sql_writer_tests.yml", + "!**.md", "!**/benches/**", + ], files): + selected["build_linux"] = True + return selected def event_from_env(): diff --git a/dev/ci/native-cache-key.py b/dev/ci/native-cache-key.py index 763471aae5d..10cc8e8ef57 100644 --- a/dev/ci/native-cache-key.py +++ b/dev/ci/native-cache-key.py @@ -24,6 +24,7 @@ """ import argparse +from fnmatch import fnmatchcase import hashlib import json import os @@ -31,9 +32,15 @@ import subprocess -SOURCE_PREFIXES = ("native/", "contrib/", "common/", ".cargo/", ".mvn/", - ".github/actions/", ".github/workflows/", "dev/ci/") -SOURCE_FILES = {"Makefile", "pom.xml", "mvnw", "rust-toolchain", "rust-toolchain.toml"} +# Direct Cargo inputs and the recipes that install/configure the native build. +# Unrelated workflows and JVM/Maven files do not enter the native library build. +INPUT_PATTERNS = ( + "native/**", "contrib/*/native/**", ".cargo/**", + ".github/actions/setup-builder/**", ".github/actions/build-native-ci/**", + ".github/workflows/pr_build_linux.yml", ".github/workflows/spark_sql_test_reusable.yml", + ".github/workflows/iceberg_spark_test_reusable.yml", ".github/workflows/spark_sql_writer_tests.yml", + "rust-toolchain", "rust-toolchain.toml", "dev/ci/native-cache-key.py", +) def digest(value): @@ -46,12 +53,13 @@ def command(args, cwd): return subprocess.check_output(args, cwd=cwd, text=True).strip() -def source_inputs(root): - """Read tracked build files and return dependency and complete input maps. +def source_inputs(root, profile="ci"): + """Return dependency and source maps for the selected native build profile. Each map contains relative names, Git modes and content digests. Untracked - generated Rust and target files are excluded. Trust only this checkout for - the Git read: container steps can run as a different owner than checkout. + generated Rust, target files and documentation are excluded. CI library + builds omit benchmarks; debug checks compile them. Trust only this checkout + for the Git read: container steps can run as a different owner than checkout. """ inventory = command(["git", "-c", f"safe.directory={root}", "ls-files", "--stage", "-z"], root) @@ -60,7 +68,9 @@ def source_inputs(root): if not record: continue metadata, name = record.split("\t", 1) - if name in SOURCE_FILES or name.startswith(SOURCE_PREFIXES): + if name.endswith(".md") or (profile == "ci" and "/benches/" in name): + continue + if any(fnmatchcase(name, pattern) for pattern in INPUT_PATTERNS): sources[name] = [metadata.split()[0], hashlib.sha256((root / name).read_bytes()).hexdigest()] dependencies = {name: value for name, value in sources.items() @@ -74,8 +84,8 @@ def environment_inputs(root, env): Rust's versions include the compiler commit; dpkg identifies the installed C/C++/protobuf tools and system libraries. The JDK release file identifies the vendor/build supplying JNI headers and libjvm. Paths and RUSTFLAGS are - included because linking can embed them. Workflow/action files in the source - map cover changes to how these tools are installed and invoked. + included because linking can embed them. Native producer workflows/actions + in the source map cover how these tools are configured and invoked. """ java_home = Path(env["JAVA_HOME"]) return { @@ -121,7 +131,7 @@ def main(): cwd = Path.cwd().resolve() root = Path(command(["git", "-c", f"safe.directory={cwd}", "rev-parse", "--show-toplevel"], cwd)) - dependencies, sources = source_inputs(root) + dependencies, sources = source_inputs(root, args.profile) environment = environment_inputs(root, os.environ) keys = cache_keys(args.profile, dependencies, sources, environment) output = "".join(f"{key}={value}\n" for key, value in keys.items()) diff --git a/dev/ci/test-native-cache-key.py b/dev/ci/test-native-cache-key.py index 1b5e7413401..53c8a059ba9 100644 --- a/dev/ci/test-native-cache-key.py +++ b/dev/ci/test-native-cache-key.py @@ -22,6 +22,7 @@ import io import os from pathlib import Path +import runpy import subprocess import sys import tempfile @@ -45,7 +46,11 @@ def setUp(self): subprocess.run(["git", "init", "--quiet", str(self.root)], check=True) self.inputs = {"native/Cargo.toml": "[workspace]\n", "native/Cargo.lock": "version = 4\n", "native/lib.rs": "fn native() {}\n", "native/proto/expr.proto": "message Expr {}\n", - "spark/Plan.scala": "object Plan {}\n", "README.md": "Comet\n"} + "spark/Plan.scala": "object Plan {}\n", "README.md": "Comet\n", + ".github/workflows/README.md": "CI documentation\n", + ".github/workflows/spark_sql_test_reusable.yml": "jobs: {}\n", + ".github/workflows/check_pr_title.yml": "jobs: {}\n", + "native/core/benches/perf.rs": "fn benchmark() {}\n"} for name, content in self.inputs.items(): self.write(name, content) subprocess.run(["git", "add", "."], cwd=self.root, check=True) @@ -64,7 +69,7 @@ def write(self, name, content): def keys(self, profile="ci"): """Return keys from real tracked files and deterministic tool version responses.""" - dependencies, sources = CACHE.source_inputs(self.root) + dependencies, sources = CACHE.source_inputs(self.root, profile) with patch.object(CACHE, "command", side_effect=lambda args, cwd: self.versions[args[0]]): environment = CACHE.environment_inputs(self.root, self.env) return CACHE.cache_keys(profile, dependencies, sources, environment) @@ -72,7 +77,8 @@ def keys(self, profile="ci"): def test_source_and_dependency_changes_invalidate_the_right_keys(self): """Native/protobuf edits retain the dependency prefix; dependency edits replace it.""" before = self.keys() - for name in ("native/lib.rs", "native/proto/expr.proto", "native/Cargo.toml", "native/Cargo.lock"): + for name in ("native/lib.rs", "native/proto/expr.proto", "native/Cargo.toml", "native/Cargo.lock", + ".github/workflows/spark_sql_test_reusable.yml"): with self.subTest(name=name): self.write(name, self.inputs[name] + "changed\n") after = self.keys() @@ -85,13 +91,27 @@ def test_source_and_dependency_changes_invalidate_the_right_keys(self): self.write(name, self.inputs[name]) def test_generated_files_and_unrelated_jvm_edits_preserve_keys(self): - """Untracked generated Rust/artifacts and unrelated JVM/docs edits permit native reuse.""" + """Generated files and non-build edits preserve reuse; debug still tracks benchmarks.""" before = self.keys() + debug = self.keys("debug") self.write("native/proto/src/generated/expr.rs", "generated Rust") self.write("native/target/ci/libcomet.so", "compiled library") self.write("spark/Plan.scala", "object NewPlan {}") self.write("README.md", "updated docs") + self.write(".github/workflows/README.md", "updated CI docs") + self.write(".github/workflows/check_pr_title.yml", "jobs: {changed: {}}") + self.write("native/core/benches/perf.rs", "fn changed_benchmark() {}") self.assertEqual(before, self.keys()) + self.assertNotEqual(debug["source-key"], self.keys("debug")["source-key"]) + + def test_every_binary_key_input_has_a_main_cache_warmer(self): + """Check real tracked inputs against push routing, plus inputs absent from today's tree.""" + project = Path(__file__).resolve().parents[2] + route = runpy.run_path(str(project / "dev/ci/compute-changes.py"))["compute"] + _, sources = CACHE.source_inputs(project) + for name in [*sources, ".cargo/config.toml", "rust-toolchain", "contrib/new/native/src/lib.rs"]: + self.assertTrue(route([name], {"name": "push"})["build_linux"], name) + self.assertFalse(route(["contrib/new/native/src/lib.rs"], {"name": "pull_request"})["build_linux"]) def test_tools_jdk_flags_and_tracked_build_configuration_invalidate(self): """Observed tool/package versions, Java metadata, flags and tracked configs enter keys.""" From aff75aeafbb88635647a3c4eb00c3a911ae67c8c Mon Sep 17 00:00:00 2001 From: Chao Sun Date: Wed, 16 Sep 2026 15:47:24 +0000 Subject: [PATCH 5/8] ci: align native cache selection with review feedback --- .github/workflows/README.md | 21 ++++++++++++----- .github/workflows/pr_build_linux.yml | 2 ++ dev/ci/compute-changes.py | 34 +++++++++++++++++++--------- dev/ci/native-cache-key.py | 20 ++++++---------- dev/ci/test-native-cache-key.py | 17 ++++++++++++-- 5 files changed, 62 insertions(+), 32 deletions(-) diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 8a610b7eb8b..f670ee3f073 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -411,6 +411,8 @@ The Linux, Spark SQL, Iceberg and manual writer workflows call cache hit restores `native/target/ci/libcomet.so` and skips Cargo. A miss restores an incremental cache and runs `cargo build --locked --profile ci`. Artifacts and downstream tests use the same paths in either case. +`--locked` deliberately fails when a manifest change requires updating +`native/Cargo.lock`; contributors must commit that lockfile update with the change. `dev/ci/native-cache-key.py` snapshots tracked native/protobuf/dependency files, Cargo configuration and the native build recipes before Cargo generates source @@ -418,14 +420,21 @@ files. The key also includes Rust versions, installed system package versions, architecture, JDK release/path and compiler flags. The helper targets our official Rust container and `setup-builder`, not arbitrary local toolchains. Spark-only edits, documentation, unrelated workflows and generated files preserve -the key; native/protobuf changes invalidate it. Benchmarks enter the debug cache -key but not the library key. The shared action uses portable `x86-64-v3` code -generation. +the key; native/protobuf changes invalidate it. Optional contrib crates contribute +their manifests, which Cargo resolves even with their features disabled, but not +their Rust sources or standalone lockfiles. Benchmarks enter the debug cache key +but not the library key. The input lists and glob matcher are shared with main's +cache routing in `compute-changes.py`. The shared action uses portable `x86-64-v3` +code generation. The incremental cache contains the effective `CARGO_HOME` registry/git directories -and `native/target`. Its dependency prefix permits reuse after source changes, -but every restore still invokes Cargo. The Rust test job uses a separate debug -key and continues to run all checks and tests. +and `native/target`. Its dependency prefix permits reuse after source changes +within the same build environment, but every restore still invokes Cargo. +Environment changes also invalidate this fallback: native dependencies compile C +against JNI headers and cache build-script outputs that Cargo does not fully +invalidate after external compiler or JDK changes. This can miss after unrelated +package updates, but prevents reusing those objects under a new library key. +The Rust test job uses a separate debug key and continues to run all checks and tests. Only pushes to `main` save either cache. Main always compiles to keep the incremental cache warm. Other runs consume matching entries; a cold or evicted diff --git a/.github/workflows/pr_build_linux.yml b/.github/workflows/pr_build_linux.yml index dc798fbb8c9..e9e6cec44ec 100644 --- a/.github/workflows/pr_build_linux.yml +++ b/.github/workflows/pr_build_linux.yml @@ -404,6 +404,8 @@ jobs: jdk-version: 17 - name: Fingerprint Rust test build inputs + # Default HDFS support compiles against JNI headers and links libjvm. + # Cached build-script outputs also contain JDK paths (core/build.rs). id: cargo-key run: python3 dev/ci/native-cache-key.py --profile debug --github-output "$GITHUB_OUTPUT" diff --git a/dev/ci/compute-changes.py b/dev/ci/compute-changes.py index cde356795ff..f445ef6b78f 100644 --- a/dev/ci/compute-changes.py +++ b/dev/ci/compute-changes.py @@ -38,6 +38,25 @@ import sys from pathlib import Path +# Shared cache recipes affect every native producer. Tests exercise the recipes +# but do not affect the resulting library, so they are routed separately below. +NATIVE_CACHE_RECIPES = ( + ".github/actions/build-native-ci/**", + "dev/ci/native-cache-key.py", "dev/ci/compute-changes.py", +) + +# Cargo validates optional contrib manifests against native/Cargo.lock even +# with their features disabled. Their Rust sources and standalone lockfiles +# do not enter the default CI/debug builds. +NATIVE_BUILD_INPUTS = ( + "native/**", "contrib/*/native/Cargo.toml", ".cargo/**", + ".github/actions/setup-builder/**", *NATIVE_CACHE_RECIPES, + ".github/workflows/pr_build_linux.yml", ".github/workflows/spark_sql_test_reusable.yml", + ".github/workflows/iceberg_spark_test_reusable.yml", ".github/workflows/spark_sql_writer_tests.yml", + "rust-toolchain", "rust-toolchain.toml", "!**.md", +) +NATIVE_LIBRARY_INPUTS = (*NATIVE_BUILD_INPUTS, "!**/benches/**") + FILTERS = { "build_linux": [ "native/**", @@ -395,8 +414,7 @@ "iceberg_1_8", "iceberg_1_9", "iceberg_1_10", "iceberg_1_11", ): FILTERS[_native_consumer].extend([ - ".github/actions/build-native-ci/**", - "dev/ci/native-cache-key.py", + *NATIVE_CACHE_RECIPES, "dev/ci/test-native-cache-key.py", ]) @@ -563,15 +581,9 @@ def compute(files, event): name: event_allows(name, event) and matches(patterns, files) for name, patterns in FILTERS.items() } - # These native-key inputs have no ordinary Linux route. Warm main after - # they change, without adding the full Linux pipeline to contrib-only PRs. - if event.get("name") == "push" and matches([ - "contrib/*/native/**", ".cargo/**", "rust-toolchain", - ".github/workflows/spark_sql_test_reusable.yml", - ".github/workflows/iceberg_spark_test_reusable.yml", - ".github/workflows/spark_sql_writer_tests.yml", - "!**.md", "!**/benches/**", - ], files): + # Use the fingerprint's exact patterns and matcher for main's producer, + # including inputs owned by other workflows, without broadening PR jobs. + if event.get("name") == "push" and matches(NATIVE_LIBRARY_INPUTS, files): selected["build_linux"] = True return selected diff --git a/dev/ci/native-cache-key.py b/dev/ci/native-cache-key.py index 10cc8e8ef57..03fe6e550c1 100644 --- a/dev/ci/native-cache-key.py +++ b/dev/ci/native-cache-key.py @@ -24,23 +24,16 @@ """ import argparse -from fnmatch import fnmatchcase import hashlib import json import os from pathlib import Path +import runpy import subprocess -# Direct Cargo inputs and the recipes that install/configure the native build. -# Unrelated workflows and JVM/Maven files do not enter the native library build. -INPUT_PATTERNS = ( - "native/**", "contrib/*/native/**", ".cargo/**", - ".github/actions/setup-builder/**", ".github/actions/build-native-ci/**", - ".github/workflows/pr_build_linux.yml", ".github/workflows/spark_sql_test_reusable.yml", - ".github/workflows/iceberg_spark_test_reusable.yml", ".github/workflows/spark_sql_writer_tests.yml", - "rust-toolchain", "rust-toolchain.toml", "dev/ci/native-cache-key.py", -) +# Share both the input patterns and their glob semantics with main's warmer. +CHANGES = runpy.run_path(str(Path(__file__).with_name("compute-changes.py"))) def digest(value): @@ -61,6 +54,7 @@ def source_inputs(root, profile="ci"): builds omit benchmarks; debug checks compile them. Trust only this checkout for the Git read: container steps can run as a different owner than checkout. """ + patterns = CHANGES["NATIVE_LIBRARY_INPUTS" if profile == "ci" else "NATIVE_BUILD_INPUTS"] inventory = command(["git", "-c", f"safe.directory={root}", "ls-files", "--stage", "-z"], root) sources = {} @@ -68,9 +62,7 @@ def source_inputs(root, profile="ci"): if not record: continue metadata, name = record.split("\t", 1) - if name.endswith(".md") or (profile == "ci" and "/benches/" in name): - continue - if any(fnmatchcase(name, pattern) for pattern in INPUT_PATTERNS): + if CHANGES["matches"](patterns, [name]): sources[name] = [metadata.split()[0], hashlib.sha256((root / name).read_bytes()).hexdigest()] dependencies = {name: value for name, value in sources.items() @@ -108,6 +100,8 @@ def cache_keys(profile, dependencies, sources, environment): Only the incremental Cargo cache has a source-independent restore prefix. The library key includes all tracked build inputs and never uses fallback. + Both retain the environment: native build scripts can reuse C objects + without detecting changes to external compiler binaries or JNI headers. """ prefix = f"Linux-cargo-{profile}-v3-{digest([environment, dependencies])}-" return { diff --git a/dev/ci/test-native-cache-key.py b/dev/ci/test-native-cache-key.py index 53c8a059ba9..e20bad5b1a6 100644 --- a/dev/ci/test-native-cache-key.py +++ b/dev/ci/test-native-cache-key.py @@ -50,6 +50,11 @@ def setUp(self): ".github/workflows/README.md": "CI documentation\n", ".github/workflows/spark_sql_test_reusable.yml": "jobs: {}\n", ".github/workflows/check_pr_title.yml": "jobs: {}\n", + "dev/ci/compute-changes.py": "# shared native input rules\n", + "contrib/delta/native/Cargo.toml": '[package]\nname = "delta"\n', + "contrib/delta/native/src/lib.rs": "fn delta() {}\n", + "contrib/delta/native/Cargo.lock": "version = 4\n", + "contrib/a/b/native/Cargo.toml": '[package]\nname = "nested"\n', "native/core/benches/perf.rs": "fn benchmark() {}\n"} for name, content in self.inputs.items(): self.write(name, content) @@ -78,6 +83,7 @@ def test_source_and_dependency_changes_invalidate_the_right_keys(self): """Native/protobuf edits retain the dependency prefix; dependency edits replace it.""" before = self.keys() for name in ("native/lib.rs", "native/proto/expr.proto", "native/Cargo.toml", "native/Cargo.lock", + "contrib/delta/native/Cargo.toml", "dev/ci/compute-changes.py", ".github/workflows/spark_sql_test_reusable.yml"): with self.subTest(name=name): self.write(name, self.inputs[name] + "changed\n") @@ -100,6 +106,9 @@ def test_generated_files_and_unrelated_jvm_edits_preserve_keys(self): self.write("README.md", "updated docs") self.write(".github/workflows/README.md", "updated CI docs") self.write(".github/workflows/check_pr_title.yml", "jobs: {changed: {}}") + self.write("contrib/delta/native/src/lib.rs", "fn changed_delta() {}") + self.write("contrib/delta/native/Cargo.lock", "version = 3\n") + self.write("contrib/a/b/native/Cargo.toml", '[package]\nname = "changed_nested"\n') self.write("native/core/benches/perf.rs", "fn changed_benchmark() {}") self.assertEqual(before, self.keys()) self.assertNotEqual(debug["source-key"], self.keys("debug")["source-key"]) @@ -109,9 +118,12 @@ def test_every_binary_key_input_has_a_main_cache_warmer(self): project = Path(__file__).resolve().parents[2] route = runpy.run_path(str(project / "dev/ci/compute-changes.py"))["compute"] _, sources = CACHE.source_inputs(project) - for name in [*sources, ".cargo/config.toml", "rust-toolchain", "contrib/new/native/src/lib.rs"]: + for name in [*sources, ".cargo/config.toml", "rust-toolchain", "contrib/new/native/Cargo.toml"]: self.assertTrue(route([name], {"name": "push"})["build_linux"], name) - self.assertFalse(route(["contrib/new/native/src/lib.rs"], {"name": "pull_request"})["build_linux"]) + for name in ("contrib/delta/native/src/lib.rs", "contrib/delta/native/Cargo.lock", + "contrib/a/b/native/Cargo.toml", "contrib/a/b/native/x.rs"): + self.assertFalse(route([name], {"name": "push"})["build_linux"], name) + self.assertFalse(route(["contrib/new/native/Cargo.toml"], {"name": "pull_request"})["build_linux"]) def test_tools_jdk_flags_and_tracked_build_configuration_invalidate(self): """Observed tool/package versions, Java metadata, flags and tracked configs enter keys.""" @@ -122,6 +134,7 @@ def test_tools_jdk_flags_and_tracked_build_configuration_invalidate(self): self.versions[tool] += "changed\n" self.assertNotEqual(before["source-key"], self.keys()["source-key"]) self.assertNotEqual(before["binary-key"], self.keys()["binary-key"]) + self.assertNotEqual(before["restore-prefix"], self.keys()["restore-prefix"]) self.versions[tool] = old self.write("jdk/release", 'JAVA_VERSION="17.0.2"\n') self.assertNotEqual(before["binary-key"], self.keys()["binary-key"]) From bd4ee8a14bfd42fe7a35e6219985f667936c3bc9 Mon Sep 17 00:00:00 2001 From: Chao Sun Date: Wed, 16 Sep 2026 17:34:38 +0000 Subject: [PATCH 6/8] ci: narrow native cache inputs and test routing --- .github/actions/build-native-ci/action.yaml | 8 ++--- .github/workflows/README.md | 28 ++++++++++++---- dev/ci/compute-changes.py | 11 ++---- dev/ci/native-cache-key.py | 26 +++++++++------ dev/ci/test-native-cache-key.py | 37 +++++++++++++++++---- 5 files changed, 74 insertions(+), 36 deletions(-) diff --git a/.github/actions/build-native-ci/action.yaml b/.github/actions/build-native-ci/action.yaml index 630e8e57bed..f8b1106a527 100644 --- a/.github/actions/build-native-ci/action.yaml +++ b/.github/actions/build-native-ci/action.yaml @@ -20,13 +20,15 @@ description: 'Reuse an exact-input native library, otherwise build it with the C runs: using: composite steps: + - name: Pin native build flags + shell: bash + run: echo 'RUSTFLAGS=-Ctarget-cpu=x86-64-v3 -Clink-arg=-fuse-ld=bfd' >> "$GITHUB_ENV" + # Call after checkout and setup-builder. Compute once, before Cargo writes # generated Rust files, and use the same keys for both restore and save. - name: Fingerprint native build inputs id: key shell: bash - env: - RUSTFLAGS: '-Ctarget-cpu=x86-64-v3 -Clink-arg=-fuse-ld=bfd' run: python3 dev/ci/native-cache-key.py --profile ci --github-output "$GITHUB_OUTPUT" - name: Restore native library cache @@ -54,8 +56,6 @@ runs: - name: Build native library (CI profile) if: steps.binary-cache.outputs.cache-hit != 'true' || (github.event_name == 'push' && github.ref == 'refs/heads/main') shell: bash - env: - RUSTFLAGS: '-Ctarget-cpu=x86-64-v3 -Clink-arg=-fuse-ld=bfd' run: | cd native # A target-directory cache is only an incremental build aid. Cargo diff --git a/.github/workflows/README.md b/.github/workflows/README.md index f670ee3f073..bd23911e10a 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -417,10 +417,18 @@ downstream tests use the same paths in either case. `dev/ci/native-cache-key.py` snapshots tracked native/protobuf/dependency files, Cargo configuration and the native build recipes before Cargo generates source files. The key also includes Rust versions, installed system package -versions, architecture, JDK release/path and compiler flags. The helper targets -our official Rust container and `setup-builder`, not arbitrary local toolchains. -Spark-only edits, documentation, unrelated workflows and generated files preserve -the key; native/protobuf changes invalidate it. Optional contrib crates contribute +versions, architecture, JDK release/path and 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 helper targets +our official Rust container and `setup-builder`. Adding external tools or files +requires updating this contract; recording an override's path does not identify +arbitrary contents stored there. + +The shared build and setup actions are fingerprinted; the four caller workflows +are not. Their selected Rust/JDK versions and build environment are observed +directly, so editing a test matrix or shard does not force a native rebuild. +Spark-only edits, documentation and generated files also preserve the key; +native/protobuf changes invalidate it. Optional contrib crates contribute their manifests, which Cargo resolves even with their features disabled, but not their Rust sources or standalone lockfiles. Benchmarks enter the debug cache key but not the library key. The input lists and glob matcher are shared with main's @@ -428,7 +436,11 @@ cache routing in `compute-changes.py`. The shared action uses portable `x86-64-v code generation. The incremental cache contains the effective `CARGO_HOME` registry/git directories -and `native/target`. Its dependency prefix permits reuse after source changes +and `native/target`. In the Rust container, correcting `~/.cargo` to +`/usr/local/cargo` adds the registry and Git checkouts that the old entry did not +contain. The incremental entry therefore grows alongside the addition of the +separate finished-library entry. +Its dependency prefix permits reuse after source changes within the same build environment, but every restore still invokes Cargo. Environment changes also invalidate this fallback: native dependencies compile C against JNI headers and cache build-script outputs that Cargo does not fully @@ -443,8 +455,10 @@ also trigger main's cache warmer. GitHub Actions handles cache storage and restoration. Preflight tests key invalidation, generated-file stability, container checkout -ownership, and that every binary-key input triggers main's cache warmer. After -main populates the new namespace, verify a hosted library hit +ownership, and that every binary-key input triggers main's cache warmer. On the +first main push that populates these namespaces, report the compressed cache +sizes in bytes for both the finished library and the incremental Cargo entry, +using the cache-save logs or Actions cache API. Then verify a hosted library hit by checking that `Restore native library cache` reports an exact hit and the Cargo steps skip, while the normal artifact upload and downstream tests pass. diff --git a/dev/ci/compute-changes.py b/dev/ci/compute-changes.py index f445ef6b78f..2789d7b0755 100644 --- a/dev/ci/compute-changes.py +++ b/dev/ci/compute-changes.py @@ -38,8 +38,8 @@ import sys from pathlib import Path -# Shared cache recipes affect every native producer. Tests exercise the recipes -# but do not affect the resulting library, so they are routed separately below. +# Shared cache recipes affect every native producer. Their tests run in +# Preflight and retain the existing dev/ci/** routes, without adding consumers. NATIVE_CACHE_RECIPES = ( ".github/actions/build-native-ci/**", "dev/ci/native-cache-key.py", "dev/ci/compute-changes.py", @@ -51,8 +51,6 @@ NATIVE_BUILD_INPUTS = ( "native/**", "contrib/*/native/Cargo.toml", ".cargo/**", ".github/actions/setup-builder/**", *NATIVE_CACHE_RECIPES, - ".github/workflows/pr_build_linux.yml", ".github/workflows/spark_sql_test_reusable.yml", - ".github/workflows/iceberg_spark_test_reusable.yml", ".github/workflows/spark_sql_writer_tests.yml", "rust-toolchain", "rust-toolchain.toml", "!**.md", ) NATIVE_LIBRARY_INPUTS = (*NATIVE_BUILD_INPUTS, "!**/benches/**") @@ -413,10 +411,7 @@ "build_linux", "spark_3_4", "spark_3_5", "spark_4_0", "spark_4_1", "iceberg_1_8", "iceberg_1_9", "iceberg_1_10", "iceberg_1_11", ): - FILTERS[_native_consumer].extend([ - *NATIVE_CACHE_RECIPES, - "dev/ci/test-native-cache-key.py", - ]) + FILTERS[_native_consumer].extend(NATIVE_CACHE_RECIPES) FILTERS["spark_4_1_hive"] = FILTERS["spark_4_1"] FILTERS["build_linux_full"] = FILTERS["build_linux"] diff --git a/dev/ci/native-cache-key.py b/dev/ci/native-cache-key.py index 03fe6e550c1..ca2cb107352 100644 --- a/dev/ci/native-cache-key.py +++ b/dev/ci/native-cache-key.py @@ -25,24 +25,24 @@ import argparse import hashlib +import importlib.util import json import os from pathlib import Path -import runpy import subprocess # Share both the input patterns and their glob semantics with main's warmer. -CHANGES = runpy.run_path(str(Path(__file__).with_name("compute-changes.py"))) +SPEC = importlib.util.spec_from_file_location("compute_changes", Path(__file__).with_name("compute-changes.py")) +CHANGES = importlib.util.module_from_spec(SPEC) +SPEC.loader.exec_module(CHANGES) def digest(value): - """Return a stable SHA-256 for JSON-compatible build inputs.""" return hashlib.sha256(json.dumps(value, sort_keys=True).encode()).hexdigest() def command(args, cwd): - """Read command stdout in cwd; missing tools or unsuccessful commands fail CI.""" return subprocess.check_output(args, cwd=cwd, text=True).strip() @@ -54,7 +54,7 @@ def source_inputs(root, profile="ci"): builds omit benchmarks; debug checks compile them. Trust only this checkout for the Git read: container steps can run as a different owner than checkout. """ - patterns = CHANGES["NATIVE_LIBRARY_INPUTS" if profile == "ci" else "NATIVE_BUILD_INPUTS"] + patterns = CHANGES.NATIVE_LIBRARY_INPUTS if profile == "ci" else CHANGES.NATIVE_BUILD_INPUTS inventory = command(["git", "-c", f"safe.directory={root}", "ls-files", "--stage", "-z"], root) sources = {} @@ -62,7 +62,7 @@ def source_inputs(root, profile="ci"): if not record: continue metadata, name = record.split("\t", 1) - if CHANGES["matches"](patterns, [name]): + if CHANGES.matches(patterns, [name]): sources[name] = [metadata.split()[0], hashlib.sha256((root / name).read_bytes()).hexdigest()] dependencies = {name: value for name, value in sources.items() @@ -75,9 +75,10 @@ def environment_inputs(root, env): Rust's versions include the compiler commit; dpkg identifies the installed C/C++/protobuf tools and system libraries. The JDK release file identifies - the vendor/build supplying JNI headers and libjvm. Paths and RUSTFLAGS are - included because linking can embed them. Native producer workflows/actions - in the source map cover how these tools are configured and invoked. + the vendor/build supplying JNI headers and libjvm. Record build overrides, + including target-qualified cc variables and HDFS linking options, without + including unrelated per-run GitHub variables. The shared setup/build actions + are hashed separately; caller test configuration does not affect the library. """ java_home = Path(env["JAVA_HOME"]) return { @@ -91,7 +92,12 @@ def environment_inputs(root, env): "java_home": str(java_home), "java_release": (java_home / "release").read_text(), "cargo_home": env.get("CARGO_HOME", str(Path.home() / ".cargo")), - "rustflags": env["RUSTFLAGS"], + "env": {name: value for name, value in env.items() + if name.startswith(("CARGO_", "RUST", "HOST_", "TARGET_", "HDFS_")) + or name.split("_", 1)[0] in {"CC", "CXX", "CFLAGS", "CXXFLAGS", "CXXSTDLIB", + "LDFLAGS", "AR", "ARFLAGS", "RANLIB", "RANLIBFLAGS", "PROTOC"} + or name in {"JAVA_HOME", "PATH", "HADOOP_HOME", "DOCS_RS", + "CRATE_CC_NO_DEFAULTS", "CROSS_COMPILE"}}, } diff --git a/dev/ci/test-native-cache-key.py b/dev/ci/test-native-cache-key.py index e20bad5b1a6..b8c93f3c905 100644 --- a/dev/ci/test-native-cache-key.py +++ b/dev/ci/test-native-cache-key.py @@ -22,7 +22,6 @@ import io import os from pathlib import Path -import runpy import subprocess import sys import tempfile @@ -48,8 +47,13 @@ def setUp(self): "native/lib.rs": "fn native() {}\n", "native/proto/expr.proto": "message Expr {}\n", "spark/Plan.scala": "object Plan {}\n", "README.md": "Comet\n", ".github/workflows/README.md": "CI documentation\n", + ".github/workflows/pr_build_linux.yml": "jobs: {}\n", ".github/workflows/spark_sql_test_reusable.yml": "jobs: {}\n", + ".github/workflows/iceberg_spark_test_reusable.yml": "jobs: {}\n", + ".github/workflows/spark_sql_writer_tests.yml": "jobs: {}\n", ".github/workflows/check_pr_title.yml": "jobs: {}\n", + ".github/actions/build-native-ci/action.yaml": "runs: {}\n", + ".github/actions/setup-builder/action.yaml": "runs: {}\n", "dev/ci/compute-changes.py": "# shared native input rules\n", "contrib/delta/native/Cargo.toml": '[package]\nname = "delta"\n', "contrib/delta/native/src/lib.rs": "fn delta() {}\n", @@ -84,7 +88,7 @@ def test_source_and_dependency_changes_invalidate_the_right_keys(self): before = self.keys() for name in ("native/lib.rs", "native/proto/expr.proto", "native/Cargo.toml", "native/Cargo.lock", "contrib/delta/native/Cargo.toml", "dev/ci/compute-changes.py", - ".github/workflows/spark_sql_test_reusable.yml"): + ".github/actions/build-native-ci/action.yaml", ".github/actions/setup-builder/action.yaml"): with self.subTest(name=name): self.write(name, self.inputs[name] + "changed\n") after = self.keys() @@ -100,12 +104,16 @@ def test_generated_files_and_unrelated_jvm_edits_preserve_keys(self): """Generated files and non-build edits preserve reuse; debug still tracks benchmarks.""" before = self.keys() debug = self.keys("debug") + for name in self.inputs: + if name.startswith(".github/workflows/"): + self.write(name, "unrelated test configuration\n") + self.env["GITHUB_RUN_ID"] = "12345" + self.assertEqual(before, self.keys()) + self.assertEqual(debug, self.keys("debug")) self.write("native/proto/src/generated/expr.rs", "generated Rust") self.write("native/target/ci/libcomet.so", "compiled library") self.write("spark/Plan.scala", "object NewPlan {}") self.write("README.md", "updated docs") - self.write(".github/workflows/README.md", "updated CI docs") - self.write(".github/workflows/check_pr_title.yml", "jobs: {changed: {}}") self.write("contrib/delta/native/src/lib.rs", "fn changed_delta() {}") self.write("contrib/delta/native/Cargo.lock", "version = 3\n") self.write("contrib/a/b/native/Cargo.toml", '[package]\nname = "changed_nested"\n') @@ -113,10 +121,10 @@ def test_generated_files_and_unrelated_jvm_edits_preserve_keys(self): self.assertEqual(before, self.keys()) self.assertNotEqual(debug["source-key"], self.keys("debug")["source-key"]) - def test_every_binary_key_input_has_a_main_cache_warmer(self): - """Check real tracked inputs against push routing, plus inputs absent from today's tree.""" + def test_native_input_routing(self): + """Library inputs warm main; helper tests retain Linux coverage without extra consumers.""" project = Path(__file__).resolve().parents[2] - route = runpy.run_path(str(project / "dev/ci/compute-changes.py"))["compute"] + route = CACHE.CHANGES.compute _, sources = CACHE.source_inputs(project) for name in [*sources, ".cargo/config.toml", "rust-toolchain", "contrib/new/native/Cargo.toml"]: self.assertTrue(route([name], {"name": "push"})["build_linux"], name) @@ -124,6 +132,11 @@ def test_every_binary_key_input_has_a_main_cache_warmer(self): "contrib/a/b/native/Cargo.toml", "contrib/a/b/native/x.rs"): self.assertFalse(route([name], {"name": "push"})["build_linux"], name) self.assertFalse(route(["contrib/new/native/Cargo.toml"], {"name": "pull_request"})["build_linux"]) + for event in ("merge_group", "schedule"): + routed = route(["dev/ci/test-native-cache-key.py"], {"name": event}) + self.assertTrue(routed["build_linux" if event == "merge_group" else "build_linux_all_profiles"]) + self.assertFalse(any(selected for name, selected in routed.items() + if name.startswith(("spark_", "iceberg_")))) def test_tools_jdk_flags_and_tracked_build_configuration_invalidate(self): """Observed tool/package versions, Java metadata, flags and tracked configs enter keys.""" @@ -142,6 +155,16 @@ def test_tools_jdk_flags_and_tracked_build_configuration_invalidate(self): self.env["RUSTFLAGS"] += " -Copt-level=1" self.assertNotEqual(before["binary-key"], self.keys()["binary-key"]) self.env["RUSTFLAGS"] = "-Ctarget-cpu=x86-64-v3 -Clink-arg=-fuse-ld=bfd" + for name in ("CC", "CXX", "CFLAGS", "LDFLAGS", "AR", "PROTOC", "PROTOC_INCLUDE", + "RUSTC_WRAPPER", "CARGO_BUILD_TARGET", "CARGO_PROFILE_CI_OPT_LEVEL", + "CC_x86_64_unknown_linux_gnu", "HOST_CC", "TARGET_CFLAGS", + "HDFS_LIB_DIR", "HADOOP_HOME", "HDFS_STATIC", "DOCS_RS", "PATH"): + with self.subTest(environment=name): + self.env[name] = "build override" + after = self.keys() + for key in ("binary-key", "source-key", "restore-prefix"): + self.assertNotEqual(before[key], after[key]) + del self.env[name] self.write(".cargo/config.toml", "[build]\nincremental = false\n") subprocess.run(["git", "add", ".cargo/config.toml"], cwd=self.root, check=True) self.assertNotEqual(before["binary-key"], self.keys()["binary-key"]) From 1dc4bf31839c613c12b65075f97ca58ee6ad1549 Mon Sep 17 00:00:00 2001 From: Chao Sun Date: Thu, 17 Sep 2026 17:18:04 +0000 Subject: [PATCH 7/8] ci: skip redundant native builds on main cache hits --- .github/actions/build-native-ci/action.yaml | 12 +-- .github/workflows/README.md | 87 ++++++++------------- .github/workflows/ci.yml | 6 +- dev/ci/compute-changes.py | 10 ++- dev/ci/test-native-cache-key.py | 41 ++++++---- dev/ci/test-native-cache-workflow.py | 84 ++++++++++++++++++++ 6 files changed, 159 insertions(+), 81 deletions(-) create mode 100644 dev/ci/test-native-cache-workflow.py diff --git a/.github/actions/build-native-ci/action.yaml b/.github/actions/build-native-ci/action.yaml index f8b1106a527..4914ddf5370 100644 --- a/.github/actions/build-native-ci/action.yaml +++ b/.github/actions/build-native-ci/action.yaml @@ -37,8 +37,8 @@ runs: with: path: native/target/ci/libcomet.so key: ${{ steps.key.outputs.binary-key }} - # Main still builds to keep its incremental Cargo cache warm. Lookup - # only avoids downloading a library that this run will not execute. + # Main restores the incremental cache below, including libcomet.so. + # A lookup avoids downloading the same library separately. lookup-only: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }} - name: Restore incremental Cargo cache @@ -54,13 +54,13 @@ runs: restore-keys: ${{ steps.key.outputs.restore-prefix }} - name: Build native library (CI profile) - if: steps.binary-cache.outputs.cache-hit != 'true' || (github.event_name == 'push' && github.ref == 'refs/heads/main') + if: steps.binary-cache.outputs.cache-hit != 'true' || (github.event_name == 'push' && github.ref == 'refs/heads/main' && steps.cargo-cache.outputs.cache-hit != 'true') shell: bash run: | cd native - # A target-directory cache is only an incremental build aid. Cargo - # must run even on an exact target-cache hit; only the separately - # keyed library cache can replace compilation. + # A library miss always builds, even on an exact incremental hit. + # Main also builds to populate a missing incremental entry; two exact + # hits already supply the library and leave neither cache to publish. cargo build --locked --profile ci - name: Save native library cache diff --git a/.github/workflows/README.md b/.github/workflows/README.md index bd23911e10a..32f4c489512 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -407,60 +407,39 @@ which jobs do run on main and therefore do write. ## Reusing Linux native builds The Linux, Spark SQL, Iceberg and manual writer workflows call -`.github/actions/build-native-ci` after checkout and `setup-builder`. An exact -cache hit restores `native/target/ci/libcomet.so` and skips Cargo. A miss restores -an incremental cache and runs `cargo build --locked --profile ci`. Artifacts and -downstream tests use the same paths in either case. -`--locked` deliberately fails when a manifest change requires updating -`native/Cargo.lock`; contributors must commit that lockfile update with the change. - -`dev/ci/native-cache-key.py` snapshots tracked native/protobuf/dependency files, -Cargo configuration and the native build recipes before Cargo generates source -files. The key also includes Rust versions, installed system package -versions, architecture, JDK release/path and 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 helper targets -our official Rust container and `setup-builder`. Adding external tools or files -requires updating this contract; recording an override's path does not identify -arbitrary contents stored there. - -The shared build and setup actions are fingerprinted; the four caller workflows -are not. Their selected Rust/JDK versions and build environment are observed -directly, so editing a test matrix or shard does not force a native rebuild. -Spark-only edits, documentation and generated files also preserve the key; -native/protobuf changes invalidate it. Optional contrib crates contribute -their manifests, which Cargo resolves even with their features disabled, but not -their Rust sources or standalone lockfiles. Benchmarks enter the debug cache key -but not the library key. The input lists and glob matcher are shared with main's -cache routing in `compute-changes.py`. The shared action uses portable `x86-64-v3` -code generation. - -The incremental cache contains the effective `CARGO_HOME` registry/git directories -and `native/target`. In the Rust container, correcting `~/.cargo` to -`/usr/local/cargo` adds the registry and Git checkouts that the old entry did not -contain. The incremental entry therefore grows alongside the addition of the -separate finished-library entry. -Its dependency prefix permits reuse after source changes -within the same build environment, but every restore still invokes Cargo. -Environment changes also invalidate this fallback: native dependencies compile C -against JNI headers and cache build-script outputs that Cargo does not fully -invalidate after external compiler or JDK changes. This can miss after unrelated -package updates, but prevents reusing those objects under a new library key. -The Rust test job uses a separate debug key and continues to run all checks and tests. - -Only pushes to `main` save either cache. Main always compiles to keep the -incremental cache warm. Other runs consume matching entries; a cold or evicted -cache builds normally. Changes to shared native inputs owned by other workflows -also trigger main's cache warmer. GitHub Actions handles cache storage and -restoration. - -Preflight tests key invalidation, generated-file stability, container checkout -ownership, and that every binary-key input triggers main's cache warmer. On the -first main push that populates these namespaces, report the compressed cache -sizes in bytes for both the finished library and the incremental Cargo entry, -using the cache-save logs or Actions cache API. Then verify a hosted library hit -by checking that `Restore native library cache` reports an exact hit and the -Cargo steps skip, while the normal artifact upload and downstream tests pass. +`.github/actions/build-native-ci` after checkout and `setup-builder`. PR, queue, +scheduled and manual runs restore `native/target/ci/libcomet.so` and skip Cargo +on an exact library match. Only +pushes to `main` save caches: main skips Cargo when both the library and incremental +cache match exactly, and builds when either lacks an exact match to replenish it. +An incremental cache hit alone never replaces compilation. Builds use +`cargo build --locked --profile ci`; manifest changes requiring a lockfile update +must include that update to `native/Cargo.lock`. Artifact paths remain unchanged. + +`dev/ci/native-cache-key.py` snapshots native sources, protobufs, dependencies, +Cargo configuration and shared build/setup actions before source generation. +It includes Rust versions, installed package versions, architecture, JDK +release/path, and Cargo/Rust, C/C++ compiler/flag and HDFS environment overrides. +Caller workflows are excluded because their selected tools and environment are +observed directly. Spark edits, documentation, generated files and disabled +contrib sources preserve the key; contrib manifests remain inputs for `--locked`. +Benchmarks enter only the debug key. The input lists and glob matcher are shared +with main's routing in `compute-changes.py`; code generation uses `x86-64-v3`. + +The helper supports the official Rust container and `setup-builder`. Introducing +external tools or files requires updating that contract: an override's path does +not identify arbitrary contents stored there. Both binary and incremental keys +retain package and JDK identity because native dependencies compile against JNI +headers and link `libjvm`, and Cargo does not fully track external tool/header +changes. Unrelated package updates can therefore cause conservative misses. + +The incremental cache holds `native/target` and the effective `CARGO_HOME` +registry/Git directories. Correcting the container's path to `/usr/local/cargo` +adds previously uncached dependencies, increasing the shared cache budget needed +alongside the finished library. Its fallback permits source changes within the +same dependency/build environment. Rust checks and tests always run with their +separate debug cache. Preflight checks fingerprint invalidation, main's routing, +and the action's cache-hit/miss behavior. ## Retrying flaky network operations diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d97e7e55aff..7e8267bcb56 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -134,8 +134,10 @@ jobs: - name: Check Iceberg shard inventory validation run: python3 dev/ci/test-iceberg-shards.py - - name: Check native cache keys - run: python3 dev/ci/test-native-cache-key.py + - name: Check native caching + run: | + python3 dev/ci/test-native-cache-key.py + python3 dev/ci/test-native-cache-workflow.py - name: Check CI config invariants run: python3 dev/ci/check-ci-config.py diff --git a/dev/ci/compute-changes.py b/dev/ci/compute-changes.py index 2789d7b0755..57eb13ff7c0 100644 --- a/dev/ci/compute-changes.py +++ b/dev/ci/compute-changes.py @@ -71,6 +71,7 @@ ".github/workflows/ci.yml", ".github/workflows/pr_build_linux.yml", ".github/actions/setup-builder/**", + ".github/actions/build-native-ci/**", ".github/actions/java-test/**", ".github/actions/maven-bootstrap/**", ".github/actions/rust-test/**", @@ -405,10 +406,10 @@ "mvnw", ], } -# These inputs are shared by the Linux native producers. Keep the routes in -# one place so an action-only cache change exercises each applicable consumer. +# Spark and Iceberg producers share these recipes. Linux routes the action +# above and already covers the Python helpers through dev/ci/**. for _native_consumer in ( - "build_linux", "spark_3_4", "spark_3_5", "spark_4_0", "spark_4_1", + "spark_3_4", "spark_3_5", "spark_4_0", "spark_4_1", "iceberg_1_8", "iceberg_1_9", "iceberg_1_10", "iceberg_1_11", ): FILTERS[_native_consumer].extend(NATIVE_CACHE_RECIPES) @@ -578,7 +579,8 @@ def compute(files, event): } # Use the fingerprint's exact patterns and matcher for main's producer, # including inputs owned by other workflows, without broadening PR jobs. - if event.get("name") == "push" and matches(NATIVE_LIBRARY_INPUTS, files): + if (event.get("name") == "push" and event_allows("build_linux", event) + and matches(NATIVE_LIBRARY_INPUTS, files)): selected["build_linux"] = True return selected diff --git a/dev/ci/test-native-cache-key.py b/dev/ci/test-native-cache-key.py index b8c93f3c905..7c06ab82ea6 100644 --- a/dev/ci/test-native-cache-key.py +++ b/dev/ci/test-native-cache-key.py @@ -123,14 +123,23 @@ def test_generated_files_and_unrelated_jvm_edits_preserve_keys(self): def test_native_input_routing(self): """Library inputs warm main; helper tests retain Linux coverage without extra consumers.""" - project = Path(__file__).resolve().parents[2] route = CACHE.CHANGES.compute - _, sources = CACHE.source_inputs(project) - for name in [*sources, ".cargo/config.toml", "rust-toolchain", "contrib/new/native/Cargo.toml"]: + inputs = ("native/core/src/lib.rs", "native/proto/expr.proto", + "contrib/new/native/Cargo.toml", ".cargo/config.toml", + ".github/actions/setup-builder/action.yaml", + ".github/actions/build-native-ci/action.yaml", + "dev/ci/native-cache-key.py", "dev/ci/compute-changes.py", + "rust-toolchain", "rust-toolchain.toml") + for name in inputs: + self.assertTrue(CACHE.CHANGES.matches(CACHE.CHANGES.NATIVE_LIBRARY_INPUTS, [name]), name) self.assertTrue(route([name], {"name": "push"})["build_linux"], name) - for name in ("contrib/delta/native/src/lib.rs", "contrib/delta/native/Cargo.lock", + for name in ("native/core/README.md", "native/core/benches/perf.rs", + "contrib/delta/native/src/lib.rs", "contrib/delta/native/Cargo.lock", "contrib/a/b/native/Cargo.toml", "contrib/a/b/native/x.rs"): + self.assertFalse(CACHE.CHANGES.matches(CACHE.CHANGES.NATIVE_LIBRARY_INPUTS, [name]), name) self.assertFalse(route([name], {"name": "push"})["build_linux"], name) + with patch.dict(CACHE.CHANGES.POLICY, {"build_linux": ["pr", "queue"]}): + self.assertFalse(route([".cargo/config.toml"], {"name": "push"})["build_linux"]) self.assertFalse(route(["contrib/new/native/Cargo.toml"], {"name": "pull_request"})["build_linux"]) for event in ("merge_group", "schedule"): routed = route(["dev/ci/test-native-cache-key.py"], {"name": event}) @@ -139,15 +148,13 @@ def test_native_input_routing(self): if name.startswith(("spark_", "iceberg_")))) def test_tools_jdk_flags_and_tracked_build_configuration_invalidate(self): - """Observed tool/package versions, Java metadata, flags and tracked configs enter keys.""" + """Capture build overrides only; tools, Java metadata and tracked configs invalidate keys.""" before = self.keys() for tool in self.versions: with self.subTest(tool=tool): old = self.versions[tool] self.versions[tool] += "changed\n" - self.assertNotEqual(before["source-key"], self.keys()["source-key"]) self.assertNotEqual(before["binary-key"], self.keys()["binary-key"]) - self.assertNotEqual(before["restore-prefix"], self.keys()["restore-prefix"]) self.versions[tool] = old self.write("jdk/release", 'JAVA_VERSION="17.0.2"\n') self.assertNotEqual(before["binary-key"], self.keys()["binary-key"]) @@ -155,16 +162,20 @@ def test_tools_jdk_flags_and_tracked_build_configuration_invalidate(self): self.env["RUSTFLAGS"] += " -Copt-level=1" self.assertNotEqual(before["binary-key"], self.keys()["binary-key"]) self.env["RUSTFLAGS"] = "-Ctarget-cpu=x86-64-v3 -Clink-arg=-fuse-ld=bfd" - for name in ("CC", "CXX", "CFLAGS", "LDFLAGS", "AR", "PROTOC", "PROTOC_INCLUDE", + overrides = ("CC", "CXX", "CFLAGS", "LDFLAGS", "AR", "PROTOC", "PROTOC_INCLUDE", "RUSTC_WRAPPER", "CARGO_BUILD_TARGET", "CARGO_PROFILE_CI_OPT_LEVEL", "CC_x86_64_unknown_linux_gnu", "HOST_CC", "TARGET_CFLAGS", - "HDFS_LIB_DIR", "HADOOP_HOME", "HDFS_STATIC", "DOCS_RS", "PATH"): - with self.subTest(environment=name): - self.env[name] = "build override" - after = self.keys() - for key in ("binary-key", "source-key", "restore-prefix"): - self.assertNotEqual(before[key], after[key]) - del self.env[name] + "HDFS_LIB_DIR", "HADOOP_HOME", "HDFS_STATIC", "DOCS_RS", "PATH") + build_env = {**self.env, **dict.fromkeys(overrides, "build override")} + with patch.object(CACHE, "command", side_effect=lambda args, cwd: self.versions[args[0]]): + environment = CACHE.environment_inputs( + self.root, {**build_env, "GITHUB_RUN_ID": "12345", "UNRELATED": "ignored"}) + self.assertEqual(environment["env"], build_env) + self.env["TARGET_CFLAGS"] = "build override" + after = self.keys() + for key in ("binary-key", "source-key", "restore-prefix"): + self.assertNotEqual(before[key], after[key]) + del self.env["TARGET_CFLAGS"] self.write(".cargo/config.toml", "[build]\nincremental = false\n") subprocess.run(["git", "add", ".cargo/config.toml"], cwd=self.root, check=True) self.assertNotEqual(before["binary-key"], self.keys()["binary-key"]) diff --git a/dev/ci/test-native-cache-workflow.py b/dev/ci/test-native-cache-workflow.py new file mode 100644 index 00000000000..9655cc9f02a --- /dev/null +++ b/dev/ci/test-native-cache-workflow.py @@ -0,0 +1,84 @@ +#!/usr/bin/env python3 +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Check the native action's actual conditions for cache hits and misses.""" + +from pathlib import Path +import re +import shlex +import subprocess +import unittest + + +def condition_matches(expression, context): + """Evaluate the action's string comparisons and boolean operators with Bash. + + Substitute fixed test context values into the expression read from YAML. + Bash [[ ]] supports the comparisons, parentheses, && and || used here; + unsupported syntax fails the test instead of emulating the Actions runner. + """ + expression = expression.removeprefix("${{ ").removesuffix(" }}") + for name, value in context.items(): + expression = expression.replace(name, shlex.quote(value)) + result = subprocess.run(["bash", "-c", f"[[ {expression} ]]"], capture_output=True, text=True) + if result.returncode not in (0, 1): + raise AssertionError(result.stderr) + return result.returncode == 0 + + +class NativeCacheWorkflowTests(unittest.TestCase): + def test_cache_hit_decisions(self): + """Protect compilation, lookup-only restores, and main-only publication.""" + project = Path(__file__).resolve().parents[2] + action = (project / ".github/actions/build-native-ci/action.yaml").read_text() + conditions = {} + for block in re.split(r"^ - name: ", action, flags=re.MULTILINE)[1:]: + name, _, body = block.partition("\n") + for line in body.splitlines(): + if line.startswith(" if: "): + conditions[name] = line.removeprefix(" if: ") + elif line.startswith(" lookup-only: "): + conditions["lookup-only"] = line.removeprefix(" lookup-only: ") + names = ("lookup-only", "Restore incremental Cargo cache", "Build native library (CI profile)", + "Save native library cache", "Save incremental Cargo cache") + self.assertEqual(set(conditions), set(names)) + # Expected: lookup only, restore Cargo, build, save library, save Cargo. + cases = [ + ("pull_request", "refs/pull/1/merge", "true", "", (False, False, False, False, False)), + ("pull_request", "refs/pull/1/merge", "", "true", (False, True, True, False, False)), + ("merge_group", "refs/heads/gh-readonly-queue/main/test", "true", "", (False, False, False, False, False)), + ("merge_group", "refs/heads/gh-readonly-queue/main/test", "", "false", (False, True, True, False, False)), + ("push", "refs/heads/main", "true", "true", (True, True, False, False, False)), + ("push", "refs/heads/main", "true", "false", (True, True, True, False, True)), + ("push", "refs/heads/main", "true", "", (True, True, True, False, True)), + ("push", "refs/heads/main", "", "true", (True, True, True, True, False)), + ("push", "refs/heads/main", "", "", (True, True, True, True, True)), + ("push", "refs/heads/branch", "", "", (False, True, True, False, False)), + ("schedule", "refs/heads/main", "", "", (False, True, True, False, False)), + ("workflow_dispatch", "refs/heads/main", "", "", (False, True, True, False, False)), + ] + for event, ref, library_hit, cargo_hit, expected in cases: + with self.subTest(event=event, ref=ref, library_hit=library_hit, cargo_hit=cargo_hit): + context = {"github.event_name": event, "github.ref": ref, + "steps.binary-cache.outputs.cache-hit": library_hit, + "steps.cargo-cache.outputs.cache-hit": cargo_hit} + self.assertEqual(tuple(condition_matches(conditions[name], context) for name in names), expected) + + +if __name__ == "__main__": + unittest.main() From 80296cb7bcddcd539fbf80e6cdeae67ba64ab877 Mon Sep 17 00:00:00 2001 From: Chao Sun Date: Thu, 17 Sep 2026 17:39:52 +0000 Subject: [PATCH 8/8] ci: keep native caches limited to compiled targets --- .github/actions/build-native-ci/action.yaml | 10 ++-------- .github/workflows/README.md | 19 +++++++++---------- .github/workflows/pr_build_linux.yml | 10 ++-------- dev/ci/native-cache-key.py | 1 - dev/ci/test-native-cache-key.py | 1 - 5 files changed, 13 insertions(+), 28 deletions(-) diff --git a/.github/actions/build-native-ci/action.yaml b/.github/actions/build-native-ci/action.yaml index 4914ddf5370..9b7ece38070 100644 --- a/.github/actions/build-native-ci/action.yaml +++ b/.github/actions/build-native-ci/action.yaml @@ -46,10 +46,7 @@ runs: if: steps.binary-cache.outputs.cache-hit != 'true' || (github.event_name == 'push' && github.ref == 'refs/heads/main') uses: actions/cache/restore@v6 with: - path: | - ${{ steps.key.outputs.cargo-home }}/registry - ${{ steps.key.outputs.cargo-home }}/git - native/target + path: native/target key: ${{ steps.key.outputs.source-key }} restore-keys: ${{ steps.key.outputs.restore-prefix }} @@ -74,8 +71,5 @@ runs: if: github.event_name == 'push' && github.ref == 'refs/heads/main' && steps.cargo-cache.outputs.cache-hit != 'true' uses: actions/cache/save@v6 with: - path: | - ${{ steps.key.outputs.cargo-home }}/registry - ${{ steps.key.outputs.cargo-home }}/git - native/target + path: native/target key: ${{ steps.key.outputs.source-key }} diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 32f4c489512..c7e3fb4f571 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -409,9 +409,9 @@ which jobs do run on main and therefore do write. The Linux, Spark SQL, Iceberg and manual writer workflows call `.github/actions/build-native-ci` after checkout and `setup-builder`. PR, queue, scheduled and manual runs restore `native/target/ci/libcomet.so` and skip Cargo -on an exact library match. Only -pushes to `main` save caches: main skips Cargo when both the library and incremental -cache match exactly, and builds when either lacks an exact match to replenish it. +on an exact library match. Only pushes to `main` save caches. Main skips Cargo +when both the library and incremental cache match exactly, and builds when either +lacks an exact match to replenish it. An incremental cache hit alone never replaces compilation. Builds use `cargo build --locked --profile ci`; manifest changes requiring a lockfile update must include that update to `native/Cargo.lock`. Artifact paths remain unchanged. @@ -433,13 +433,12 @@ retain package and JDK identity because native dependencies compile against JNI headers and link `libjvm`, and Cargo does not fully track external tool/header changes. Unrelated package updates can therefore cause conservative misses. -The incremental cache holds `native/target` and the effective `CARGO_HOME` -registry/Git directories. Correcting the container's path to `/usr/local/cargo` -adds previously uncached dependencies, increasing the shared cache budget needed -alongside the finished library. Its fallback permits source changes within the -same dependency/build environment. Rust checks and tests always run with their -separate debug cache. Preflight checks fingerprint invalidation, main's routing, -and the action's cache-hit/miss behavior. +The CI and debug incremental caches hold only `native/target`, including compiled +dependencies. Cargo fetches registry and Git dependency sources as needed; those +downloads are not duplicated in the repository's limited cache storage. Fallback +restores permit source changes within the same dependency/build environment. +Rust checks and tests always run with their separate debug cache. Preflight checks +fingerprint invalidation, main's routing, and the action's cache-hit/miss behavior. ## Retrying flaky network operations diff --git a/.github/workflows/pr_build_linux.yml b/.github/workflows/pr_build_linux.yml index e9e6cec44ec..3fb9ba3249c 100644 --- a/.github/workflows/pr_build_linux.yml +++ b/.github/workflows/pr_build_linux.yml @@ -413,10 +413,7 @@ jobs: id: cargo-cache uses: actions/cache/restore@v6 with: - path: | - ${{ steps.cargo-key.outputs.cargo-home }}/registry - ${{ steps.cargo-key.outputs.cargo-home }}/git - native/target + path: native/target key: ${{ steps.cargo-key.outputs.source-key }} restore-keys: ${{ steps.cargo-key.outputs.restore-prefix }} @@ -429,10 +426,7 @@ jobs: # at the same sha would only re-archive an entry that already exists. if: github.event_name == 'push' && github.ref == 'refs/heads/main' && steps.cargo-cache.outputs.cache-hit != 'true' with: - path: | - ${{ steps.cargo-key.outputs.cargo-home }}/registry - ${{ steps.cargo-key.outputs.cargo-home }}/git - native/target + path: native/target key: ${{ steps.cargo-key.outputs.source-key }} linux-test: diff --git a/dev/ci/native-cache-key.py b/dev/ci/native-cache-key.py index ca2cb107352..f07cad84a30 100644 --- a/dev/ci/native-cache-key.py +++ b/dev/ci/native-cache-key.py @@ -111,7 +111,6 @@ def cache_keys(profile, dependencies, sources, environment): """ prefix = f"Linux-cargo-{profile}-v3-{digest([environment, dependencies])}-" return { - "cargo-home": environment["cargo_home"], "source-key": prefix + digest(sources), "restore-prefix": prefix, "binary-key": f"Linux-native-ci-v2-{digest([environment, sources])}" if profile == "ci" else "", diff --git a/dev/ci/test-native-cache-key.py b/dev/ci/test-native-cache-key.py index 7c06ab82ea6..a5f7a004b7d 100644 --- a/dev/ci/test-native-cache-key.py +++ b/dev/ci/test-native-cache-key.py @@ -188,7 +188,6 @@ def test_profiles_have_separate_cargo_caches(self): self.assertEqual(debug["binary-key"], "") self.assertTrue(ci["binary-key"].startswith("Linux-native-ci-")) self.assertTrue(ci["source-key"].startswith(ci["restore-prefix"])) - self.assertEqual(ci["cargo-home"], self.env["CARGO_HOME"]) def test_container_ownership_works_without_global_git_config_changes(self): """A differently owned checkout permits helper root/inventory reads without global trust."""