From a816fbf5c850c623e3780e0c540c0c3392e1b6df Mon Sep 17 00:00:00 2001 From: Maxwell Elliott Date: Fri, 30 Oct 2026 11:53:51 -0400 Subject: [PATCH 1/2] feat(cli): opt-in --partialGraph for generate-hashes, deriving from --baseHashes without a bazel query Adopts serve's partial-graph derivation in the one-shot CLI. Under the flag, generate-hashes hashes the working tree's own git tree object (committed or not; a clean tree is HEAD's), records every rule's recipe and the tree id in the file, and with --baseHashes re-evaluates the base's recipes against that tree when `git diff` between the two trees only modifies regular file content. Anything else falls back to a full generation with the reason logged; either way the file is byte-identical to a full generation's, so derived files are bases for later runs. Details that fell out of making it usable: - the run's own output, --baseHashes and --depEdgesFile files are left out of the snapshot when they lie inside the workspace, otherwise a hashes file written into the repo forces a fallback every time; - partial-graph files always carry target types, and the dependency edges when --depEdgesFile is given, since recipes cannot be evaluated without kinds and a derivation cannot invent edges; --targetType is rejected under the flag; - get-impacted-targets and explain refuse a pair of files of which only one carries recipes, as the two formulas are not comparable. Refactor: git plumbing (resolution, ls-tree indexing, working-tree snapshot, checkout, classification) moves from server.rs to git.rs, and the configuration fingerprint and cache key to cache.rs, so a later CLI cache shares serve's key space. TreeIndex carries its tree id and hash files record it as metadata.tree in partial mode only. Tests: git.rs and cache.rs unit tests, CLI tests for the flags and the derive/fallback paths, and e2e core::generate_hashes_partial_graph_derives_from_base_hashes asserting byte-identical output and impacted-set equality with the default mode. README gains a "Partial Graph Updates" section. Co-Authored-By: Claude Fable 5.1 --- README.md | 65 +++- src/cache.rs | 188 +++++++++++ src/git.rs | 613 +++++++++++++++++++++++++++++++++++ src/hash.rs | 6 +- src/lib.rs | 2 + src/main.rs | 487 +++++++++++++++++++++++++++- src/model.rs | 32 +- src/partial.rs | 54 ++- src/server.rs | 284 +++------------- tests/e2e/core.rs | 129 ++++++++ tools/e2e/rust_e2e_cases.bzl | 4 + tools/readme_template.md | 57 +++- 12 files changed, 1667 insertions(+), 254 deletions(-) create mode 100644 src/cache.rs create mode 100644 src/git.rs diff --git a/README.md b/README.md index e9f2a91c..c4f1cf8c 100644 --- a/README.md +++ b/README.md @@ -68,6 +68,59 @@ See https://github.com/aspect-extensions/impacted * We run `bazel-diff` on the starting and final JSON hash filepaths to get our impacted set of targets. This impacted set of targets is written to a file. +## Partial Graph Updates + +A full `generate-hashes` pays for a `bazel query` over the whole graph even when the change +cannot have altered that graph: editing the body of a source file changes what Bazel builds, +never which targets exist or how they depend on each other. `generate-hashes --partialGraph` +skips that query when it can: + +* The hashes describe the workspace's *git tree* rather than the files on disk: the working tree + as it stands, committed or not, is written as a tree object (an untracked file `.gitignore` + does not exclude is part of it, an ignored one is not, and neither are the hash files the run + itself reads and writes: the output, `--baseHashes` and `--depEdgesFile`), and source content + is read from that tree. A clean working tree yields the commit's own tree. The file records + the tree's id under `metadata.tree`, and every rule's hashing *recipe* (the digest of + everything that feeds its hash except source content, plus its ordered source inputs and + dependencies) under `metadata.partialGraph`. The file always includes every target's type, as + `--includeTargetType` would. +* With `--baseHashes ` naming a file written this way, the recipes it carries are + re-evaluated against the workspace's tree when `git diff` between the two trees only modifies + the content of regular files -- no `bazel query`, no Bazel at all. Anything else falls back to a + full generation: an added, deleted, renamed or type-changed path; a symlink or submodule on + either side; a change to a file Bazel reads while loading (`BUILD`, `BUILD.bazel`, `*.bzl`, + `*.scl`, `MODULE.bazel`, `MODULE.bazel.lock`, `WORKSPACE*`, `REPO.bazel`, `VENDOR.bazel`, + `*bazelrc`, `.bazelversion`, `.bazelignore`, `.bazeliskrc`, `tools/bazel`, `*.BUILD`, + `BUILD.*`, `*.patch`, `*.diff`); a file `MODULE.bazel` or a `WORKSPACE` file names by label; + anything a repository rule read from the workspace while the graph was generated; a + `--seed-filepaths` file; a path matching `--partialGraphRebuildPattern` (a glob: `*` within a + path segment, `**` across, a pattern without `/` matches the file name; repeatable); or a base + whose tree the local clone does not have. A fallback costs speed, never correctness, and the + log says which path each run took. A modified file no target reads impacts nothing, as it + would in a full generation. +* The file a derivation writes is byte-identical to the one a full generation under the flag + writes for the same tree, so derived files are themselves bases for later runs. A base that + already describes the workspace's tree is written back as is. With `--depEdgesFile` the hash + file also carries the dependency edges, so a derivation can write them; a base without them + cannot feed `--depEdgesFile` and forces a full generation. +* Hashes under the flag follow a slightly different formula from the default mode's, so + `get-impacted-targets` and `explain` refuse a pair of files of which only one carries recipes. + Not supported together with `--useCquery`, `--fineGrainedHashExternalRepos`, + `--alwaysAffectedTags`, `--contentHashPath`, `--modified-filepaths` or `--targetType` (a file + missing targets cannot be derived from; filter in `get-impacted-targets` instead): a derivation + reads only the git tree and keys recipes by bare label. The workspace must be the repository + root, and `--gitPath` names the git binary (default `git`). + +A typical CI job hashes the merge base once on the main branch and keeps the file; a pull +request run then needs no Bazel for the common case of a source-only change: + +```terminal +bazel-diff generate-hashes -w . -b bazel --partialGraph --baseHashes main_hashes.json pr_hashes.json +bazel-diff get-impacted-targets --startingHashes main_hashes.json --finalHashes pr_hashes.json -o impacted.txt +``` + +The [query service](#query-service) runs the same derivation server-side behind a cache. + ## Build Graph Distance Metrics `bazel-diff` can optionally compute build graph distance metrics between two revisions. This is @@ -452,7 +505,9 @@ Notes and operational guidance: with `--useCquery`, `--fineGrainedHashExternalRepos` or `--alwaysAffectedTags` (the server refuses to start): a derivation reads only the git tree and keys recipes by bare label. The nightly [`partial-graph-bench`](.github/workflows/partial-graph-bench.yml) workflow checks the - mode's speed and its exact agreement with the default mode (see `tools/perf/README.md`). + mode's speed and its exact agreement with the default mode (see `tools/perf/README.md`). The + one-shot `generate-hashes --partialGraph` writes the same entries (see [Partial Graph + Updates](#partial-graph-updates)). * The service checks out revisions inside `--workspacePath`, so point it at a dedicated clone, not a working tree you edit. All workspace-mutating work (git checkout + `bazel query`) is serialized, @@ -581,6 +636,14 @@ Options: -m, --modified-filepaths + --partialGraph[=] + Opt-in: hash the workspace's git tree (the working tree as it is, committed or not) and record every rule's hashing recipe, so a later run can derive a revision's hashes from this file with --baseHashes and no `bazel query` when the diff between the two only edits regular source files. The file always includes target types. Incompatible with --useCquery, --fineGrainedHashExternalRepos, --alwaysAffectedTags, --contentHashPath, --modified-filepaths and --targetType [default: false] [possible values: true, false] + --partialGraphRebuildPattern + Glob (`*` within a path segment, `**` across, a pattern without `/` matches the file name) of workspace paths whose change always forces a full generation under --partialGraph + --baseHashes + A hash file written with --partialGraph to derive this revision's hashes from, when the diff between its tree and the workspace's allows it; otherwise a full generation runs + --gitPath + Path to the git executable --partialGraph reads the workspace's tree with [default: git] -h, --help Print help -V, --version diff --git a/src/cache.rs b/src/cache.rs new file mode 100644 index 00000000..e6604ffb --- /dev/null +++ b/src/cache.rs @@ -0,0 +1,188 @@ +//! The cache key contract every producer of hash entries shares. +//! +//! `serve` and the one-shot CLI must agree on it exactly, so an entry one of them wrote is found +//! and trusted by the other: the key is the revision's git *tree* (not its commit, so a rebase or +//! an amended message shares the entry) joined with a digest of everything in the configuration +//! that can change hash content. Retention, tiers and remote locations are deliberately not in +//! it. + +use crate::hash::HashOptions; +use crate::partial; +use sha2::{Digest, Sha256}; +use std::collections::BTreeSet; +use std::path::PathBuf; + +/// Digests the hash-affecting configuration: the package version, every query and hashing +/// option, whether dependency edges are recorded, and, only when on, partial-graph mode (its +/// entries hash under a different formula and carry recipes, so they are never served in place +/// of a default-mode entry; folding the flag in only when enabled keeps existing keys put). +pub fn configuration_fingerprint( + options: &HashOptions, + track_deps: bool, + partial_graph: bool, +) -> String { + let mut hasher = Sha256::new(); + hasher.update(env!("CARGO_PKG_VERSION").as_bytes()); + hasher.update([0]); + for value in [ + options.bazel.startup_options.join("\0"), + options.bazel.command_options.join("\0"), + options.bazel.cquery_options.join("\0"), + options.bazel.use_cquery.to_string(), + options.bazel.cquery_expression.clone().unwrap_or_default(), + options.bazel.keep_going.to_string(), + options.bazel.exclude_external_targets.to_string(), + options + .bazel + .exclude_targets_query + .clone() + .unwrap_or_default(), + options + .bazel + .fine_grained_external_repos + .iter() + .cloned() + .collect::>() + .join(","), + options + .bazel + .label_only_module_extensions + .iter() + .cloned() + .collect::>() + .join(","), + options + .ignored_attributes + .iter() + .cloned() + .collect::>() + .into_iter() + .collect::>() + .join(","), + options + .seed_filepaths + .iter() + .map(|path| path.to_string_lossy()) + .collect::>() + .join("\0"), + options + .always_affected_tags + .iter() + .cloned() + .collect::>() + .into_iter() + .collect::>() + .join(","), + track_deps.to_string(), + // Full-content entries hash git blob ids rather than file bytes; keep them apart from + // entries a build without this seeding wrote under the same package version. + String::from("sourceContent=git-blob"), + ] { + hasher.update(value.as_bytes()); + hasher.update([0]); + } + if partial_graph { + hasher.update(format!("partialGraph={}", partial::RECIPE_VERSION).as_bytes()); + hasher.update([0]); + } + hex::encode(hasher.finalize())[..12].to_owned() +} + +/// The key of a revision's entry: its tree and the configuration fingerprint, plus a digest of +/// the `modifiedFilepaths` set for a scoped entry so it is never served as the full one. +pub fn cache_key(tree: &str, fingerprint: &str, modified: &BTreeSet) -> String { + let base = format!("{tree}.{fingerprint}"); + if modified.is_empty() { + return base; + } + let mut hasher = Sha256::new(); + for path in modified { + hasher.update(path.to_string_lossy().as_bytes()); + hasher.update(b"\n"); + } + format!("{base}.{}", &hex::encode(hasher.finalize())[..12]) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::bazel::BazelOptions; + use std::collections::HashSet; + + fn options() -> HashOptions { + HashOptions { + bazel: BazelOptions { + workspace: PathBuf::from("."), + bazel: PathBuf::from("bazel"), + startup_options: Vec::new(), + command_options: Vec::new(), + cquery_options: Vec::new(), + use_cquery: false, + cquery_expression: None, + keep_going: false, + fine_grained_external_repos: BTreeSet::new(), + label_only_module_extensions: BTreeSet::new(), + exclude_external_targets: false, + exclude_targets_query: None, + no_bazelrc: false, + verbose: false, + cancellation: None, + }, + content_hashes: None, + ignored_attributes: HashSet::new(), + seed_filepaths: BTreeSet::new(), + modified_filepaths: BTreeSet::new(), + track_deps: false, + always_affected_tags: HashSet::new(), + partial_tree: None, + } + } + + #[test] + fn fingerprint_moves_with_hash_affecting_options_only() { + let base = configuration_fingerprint(&options(), false, false); + assert_eq!(base.len(), 12); + assert_eq!(configuration_fingerprint(&options(), false, false), base); + + let mut cquery = options(); + cquery.bazel.use_cquery = true; + assert_ne!(configuration_fingerprint(&cquery, false, false), base); + let mut seeded = options(); + seeded.seed_filepaths.insert(PathBuf::from("seed")); + assert_ne!(configuration_fingerprint(&seeded, false, false), base); + let mut ignored = options(); + ignored.ignored_attributes.insert("visibility".into()); + assert_ne!(configuration_fingerprint(&ignored, false, false), base); + assert_ne!(configuration_fingerprint(&options(), true, false), base); + + // Partial-graph mode has its own key space; the default mode's keys stay put. + let partial = configuration_fingerprint(&options(), false, true); + assert_ne!(partial, base); + assert_eq!(configuration_fingerprint(&options(), false, true), partial); + } + + #[test] + fn keys_join_tree_and_fingerprint_and_scope_modified_sets() { + let full = cache_key("abc", "fp", &BTreeSet::new()); + assert_eq!(full, "abc.fp"); + let scoped = cache_key( + "abc", + "fp", + &BTreeSet::from([PathBuf::from("b"), PathBuf::from("a")]), + ); + assert!(scoped.starts_with("abc.fp.")); + assert_eq!(scoped.len(), "abc.fp.".len() + 12); + assert_eq!( + scoped, + cache_key( + "abc", + "fp", + &BTreeSet::from([PathBuf::from("a"), PathBuf::from("b")]) + ) + ); + assert_ne!( + scoped, + cache_key("abc", "fp", &BTreeSet::from([PathBuf::from("a")])) + ); + } +} diff --git a/src/git.rs b/src/git.rs new file mode 100644 index 00000000..23a4010f --- /dev/null +++ b/src/git.rs @@ -0,0 +1,613 @@ +//! Git plumbing shared by `serve` and the one-shot CLI. +//! +//! Everything partial-graph hashing reads from git goes through here: resolving a revision to +//! its commit or tree, listing a tree, snapshotting the working tree as a tree object, checking +//! a revision out, and deciding whether one revision's hashes can be derived from another's +//! (see [`crate::partial`]). The workspace is expected to be the repository root, as the paths +//! `git ls-tree` prints are compared with workspace-relative Bazel paths. + +use crate::cancel::{self, Cancellation}; +use crate::model::HashFileData; +use crate::partial::{self, Classification, ClassifyInputs, Glob, TreeIndex, MODULE_FILES}; +use anyhow::{bail, Context, Result}; +use std::collections::BTreeSet; +use std::ffi::{OsStr, OsString}; +use std::fs; +use std::path::{Path, PathBuf}; +use std::process::{Command, Output, Stdio}; +use std::sync::Arc; + +/// A git binary bound to one workspace. +/// +/// Subprocesses belong to `cancellation` when it is set, so `serve` kills them with the query +/// that holds the workspace; the CLI runs them plainly. +#[derive(Clone, Debug)] +pub struct Git { + path: PathBuf, + workspace: PathBuf, + cancellation: Option>, +} + +impl Git { + pub fn new(path: impl Into, workspace: impl Into) -> Self { + Self { + path: path.into(), + workspace: workspace.into(), + cancellation: None, + } + } + + pub fn with_cancellation(mut self, cancellation: Option>) -> Self { + self.cancellation = cancellation; + self + } + + pub fn workspace(&self) -> &Path { + &self.workspace + } + + /// Runs git in the workspace and returns its output, failing only when it cannot be spawned. + pub fn output(&self, args: I) -> Result + where + I: IntoIterator, + S: AsRef, + { + self.output_with_env(args, &[]) + } + + fn output_with_env(&self, args: I, env: &[(&str, &OsStr)]) -> Result + where + I: IntoIterator, + S: AsRef, + { + let mut command = Command::new(&self.path); + command + .current_dir(&self.workspace) + .args(args) + .envs(env.iter().copied()) + .stdin(Stdio::null()); + cancel::output(self.cancellation.as_deref(), &mut command) + .with_context(|| format!("failed to execute {}", self.path.display())) + } + + /// Runs git, failing on a non-zero exit with the command and its stderr. + pub fn run(&self, args: I) -> Result<()> + where + I: IntoIterator, + S: AsRef, + { + self.stdout(args).map(drop) + } + + /// Runs git and returns its stdout, failing on a non-zero exit with the command and stderr. + pub fn stdout(&self, args: I) -> Result> + where + I: IntoIterator, + S: AsRef, + { + self.stdout_with_env(args, &[]) + } + + fn stdout_with_env(&self, args: I, env: &[(&str, &OsStr)]) -> Result> + where + I: IntoIterator, + S: AsRef, + { + let args = args + .into_iter() + .map(|arg| arg.as_ref().to_owned()) + .collect::>(); + let output = self.output_with_env(&args, env)?; + if !output.status.success() { + bail!( + "git {} failed: {}", + args.iter() + .map(|arg| arg.to_string_lossy()) + .collect::>() + .join(" "), + String::from_utf8_lossy(&output.stderr) + ); + } + Ok(output.stdout) + } + + fn stdout_line(&self, args: I, env: &[(&str, &OsStr)]) -> Result + where + I: IntoIterator, + S: AsRef, + { + Ok(String::from_utf8_lossy(&self.stdout_with_env(args, env)?) + .trim() + .to_owned()) + } + + /// Resolves a revision to its commit SHA. + pub fn resolve_sha(&self, revision: &str) -> Result { + let output = self.output(["rev-parse", "--verify", &format!("{revision}^{{commit}}")])?; + if !output.status.success() { + bail!("revision '{revision}' is missing from the local clone"); + } + let sha = String::from_utf8_lossy(&output.stdout).trim().to_owned(); + if sha.len() != 40 { + bail!("revision '{revision}' resolved to invalid SHA '{sha}'"); + } + Ok(sha) + } + + /// Resolves the tree object a revision points at; distinct commits with identical content + /// share it. A tree id resolves to itself. + pub fn resolve_tree(&self, revision: &str) -> Result { + let output = self.output(["rev-parse", "--verify", &format!("{revision}^{{tree}}")])?; + if !output.status.success() { + bail!("tree for revision '{revision}' is missing from the local clone"); + } + let tree = String::from_utf8_lossy(&output.stdout).trim().to_owned(); + if tree.len() != 40 { + bail!("revision '{revision}' resolved to invalid tree '{tree}'"); + } + Ok(tree) + } + + /// Every tracked path at a revision with its object id and mode, from one `git ls-tree`, + /// tagged with the revision's tree id. + pub fn tree_index(&self, revision: &str) -> Result { + let tree = self.resolve_tree(revision)?; + let output = self.stdout(["ls-tree", "-r", "-z", &tree])?; + Ok(TreeIndex::parse_ls_tree(&output).with_id(tree)) + } + + /// The content of `path` at a revision. + pub fn show(&self, revision: &str, path: &str) -> Result> { + self.stdout(["show", &format!("{revision}:{path}")]) + } + + /// Writes the working tree as a tree object and lists it: every tracked file as it is on + /// disk, plus untracked files `.gitignore` does not exclude, with staged and unstaged + /// deletions applied. A clean working tree yields `HEAD`'s own tree, so an entry generated + /// from it is keyed exactly as one generated from the commit; a dirty one yields a tree of + /// its own that `git diff` and `git ls-tree` accept like any other. + /// + /// `excluded` names files that are not part of the snapshot even when they lie inside the + /// workspace, tracked or not: the hash files bazel-diff itself reads and writes, which would + /// otherwise show up as an added or modified path in every diff. + /// + /// The repository's index is staged into a temporary copy (so its stat cache spares the + /// rehash of unchanged files) and the repository's own index is never touched. + pub fn snapshot_working_tree(&self, excluded: &[&Path]) -> Result { + let top_level = self.stdout_line(["rev-parse", "--show-toplevel"], &[])?; + let workspace = match ( + fs::canonicalize(&top_level), + fs::canonicalize(&self.workspace), + ) { + (Ok(root), Ok(workspace)) if root == workspace => workspace, + _ => bail!( + "--partialGraph needs the workspace to be the repository root: {} is inside the repository at {top_level}", + self.workspace.display() + ), + }; + let staging = tempfile::tempdir().context("failed to create a temporary index")?; + let index = staging.path().join("index"); + let repository_index = self.stdout_line(["rev-parse", "--git-path", "index"], &[])?; + let repository_index = self.workspace.join(repository_index); + if repository_index.is_file() { + fs::copy(&repository_index, &index).with_context(|| { + format!( + "failed to copy {} to a temporary index", + repository_index.display() + ) + })?; + } + let env = [("GIT_INDEX_FILE", index.as_os_str())]; + self.stdout_with_env(["add", "--all"], &env)?; + let excluded = excluded + .iter() + .filter_map(|path| workspace_relative_file(&workspace, path)) + .collect::>(); + if !excluded.is_empty() { + let mut args = vec![ + OsString::from("update-index"), + OsString::from("--force-remove"), + OsString::from("--"), + ]; + args.extend(excluded.into_iter().map(OsString::from)); + self.stdout_with_env(&args, &env)?; + } + let tree = self.stdout_line(["write-tree"], &env)?; + if tree.len() != 40 { + bail!("git write-tree produced invalid tree '{tree}'"); + } + self.tree_index(&tree) + } + + /// Checks `sha` out, clearing a stale `index.lock` a killed git left behind once. + pub fn checkout(&self, sha: &str) -> Result<()> { + if let Ok(output) = self.output(["rev-parse", "HEAD"]) { + if output.status.success() && String::from_utf8_lossy(&output.stdout).trim() == sha { + return Ok(()); + } + } + let args = [ + "-c", + "advice.detachedHead=false", + "checkout", + "--force", + sha, + ]; + if self.run(args).is_ok() { + return Ok(()); + } + let index_lock = self.workspace.join(".git/index.lock"); + if !index_lock.is_file() { + bail!("git checkout failed for revision {sha}"); + } + fs::remove_file(&index_lock).context("failed to remove stale .git/index.lock")?; + eprintln!( + "[Warn] cleared stale git index.lock in {}; retrying checkout of {sha}", + self.workspace.display() + ); + self.run(args) + } +} + +/// `path` relative to the canonical `workspace` when it lies inside it, with the file itself +/// allowed not to exist yet (an output about to be written): its parent is canonicalized and +/// the file name appended. +fn workspace_relative_file(workspace: &Path, path: &Path) -> Option { + let absolute = if path.is_absolute() { + path.to_path_buf() + } else { + std::env::current_dir().ok()?.join(path) + }; + let name = absolute.file_name()?; + let parent = fs::canonicalize(absolute.parent()?).ok()?; + let relative = parent + .join(name) + .strip_prefix(workspace) + .ok()? + .to_path_buf(); + Some(relative.to_string_lossy().replace('\\', "/")) +} + +/// The configuration a derivation decision depends on besides the two revisions. +#[derive(Clone, Copy, Debug)] +pub struct DerivationPolicy<'a> { + pub seed_filepaths: &'a BTreeSet, + pub rebuild_patterns: &'a [Glob], +} + +/// Decides whether `revision` can be derived from the entry generated at `base_revision`: the +/// raw diff between the two, the module and workspace files' label references at `revision`, +/// and the policy's rebuild patterns and seed files go through [`partial::classify`]. Both +/// arguments may be commits or tree ids. +pub fn classify_revisions( + git: &Git, + base_revision: &str, + revision: &str, + base: &HashFileData, + tree: &TreeIndex, + policy: &DerivationPolicy<'_>, +) -> Result { + let diff = git.stdout([ + "diff", + "--raw", + "--no-renames", + "-z", + base_revision, + revision, + ])?; + let diff = partial::parse_diff_raw(&diff)?; + let mut module_referenced = BTreeSet::new(); + for file in MODULE_FILES { + if tree.get(file).is_none() { + continue; + } + let text = git.show(revision, file)?; + module_referenced.extend(partial::referenced_main_repo_paths( + &String::from_utf8_lossy(&text), + )); + } + Ok(partial::classify(&ClassifyInputs { + diff: &diff, + tree, + base, + workspace: git.workspace(), + seed_filepaths: policy.seed_filepaths, + rebuild_patterns: policy.rebuild_patterns, + module_referenced: &module_referenced, + })) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::partial::TreeKind; + + fn git_command(repo: &Path, args: &[&str]) -> String { + let output = Command::new("git") + .current_dir(repo) + .args(args) + .output() + .unwrap(); + assert!( + output.status.success(), + "git {:?}: {}", + args, + String::from_utf8_lossy(&output.stderr) + ); + String::from_utf8_lossy(&output.stdout).trim().to_owned() + } + + fn repository() -> (tempfile::TempDir, Git) { + let repo = tempfile::tempdir().unwrap(); + git_command(repo.path(), &["init", "-q"]); + git_command(repo.path(), &["config", "user.email", "test@example.com"]); + git_command(repo.path(), &["config", "user.name", "Test"]); + fs::write(repo.path().join("file.txt"), "one").unwrap(); + fs::write(repo.path().join(".gitignore"), "ignored/\n").unwrap(); + git_command(repo.path(), &["add", "-A"]); + git_command(repo.path(), &["commit", "-q", "-m", "one"]); + let git = Git::new("git", repo.path()); + (repo, git) + } + + #[test] + fn resolves_commits_and_trees() { + let (repo, git) = repository(); + let head = git_command(repo.path(), &["rev-parse", "HEAD"]); + let tree = git_command(repo.path(), &["rev-parse", "HEAD^{tree}"]); + assert_eq!(git.resolve_sha("HEAD").unwrap(), head); + assert_eq!(git.resolve_tree("HEAD").unwrap(), tree); + assert_eq!(git.resolve_tree(&tree).unwrap(), tree); + assert_eq!( + git.resolve_sha("missing").unwrap_err().to_string(), + "revision 'missing' is missing from the local clone" + ); + assert!(git + .resolve_tree("missing") + .unwrap_err() + .to_string() + .contains("missing")); + assert!(git.resolve_sha(&tree).is_err(), "a tree is not a commit"); + + let index = git.tree_index("HEAD").unwrap(); + assert_eq!(index.id(), Some(tree.as_str())); + assert_eq!( + index.content_hashes()["file.txt"], + git_command(repo.path(), &["rev-parse", "HEAD:file.txt"]) + ); + assert_eq!(git.show("HEAD", "file.txt").unwrap(), b"one"); + assert!(git.show("HEAD", "nope.txt").is_err()); + assert!(git.tree_index("missing").is_err()); + assert!(git + .run(["not-a-command"]) + .unwrap_err() + .to_string() + .contains("git not-a-command failed")); + + let missing_binary = Git::new("/definitely/missing/git", repo.path()); + assert!(missing_binary.output::<[&str; 0], &str>([]).is_err()); + } + + #[test] + fn working_tree_snapshot_matches_head_when_clean_and_captures_edits() { + let (repo, git) = repository(); + let head_tree = git_command(repo.path(), &["rev-parse", "HEAD^{tree}"]); + let clean = git.snapshot_working_tree(&[]).unwrap(); + assert_eq!(clean.id(), Some(head_tree.as_str())); + + // A modified tracked file, an untracked file and an ignored one, with nothing staged. + fs::write(repo.path().join("file.txt"), "two").unwrap(); + fs::create_dir_all(repo.path().join("new dir")).unwrap(); + fs::write(repo.path().join("new dir/added.txt"), "added").unwrap(); + fs::create_dir_all(repo.path().join("ignored")).unwrap(); + fs::write(repo.path().join("ignored/skip.txt"), "skip").unwrap(); + let dirty = git.snapshot_working_tree(&[]).unwrap(); + assert_ne!(dirty.id(), clean.id()); + let hashes = dirty.content_hashes(); + assert_eq!( + hashes["file.txt"], + git_command(repo.path(), &["hash-object", "file.txt"]) + ); + assert_eq!( + hashes["new dir/added.txt"], + git_command(repo.path(), &["hash-object", "new dir/added.txt"]) + ); + assert!(!hashes.contains_key("ignored/skip.txt")); + assert_eq!( + dirty.get("file.txt").unwrap().kind, + TreeKind::File { executable: false } + ); + // The repository's own index and HEAD are untouched. + assert_eq!( + git_command(repo.path(), &["rev-parse", "HEAD^{tree}"]), + head_tree + ); + assert_eq!( + git_command(repo.path(), &["diff", "--cached", "--name-only"]), + "" + ); + assert_eq!( + git_command(repo.path(), &["status", "--porcelain"]) + .lines() + .count(), + 2 + ); + // The snapshot is a real tree object: the same edits committed yield the same id. + git_command(repo.path(), &["add", "-A"]); + git_command(repo.path(), &["commit", "-q", "-m", "two"]); + assert_eq!( + git_command(repo.path(), &["rev-parse", "HEAD^{tree}"]), + dirty.id().unwrap() + ); + + // A deletion is applied even when unstaged. + fs::remove_file(repo.path().join("file.txt")).unwrap(); + assert!(git + .snapshot_working_tree(&[]) + .unwrap() + .get("file.txt") + .is_none()); + } + + #[test] + fn working_tree_snapshot_leaves_excluded_files_out() { + let (repo, git) = repository(); + let head_tree = git_command(repo.path(), &["rev-parse", "HEAD^{tree}"]); + // An untracked output inside the workspace, one that does not exist yet, a path outside + // the workspace, and a relative path (against the process working directory). + let output = repo.path().join("out/hashes.json"); + fs::create_dir_all(output.parent().unwrap()).unwrap(); + fs::write(&output, "{}").unwrap(); + let pending = repo.path().join("pending.json"); + let outside = tempfile::tempdir().unwrap(); + let elsewhere = outside.path().join("hashes.json"); + let snapshot = git + .snapshot_working_tree(&[&output, &pending, &elsewhere, Path::new("relative.json")]) + .unwrap(); + assert_eq!( + snapshot.id(), + Some(head_tree.as_str()), + "{:?}", + snapshot.content_hashes() + ); + // A tracked file is left out too, and the repository's index is untouched. + let tracked = repo.path().join("file.txt"); + let without_tracked = git.snapshot_working_tree(&[&tracked]).unwrap(); + assert!(without_tracked.get("file.txt").is_none()); + assert_ne!(without_tracked.id(), snapshot.id()); + assert_eq!( + git_command(repo.path(), &["status", "--porcelain", "file.txt"]), + "" + ); + let workspace = fs::canonicalize(repo.path()).unwrap(); + assert_eq!(workspace_relative_file(&workspace, outside.path()), None); + assert_eq!( + workspace_relative_file(&workspace, &repo.path().join("missing dir/x.json")), + None + ); + assert_eq!( + workspace_relative_file(&workspace, &output).as_deref(), + Some("out/hashes.json") + ); + } + + #[test] + fn working_tree_snapshot_requires_the_repository_root() { + let (repo, _) = repository(); + let nested = repo.path().join("nested"); + fs::create_dir_all(&nested).unwrap(); + let error = Git::new("git", &nested) + .snapshot_working_tree(&[]) + .unwrap_err() + .to_string(); + assert!(error.contains("repository root"), "{error}"); + let outside = tempfile::tempdir().unwrap(); + assert!(Git::new("git", outside.path()) + .snapshot_working_tree(&[]) + .is_err()); + } + + #[test] + fn working_tree_snapshot_without_an_index_file() { + let repo = tempfile::tempdir().unwrap(); + git_command(repo.path(), &["init", "-q"]); + fs::write(repo.path().join("file.txt"), "one").unwrap(); + assert!(!repo.path().join(".git/index").exists()); + let snapshot = Git::new("git", repo.path()) + .snapshot_working_tree(&[]) + .unwrap(); + assert_eq!(snapshot.len(), 1); + assert!(snapshot.get("file.txt").is_some()); + } + + #[test] + fn checkout_switches_revisions_and_clears_a_stale_index_lock() { + let (repo, git) = repository(); + let first = git_command(repo.path(), &["rev-parse", "HEAD"]); + fs::write(repo.path().join("file.txt"), "two").unwrap(); + git_command(repo.path(), &["commit", "-q", "-am", "two"]); + let second = git_command(repo.path(), &["rev-parse", "HEAD"]); + git.checkout(&first).unwrap(); + assert_eq!( + fs::read_to_string(repo.path().join("file.txt")).unwrap(), + "one" + ); + // Already there: a no-op even with a stale lock in place. + fs::write(repo.path().join(".git/index.lock"), "").unwrap(); + git.checkout(&first).unwrap(); + git.checkout(&second).unwrap(); + assert!(!repo.path().join(".git/index.lock").exists()); + assert_eq!( + fs::read_to_string(repo.path().join("file.txt")).unwrap(), + "two" + ); + assert!(git.checkout("missing").is_err()); + } + + #[test] + fn classifies_source_edits_against_a_base_entry() { + let (repo, git) = repository(); + let base_tree = git.resolve_tree("HEAD").unwrap(); + let base = HashFileData { + partial: Some(partial::PartialRecipes { + version: partial::RECIPE_VERSION, + seed_hash: "00".repeat(32), + package_seeds: true, + ..Default::default() + }), + ..Default::default() + }; + let seeds = BTreeSet::new(); + let policy = DerivationPolicy { + seed_filepaths: &seeds, + rebuild_patterns: &[], + }; + fs::write(repo.path().join("file.txt"), "two").unwrap(); + let edited = git.snapshot_working_tree(&[]).unwrap(); + let derivable = classify_revisions( + &git, + &base_tree, + edited.id().unwrap(), + &base, + &edited, + &policy, + ) + .unwrap(); + assert!( + matches!(&derivable, Classification::Derivable { unmatched_paths, .. } if unmatched_paths == &["file.txt"]), + "{derivable:?}" + ); + fs::write(repo.path().join("BUILD"), "").unwrap(); + let rebuilt = git.snapshot_working_tree(&[]).unwrap(); + let rebuild = classify_revisions( + &git, + &base_tree, + rebuilt.id().unwrap(), + &base, + &rebuilt, + &policy, + ) + .unwrap(); + assert!( + matches!(rebuild, Classification::Rebuild { .. }), + "{rebuild:?}" + ); + // A module file at the revision contributes its label references. + fs::write( + repo.path().join("MODULE.bazel"), + "bazel_dep(name = \"x\")\nuse_repo_rule(\"//:deps.bzl\")\n", + ) + .unwrap(); + let with_module = git.snapshot_working_tree(&[]).unwrap(); + assert!(classify_revisions( + &git, + &base_tree, + with_module.id().unwrap(), + &base, + &with_module, + &policy + ) + .is_ok()); + assert!(classify_revisions(&git, "missing", &base_tree, &base, &edited, &policy).is_err()); + } +} diff --git a/src/hash.rs b/src/hash.rs index 0749f4b9..291b6763 100644 --- a/src/hash.rs +++ b/src/hash.rs @@ -56,7 +56,7 @@ pub struct HashOptions { pub modified_filepaths: BTreeSet, pub track_deps: bool, pub always_affected_tags: HashSet, - /// Partial-graph mode (`serve --partialGraph`): the hashed revision's `git ls-tree`. When + /// Partial-graph mode (`--partialGraph`): the hashed revision's `git ls-tree`. When /// set, main-repository sources are digested from the tree instead of the working tree, every /// rule's hashing recipe is recorded, and the hashes are computed by evaluating those recipes /// (see [`crate::partial`]), so that an entry derived from this one later is identical to @@ -513,6 +513,10 @@ fn hash_targets_with_environment( dependency_fingerprint: None, dep_edges, partial, + tree: options + .partial_tree + .as_ref() + .and_then(|tree| tree.id().map(str::to_owned)), }) } diff --git a/src/lib.rs b/src/lib.rs index 0f24613a..81dd1219 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -5,9 +5,11 @@ //! tests; its module-level API is not a stability commitment. pub mod bazel; +pub mod cache; pub mod cancel; pub mod explain; pub mod fingerprint; +pub mod git; pub mod hash; pub mod model; pub mod module_graph; diff --git a/src/main.rs b/src/main.rs index fde70a7e..3283289b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -2,17 +2,20 @@ use anyhow::{bail, Context, Result}; use bazel_diff::bazel::BazelOptions; use bazel_diff::explain::{self, ExplainFormat, ExplainLimits}; use bazel_diff::fingerprint; +use bazel_diff::git::{self, DerivationPolicy, Git}; use bazel_diff::hash::{generate_hashes, HashOptions}; use bazel_diff::model::{ filter_and_sort_labels, impacted_targets, impacted_targets_with_distances, HashFileData, }; use bazel_diff::module_graph::impacted_with_module_changes; +use bazel_diff::partial::{self, Classification, Glob}; use clap::{ArgAction, Args, Parser, Subcommand, ValueEnum}; use serde::Serialize; use std::collections::{BTreeMap, BTreeSet, HashSet}; use std::fs::{self, File}; use std::io::{self, BufWriter, Write}; use std::path::{Path, PathBuf}; +use std::sync::Arc; use std::time::Duration; // Use mimalloc's per-thread heaps for parallel protobuf decoding in generate-hashes. @@ -194,6 +197,36 @@ struct GenerateHashesArgs { #[arg(short = 'm', long = "modified-filepaths")] modified_filepaths: Option, + /// Opt-in: hash the workspace's git tree (the working tree as it is, committed or not) and + /// record every rule's hashing recipe, so a later run can derive a revision's hashes from + /// this file with --baseHashes and no `bazel query` when the diff between the two only edits + /// regular source files. The file always includes target types. Incompatible with + /// --useCquery, --fineGrainedHashExternalRepos, --alwaysAffectedTags, --contentHashPath, + /// --modified-filepaths and --targetType + #[arg( + long = "partialGraph", + action = ArgAction::Set, + num_args = 0..=1, + require_equals = true, + default_missing_value = "true", + default_value_t = false + )] + partial_graph: bool, + + /// Glob (`*` within a path segment, `**` across, a pattern without `/` matches the file + /// name) of workspace paths whose change always forces a full generation under --partialGraph + #[arg(long = "partialGraphRebuildPattern", value_delimiter = ',')] + partial_graph_rebuild_patterns: Vec, + + /// A hash file written with --partialGraph to derive this revision's hashes from, when the + /// diff between its tree and the workspace's allows it; otherwise a full generation runs + #[arg(long = "baseHashes")] + base_hashes: Option, + + /// Path to the git executable --partialGraph reads the workspace's tree with + #[arg(long = "gitPath", default_value = "git")] + git_path: PathBuf, + output_path: Option, } @@ -686,21 +719,32 @@ fn run_generate_with( verbose: bool, generate: impl FnOnce(&HashOptions) -> Result, ) -> Result { - let options = hash_options( + let mut options = hash_options( &args.hashing, args.content_hash_path.as_deref(), args.modified_filepaths.as_deref(), args.dep_edges_file.is_some(), verbose, )?; - let mut data = generate(&options)?; + let mut data = if args.partial_graph { + partial_graph_hashes(args, &mut options, generate)? + } else if args.base_hashes.is_some() { + bail!("--baseHashes needs --partialGraph"); + } else { + generate(&options)? + }; if !args.target_type.is_empty() { let types = args.target_type.iter().cloned().collect::>(); data.hashes.retain(|_, hash| types.contains(&hash.kind)); } + // A partial-graph file carries every target's type and its dependency edges, which a + // derivation from it needs to evaluate the recipes and to write --depEdgesFile. write_json( args.output_path.as_deref(), - &data.serialized(args.include_target_type, false), + &data.serialized( + args.include_target_type || args.partial_graph, + args.partial_graph && args.dep_edges_file.is_some(), + ), )?; if let Some(path) = &args.dep_edges_file { let context = || format!("failed to write {}", path.display()); @@ -710,6 +754,121 @@ fn run_generate_with( Ok(data) } +/// `--partialGraph` for `generate-hashes`: the hashes describe the working tree's git tree, and +/// come from re-evaluating the `--baseHashes` entry's recipes against it when the diff between +/// the two allows it (see [`bazel_diff::partial`]), or from a full generation that records the +/// recipes otherwise. Either way the file written is the same. +fn partial_graph_hashes( + args: &GenerateHashesArgs, + options: &mut HashOptions, + generate: impl FnOnce(&HashOptions) -> Result, +) -> Result { + partial::validate_hash_options(options)?; + if args.content_hash_path.is_some() { + bail!("--partialGraph does not support --contentHashPath: source content is read from the git tree"); + } + if args.modified_filepaths.is_some() { + bail!("--partialGraph does not support --modified-filepaths"); + } + if !args.target_type.is_empty() { + bail!("--partialGraph does not support --targetType: a file missing targets cannot be derived from (filter in get-impacted-targets instead)"); + } + let rebuild_patterns = args + .partial_graph_rebuild_patterns + .iter() + .map(|pattern| Glob::new(pattern)) + .collect::>>()?; + let git = Git::new(&args.git_path, &args.hashing.workspace_path); + // bazel-diff's own files are not part of the revision, wherever the user keeps them. + let own_files = [ + args.output_path.as_deref(), + args.base_hashes.as_deref(), + args.dep_edges_file.as_deref(), + ] + .into_iter() + .flatten() + .collect::>(); + let tree = git.snapshot_working_tree(&own_files)?; + let tree_id = tree + .id() + .expect("a working tree snapshot carries its tree id") + .to_owned(); + eprintln!( + "[Info] partial graph: workspace tree {tree_id} ({} tracked paths)", + tree.len() + ); + let fallback = match &args.base_hashes { + Some(path) => { + let base = HashFileData::read(path) + .with_context(|| format!("failed to read --baseHashes {}", path.display()))?; + match base.tree.as_deref() { + _ if base.partial.is_none() => { + format!("{} carries no partial-graph recipes", path.display()) + } + _ if args.dep_edges_file.is_some() + && base.dep_edges.is_empty() + && !base.hashes.is_empty() => + { + format!( + "{} records no dependency edges, which --depEdgesFile needs", + path.display() + ) + } + None => format!("{} records no git tree", path.display()), + Some(base_tree) if base_tree == tree_id => { + eprintln!( + "[Info] partial graph: {} already describes tree {tree_id}", + path.display() + ); + return Ok(base); + } + Some(base_tree) => { + let policy = DerivationPolicy { + seed_filepaths: &options.seed_filepaths, + rebuild_patterns: &rebuild_patterns, + }; + match git::classify_revisions(&git, base_tree, &tree_id, &base, &tree, &policy) + { + Ok(Classification::Derivable { + changed_sources, + unmatched_paths, + }) => match partial::derive(&base, &tree) { + Ok(data) => { + eprintln!( + "[Info] partial graph: derived {tree_id} from {base_tree}: {} changed source targets, {} changed paths outside the graph, {} targets", + changed_sources.len(), + unmatched_paths.len(), + data.hashes.len() + ); + return Ok(data); + } + Err(error) => format!("derivation failed: {error:#}"), + }, + Ok(Classification::Rebuild { reason }) => reason, + Err(error) => format!("classification failed: {error:#}"), + } + } + } + } + None => "no --baseHashes to derive from".to_owned(), + }; + eprintln!("[Info] partial graph: full generation for {tree_id}: {fallback}"); + options.content_hashes = Some(tree.content_hashes()); + options.partial_tree = Some(Arc::new(tree)); + generate(options) +} + +/// Hashes of the two modes follow different formulas, so comparing a `--partialGraph` file with +/// a default-mode one would report every target as impacted. +fn ensure_same_mode(from: &HashFileData, to: &HashFileData) -> Result<()> { + if from.partial.is_some() != to.partial.is_some() { + bail!( + "the starting and final hashes were not generated in the same mode: one carries --partialGraph recipes and the other does not, and hashes of the two modes are not comparable" + ); + } + Ok(()) +} + fn get_bazel_options(args: &GetImpactedTargetsArgs, verbose: bool) -> BazelOptions { BazelOptions { workspace: args.workspace_path.clone(), @@ -733,6 +892,7 @@ fn get_bazel_options(args: &GetImpactedTargetsArgs, verbose: bool) -> BazelOptio fn run_get_impacted(args: &GetImpactedTargetsArgs, verbose: bool) -> Result<()> { let from = HashFileData::read(&args.starting_hashes)?; let to = HashFileData::read(&args.final_hashes)?; + ensure_same_mode(&from, &to)?; let bazel = get_bazel_options(args, verbose); let exclude_external = args .exclude_external_targets @@ -782,6 +942,7 @@ fn run_explain(args: &ExplainArgs, verbose: bool) -> Result<()> { .context("Incorrect starting hashes: file doesn't exist or can't be read.")?; let to = HashFileData::read(&args.final_hashes) .context("Incorrect final hashes: file doesn't exist or can't be read.")?; + ensure_same_mode(&from, &to)?; let dep_edges: BTreeMap> = serde_json::from_slice( &fs::read(&args.dep_edges_file) .context("Incorrect dep edges file: file doesn't exist or can't be read.")?, @@ -846,6 +1007,10 @@ fn run_warmup_with( target_type: args.generate.target_type.clone(), dep_edges_file: args.generate.dep_edges_file.clone(), modified_filepaths: args.generate.modified_filepaths.clone(), + partial_graph: args.generate.partial_graph, + partial_graph_rebuild_patterns: args.generate.partial_graph_rebuild_patterns.clone(), + base_hashes: args.generate.base_hashes.clone(), + git_path: args.generate.git_path.clone(), output_path: Some(args.base_hashes.clone()), }; generate.output_path = Some(args.base_hashes.clone()); @@ -1649,6 +1814,10 @@ mod tests { target_type: vec!["Rule".into()], dep_edges_file: Some(deps.clone()), modified_filepaths: None, + partial_graph: false, + partial_graph_rebuild_patterns: Vec::new(), + base_hashes: None, + git_path: PathBuf::from("git"), output_path: Some(output.clone()), }, false, @@ -1890,6 +2059,10 @@ mod tests { target_type: vec!["Rule".into()], dep_edges_file: None, modified_filepaths: None, + partial_graph: false, + partial_graph_rebuild_patterns: Vec::new(), + base_hashes: None, + git_path: PathBuf::from("git"), output_path: None, }, base_hashes: workspace.path().join("nested/base.json"), @@ -1925,4 +2098,312 @@ mod tests { .is_err()); assert!(!fingerprint_called.get()); } + + fn git_in(repo: &Path, args: &[&str]) -> String { + let output = std::process::Command::new("git") + .current_dir(repo) + .args(args) + .output() + .unwrap(); + assert!( + output.status.success(), + "git {args:?}: {}", + String::from_utf8_lossy(&output.stderr) + ); + String::from_utf8_lossy(&output.stdout).trim().to_owned() + } + + fn partial_generate_args(workspace: &Path, output: &Path) -> GenerateHashesArgs { + GenerateHashesArgs { + hashing: hashing_args(workspace), + content_hash_path: None, + include_target_type: false, + target_type: Vec::new(), + dep_edges_file: None, + modified_filepaths: None, + partial_graph: true, + partial_graph_rebuild_patterns: Vec::new(), + base_hashes: None, + git_path: PathBuf::from("git"), + output_path: Some(output.to_path_buf()), + } + } + + /// A partial-graph entry for the repository's current tree whose only target is the source + /// file `file.txt`, as a full generation in the mode would write it. + fn partial_base(repo: &Path, path: &Path) -> HashFileData { + let git = Git::new("git", repo); + let tree = git.snapshot_working_tree(&[]).unwrap(); + let recipes = bazel_diff::partial::PartialRecipes { + version: bazel_diff::partial::RECIPE_VERSION, + seed_hash: "00".repeat(32), + package_seeds: true, + ..Default::default() + }; + let targets = BTreeMap::from([("//:file.txt".to_owned(), "SourceFile".to_owned())]); + let data = HashFileData { + hashes: bazel_diff::partial::evaluate(&recipes, &targets, &tree).unwrap(), + partial: Some(recipes), + tree: tree.id().map(str::to_owned), + ..Default::default() + }; + fs::write( + path, + serde_json::to_vec(&data.serialized(true, false)).unwrap(), + ) + .unwrap(); + data + } + + #[test] + fn generate_partial_graph_flags_parse() { + let cli = parse(&[ + "bazel-diff", + "generate-hashes", + "-w", + ".", + "--partialGraph", + "--partialGraphRebuildPattern", + "*.lock,third_party/**", + "--baseHashes", + "base.json", + "--gitPath", + "/opt/git", + "out.json", + ]); + let Commands::GenerateHashes(args) = cli.command else { + panic!("expected generate-hashes"); + }; + assert!(args.partial_graph); + assert_eq!( + args.partial_graph_rebuild_patterns, + ["*.lock", "third_party/**"] + ); + assert_eq!(args.base_hashes, Some(PathBuf::from("base.json"))); + assert_eq!(args.git_path, PathBuf::from("/opt/git")); + let Commands::GenerateHashes(defaults) = + parse(&["bazel-diff", "generate-hashes", "-w", "."]).command + else { + panic!("expected generate-hashes"); + }; + assert!(!defaults.partial_graph); + assert!(defaults.base_hashes.is_none()); + assert_eq!(defaults.git_path, PathBuf::from("git")); + } + + #[test] + fn generate_partial_graph_rejects_incompatible_options() { + let workspace = tempfile::tempdir().unwrap(); + let output = workspace.path().join("out.json"); + let never = |_: &HashOptions| -> Result { panic!("must not generate") }; + let error = |args: &GenerateHashesArgs| { + run_generate_with(args, false, never) + .unwrap_err() + .to_string() + }; + let mut cquery = partial_generate_args(workspace.path(), &output); + cquery.hashing.use_cquery = true; + assert!(error(&cquery).contains("--useCquery")); + let mut content = partial_generate_args(workspace.path(), &output); + content.content_hash_path = Some(workspace.path().join("content.json")); + fs::write(workspace.path().join("content.json"), "{}").unwrap(); + assert!(error(&content).contains("--contentHashPath")); + let mut modified = partial_generate_args(workspace.path(), &output); + modified.modified_filepaths = Some(workspace.path().join("modified.txt")); + fs::write(workspace.path().join("modified.txt"), "").unwrap(); + assert!(error(&modified).contains("--modified-filepaths")); + let mut typed = partial_generate_args(workspace.path(), &output); + typed.target_type = vec!["Rule".into()]; + assert!(error(&typed).contains("--targetType")); + let mut pattern = partial_generate_args(workspace.path(), &output); + pattern.partial_graph_rebuild_patterns = vec!["[".into()]; + assert!(!error(&pattern).is_empty()); + let mut without_mode = partial_generate_args(workspace.path(), &output); + without_mode.partial_graph = false; + without_mode.base_hashes = Some(workspace.path().join("base.json")); + assert!(error(&without_mode).contains("--baseHashes needs --partialGraph")); + // Not a git repository: the snapshot fails before any generation. + assert!(!error(&partial_generate_args(workspace.path(), &output)).is_empty()); + } + + #[test] + fn generate_partial_graph_derives_from_base_or_falls_back() { + let repo = tempfile::tempdir().unwrap(); + git_in(repo.path(), &["init", "-q"]); + git_in(repo.path(), &["config", "user.email", "test@example.com"]); + git_in(repo.path(), &["config", "user.name", "Test"]); + fs::write(repo.path().join("file.txt"), "one").unwrap(); + git_in(repo.path(), &["add", "-A"]); + git_in(repo.path(), &["commit", "-q", "-m", "one"]); + let base_path = repo.path().join("base.json"); + let base = partial_base(repo.path(), &base_path); + let output = repo.path().join("out.json"); + let never = |_: &HashOptions| -> Result { panic!("must not generate") }; + let mut args = partial_generate_args(repo.path(), &output); + args.base_hashes = Some(base_path.clone()); + + // The base already describes the working tree: returned as is, no generation. + let same = run_generate_with(&args, false, never).unwrap(); + assert_eq!(same.hashes, base.hashes); + assert_eq!(same.tree, base.tree); + + // An uncommitted source edit is derived: no generation, a new tree, a moved hash. + fs::write(repo.path().join("file.txt"), "two").unwrap(); + let derived = run_generate_with(&args, false, never).unwrap(); + assert_ne!(derived.tree, base.tree); + assert_ne!(derived.hashes["//:file.txt"], base.hashes["//:file.txt"]); + assert_eq!(derived.partial, base.partial); + let written: serde_json::Value = + serde_json::from_slice(&fs::read(&output).unwrap()).unwrap(); + assert_eq!(written["metadata"]["tree"], derived.tree.clone().unwrap()); + assert!( + written["hashes"]["//:file.txt"] + .as_str() + .unwrap() + .starts_with("SourceFile#"), + "partial-graph files always carry target types: {written}" + ); + // The hash files inside the workspace are not part of the snapshot. + let expected_tree = |excluded: &[&Path]| { + Git::new("git", repo.path()) + .snapshot_working_tree(excluded) + .unwrap() + .id() + .map(str::to_owned) + }; + assert_eq!(derived.tree, expected_tree(&[&output, &base_path])); + assert_ne!(derived.tree, expected_tree(&[])); + + // A graph-defining edit falls back to a full generation that hashes from the tree. + fs::write(repo.path().join("BUILD"), "").unwrap(); + let generated = std::cell::Cell::new(false); + let full = run_generate_with(&args, false, |options| { + generated.set(true); + let tree = options.partial_tree.as_ref().expect("tree for generation"); + assert!(tree.get("BUILD").is_some()); + assert_eq!( + options.content_hashes.as_ref().unwrap()["file.txt"], + tree.content_hashes()["file.txt"] + ); + Ok(HashFileData { + tree: tree.id().map(str::to_owned), + ..Default::default() + }) + }) + .unwrap(); + assert!(generated.get()); + assert_eq!(full.tree, expected_tree(&[&output, &base_path])); + fs::remove_file(repo.path().join("BUILD")).unwrap(); + + // A base without recipes or without a tree, and no base at all, all generate. + let counted = |args: &GenerateHashesArgs| { + let calls = std::cell::Cell::new(0); + run_generate_with(args, false, |_| { + calls.set(calls.get() + 1); + Ok(HashFileData::default()) + }) + .unwrap(); + calls.get() + }; + let mut no_recipes = base.clone(); + no_recipes.partial = None; + fs::write( + &base_path, + serde_json::to_vec(&no_recipes.serialized(true, false)).unwrap(), + ) + .unwrap(); + assert_eq!(counted(&args), 1); + let mut with_edges = partial_generate_args(repo.path(), &output); + with_edges.base_hashes = Some(base_path.clone()); + with_edges.dep_edges_file = Some(repo.path().join("deps.json")); + fs::write( + &base_path, + serde_json::to_vec(&base.serialized(true, false)).unwrap(), + ) + .unwrap(); + assert_eq!( + counted(&with_edges), + 1, + "a base without edges cannot feed -d" + ); + let mut edged = base.clone(); + edged.dep_edges = BTreeMap::from([("//:file.txt".to_owned(), vec![])]); + fs::write( + &base_path, + serde_json::to_vec(&edged.serialized(true, true)).unwrap(), + ) + .unwrap(); + assert_eq!(counted(&with_edges), 0, "a base with edges derives"); + let written: serde_json::Value = + serde_json::from_slice(&fs::read(&output).unwrap()).unwrap(); + assert_eq!( + written["metadata"]["depEdges"]["//:file.txt"], + serde_json::json!([]) + ); + assert_eq!( + fs::read(repo.path().join("deps.json")).unwrap(), + br#"{"//:file.txt":[]}"# + ); + let mut no_tree = base.clone(); + no_tree.tree = None; + fs::write( + &base_path, + serde_json::to_vec(&no_tree.serialized(true, false)).unwrap(), + ) + .unwrap(); + assert_eq!(counted(&args), 1); + let mut unknown_tree = base.clone(); + unknown_tree.tree = Some("0".repeat(40)); + fs::write( + &base_path, + serde_json::to_vec(&unknown_tree.serialized(true, false)).unwrap(), + ) + .unwrap(); + assert_eq!( + counted(&args), + 1, + "a base tree missing from the clone falls back" + ); + assert_eq!(counted(&partial_generate_args(repo.path(), &output)), 1); + // A base file that cannot be read is a misconfiguration, not a fallback. + args.base_hashes = Some(repo.path().join("missing.json")); + assert!(run_generate_with(&args, false, never) + .unwrap_err() + .to_string() + .contains("missing.json")); + } + + #[test] + fn impacted_and_explain_refuse_mixed_modes() { + let dir = tempfile::tempdir().unwrap(); + let explain = explain_args(dir.path(), &[]); + let (from, to, deps) = explain_fixture(dir.path()); + let mut partial = HashFileData::read(&to).unwrap(); + partial.partial = Some(bazel_diff::partial::PartialRecipes::default()); + fs::write( + &to, + serde_json::to_vec(&partial.serialized(true, false)).unwrap(), + ) + .unwrap(); + let error = run_get_impacted( + &GetImpactedTargetsArgs { + starting_hashes: from.clone(), + final_hashes: to.clone(), + dep_edges_file: Some(deps.clone()), + target_type: Vec::new(), + output: None, + workspace_path: dir.path().to_path_buf(), + bazel_path: PathBuf::from("/definitely/missing/bazel"), + bazel_startup_options: Vec::new(), + no_bazelrc: false, + exclude_external_targets: Some(false), + }, + false, + ) + .unwrap_err() + .to_string(); + assert!(error.contains("same mode"), "{error}"); + let error = run_explain(&explain, false).unwrap_err().to_string(); + assert!(error.contains("same mode"), "{error}"); + } } diff --git a/src/model.rs b/src/model.rs index 29e38587..de0d9529 100644 --- a/src/model.rs +++ b/src/model.rs @@ -217,7 +217,8 @@ impl Serialize for SerializedMetadata<'_> { usize::from(self.data.module_graph_json.is_some()) + usize::from(include_dep_edges) + usize::from(self.data.dependency_fingerprint.is_some()) - + usize::from(self.data.partial.is_some()), + + usize::from(self.data.partial.is_some()) + + usize::from(self.data.tree.is_some()), )?; if let Some(module_graph_json) = &self.data.module_graph_json { state.serialize_field("moduleGraphJson", module_graph_json)?; @@ -231,6 +232,9 @@ impl Serialize for SerializedMetadata<'_> { if let Some(partial) = &self.data.partial { state.serialize_field("partialGraph", partial)?; } + if let Some(tree) = &self.data.tree { + state.serialize_field("tree", tree)?; + } state.end() } } @@ -253,6 +257,7 @@ impl Serialize for SerializedHashFile<'_> { if self.data.module_graph_json.is_none() && self.data.dependency_fingerprint.is_none() && self.data.partial.is_none() + && self.data.tree.is_none() && (!self.include_deps || self.data.dep_edges.is_empty()) { return hashes.serialize(serializer); @@ -276,8 +281,11 @@ pub struct HashFileData { pub module_graph_json: Option, pub dependency_fingerprint: Option, pub dep_edges: BTreeMap>, - /// Partial-graph mode (`serve --partialGraph`): the recipes a derivation re-evaluates. + /// Partial-graph mode (`--partialGraph`): the recipes a derivation re-evaluates. pub partial: Option, + /// Partial-graph mode: the id of the git tree the hashes describe, the base of a later + /// derivation's `git diff`. + pub tree: Option, } /// A JSON string borrowed from the input when it has no escapes, copied otherwise. Hash values @@ -349,6 +357,8 @@ struct Metadata { dep_edges: BTreeMap>, #[serde(default)] partial_graph: Option, + #[serde(default)] + tree: Option, } impl<'de> Deserialize<'de> for HashFileData { @@ -375,6 +385,7 @@ impl<'de> Deserialize<'de> for HashFileData { data.dependency_fingerprint = metadata.dependency_fingerprint; data.dep_edges = metadata.dep_edges; data.partial = metadata.partial_graph; + data.tree = metadata.tree; } label => { let value = map.next_value::>()?; @@ -432,6 +443,7 @@ impl HashFileData { if self.module_graph_json.is_none() && self.dependency_fingerprint.is_none() && self.partial.is_none() + && self.tree.is_none() && (!include_deps || self.dep_edges.is_empty()) { return Value::Object(hashes); @@ -461,6 +473,9 @@ impl HashFileData { serde_json::to_value(partial).expect("serializing recipes cannot fail"), ); } + if let Some(tree) = &self.tree { + metadata.insert("tree".to_owned(), Value::String(tree.clone())); + } serde_json::json!({"hashes": hashes, "metadata": metadata}) } } @@ -789,6 +804,19 @@ mod tests { let parsed = HashFileData::from_slice(streamed.to_string().as_bytes()).unwrap(); assert_eq!(parsed.partial, data.partial); assert_eq!(parsed.hashes, data.hashes); + assert!(parsed.tree.is_none()); + + data.tree = Some("f".repeat(40)); + let streamed = serde_json::to_value(data.serialized(true, false)).unwrap(); + assert_eq!(streamed, data.to_value(true, false)); + assert_eq!(streamed["metadata"]["tree"], "f".repeat(40)); + let parsed = HashFileData::from_slice(streamed.to_string().as_bytes()).unwrap(); + assert_eq!(parsed.tree, data.tree); + // The tree alone is metadata enough to leave the legacy bare shape. + data.partial = None; + let streamed = serde_json::to_value(data.serialized(false, false)).unwrap(); + assert_eq!(streamed["metadata"]["tree"], "f".repeat(40)); + assert_eq!(streamed, data.to_value(false, false)); } #[test] diff --git a/src/partial.rs b/src/partial.rs index a36aba8c..e63c66fe 100644 --- a/src/partial.rs +++ b/src/partial.rs @@ -1,4 +1,4 @@ -//! Opt-in partial graph updates for `bazel-diff serve` (`--partialGraph`). +//! Opt-in partial graph updates (`--partialGraph`, for `generate-hashes` and `serve`). //! //! A full hash generation pays for a `bazel query` over the whole graph even when the change //! between two revisions cannot have altered that graph: editing the body of a source file @@ -28,7 +28,7 @@ //! A fallback costs speed, never correctness. use crate::bazel::{attribute_main_repo_labels, target_label}; -use crate::hash::{hash_pool, is_main_repo, label_package, main_repo_path}; +use crate::hash::{hash_pool, is_main_repo, label_package, main_repo_path, HashOptions}; use crate::model::{HashFileData, TargetHash}; use crate::proto::blaze_query::Rule; use anyhow::{anyhow, bail, Context, Result}; @@ -73,6 +73,9 @@ pub struct TreeEntry { #[derive(Clone, Debug, Default, Eq, PartialEq)] pub struct TreeIndex { entries: BTreeMap, + /// The tree object's id, when the index was listed from one; a hash file records it so a + /// later derivation knows which tree its hashes describe. + id: Option, } impl TreeIndex { @@ -100,7 +103,18 @@ impl TreeIndex { Some((path, TreeEntry { object, kind })) }) .collect(); - Self { entries } + Self { entries, id: None } + } + + /// Tags the index with the id of the tree it was listed from. + pub fn with_id(mut self, id: impl Into) -> Self { + self.id = Some(id.into()); + self + } + + /// The id of the tree the index was listed from, if known. + pub fn id(&self) -> Option<&str> { + self.id.as_deref() } pub fn get(&self, path: &str) -> Option<&TreeEntry> { @@ -655,9 +669,9 @@ impl Evaluator<'_> { } } -/// Re-hashes a cached entry against another revision's tree. Everything but the hashes is -/// carried over: the graph did not change, so neither did the dependency edges, the module -/// graph, the dependency fingerprint, or the recipes themselves. +/// Re-hashes a cached entry against another revision's tree. Everything but the hashes and the +/// tree id is carried over: the graph did not change, so neither did the dependency edges, the +/// module graph, the dependency fingerprint, or the recipes themselves. pub fn derive(base: &HashFileData, tree: &TreeIndex) -> Result { let recipes = base .partial @@ -675,9 +689,28 @@ pub fn derive(base: &HashFileData, tree: &TreeIndex) -> Result { dependency_fingerprint: base.dependency_fingerprint.clone(), dep_edges: base.dep_edges.clone(), partial: base.partial.clone(), + tree: tree.id().map(str::to_owned), }) } +/// Rejects hashing options partial-graph mode cannot honour. The derivation evaluates recipes +/// keyed by bare label from the revision's git tree alone, so it has no configured instances +/// (cquery), no files outside the tree (fine-grained external repositories) and no +/// per-invocation seeds (always-affected tags); each of those would make a derived entry differ +/// from a full generation, which is the one property the mode rests on. +pub fn validate_hash_options(options: &HashOptions) -> Result<()> { + if options.bazel.use_cquery { + bail!("--partialGraph does not support --useCquery"); + } + if !options.bazel.fine_grained_external_repos.is_empty() { + bail!("--partialGraph does not support --fineGrainedHashExternalRepos"); + } + if !options.always_affected_tags.is_empty() { + bail!("--partialGraph does not support --alwaysAffectedTags"); + } + Ok(()) +} + // --------------------------------------------------------------------------------------------- // Change classification // --------------------------------------------------------------------------------------------- @@ -1027,6 +1060,7 @@ mod tests { .map(|(label, kind)| ((*label).to_owned(), hash(kind))) .collect(), partial: recipes.then(PartialRecipes::default), + tree: None, ..Default::default() } } @@ -1341,14 +1375,16 @@ mod tests { dependency_fingerprint: Some("fp".into()), dep_edges: BTreeMap::from([("//pkg:bin".to_owned(), vec!["//pkg:lib".to_owned()])]), partial: Some(recipes), + tree: None, }; - let mut edited = fixture_tree(); + let mut edited = fixture_tree().with_id("edited-tree"); edited.entries.get_mut("other/c.txt").unwrap().object = "c2".into(); let derived = derive(&base, &edited).unwrap(); assert_eq!(derived.module_graph_json.as_deref(), Some("{}")); assert_eq!(derived.dependency_fingerprint.as_deref(), Some("fp")); assert_eq!(derived.dep_edges, base.dep_edges); assert_eq!(derived.partial, base.partial); + assert_eq!(derived.tree.as_deref(), Some("edited-tree")); assert_ne!( derived.hashes["//other:c.txt"].hash, base.hashes["//other:c.txt"].hash @@ -1357,7 +1393,9 @@ mod tests { derived.hashes["//pkg:bin"].hash, base.hashes["//pkg:bin"].hash ); - assert_eq!(derive(&base, &fixture_tree()).unwrap().hashes, base.hashes); + let same = derive(&base, &fixture_tree()).unwrap(); + assert_eq!(same.hashes, base.hashes); + assert_eq!(same.tree, None); base.partial = None; assert!(derive(&base, &edited).is_err()); } diff --git a/src/server.rs b/src/server.rs index 046ca965..e5519451 100644 --- a/src/server.rs +++ b/src/server.rs @@ -1,11 +1,13 @@ use crate::bazel::BazelOptions; -use crate::cancel::{self, Cancellation, CANCELLED}; +use crate::cache; +use crate::cancel::{Cancellation, CANCELLED}; +use crate::git::{self, DerivationPolicy, Git}; use crate::hash::{generate_hashes_timed, HashOptions}; use crate::model::{ filter_and_sort_labels, impacted_targets, impacted_targets_with_distances, HashFileData, }; use crate::module_graph::impacted_with_module_changes; -use crate::partial::{self, Classification, ClassifyInputs, Glob, TreeIndex, MODULE_FILES}; +use crate::partial::{self, Classification, Glob, TreeIndex}; use anyhow::{anyhow, bail, Context, Result}; use flate2::read::GzDecoder; use flate2::write::GzEncoder; @@ -17,13 +19,11 @@ use s3::request::ResponseData; use s3::{Bucket, Region}; use serde::{Deserialize, Serialize}; use serde_json::{json, Value}; -use sha2::{Digest, Sha256}; use std::collections::{BTreeSet, HashSet, VecDeque}; use std::fmt; use std::fs; use std::io::{Read, Write}; use std::path::{Path, PathBuf}; -use std::process::{Command, Stdio}; use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use std::sync::{mpsc, Arc, Condvar, Mutex}; use std::time::{Duration, Instant, SystemTime}; @@ -187,6 +187,16 @@ impl State { } } + /// Git bound to the query currently holding the workspace, so the subprocesses it spawns die + /// with that query; outside the lock (startup fetch) they run plainly. + fn git(&self) -> Git { + Git::new( + &self.config.git_path, + &self.config.hash_options.bazel.workspace, + ) + .with_cancellation(self.workspace.holder()) + } + /// The server's Bazel options bound to the query currently holding the workspace, so the /// Bazel subprocesses it spawns die with that query. fn bazel_options(&self) -> BazelOptions { @@ -735,26 +745,13 @@ pub fn parse_byte_size(value: &str) -> Result { .ok_or_else(|| anyhow!("size overflow: {value}")) } -/// Rejects configurations partial-graph mode cannot honour. The derivation evaluates recipes -/// keyed by bare label from the revision's git tree alone, so it has no configured instances -/// (cquery), no files outside the tree (fine-grained external repositories) and no -/// per-invocation seeds (always-affected tags); each of those would make a derived entry differ -/// from a full generation, which is the one property the mode rests on. +/// Rejects configurations partial-graph mode cannot honour (see +/// [`partial::validate_hash_options`]). pub fn validate_partial_graph(config: &ServerConfig) -> Result<()> { if !config.partial_graph { return Ok(()); } - let bazel = &config.hash_options.bazel; - if bazel.use_cquery { - bail!("--partialGraph does not support --useCquery"); - } - if !bazel.fine_grained_external_repos.is_empty() { - bail!("--partialGraph does not support --fineGrainedHashExternalRepos"); - } - if !config.hash_options.always_affected_tags.is_empty() { - bail!("--partialGraph does not support --alwaysAffectedTags"); - } - Ok(()) + partial::validate_hash_options(&config.hash_options) } pub fn serve(config: ServerConfig) -> Result<()> { @@ -1563,89 +1560,18 @@ fn dependency_fingerprint_for_workspace(state: &State) -> Result { }) } +/// The hash-affecting configuration's digest, as [`cache::configuration_fingerprint`] +/// computes it for every producer of cache entries. fn configuration_fingerprint(config: &ServerConfig) -> String { - let options = &config.hash_options; - let mut hasher = Sha256::new(); - hasher.update(env!("CARGO_PKG_VERSION").as_bytes()); - hasher.update([0]); - for value in [ - options.bazel.startup_options.join("\0"), - options.bazel.command_options.join("\0"), - options.bazel.cquery_options.join("\0"), - options.bazel.use_cquery.to_string(), - options.bazel.cquery_expression.clone().unwrap_or_default(), - options.bazel.keep_going.to_string(), - options.bazel.exclude_external_targets.to_string(), - options - .bazel - .exclude_targets_query - .clone() - .unwrap_or_default(), - options - .bazel - .fine_grained_external_repos - .iter() - .cloned() - .collect::>() - .join(","), - options - .bazel - .label_only_module_extensions - .iter() - .cloned() - .collect::>() - .join(","), - options - .ignored_attributes - .iter() - .cloned() - .collect::>() - .into_iter() - .collect::>() - .join(","), - options - .seed_filepaths - .iter() - .map(|path| path.to_string_lossy()) - .collect::>() - .join("\0"), - options - .always_affected_tags - .iter() - .cloned() - .collect::>() - .into_iter() - .collect::>() - .join(","), - config.track_deps.to_string(), - // Full-content entries hash git blob ids rather than file bytes; keep them apart from - // entries a build without this seeding wrote under the same package version. - String::from("sourceContent=git-blob"), - ] { - hasher.update(value.as_bytes()); - hasher.update([0]); - } - if config.partial_graph { - // Partial-graph entries hash under a different formula and carry recipes; they are never - // compared with, or served in place of, an entry of the default mode. Only folded in when - // enabled so existing keys do not move. - hasher.update(format!("partialGraph={}", partial::RECIPE_VERSION).as_bytes()); - hasher.update([0]); - } - hex::encode(hasher.finalize())[..12].to_owned() + cache::configuration_fingerprint( + &config.hash_options, + config.track_deps, + config.partial_graph, + ) } fn cache_key(state: &State, sha: &str, modified: &BTreeSet) -> String { - let base = format!("{sha}.{}", state.fingerprint); - if modified.is_empty() { - return base; - } - let mut hasher = Sha256::new(); - for path in modified { - hasher.update(path.to_string_lossy().as_bytes()); - hasher.update(b"\n"); - } - format!("{base}.{}", &hex::encode(hasher.finalize())[..12]) + cache::cache_key(sha, &state.fingerprint, modified) } fn resolve_both(state: &State, from: &str, to: &str) -> Result<(String, String)> { @@ -1692,63 +1618,22 @@ fn fetch_revision(state: &State, revision: &str) -> bool { }) } +/// Thin wrappers over [`Git`] bound to the query holding the workspace, so the subprocesses +/// die with it (see [`State::git`]). fn resolve_sha(state: &State, revision: &str) -> Result { - let output = git_output( - state, - &[ - String::from("rev-parse"), - String::from("--verify"), - format!("{revision}^{{commit}}"), - ], - )?; - if !output.status.success() { - bail!("revision '{revision}' is missing from the local clone"); - } - let sha = String::from_utf8_lossy(&output.stdout).trim().to_owned(); - if sha.len() != 40 { - bail!("revision '{revision}' resolved to invalid SHA '{sha}'"); - } - Ok(sha) + state.git().resolve_sha(revision) } -/// Resolves the tree object a commit points at; distinct commits with identical content share it. fn resolve_tree(state: &State, sha: &str) -> Result { - let output = git_output( - state, - &[ - String::from("rev-parse"), - String::from("--verify"), - format!("{sha}^{{tree}}"), - ], - )?; - if !output.status.success() { - bail!("tree for revision '{sha}' is missing from the local clone"); - } - let tree = String::from_utf8_lossy(&output.stdout).trim().to_owned(); - if tree.len() != 40 { - bail!("revision '{sha}' resolved to invalid tree '{tree}'"); - } - Ok(tree) + state.git().resolve_tree(sha) } -/// Every tracked path at `sha` with its object id and mode, from one `git ls-tree`. -/// [`TreeIndex::content_hashes`] is the `{path: blob}` map `--contentHashPath` accepts. fn git_tree_index(state: &State, sha: &str) -> Result { - let output = git_success( - state, - &[ - String::from("ls-tree"), - String::from("-r"), - String::from("-z"), - sha.to_owned(), - ], - )?; - Ok(TreeIndex::parse_ls_tree(&output)) + state.git().tree_index(sha) } -/// Decides whether `sha` can be derived from the cached `base_sha` entry: the raw diff between -/// the two, the module and workspace files' label references at `sha`, and the server's -/// rebuild patterns and seed files go through [`partial::classify`]. +/// Decides whether `sha` can be derived from the cached `base_sha` entry under the server's +/// rebuild patterns and seed files (see [`git::classify_revisions`]). fn classify_revisions( state: &State, base_sha: &str, @@ -1756,107 +1641,29 @@ fn classify_revisions( base: &HashFileData, tree: &TreeIndex, ) -> Result { - let diff = git_success( - state, - &[ - String::from("diff"), - String::from("--raw"), - String::from("--no-renames"), - String::from("-z"), - base_sha.to_owned(), - sha.to_owned(), - ], - )?; - let diff = partial::parse_diff_raw(&diff)?; - let mut module_referenced = BTreeSet::new(); - for file in MODULE_FILES { - if tree.get(file).is_none() { - continue; - } - let text = git_success(state, &[String::from("show"), format!("{sha}:{file}")])?; - module_referenced.extend(partial::referenced_main_repo_paths( - &String::from_utf8_lossy(&text), - )); - } - Ok(partial::classify(&ClassifyInputs { - diff: &diff, - tree, + git::classify_revisions( + &state.git(), + base_sha, + sha, base, - workspace: &state.config.hash_options.bazel.workspace, - seed_filepaths: &state.config.hash_options.seed_filepaths, - rebuild_patterns: &state.config.partial_graph_rebuild_patterns, - module_referenced: &module_referenced, - })) -} - -/// Runs git and returns its stdout, failing on a non-zero exit with the command and stderr. -fn git_success(state: &State, args: &[String]) -> Result> { - let output = git_output(state, args)?; - if !output.status.success() { - bail!( - "git {} failed: {}", - args.join(" "), - String::from_utf8_lossy(&output.stderr) - ); - } - Ok(output.stdout) + tree, + &DerivationPolicy { + seed_filepaths: &state.config.hash_options.seed_filepaths, + rebuild_patterns: &state.config.partial_graph_rebuild_patterns, + }, + ) } fn checkout(state: &State, sha: &str) -> Result<()> { - if let Ok(output) = git_output(state, &[String::from("rev-parse"), String::from("HEAD")]) { - if output.status.success() && String::from_utf8_lossy(&output.stdout).trim() == sha { - return Ok(()); - } - } - let args = [ - String::from("-c"), - String::from("advice.detachedHead=false"), - String::from("checkout"), - String::from("--force"), - sha.to_owned(), - ]; - if git(state, &args).is_ok() { - return Ok(()); - } - let index_lock = state - .config - .hash_options - .bazel - .workspace - .join(".git/index.lock"); - if !index_lock.is_file() { - bail!("git checkout failed for revision {sha}"); - } - fs::remove_file(&index_lock).context("failed to remove stale .git/index.lock")?; - eprintln!( - "[Warn] cleared stale git index.lock in {}; retrying checkout of {sha}", - state.config.hash_options.bazel.workspace.display() - ); - git(state, &args) + state.git().checkout(sha) } fn git(state: &State, args: &[String]) -> Result<()> { - let output = git_output(state, args)?; - if !output.status.success() { - bail!( - "git {} failed: {}", - args.join(" "), - String::from_utf8_lossy(&output.stderr) - ); - } - Ok(()) + state.git().run(args) } -/// Runs git in the workspace. While a query holds the workspace the subprocess belongs to it and -/// is killed if that query is cancelled; outside the lock (startup fetch) it runs plainly. fn git_output(state: &State, args: &[String]) -> Result { - let mut command = Command::new(&state.config.git_path); - command - .current_dir(&state.config.hash_options.bazel.workspace) - .args(args) - .stdin(Stdio::null()); - cancel::output(state.workspace.holder().as_deref(), &mut command) - .with_context(|| format!("failed to execute {}", state.config.git_path.display())) + state.git().output(args) } fn metrics(state: &State) -> Value { @@ -2019,6 +1826,7 @@ mod tests { use std::collections::HashSet; use std::io::{Read, Write}; use std::net::TcpStream; + use std::process::Command; use std::sync::mpsc as std_mpsc; use std::thread; diff --git a/tests/e2e/core.rs b/tests/e2e/core.rs index b19ef7dd..52655a12 100644 --- a/tests/e2e/core.rs +++ b/tests/e2e/core.rs @@ -354,6 +354,135 @@ fn serve_partial_graph_derives_source_edits() { ); } +/// `generate-hashes --partialGraph`: a source edit is hashed from the `--baseHashes` entry with +/// no `bazel query`, as the working tree stands (committed or not), with the same impacted set +/// as the default mode and a file byte-identical to the one a full generation in the mode +/// writes; the two modes' files are refused side by side; a `BUILD` edit falls back to a full +/// generation. +#[test] +fn generate_hashes_partial_graph_derives_from_base_hashes() { + let workspace = copy_workspace("distance_metrics"); + init_git(workspace.path()); + let out = tempfile::tempdir().unwrap(); + let path = |name: &str| out.path().join(name); + let generated = |output: &Path, extra: &[&str]| -> String { + let result = generate(workspace.path(), output, extra); + assert!(result.status.success(), "generate-hashes {extra:?}"); + String::from_utf8_lossy(&result.stderr).into_owned() + }; + let base_default = path("base_default.json"); + let base_partial = path("base_partial.json"); + let base_deps = path("base_deps.json"); + generated(&base_default, &[]); + let log = generated( + &base_partial, + &["--partialGraph", "-d", &base_deps.display().to_string()], + ); + assert!(log.contains("full generation"), "{log}"); + assert!(log.contains("no --baseHashes"), "{log}"); + let base_partial_arg = base_partial.display().to_string(); + + // An uncommitted source edit: derived from the base, no query. + fs::write(workspace.path().join("lib.sh"), "echo changed\n").unwrap(); + let to_default = path("to_default.json"); + let to_derived = path("to_derived.json"); + let to_full = path("to_full.json"); + let derived_deps = path("derived_deps.json"); + let full_deps = path("full_deps.json"); + generated(&to_default, &[]); + let log = generated( + &to_derived, + &[ + "--partialGraph", + "--baseHashes", + &base_partial_arg, + "-d", + &derived_deps.display().to_string(), + ], + ); + assert!(log.contains("partial graph: derived"), "{log}"); + assert!(!log.contains("bazel query"), "{log}"); + generated( + &to_full, + &["--partialGraph", "-d", &full_deps.display().to_string()], + ); + assert_eq!(dep_edges(&derived_deps), dep_edges(&full_deps)); + assert!(dep_edges(&derived_deps).contains_key("//:lib")); + assert_eq!( + fs::read(&to_derived).unwrap(), + fs::read(&to_full).unwrap(), + "derived and fully generated files differ" + ); + let tree: Value = serde_json::from_slice(&fs::read(&to_full).unwrap()).unwrap(); + + // The same impacted set as the default mode, including the edited script's dependents. + let expected = path("expected.txt"); + let actual = path("actual.txt"); + assert!( + impacted(workspace.path(), &base_default, &to_default, &expected, &[]) + .status + .success() + ); + assert!( + impacted(workspace.path(), &base_partial, &to_derived, &actual, &[]) + .status + .success() + ); + let expected = filter_internal(read_lines(&expected)); + assert!(expected.contains("//:lib"), "{expected:?}"); + assert_eq!(filter_internal(read_lines(&actual)), expected); + let mixed = impacted( + workspace.path(), + &base_default, + &to_derived, + &path("mixed.txt"), + &[], + ); + assert!(!mixed.status.success()); + assert!(String::from_utf8_lossy(&mixed.stderr).contains("same mode")); + + // Committing the edit leaves the tree, and so the file, unchanged: the file written from the + // dirty working tree already recorded the tree the commit now points at. + let to = commit(workspace.path(), "source edit"); + assert_eq!( + tree["metadata"]["tree"], + git(workspace.path(), &["rev-parse", &format!("{to}^{{tree}}")]).as_str(), + "the file records the working tree's tree id" + ); + let committed = path("committed.json"); + generated( + &committed, + &[ + "--partialGraph", + "--baseHashes", + &base_partial_arg, + "-d", + &path("committed_deps.json").display().to_string(), + ], + ); + assert_eq!(fs::read(&committed).unwrap(), fs::read(&to_full).unwrap()); + + // A BUILD edit cannot be derived: a full generation runs and still records the tree. + let build = workspace.path().join("BUILD"); + let original = fs::read_to_string(&build).unwrap(); + fs::write(&build, format!("{original}\n# touched\n")).unwrap(); + let rebuilt = path("rebuilt.json"); + let log = generated( + &rebuilt, + &[ + "--partialGraph", + "--baseHashes", + &to_derived.display().to_string(), + ], + ); + assert!(log.contains("full generation"), "{log}"); + assert!(log.contains("graph-defining file BUILD"), "{log}"); + let rebuilt: Value = serde_json::from_slice(&fs::read(&rebuilt).unwrap()).unwrap(); + assert_eq!(rebuilt["metadata"]["tree"].as_str().unwrap().len(), 40); + assert_ne!(rebuilt["metadata"]["tree"], tree["metadata"]["tree"]); + assert!(rebuilt["metadata"]["partialGraph"].is_object()); +} + /// `--partialGraph` must not derive across an edit to a file a repository rule reads while /// loading: `dep_repo/hello` is a regular source file to git, but its content sits in the /// `//external:dep_repo` node's definition, so the entry records the local repository's root diff --git a/tools/e2e/rust_e2e_cases.bzl b/tools/e2e/rust_e2e_cases.bzl index 45a62d17..a93df5c1 100644 --- a/tools/e2e/rust_e2e_cases.bzl +++ b/tools/e2e/rust_e2e_cases.bzl @@ -23,6 +23,10 @@ RUST_E2E_SUITES = [ "name": "core::exclude_targets_query_filters_manual_targets", "timeout": "moderate", }, + { + "name": "core::generate_hashes_partial_graph_derives_from_base_hashes", + "timeout": "moderate", + }, { "name": "core::integration_golden", "timeout": "moderate", diff --git a/tools/readme_template.md b/tools/readme_template.md index 15bfe31f..986aab32 100644 --- a/tools/readme_template.md +++ b/tools/readme_template.md @@ -68,6 +68,59 @@ See https://github.com/aspect-extensions/impacted * We run `bazel-diff` on the starting and final JSON hash filepaths to get our impacted set of targets. This impacted set of targets is written to a file. +## Partial Graph Updates + +A full `generate-hashes` pays for a `bazel query` over the whole graph even when the change +cannot have altered that graph: editing the body of a source file changes what Bazel builds, +never which targets exist or how they depend on each other. `generate-hashes --partialGraph` +skips that query when it can: + +* The hashes describe the workspace's *git tree* rather than the files on disk: the working tree + as it stands, committed or not, is written as a tree object (an untracked file `.gitignore` + does not exclude is part of it, an ignored one is not, and neither are the hash files the run + itself reads and writes: the output, `--baseHashes` and `--depEdgesFile`), and source content + is read from that tree. A clean working tree yields the commit's own tree. The file records + the tree's id under `metadata.tree`, and every rule's hashing *recipe* (the digest of + everything that feeds its hash except source content, plus its ordered source inputs and + dependencies) under `metadata.partialGraph`. The file always includes every target's type, as + `--includeTargetType` would. +* With `--baseHashes ` naming a file written this way, the recipes it carries are + re-evaluated against the workspace's tree when `git diff` between the two trees only modifies + the content of regular files -- no `bazel query`, no Bazel at all. Anything else falls back to a + full generation: an added, deleted, renamed or type-changed path; a symlink or submodule on + either side; a change to a file Bazel reads while loading (`BUILD`, `BUILD.bazel`, `*.bzl`, + `*.scl`, `MODULE.bazel`, `MODULE.bazel.lock`, `WORKSPACE*`, `REPO.bazel`, `VENDOR.bazel`, + `*bazelrc`, `.bazelversion`, `.bazelignore`, `.bazeliskrc`, `tools/bazel`, `*.BUILD`, + `BUILD.*`, `*.patch`, `*.diff`); a file `MODULE.bazel` or a `WORKSPACE` file names by label; + anything a repository rule read from the workspace while the graph was generated; a + `--seed-filepaths` file; a path matching `--partialGraphRebuildPattern` (a glob: `*` within a + path segment, `**` across, a pattern without `/` matches the file name; repeatable); or a base + whose tree the local clone does not have. A fallback costs speed, never correctness, and the + log says which path each run took. A modified file no target reads impacts nothing, as it + would in a full generation. +* The file a derivation writes is byte-identical to the one a full generation under the flag + writes for the same tree, so derived files are themselves bases for later runs. A base that + already describes the workspace's tree is written back as is. With `--depEdgesFile` the hash + file also carries the dependency edges, so a derivation can write them; a base without them + cannot feed `--depEdgesFile` and forces a full generation. +* Hashes under the flag follow a slightly different formula from the default mode's, so + `get-impacted-targets` and `explain` refuse a pair of files of which only one carries recipes. + Not supported together with `--useCquery`, `--fineGrainedHashExternalRepos`, + `--alwaysAffectedTags`, `--contentHashPath`, `--modified-filepaths` or `--targetType` (a file + missing targets cannot be derived from; filter in `get-impacted-targets` instead): a derivation + reads only the git tree and keys recipes by bare label. The workspace must be the repository + root, and `--gitPath` names the git binary (default `git`). + +A typical CI job hashes the merge base once on the main branch and keeps the file; a pull +request run then needs no Bazel for the common case of a source-only change: + +```terminal +bazel-diff generate-hashes -w . -b bazel --partialGraph --baseHashes main_hashes.json pr_hashes.json +bazel-diff get-impacted-targets --startingHashes main_hashes.json --finalHashes pr_hashes.json -o impacted.txt +``` + +The [query service](#query-service) runs the same derivation server-side behind a cache. + ## Build Graph Distance Metrics `bazel-diff` can optionally compute build graph distance metrics between two revisions. This is @@ -452,7 +505,9 @@ Notes and operational guidance: with `--useCquery`, `--fineGrainedHashExternalRepos` or `--alwaysAffectedTags` (the server refuses to start): a derivation reads only the git tree and keys recipes by bare label. The nightly [`partial-graph-bench`](.github/workflows/partial-graph-bench.yml) workflow checks the - mode's speed and its exact agreement with the default mode (see `tools/perf/README.md`). + mode's speed and its exact agreement with the default mode (see `tools/perf/README.md`). The + one-shot `generate-hashes --partialGraph` writes the same entries (see [Partial Graph + Updates](#partial-graph-updates)). * The service checks out revisions inside `--workspacePath`, so point it at a dedicated clone, not a working tree you edit. All workspace-mutating work (git checkout + `bazel query`) is serialized, From c817fcd536598e4c47a6ba1011d6e7573464895c Mon Sep 17 00:00:00 2001 From: Maxwell Elliott Date: Fri, 30 Oct 2026 13:04:46 -0400 Subject: [PATCH 2/2] fix(git): keep the repository index's mtime on the snapshot's temporary copy The working-tree snapshot copies .git/index to a temporary index before `git add --all`. On Linux the copy gets a fresh mtime, which defeats git's racy-index rule: an edit made in the second the index was last written, keeping the file's size, is trusted as unchanged and left out of the snapshot. macOS's fs::copy preserves the mtime, which is why the CLI unit test only flaked on the Linux runner (Rust candidate, Bazel 9.x). Stamp the copy with the original's mtime so the snapshot re-reads exactly the entries git itself would, and add a deterministic regression test that models the race with core.checkStat=minimal and core.trustctime=false. Co-Authored-By: Claude Fable 5.1 --- src/git.rs | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/src/git.rs b/src/git.rs index 23a4010f..1150083c 100644 --- a/src/git.rs +++ b/src/git.rs @@ -196,6 +196,24 @@ impl Git { repository_index.display() ) })?; + // Git trusts an entry's cached stat data unless the file's mtime is at or after the + // index file's own ("racy git"), in which case it re-reads the content. The copy + // must carry the original's mtime: with a fresh one every entry looks trustworthy + // and an edit made in the second the index was last written, keeping the file's + // size, would go unseen. + let modified = fs::metadata(&repository_index) + .and_then(|metadata| metadata.modified()) + .with_context(|| { + format!( + "failed to read the modification time of {}", + repository_index.display() + ) + })?; + fs::File::options() + .write(true) + .open(&index) + .and_then(|file| file.set_modified(modified)) + .context("failed to set the temporary index's modification time")?; } let env = [("GIT_INDEX_FILE", index.as_os_str())]; self.stdout_with_env(["add", "--all"], &env)?; @@ -321,6 +339,7 @@ pub fn classify_revisions( mod tests { use super::*; use crate::partial::TreeKind; + use std::time::{Duration, SystemTime}; fn git_command(repo: &Path, args: &[&str]) -> String { let output = Command::new("git") @@ -449,6 +468,53 @@ mod tests { .is_none()); } + #[test] + fn working_tree_snapshot_rereads_edits_made_in_the_index_write_second() { + let (repo, git) = repository(); + // Only whole-second mtimes and sizes decide whether an entry is trusted, so the edit + // below is stat-identical to the committed content without a same-second race. + git_command(repo.path(), &["config", "core.trustctime", "false"]); + git_command(repo.path(), &["config", "core.checkStat", "minimal"]); + let file = repo.path().join("file.txt"); + let index = repo.path().join(".git/index"); + let set_modified = |path: &Path, time: SystemTime| { + fs::File::options() + .write(true) + .open(path) + .unwrap() + .set_modified(time) + .unwrap(); + }; + let written = SystemTime::UNIX_EPOCH + Duration::from_secs(1_700_000_000); + set_modified(&file, written); + git_command(repo.path(), &["add", "-A"]); + git_command( + repo.path(), + &["commit", "-q", "--allow-empty", "-m", "stat"], + ); + let head_tree = git_command(repo.path(), &["rev-parse", "HEAD^{tree}"]); + assert_eq!( + git.snapshot_working_tree(&[]).unwrap().id(), + Some(head_tree.as_str()) + ); + + // A same-size edit made in the second the index was written. + fs::write(&file, "two").unwrap(); + set_modified(&file, written); + set_modified(&index, written); + let snapshot = git.snapshot_working_tree(&[]).unwrap(); + assert_ne!(snapshot.id(), Some(head_tree.as_str())); + assert_eq!( + snapshot.content_hashes()["file.txt"], + git_command(repo.path(), &["hash-object", "file.txt"]) + ); + // Git itself re-reads it too (and, run last, rewrites the index with that knowledge). + assert_eq!( + git_command(repo.path(), &["status", "--porcelain"]), + "M file.txt" + ); + } + #[test] fn working_tree_snapshot_leaves_excluded_files_out() { let (repo, git) = repository();