sunchao commented on code in PR #5976: URL: https://github.com/apache/datafusion-comet/pull/5976#discussion_r4178740753
########## dev/ci/native-cache-key.py: ########## @@ -0,0 +1,256 @@ +#!/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 inspect +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) + +# These are the outputs of native/proto/build.rs, included by proto/src/lib.rs. +# Their tracked protobuf inputs and generator are fingerprinted instead. +GENERATED_PROTO_FILES = { + f"native/proto/src/generated/spark.spark_{name}.rs" + for name in ("expression", "partitioning", "operator", "metric", "config") +} + + +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 + matches = CHANGES.compile_matcher(patterns) + inventory = command(["git", "-c", f"safe.directory={root}", + "ls-files", "--stage", "-z"], root) + # Hash the imported rules themselves, not unrelated workflow routing in + # compute-changes.py. Include the predicate as well as the glob translator: + # changing include/exclude semantics must invalidate an existing library. + sources = {"@native-input-rules": digest([ + patterns, inspect.getsource(CHANGES.glob_to_regex), + inspect.getsource(CHANGES.compile_matcher), + ])} + dependencies = {} + for record in inventory.split("\0"): + if not record: + continue + metadata, name = record.split("\t", 1) + if matches(name): + value = [metadata.split()[0], hashlib.sha256((root / name).read_bytes()).hexdigest()] + sources[name] = value + if Path(name).name in {"Cargo.toml", "Cargo.lock"}: + dependencies[name] = value + return dependencies, sources + + +def depinfo_inputs(text): + """Read Cargo's top-level .d rule (not rustc's deps/*.d format). + + Cargo emits one rule with absolute paths in our build configuration. It + escapes spaces with a backslash, but leaves #, $, colons and other + backslashes literal. A shell or generic Makefile parser would corrupt + those names. Reject other formats rather than silently drop dependencies. + """ + lines = text.splitlines() + if len(lines) != 1 or "\0" in text: + raise ValueError("expected one Cargo dependency rule") + target, separator, prerequisites = lines[0].partition(": ") + if not separator or not target or not prerequisites: + raise ValueError("Cargo dependency rule has no target or inputs") + paths, word = [], [] + index = 0 + while index < len(prerequisites): + character = prerequisites[index] + if character == "\\" and prerequisites[index:index + 2] == "\\ ": + word.append(" ") + index += 2 + continue + if character == " ": + if word: + paths.append("".join(word)) + word = [] + else: + word.append(character) + index += 1 + if word: + paths.append("".join(word)) + if not paths: + raise ValueError("Cargo dependency rule has no inputs") + return paths + + +def check_depinfo(root, depinfo, env): + """Fail if Cargo consumed files outside the tracked library fingerprint. + + Generated protobuf outputs and the installed JDK are explicit exceptions; + their inputs/tool identity are recorded by the pre-build fingerprint. A + build-script directory declaration covers its files recursively. This is + a guard for declared Cargo inputs, not an audit of undeclared script reads. + """ + root = root.resolve() + java_home = Path(env["JAVA_HOME"]).resolve(strict=True) + _, sources = source_inputs(root) + matches = CHANGES.compile_matcher(CHANGES.NATIVE_LIBRARY_INPUTS) + uncovered = set() + + def covered(path): + resolved = path.resolve(strict=True) + if resolved.is_relative_to(java_home): + return True + # Check both the named and resolved location. In particular, a symlink + # below the generated directory must not admit an arbitrary input. + if not path.is_relative_to(root) or not resolved.is_relative_to(root): + return False + name = path.relative_to(root).as_posix() + resolved_name = resolved.relative_to(root).as_posix() + if resolved.is_dir(): + if path.is_symlink() or not matches(name) or not matches(resolved_name): + return False + return all(covered(child) for child in path.iterdir()) + return (name in sources and resolved_name in sources + or name in GENERATED_PROTO_FILES and resolved_name == name) + + for name in depinfo_inputs(depinfo.read_text()): + path = Path(name) + # build.dep-info-basedir can rewrite paths relative to an arbitrary + # directory. Our official build does not set it; do not guess a base. + if not path.is_absolute(): + uncovered.add(f"{name} (relative dep-info paths are unsupported)") + continue + try: + if not covered(path): + uncovered.add(name) + except (OSError, RuntimeError): + uncovered.add(f"{name} (missing or unresolvable)") + if uncovered: + raise ValueError("Cargo inputs missing from the native cache fingerprint:\n " + + "\n ".join(sorted(uncovered))) + + +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_release": (java_home / "release").read_text(), + "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"}}, + } + + +def cache_keys(profile, dependencies, sources, environment): + """Return output keys for one pre-build snapshot. + + 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])}-" + keys = { + "cargo-key": prefix + digest(sources), Review Comment: Fixed in 5a5a6fdb3. The incremental Cargo key now contains only the dependency and builder/environment fingerprints (v4 namespace), so source-only main pushes reuse the existing large target entry without saving another one. Only the compact library key includes the source digest. The action looks up the library, restores Cargo when needed, then downloads the exact library after the target restore. That replaces any older library in the dependency cache before the double-hit path skips compilation. Build/save decisions use the actual library download result, so eviction after lookup rebuilds safely. Main still builds when its incremental entry is missing, even if the compact library hits. All 23 native-cache/configuration tests, actionlint, the CI checker and formatting checks pass. Regressions cover source-only key stability, old versus exact library contents, all main hit/miss combinations, and lookup/download eviction. The pre-fix key function fails four source-only cases; the previous lookup-only flow fails the stale-library and eviction fixtures. No live caches were uploaded or deleted during local validation. -- 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]
