From 8d7007c8d450d003559fe01f6813fde4693d3231 Mon Sep 17 00:00:00 2001 From: "@permadeath.com" Date: Mon, 24 Aug 2026 15:13:12 -0400 Subject: [PATCH] feat(repo): stamp change-ids with a commit-msg hook A change-id can only be written without rewriting history while the commit is being made, so `repo configure`, `clone` and `create` install a hook that writes one. It takes the commit-msg slot only when that slot is free or already atgc's; where a hook framework owns it, this repo's prek.toml shows the way in. Change-Id: Ic50c9232658178cb0f0982c258c8fb2f72fff756 --- CONTRIBUTING.md | 2 + README.md | 2 +- prek.toml | 18 +++ scripts/change-id-hook.sh | 55 +++++++ src/clients/git/hooks.rs | 311 ++++++++++++++++++++++++++++++++++++++ src/clients/git/mod.rs | 1 + src/cmd/repo/checkout.rs | 60 ++++++++ src/cmd/repo/mod.rs | 10 ++ src/cmd/repo/write.rs | 4 + src/cmd/stack/write.rs | 16 +- 10 files changed, 477 insertions(+), 2 deletions(-) create mode 100755 scripts/change-id-hook.sh create mode 100644 src/clients/git/hooks.rs diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a5c9a43..fe03346 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -62,6 +62,8 @@ lies about its own status, and `prek`'s doc-lint hook says so if you try. ### prek +Commits here gain a `Change-Id:` trailer automatically, from the `change-id` hook in `prek.toml` — it is what a stacked pull request is matched to its commits by, and it can only be written without rewriting history while the commit is being made. The script is `scripts/change-id-hook.sh`, and it does nothing to a commit that already has an id. In a repo with no hook framework, `atgc repo configure` installs the same script directly as `.git/hooks/commit-msg`; here prek owns that file, so it is declared as a hook instead. + Commits are checked by [prek](https://prek.j178.dev/), configured in `prek.toml`. Run `prek install` once in your checkout to enable the hooks. That installs two hook types, `pre-commit` and `commit-msg`, because `prek.toml` names both in `default_install_hook_types`; on a prek old enough not to read that key, spell it out with `prek install --hook-type pre-commit --hook-type commit-msg`. The `pre-commit` hooks cover whitespace and file hygiene plus `cargo fmt --check` and `cargo clippy -D warnings`, and they leave `vendor/` alone. `prek run --all-files` checks everything without committing: it runs the `pre-commit` stage, so it does not re-check commit messages. diff --git a/README.md b/README.md index f48166c..75ebfaf 100644 --- a/README.md +++ b/README.md @@ -99,7 +99,7 @@ what adds them. atgc repo create # create a Tangled repo from this checkout and push atgc repo create # ...or a blank one, with a checkout to start it in atgc repo clone # clone a Tangled repo and set its git identity -atgc repo configure # point this checkout at the current account +atgc repo configure # point this checkout at the current account, and hook it atgc repo edit # change a repo's description, website, spindle or topics atgc repo default-branch # point a repo's HEAD at a different branch, on the knot atgc repo delete-branch # delete a branch on the knot (irreversible) diff --git a/prek.toml b/prek.toml index 32a2b4f..78ea718 100644 --- a/prek.toml +++ b/prek.toml @@ -69,6 +69,24 @@ hooks = [ { id = "doc-lint", name = "doc lint", entry = "scripts/doc-lint.sh", language = "system", types = ["file"], files = '^(docs/.*\.md|scripts/doc-lint\.sh)$', pass_filenames = false, stages = ["pre-commit"] }, ] +# Stamp a Change-Id trailer on commits that lack one. +# +# A stacked pull request is matched to its commits by this trailer, and it can +# only be written without rewriting history while the commit is being made. In +# a repo with no hook framework `atgc repo configure` installs the same script +# straight into .git/hooks/commit-msg; here prek owns that file, so it is +# declared as a hook instead — prek's own extension point, in version control, +# where everyone who runs `prek install` gets it. +# +# Ordered before committed on purpose: hooks run top to bottom, and committed +# should see the finished message. The script itself is a no-op on a commit +# that already has an id, on a fixup!/squash!, and on an empty message. +[[repos]] +repo = "local" +hooks = [ + { id = "change-id", name = "change-id", entry = "scripts/change-id-hook.sh", language = "system", stages = ["commit-msg"], always_run = true }, +] + # Conventional Commits, checked as the message is written. # # This is not a new rule. Every commit from 5f7d819 onwards already conforms — diff --git a/scripts/change-id-hook.sh b/scripts/change-id-hook.sh new file mode 100755 index 0000000..58cd34f --- /dev/null +++ b/scripts/change-id-hook.sh @@ -0,0 +1,55 @@ +#!/bin/sh +# atgc-change-id-hook v1 +# +# Stamp a `Change-Id:` trailer on a commit that has none. The id is what a +# stacked pull request is matched to its commits by, and it is the one thing +# about a commit that survives an amend or a rebase — so it has to be written +# when the commit is, not bolted on afterwards by rewriting the branch. +# +# Installed by `atgc repo configure`. Deleting this file is a supported way +# to turn it off; atgc reinstalls it only when the slot is empty. +# +# Nothing here prompts, and nothing here fails a commit: every path that +# cannot produce an id leaves the message exactly as it found it and exits 0. + +msg="$1" +[ -n "$msg" ] || exit 0 +[ -f "$msg" ] || exit 0 + +# Already carries one. Also covers a commit being amended, whose trailer is +# already in the message being re-edited. +if grep -qi '^Change-Id:[[:space:]]*I' "$msg"; then + exit 0 +fi + +# A fixup or squash belongs to the commit it names, and will be folded into +# it: an id of its own would be one more id than there are changes. +if head -n 1 "$msg" | grep -qE '^(fixup|squash|amend)! '; then + exit 0 +fi + +# An empty message aborts the commit. Stamping one would turn that abort into +# a commit whose entire message is a trailer. +if [ -z "$(sed -e 's/#.*//' -e '/^[[:space:]]*$/d' "$msg")" ]; then + exit 0 +fi + +# Unique, not reproducible: author, time, pid and the message itself. Cut to +# the 40 hex digits a change-id is written with, since a repository using +# sha256 hashes would otherwise hand back 64. +id=$( + { + git var GIT_AUTHOR_IDENT 2>/dev/null + echo "$$" + date +%s 2>/dev/null + cat "$msg" + } | git hash-object --stdin 2>/dev/null | cut -c1-40 +) +[ -n "$id" ] || exit 0 + +# `interpret-trailers` places it in the trailer block, after a Signed-off-by +# and before nothing, and knows about comments and the scissors line — all +# the reasons not to append a line by hand here. +git interpret-trailers --if-exists doNothing --trailer "Change-Id: I$id" --in-place "$msg" 2>/dev/null + +exit 0 diff --git a/src/clients/git/hooks.rs b/src/clients/git/hooks.rs new file mode 100644 index 0000000..13bbacd --- /dev/null +++ b/src/clients/git/hooks.rs @@ -0,0 +1,311 @@ +//! Installing the `commit-msg` hook that stamps change-ids. +//! +//! A stacked pull request is matched to its commits by a `Change-Id:` +//! trailer, and the only moment that trailer can be written without +//! rewriting anybody's history is while the commit is being made. So atgc +//! installs a hook, rather than asking people to remember a flag: see +//! [`change-id-hook.sh`](../../../src/clients/git/change-id-hook.sh) for what +//! it does, which is nothing at all to a commit that already has an id. +//! +//! # Whose slot is it +//! +//! git runs exactly one file per hook name, so `.git/hooks/commit-msg` has an +//! owner, and in any repo using prek, pre-commit, husky or lefthook that owner +//! is the framework. Clobbering it would turn off whatever it runs — in this +//! project, the Conventional Commits check — and the framework would clobber +//! us back on its next install. +//! +//! So atgc takes the slot only when it is free, and otherwise takes nothing. +//! There is a tempting third option — pre-commit and prek both run a +//! `commit-msg.legacy` file beside their own, and writing one does work +//! (checked, in both install orders) — but that is their behaviour rather +//! than git's, it is undocumented as an interface, and `prek install +//! --overwrite` deletes legacy hooks. A hook that quietly stops running is +//! worse than a hook that was never installed, because the failure it +//! produces is a stack that cannot be reconciled, months later. +//! +//! What a framework repo gets instead is a sentence naming its own config as +//! the place to add the script, which is the framework's documented extension +//! point and lands in version control where the rest of the team gets it too. +//! This repository does exactly that: see the `change-id` hook in +//! `prek.toml`. + +use anyhow::{Context, Result}; +use std::path::{Path, PathBuf}; + +/// The hook itself, compiled in: atgc installs this into repositories that +/// have never heard of it, so it cannot live as a file beside the binary. +const HOOK: &str = include_str!("../../../scripts/change-id-hook.sh"); + +/// The line that says a hook file is atgc's, whatever version wrote it. The +/// script carries `atgc-change-id-hook v1`; this matches every version of +/// that, because recognizing our own older hooks is what makes an upgrade +/// replace one instead of declining the slot to it. +const MARKER: &str = "atgc-change-id-hook v"; + +/// What installing found, and did. +#[derive(Debug, PartialEq, Eq)] +pub enum Installed { + /// Written into a slot that was free. + Wrote(PathBuf), + /// Ours already, and the same version: nothing written. + Current(PathBuf), + /// Ours, from an older version of atgc, and replaced. + Updated(PathBuf), + /// A hook framework owns the slot. Nothing is written: its config is + /// the place this belongs, and the name is carried so the caller can say + /// which config that is. + Framework { path: PathBuf, name: &'static str }, + /// A hand-written hook owns the slot. Nothing is written — a repo whose + /// hooks are maintained by hand is a repo whose owner has opinions about + /// hooks. + Occupied(PathBuf), +} + +impl Installed { + /// The one-line report, or `None` when there is nothing worth saying — + /// which is the common case, a hook that was already in place. + pub fn describe(&self) -> Option { + match self { + Installed::Current(_) => None, + Installed::Wrote(path) => Some(format!( + "installed the change-id commit-msg hook: {}", + path.display() + )), + Installed::Updated(path) => Some(format!( + "updated the change-id commit-msg hook: {}", + path.display() + )), + Installed::Framework { path, name } => Some(format!( + "{name} owns {}, so the change-id hook was not installed\n\ + add it to your {name} config as a commit-msg hook running \ + scripts/change-id-hook.sh, or keep using `--add-change-ids`", + path.display() + )), + Installed::Occupied(path) => Some(format!( + "left {} alone: it is not atgc's to replace\n\ + have it run `atgc`'s scripts/change-id-hook.sh, or keep using \ + `--add-change-ids`", + path.display() + )), + } + } +} + +/// Whether a hook file is one of the dispatchers a hook framework installs. +/// +/// Recognized by the banner every one of them writes into the file, because +/// that is the only thing they have in common — the rest is each tool's own +/// shell. A framework we do not recognize is treated as a hand-written hook, +/// which is the safe direction: atgc declines the slot either way, and only +/// the wording of what it says differs. +fn framework(text: &str) -> Option<&'static str> { + [ + ("generated by prek", "prek"), + ("generated by pre-commit", "pre-commit"), + ("hook-impl", "pre-commit"), + ("husky", "husky"), + ("lefthook", "lefthook"), + ] + .iter() + .find(|(mark, _)| text.contains(mark)) + .map(|(_, name)| *name) +} + +fn is_ours(text: &str) -> bool { + text.contains(MARKER) +} + +/// Where this checkout's hooks live: `core.hooksPath` when set, the +/// repository's shared hooks directory otherwise. +/// +/// `git rev-parse --git-path hooks` answers all of it, including the case +/// this project lives in — inside a linked worktree, where it resolves to the +/// *common* directory, so one install covers every worktree of the repo. +fn hooks_dir(dir: &Path) -> Result { + let path = super::run::git_in(dir, &["rev-parse", "--git-path", "hooks"])?; + let path = PathBuf::from(path.trim()); + Ok(match path.is_absolute() { + true => path, + false => dir.join(path), + }) +} + +/// Install the hook, if a slot for it is free. Idempotent, silent, and +/// never destructive: see the module docs for which slot and why. +pub fn install_commit_msg(dir: &Path) -> Result { + let hooks = hooks_dir(dir)?; + let slot = hooks.join("commit-msg"); + + match read_hook(&slot)? { + // Nobody has it. + None => { + write_hook(&slot)?; + Ok(Installed::Wrote(slot)) + } + // Ours already: current, or an older version to replace. + Some(text) if is_ours(&text) => match text == HOOK { + true => Ok(Installed::Current(slot)), + false => { + write_hook(&slot)?; + Ok(Installed::Updated(slot)) + } + }, + Some(text) => match framework(&text) { + Some(name) => Ok(Installed::Framework { path: slot, name }), + None => Ok(Installed::Occupied(slot)), + }, + } +} + +fn read_hook(path: &Path) -> Result> { + match std::fs::read_to_string(path) { + Ok(text) => Ok(Some(text)), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(None), + // A hook that cannot be read is a hook that cannot be judged, and + // overwriting it on a permissions error is the one outcome worth + // ruling out. + Err(e) => Err(e).context(format!("reading the hook at {}", path.display())), + } +} + +fn write_hook(path: &Path) -> Result<()> { + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent) + .context(format!("creating the hooks directory {}", parent.display()))?; + } + std::fs::write(path, HOOK).context(format!("writing the hook at {}", path.display()))?; + // A hook git cannot execute is a hook git skips, silently, which is the + // failure this whole thing exists to avoid. + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mut perms = std::fs::metadata(path)?.permissions(); + perms.set_mode(0o755); + std::fs::set_permissions(path, perms) + .context(format!("making {} executable", path.display()))?; + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::testutil::TempRepo; + + /// The hook carries the marker the installer recognizes it by. Two + /// constants that have to agree, in two files, with no compiler between + /// them. + #[test] + fn the_script_carries_the_marker_the_installer_looks_for() { + assert!(HOOK.contains(MARKER), "the hook lost its version marker"); + assert!( + HOOK.contains(&format!("{MARKER}1")), + "the hook's marker carries no version" + ); + assert!(is_ours(HOOK)); + assert!( + framework(HOOK).is_none(), + "our own hook reads as a framework's" + ); + } + + /// An empty slot is taken, and a second run writes nothing. + #[test] + fn an_empty_slot_is_taken_once() { + let repo = TempRepo::new("hook-empty"); + let here = Path::new("."); + + let first = install_commit_msg(here).unwrap(); + let path = match &first { + Installed::Wrote(path) => path.clone(), + other => panic!("expected a write, got {other:?}"), + }; + assert!(path.ends_with("commit-msg"), "{path:?}"); + assert_eq!(std::fs::read_to_string(&path).unwrap(), HOOK); + + assert_eq!(install_commit_msg(here).unwrap(), Installed::Current(path)); + // And the hook actually fires: the id is on the commit, which is the + // only claim any of this is making. + repo.commit("a.txt", "one\n", "feat: a thing"); + let id = super::super::patch::change_id("HEAD").unwrap(); + assert!(id.is_some(), "the installed hook stamped nothing"); + } + + /// A framework's dispatcher keeps its slot and nothing is written: its + /// own config is where the script belongs, and the report says so. + #[test] + fn a_frameworks_hook_is_never_clobbered() { + let _repo = TempRepo::new("hook-framework"); + let here = Path::new("."); + let hooks = hooks_dir(here).unwrap(); + std::fs::create_dir_all(&hooks).unwrap(); + let dispatcher = "#!/bin/sh\n# File generated by prek: https://github.com/j178/prek\n"; + std::fs::write(hooks.join("commit-msg"), dispatcher).unwrap(); + + let installed = install_commit_msg(here).unwrap(); + assert_eq!( + installed, + Installed::Framework { + path: hooks.join("commit-msg"), + name: "prek", + } + ); + assert_eq!( + std::fs::read_to_string(hooks.join("commit-msg")).unwrap(), + dispatcher, + "the framework's dispatcher was overwritten" + ); + // Nothing beside it either: a `.legacy` file is prek's convention, + // not git's, and `prek install --overwrite` deletes those. + assert!(!hooks.join("commit-msg.legacy").exists()); + let said = installed.describe().expect("a framework repo is told why"); + assert!(said.contains("prek"), "{said}"); + assert!(said.contains("scripts/change-id-hook.sh"), "{said}"); + } + + /// A hand-written hook is left exactly as it is, and said so about. + #[test] + fn a_hand_written_hook_is_left_alone() { + let _repo = TempRepo::new("hook-foreign"); + let here = Path::new("."); + let hooks = hooks_dir(here).unwrap(); + std::fs::create_dir_all(&hooks).unwrap(); + let mine = "#!/bin/sh\necho my own hook\n"; + std::fs::write(hooks.join("commit-msg"), mine).unwrap(); + + let installed = install_commit_msg(here).unwrap(); + assert_eq!(installed, Installed::Occupied(hooks.join("commit-msg"))); + assert_eq!( + std::fs::read_to_string(hooks.join("commit-msg")).unwrap(), + mine + ); + assert!( + installed.describe().is_some(), + "silence would be wrong here" + ); + } + + /// An older copy of our own hook is replaced rather than reported. + #[test] + fn an_older_atgc_hook_is_updated_in_place() { + let _repo = TempRepo::new("hook-upgrade"); + let here = Path::new("."); + let hooks = hooks_dir(here).unwrap(); + std::fs::create_dir_all(&hooks).unwrap(); + std::fs::write( + hooks.join("commit-msg"), + "#!/bin/sh\n# atgc-change-id-hook v0\nexit 0\n", + ) + .unwrap(); + + assert_eq!( + install_commit_msg(here).unwrap(), + Installed::Updated(hooks.join("commit-msg")) + ); + assert_eq!( + std::fs::read_to_string(hooks.join("commit-msg")).unwrap(), + HOOK + ); + } +} diff --git a/src/clients/git/mod.rs b/src/clients/git/mod.rs index ee74eb9..8bcd524 100644 --- a/src/clients/git/mod.rs +++ b/src/clients/git/mod.rs @@ -14,6 +14,7 @@ //! which is scaffolding rather than a caller. pub(crate) mod config; +pub(crate) mod hooks; pub(crate) mod patch; pub(crate) mod run; pub(crate) mod ssh; diff --git a/src/cmd/repo/checkout.rs b/src/cmd/repo/checkout.rs index 51b1f45..50b6b1d 100644 --- a/src/cmd/repo/checkout.rs +++ b/src/cmd/repo/checkout.rs @@ -266,6 +266,48 @@ pub(super) fn apply_git_identity( } } +/// Install the change-id `commit-msg` hook, and say what happened. +/// +/// Called from every command that sets a checkout up — `clone`, `create` and +/// `configure` — because the hook is only useful if it is there before the +/// first commit, and a step somebody has to remember is a step that happens +/// to some checkouts and not others. It writes nothing when a hook that is +/// not atgc's holds the slot, so calling it more often costs nothing but a +/// stat. +/// +/// Returns the word `--json` reports, and never fails the command it is part +/// of: a checkout with no hook is a checkout where `stack create +/// --add-change-ids` still works, which is where everyone was before this +/// existed. +pub(super) fn install_change_id_hook(dir: &Path, skip: bool, dry_run: bool) -> &'static str { + if skip { + crate::term::say::step!(Git, "commit-msg hook: skipped (--no-hook)"); + return "skipped"; + } + if dry_run { + return "skipped"; + } + match crate::clients::git::hooks::install_commit_msg(dir) { + Ok(installed) => { + if let Some(line) = installed.describe() { + crate::term::say::step!(Git, "{line}"); + } + match installed { + crate::clients::git::hooks::Installed::Wrote(_) => "wrote", + crate::clients::git::hooks::Installed::Updated(_) => "updated", + crate::clients::git::hooks::Installed::Current(_) => "current", + crate::clients::git::hooks::Installed::Framework { .. } => "framework", + crate::clients::git::hooks::Installed::Occupied(_) => "occupied", + } + } + Err(e) => { + crate::logging::debug::dump_err("commit-msg hook install failed", &e); + crate::term::say::warning!(Git, "could not install the commit-msg hook: {e}"); + "failed" + } + } +} + #[derive(clap::Args, Debug)] pub(crate) struct CloneArgs { /// Repo to clone: owner/name, an at:// URI, or a Tangled URL @@ -281,6 +323,9 @@ pub(crate) struct CloneArgs { /// Don't set the Tangled git identity in the checkout's local config #[arg(long)] pub no_git_config: bool, + /// Don't install the commit-msg hook that stamps change-ids + #[arg(long)] + pub no_hook: bool, /// Show what would be cloned without cloning anything #[arg(long)] pub dry_run: bool, @@ -590,6 +635,7 @@ pub(super) async fn clone(args: CloneArgs) -> Result<()> { args.no_git_config, args.dry_run, )?; + install_change_id_hook(&dir, args.no_hook, args.dry_run); if args.json { return crate::term::jsonout::emit(&ClonedJson { @@ -620,6 +666,9 @@ pub(crate) struct ConfigureArgs { /// Don't pin the SSH key with core.sshCommand #[arg(long)] pub no_ssh_config: bool, + /// Don't install the commit-msg hook that stamps change-ids + #[arg(long)] + pub no_hook: bool, /// Replace an ordinary git identity, not just another Tangled one #[arg(long)] pub force: bool, @@ -658,6 +707,10 @@ pub(super) struct ConfiguredJson { /// and a DID, which is the convention every atgc command reads back. pub user_name: Option, pub user_email: String, + /// What became of the change-id hook: `wrote`, `updated`, `current`, + /// `framework` (a hook framework owns the slot; its own config is where + /// the script goes), `occupied`, `failed` or `skipped`. + pub commit_msg_hook: &'static str, } /// Point the current checkout at the currently selected account. @@ -761,6 +814,12 @@ pub(super) async fn configure(args: ConfigureArgs) -> Result<()> { configure_ssh(here, &did, &who, args.dry_run).await }; + // The hook is the reason a stack can be created from a plain-git branch + // without rewriting it first: the change-ids are already there. Not + // gated on --force or on the identity write, because it takes nothing + // away — an occupied slot is left occupied and reported. + let hook = install_change_id_hook(here, args.no_hook, args.dry_run); + if args.dry_run { if !args.json { println!("dry run; nothing written"); @@ -783,6 +842,7 @@ pub(super) async fn configure(args: ConfigureArgs) -> Result<()> { git_identity: written.label(), user_name, user_email: did, + commit_msg_hook: hook, }); } Ok(()) diff --git a/src/cmd/repo/mod.rs b/src/cmd/repo/mod.rs index 3e94478..e8451bc 100644 --- a/src/cmd/repo/mod.rs +++ b/src/cmd/repo/mod.rs @@ -53,6 +53,16 @@ pub(crate) enum Command { /// /// Writes the Tangled `[user]` identity, and pins the SSH key matching /// the account's registered keys, into this repo's local git config. + /// It also installs a `commit-msg` hook that stamps a `Change-Id:` + /// trailer on commits that have none — the identity a stacked pull + /// request is matched to its commits by, and the one thing that has to + /// be written while the commit is, not by rewriting the branch later. + /// A slot somebody else holds is never taken. Where a hook framework + /// like prek or pre-commit owns `commit-msg`, nothing is written and the + /// answer is a line naming that framework's own config as where the + /// script belongs; the same goes for a hand-written hook. `--no-hook` + /// skips it entirely. Deleting the file turns it off: atgc writes one + /// only into a slot that is empty or already its own. /// /// Unlike every other command, this one ignores the checkout's own /// `user.email` when deciding which account to act as: that is the line diff --git a/src/cmd/repo/write.rs b/src/cmd/repo/write.rs index b18ee6d..174c982 100644 --- a/src/cmd/repo/write.rs +++ b/src/cmd/repo/write.rs @@ -102,6 +102,9 @@ pub(crate) struct CreateArgs { /// Don't set the Tangled git identity in the checkout's local config #[arg(long)] pub no_git_config: bool, + /// Don't install the commit-msg hook that stamps change-ids + #[arg(long)] + pub no_hook: bool, /// Show what would be created without sending anything #[arg(long)] pub dry_run: bool, @@ -479,6 +482,7 @@ pub(crate) async fn create(args: CreateArgs) -> Result<()> { }) .with_context(|| format!("could not resolve a handle for {did}")); apply_git_identity(&dir, identity, Overwrite::Never, args.no_git_config, false)?; + super::checkout::install_change_id_hook(&dir, args.no_hook, false); if !place.pushes() { // Not a skipped push: there is no commit to push. Both ways of diff --git a/src/cmd/stack/write.rs b/src/cmd/stack/write.rs index 9da3234..876c11b 100644 --- a/src/cmd/stack/write.rs +++ b/src/cmd/stack/write.rs @@ -855,14 +855,28 @@ fn ensure_change_ids( rows.push((&sha[..7], subject_of(sha)?)); } let lines = two_column_listing(rows); + // The commits already made are this run's problem and only a rewrite + // fixes them — but the *next* ones are fixable for free, and this is + // the moment it is obvious the hook is missing. Installed quietly + // and reported only when something was written, because a refusal + // with two remedies in it reads as one remedy nobody can find. + let hook = crate::clients::git::hooks::install_commit_msg(Path::new(".")); + let hooked = match &hook { + Ok(installed) => installed.describe().map(|line| format!("\n{line}")), + Err(e) => { + crate::logging::debug::dump_err("commit-msg hook install failed", e); + None + } + }; bail!( "{} of {} commit(s) carry no change-id:\n{lines}\ a change-id is what matches each pull to its commit across rewrites\n\ rerun with --add-change-ids to rewrite {base}..HEAD with Change-Id trailers \ (shas change; `git reflog` records the old tip), or let jj >= 0.29 write \ - them via git.write-change-id-header", + them via git.write-change-id-header{}", missing.len(), commits.len(), + hooked.unwrap_or_default(), ); } if !gitpatch::working_tree_clean()? { -- 2.51.2