sunchao commented on code in PR #5976:
URL: https://github.com/apache/datafusion-comet/pull/5976#discussion_r4127437837
##########
dev/ci/compute-changes.py:
##########
@@ -546,11 +572,17 @@ 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()
}
+ # 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 event_allows("build_linux", event)
Review Comment:
Fixed in de25d50f by extending the Linux filter with NATIVE_BUILD_INPUTS and
removing the push-only override. Lance/Delta manifests, root Cargo
configuration and toolchain inputs now select Linux validation on PRs and the
merge queue as well as main; nightly and release-branch policy is preserved.
Routing regressions cover these tiers. I also reproduced the underlying failure
with real Comet cargo metadata --locked --offline --filter-platform
x86_64-unknown-linux-gnu: baseline succeeds, changing only Lance's package
version requires a lockfile update and fails. dev/local-ci.sh now uses --locked
too.
##########
dev/ci/native-cache-key.py:
##########
@@ -0,0 +1,144 @@
+#!/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.
+
+"""Fingerprint the clean Linux checkout and toolchain used by Comet CI.
+
+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
+import hashlib
+import importlib.util
+import json
+import os
+from pathlib import Path
+import subprocess
+
+
+# Share both the input patterns and their glob semantics with main's warmer.
+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 hashlib.sha256(json.dumps(value,
sort_keys=True).encode()).hexdigest()
+
+
+def command(args, cwd):
+ return subprocess.check_output(args, cwd=cwd, text=True).strip()
+
+
+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, 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.
+ """
+ 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 = {}
+ for record in inventory.split("\0"):
+ if not record:
+ continue
+ metadata, name = record.split("\t", 1)
+ 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()
+ if Path(name).name in {"Cargo.toml", "Cargo.lock"}}
+ return dependencies, sources
+
+
+def environment_inputs(root, env):
+ """Identify the official tools installed by setup-builder without
modifying them.
+
+ 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. 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 {
+ "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")),
+ "env": {name: value for name, value in env.items()
Review Comment:
Added the preflight invariant alternative in de25d50f. It discovers
native-action callers and checks their declared workflow/job/step environment
through native reuse against the known builder contract. Unknown overrides,
caller GITHUB_ENV/GITHUB_PATH writes, and unsupported setup actions/layouts
fail preflight until the contract is updated. Mutation tests exercise
AWS_LC_SYS_NO_ASM, CMAKE_BUILD_TYPE, LZMA_API_STATIC and PKG_CONFIG_PATH at the
relevant scopes. Downstream JVM/test environment remains independent.
##########
.github/workflows/README.md:
##########
@@ -404,6 +404,42 @@ 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 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.
+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
Review Comment:
Documented and enforced in de25d50f. Every consumer must match main's
runner/container, Rust selection and JDK 17 to reuse its entries; another
environment needs a compatible publisher. Preflight checks all build-native-ci
callers, including the reusable workflow's Java bindings and both manual-writer
resolver arms. Mutations to JDK, image, runner and caller bindings fail the new
tests.
##########
dev/ci/compute-changes.py:
##########
@@ -546,11 +572,17 @@ 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()
}
+ # 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 event_allows("build_linux", event)
+ and matches(NATIVE_LIBRARY_INPUTS, files)):
+ selected["build_linux"] = True
+ return selected
Review Comment:
Applied in de25d50f exactly at the shared-filter boundary:
FILTERS["build_linux"].extend(NATIVE_BUILD_INPUTS), with compute() restored to
the plain policy/filter comprehension. Using the build list retains
shuffle-benchmark routing. The duplicate helper routing test is gone; CI-config
routing cases cover contrib manifests, helper/test paths, root
configuration/toolchain inputs and the benchmark distinction. The new tier
regression also checks locked validation before main.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
