Make check_rewritable take an iterator of &CommitId instead of &Commit

This function doesn't actually need commits, it only needs their IDs. In some
contexts we may only have commit IDs, so there's no need to require an iterator
of Commits.

This commit also adds a `CommitIteratorExt` that makes it easy to convert an
iterator of `&Commit` to an iterator of `&CommitId`.

cli/src/cli_util.rs

@@ -957,14 +957,10 @@ impl WorkspaceCommandHelper {
 
     pub fn check_rewritable<'a>(
         &self,
-        commits: impl IntoIterator<Item = &'a Commit>,
+        commits: impl IntoIterator<Item = &'a CommitId>,
     ) -> Result<(), CommandError> {
-        let to_rewrite_revset = RevsetExpression::commits(
-            commits
-                .into_iter()
-                .map(|commit| commit.id().clone())
-                .collect(),
-        );
+        let to_rewrite_revset =
+            RevsetExpression::commits(commits.into_iter().cloned().collect_vec());
         let immutable = revset_util::parse_immutable_expression(&self.revset_parse_context())
             .map_err(|e| {
                 config_error_with_message("Invalid `revset-aliases.immutable_heads()`", e)

cli/src/commands/abandon.rs

@@ -15,6 +15,7 @@
 use std::io::Write;
 
 use itertools::Itertools as _;
+use jj_lib::commit::CommitIteratorExt;
 use jj_lib::object_id::ObjectId;
 use tracing::instrument;
 
@@ -58,7 +59,7 @@ pub(crate) fn cmd_abandon(
         writeln!(ui.status(), "No revisions to abandon.")?;
         return Ok(());
     }
-    workspace_command.check_rewritable(&to_abandon)?;
+    workspace_command.check_rewritable(to_abandon.iter().ids())?;
 
     let mut tx = workspace_command.start_transaction();
     for commit in &to_abandon {

cli/src/commands/chmod.rs

@@ -66,7 +66,7 @@ pub(crate) fn cmd_chmod(
         .map(|path| workspace_command.parse_file_path(path))
         .try_collect()?;
     let commit = workspace_command.resolve_single_rev(&args.revision)?;
-    workspace_command.check_rewritable([&commit])?;
+    workspace_command.check_rewritable([commit.id()])?;
 
     let mut tx = workspace_command.start_transaction();
     let tree = commit.tree()?;

cli/src/commands/describe.rs

@@ -68,7 +68,7 @@ pub(crate) fn cmd_describe(
 ) -> Result<(), CommandError> {
     let mut workspace_command = command.workspace_helper(ui)?;
     let commit = workspace_command.resolve_single_rev(&args.revision)?;
-    workspace_command.check_rewritable([&commit])?;
+    workspace_command.check_rewritable([commit.id()])?;
     let description = if args.stdin {
         let mut buffer = String::new();
         io::stdin().read_to_string(&mut buffer).unwrap();

cli/src/commands/diffedit.rs

@@ -81,7 +81,7 @@ pub(crate) fn cmd_diffedit(
         base_commits = target_commit.parents();
         diff_description = "The diff initially shows the commit's changes.".to_string();
     };
-    workspace_command.check_rewritable([&target_commit])?;
+    workspace_command.check_rewritable([target_commit.id()])?;
 
     let diff_editor = workspace_command.diff_editor(ui, args.tool.as_deref())?;
     let mut tx = workspace_command.start_transaction();

cli/src/commands/edit.rs

@@ -44,7 +44,7 @@ pub(crate) fn cmd_edit(
 ) -> Result<(), CommandError> {
     let mut workspace_command = command.workspace_helper(ui)?;
     let new_commit = workspace_command.resolve_single_rev(&args.revision)?;
-    workspace_command.check_rewritable([&new_commit])?;
+    workspace_command.check_rewritable([new_commit.id()])?;
     if workspace_command.get_wc_commit_id() == Some(new_commit.id()) {
         writeln!(ui.status(), "Already editing that commit")?;
     } else {

cli/src/commands/new.rs

@@ -16,7 +16,7 @@ use std::io::Write;
 
 use clap::ArgGroup;
 use itertools::Itertools;
-use jj_lib::commit::Commit;
+use jj_lib::commit::{Commit, CommitIteratorExt};
 use jj_lib::repo::Repo;
 use jj_lib::revset::{RevsetExpression, RevsetIteratorExt};
 use jj_lib::rewrite::{merge_commit_trees, rebase_commit};
@@ -107,8 +107,7 @@ Please use `jj new 'all:x|y'` instead of `jj new --allow-large-revsets x y`.",
         // command line, add it between the changes' parents and the changes.
         // The parents of the new commit will be the parents of the target commits
         // which are not descendants of other target commits.
-        tx.base_workspace_helper()
-            .check_rewritable(&target_commits)?;
+        tx.base_workspace_helper().check_rewritable(&target_ids)?;
         let new_children = RevsetExpression::commits(target_ids.clone());
         let new_parents = new_children.parents();
         if let Some(commit_id) = new_children
@@ -160,7 +159,7 @@ Please use `jj new 'all:x|y'` instead of `jj new --allow-large-revsets x y`.",
             vec![]
         };
         tx.base_workspace_helper()
-            .check_rewritable(&commits_to_rebase)?;
+            .check_rewritable(commits_to_rebase.iter().ids())?;
         let merged_tree = merge_commit_trees(tx.repo(), &target_commits)?;
         new_commit = tx
             .mut_repo()

cli/src/commands/next.rs

@@ -150,7 +150,7 @@ pub(crate) fn cmd_next(
     // We're editing, just move to the target commit.
     if edit {
         // We're editing, the target must be rewritable.
-        workspace_command.check_rewritable([target])?;
+        workspace_command.check_rewritable([target.id()])?;
         let mut tx = workspace_command.start_transaction();
         tx.edit(target)?;
         tx.finish(

cli/src/commands/prev.rs

@@ -118,7 +118,7 @@ pub(crate) fn cmd_prev(
     // If we're editing, just move to the revision directly.
     if edit {
         // The target must be rewritable if we're editing.
-        workspace_command.check_rewritable([target])?;
+        workspace_command.check_rewritable([target.id()])?;
         let mut tx = workspace_command.start_transaction();
         tx.edit(target)?;
         tx.finish(

cli/src/commands/rebase.rs

@@ -21,7 +21,7 @@ use clap::ArgGroup;
 use indexmap::IndexSet;
 use itertools::Itertools;
 use jj_lib::backend::CommitId;
-use jj_lib::commit::Commit;
+use jj_lib::commit::{Commit, CommitIteratorExt};
 use jj_lib::object_id::ObjectId;
 use jj_lib::repo::{ReadonlyRepo, Repo};
 use jj_lib::revset::{RevsetExpression, RevsetIteratorExt};
@@ -314,7 +314,7 @@ fn rebase_descendants_transaction(
     old_commits: &IndexSet<Commit>,
     rebase_options: RebaseOptions,
 ) -> Result<(), CommandError> {
-    workspace_command.check_rewritable(old_commits)?;
+    workspace_command.check_rewritable(old_commits.iter().ids())?;
     let (skipped_commits, old_commits) = old_commits
         .iter()
         .partition::<Vec<_>, _>(|commit| commit.parents() == new_parents);
@@ -353,7 +353,7 @@ fn rebase_revision(
     rev_arg: &RevisionArg,
 ) -> Result<(), CommandError> {
     let old_commit = workspace_command.resolve_single_rev(rev_arg)?;
-    workspace_command.check_rewritable([&old_commit])?;
+    workspace_command.check_rewritable([old_commit.id()])?;
     if new_parents.contains(&old_commit) {
         return Err(user_error(format!(
             "Cannot rebase {} onto itself",
@@ -370,7 +370,9 @@ fn rebase_revision(
         .try_collect()?;
     // Currently, immutable commits are defined so that a child of a rewriteable
     // commit is always rewriteable.
-    debug_assert!(workspace_command.check_rewritable(&child_commits).is_ok());
+    debug_assert!(workspace_command
+        .check_rewritable(child_commits.iter().ids())
+        .is_ok());
 
     // First, rebase the children of `old_commit`.
     let mut tx = workspace_command.start_transaction();

cli/src/commands/resolve.rs

@@ -92,7 +92,7 @@ pub(crate) fn cmd_resolve(
     };
 
     let (repo_path, _) = conflicts.first().unwrap();
-    workspace_command.check_rewritable([&commit])?;
+    workspace_command.check_rewritable([commit.id()])?;
     let merge_editor = workspace_command.merge_editor(ui, args.tool.as_deref())?;
     writeln!(
         ui.status(),

cli/src/commands/restore.rs

@@ -96,7 +96,7 @@ pub(crate) fn cmd_restore(
             .resolve_single_rev(args.changes_in.as_ref().unwrap_or(&RevisionArg::AT))?;
         from_tree = merge_commit_trees(workspace_command.repo().as_ref(), &to_commit.parents())?;
     }
-    workspace_command.check_rewritable([&to_commit])?;
+    workspace_command.check_rewritable([to_commit.id()])?;
 
     let matcher = workspace_command.matcher_from_values(&args.paths)?;
     let to_tree = to_commit.tree()?;

cli/src/commands/split.rs

@@ -67,7 +67,7 @@ pub(crate) fn cmd_split(
 ) -> Result<(), CommandError> {
     let mut workspace_command = command.workspace_helper(ui)?;
     let commit = workspace_command.resolve_single_rev(&args.revision)?;
-    workspace_command.check_rewritable([&commit])?;
+    workspace_command.check_rewritable([commit.id()])?;
     let matcher = workspace_command.matcher_from_values(&args.paths)?;
     let diff_selector = workspace_command.diff_selector(
         ui,

cli/src/commands/squash.rs

@@ -13,7 +13,7 @@
 // limitations under the License.
 
 use itertools::Itertools as _;
-use jj_lib::commit::Commit;
+use jj_lib::commit::{Commit, CommitIteratorExt};
 use jj_lib::matchers::Matcher;
 use jj_lib::object_id::ObjectId;
 use jj_lib::repo::Repo;
@@ -145,7 +145,7 @@ pub fn move_diff(
     path_arg: &[String],
 ) -> Result<(), CommandError> {
     tx.base_workspace_helper()
-        .check_rewritable(sources.iter().chain(std::iter::once(destination)))?;
+        .check_rewritable(sources.iter().chain(std::iter::once(destination)).ids())?;
     // Tree diffs to apply to the destination
     let mut tree_diffs = vec![];
     let mut abandoned_commits = vec![];

cli/src/commands/unsquash.rs

@@ -57,13 +57,12 @@ pub(crate) fn cmd_unsquash(
 ) -> Result<(), CommandError> {
     let mut workspace_command = command.workspace_helper(ui)?;
     let commit = workspace_command.resolve_single_rev(&args.revision)?;
-    workspace_command.check_rewritable([&commit])?;
-    let parents = commit.parents();
-    if parents.len() != 1 {
+    workspace_command.check_rewritable([commit.id()])?;
+    if commit.parent_ids().len() > 1 {
         return Err(user_error("Cannot unsquash merge commits"));
     }
-    let parent = &parents[0];
-    workspace_command.check_rewritable(&parents[..1])?;
+    let parent = commit.parents().pop().unwrap();
+    workspace_command.check_rewritable([parent.id()])?;
     let interactive_editor = if args.tool.is_some() || args.interactive {
         Some(workspace_command.diff_editor(ui, args.tool.as_deref())?)
     } else {
@@ -85,7 +84,7 @@ the parent commit. The changes you edited out will be moved into the
 child commit. If you don't make any changes, then the operation will be
 aborted.
 ",
-            tx.format_commit_summary(parent),
+            tx.format_commit_summary(&parent),
             tx.format_commit_summary(&commit)
         );
         let parent_tree = parent.tree()?;
@@ -105,7 +104,8 @@ aborted.
     // case).
     if new_parent_tree_id == parent_base_tree.id() {
         tx.mut_repo().record_abandoned_commit(parent.id().clone());
-        let description = combine_messages(tx.base_repo(), &[parent], &commit, command.settings())?;
+        let description =
+            combine_messages(tx.base_repo(), &[&parent], &commit, command.settings())?;
         // Commit the new child on top of the parent's parents.
         tx.mut_repo()
             .rewrite_commit(command.settings(), &commit)
@@ -115,7 +115,7 @@ aborted.
     } else {
         let new_parent = tx
             .mut_repo()
-            .rewrite_commit(command.settings(), parent)
+            .rewrite_commit(command.settings(), &parent)
             .set_tree_id(new_parent_tree_id)
             .set_predecessors(vec![parent.id().clone(), commit.id().clone()])
             .write()?;

lib/src/commit.rs

@@ -163,6 +163,19 @@ impl Commit {
     }
 }
 
+pub trait CommitIteratorExt<'c, I> {
+    fn ids(self) -> Box<dyn Iterator<Item = &'c CommitId> + 'c>;
+}
+
+impl<'c, I> CommitIteratorExt<'c, I> for I
+where
+    I: Iterator<Item = &'c Commit> + 'c,
+{
+    fn ids(self) -> Box<dyn Iterator<Item = &'c CommitId> + 'c> {
+        Box::new(self.map(|commit| commit.id()))
+    }
+}
+
 /// Wrapper to sort `Commit` by committer timestamp.
 #[derive(Clone, Debug, Eq, Hash, PartialEq)]
 pub(crate) struct CommitByCommitterTimestamp(pub Commit);