rewrite: make it possible to rebase descendants multiple times

Despite what the documentation said, we don't clear the record of
rewritten and abandoned commits at the end. This change fixes that,
and adds a test showing that it's possible to call
`MutableRepo::rebase_descendants()` multiple times.

lib/src/repo.rs

@@ -503,6 +503,10 @@ impl MutableRepo {
             .insert(new_id);
     }
 
+    pub fn clear_rewritten_commits(&mut self) {
+        self.rewritten_commits.clear();
+    }
+
     /// Record a commit as having been abandoned in this transaction. This
     /// record is used by `rebase_descendants()`.
     ///
@@ -513,9 +517,12 @@ impl MutableRepo {
         self.abandoned_commits.insert(old_id);
     }
 
+    pub fn clear_abandoned_commits(&mut self) {
+        self.abandoned_commits.clear();
+    }
+
     /// Creates a `DescendantRebaser` to rebase descendants of the recorded
-    /// rewritten and abandoned commits. Clears the records of rewritten and
-    /// abandoned commits.
+    /// rewritten and abandoned commits.
     pub fn create_descendant_rebaser<'settings, 'repo>(
         &'repo mut self,
         settings: &'settings UserSettings,

lib/src/rewrite.rs

@@ -365,6 +365,9 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> {
                 new_commit,
             });
         }
+        // TODO: As the TODO above says, we should probably change the API. Even if we
+        // don't, we should at least make this code not do any work if you call
+        // rebase_next() after we've returned None.
         let mut view = self.mut_repo.view().store_view().clone();
         for commit_id in &self.heads_to_remove {
             view.head_ids.remove(commit_id);
@@ -375,6 +378,8 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> {
         self.heads_to_remove.clear();
         self.heads_to_add.clear();
         self.mut_repo.set_view(view);
+        self.mut_repo.clear_rewritten_commits();
+        self.mut_repo.clear_abandoned_commits();
         None
     }
 

lib/tests/test_rewrite.rs

@@ -722,6 +722,70 @@ fn test_rebase_descendants_divergent_rewrite(use_git: bool) {
     );
 }
 
+#[test_case(false ; "local backend")]
+#[test_case(true ; "git backend")]
+fn test_rebase_descendants_repeated(use_git: bool) {
+    let settings = testutils::user_settings();
+    let test_workspace = testutils::init_repo(&settings, use_git);
+    let repo = &test_workspace.repo;
+
+    // Commit B was replaced by commit B2. Commit C should get rebased. Rebasing
+    // descendants again should have no effect (C should not get rebased again).
+    // We then replace B2 by B3. C should now get rebased onto B3.
+    //
+    // C
+    // B
+    // | B3
+    // |/
+    // | B2
+    // |/
+    // A
+    let mut tx = repo.start_transaction("test");
+    let mut graph_builder = CommitGraphBuilder::new(&settings, tx.mut_repo());
+    let commit_a = graph_builder.initial_commit();
+    let commit_b = graph_builder.commit_with_parents(&[&commit_a]);
+    let commit_c = graph_builder.commit_with_parents(&[&commit_b]);
+
+    let commit_b2 = CommitBuilder::for_rewrite_from(&settings, repo.store(), &commit_b)
+        .set_description("b2".to_string())
+        .write_to_repo(tx.mut_repo());
+    let mut rebaser = tx.mut_repo().create_descendant_rebaser(&settings);
+    let commit_c2 = assert_rebased(rebaser.rebase_next(), &commit_c, &[&commit_b2]);
+    assert!(rebaser.rebase_next().is_none());
+    assert_eq!(rebaser.rebased().len(), 1);
+
+    assert_eq!(
+        *tx.mut_repo().view().heads(),
+        hashset! {
+            repo.view().checkout().clone(),
+            commit_c2.id().clone(),
+        }
+    );
+
+    // We made no more changes, so nothing should be rebased.
+    let mut rebaser = tx.mut_repo().create_descendant_rebaser(&settings);
+    assert!(rebaser.rebase_next().is_none());
+    assert_eq!(rebaser.rebased().len(), 0);
+
+    // Now mark B3 as rewritten from B2 and rebase descendants again.
+    let commit_b3 = CommitBuilder::for_rewrite_from(&settings, repo.store(), &commit_b2)
+        .set_description("b3".to_string())
+        .write_to_repo(tx.mut_repo());
+    let mut rebaser = tx.mut_repo().create_descendant_rebaser(&settings);
+    let commit_c3 = assert_rebased(rebaser.rebase_next(), &commit_c2, &[&commit_b3]);
+    assert!(rebaser.rebase_next().is_none());
+    assert_eq!(rebaser.rebased().len(), 1);
+
+    assert_eq!(
+        *tx.mut_repo().view().heads(),
+        hashset! {
+            repo.view().checkout().clone(),
+            // commit_b.id().clone(),
+            commit_c3.id().clone(),
+        }
+    );
+}
+
 #[test_case(false ; "local backend")]
 #[test_case(true ; "git backend")]
 fn test_rebase_descendants_contents(use_git: bool) {