(arxanas#1088) post-rewrite hook does not track intermediate commits from a rebase

Author: cshinaver

git-branchless-hook/tests/test_hook.rs

@@ -105,7 +105,6 @@ fn test_rebase_no_process_new_commits_until_conclusion() -> eyre::Result<()> {
             insta::assert_snapshot!(stdout, @"");
         }
 
-        git.commit_file("test4", 4)?;
         {
             let (stdout, stderr) = git.run(&["rebase", "--continue"])?;
             insta::assert_snapshot!(stderr, @r###"
@@ -133,8 +132,6 @@ fn test_rebase_no_process_new_commits_until_conclusion() -> eyre::Result<()> {
             O f777ecc (master) create initial.txt
             |\
             | o 047b7ad create test1.txt
-            | |
-            | o ecab41f create test4.txt
             |
             x 62fc20d (rewritten as 047b7ad7) create test1.txt
             |

git-branchless-lib/src/core/rewrite/rewrite_hooks.rs

@@ -3,10 +3,9 @@
 use std::collections::{HashMap, HashSet};
 
 use std::fmt::Write;
-use std::fs::{self, File};
-use std::io::{self, stdin, BufRead, BufReader, Read, Write as WriteIo};
+use std::fs::File;
+use std::io::{stdin, BufRead, BufReader, Read, Write as WriteIo};
 use std::path::{Path, PathBuf};
-use std::str::FromStr;
 use std::time::SystemTime;
 
 use console::style;
@@ -44,20 +43,6 @@ pub fn get_deferred_commits_path(repo: &Repo) -> PathBuf {
     repo.get_rebase_state_dir_path().join("deferred-commits")
 }
 
-fn read_deferred_commits(repo: &Repo) -> eyre::Result<Vec<NonZeroOid>> {
-    let deferred_commits_path = get_deferred_commits_path(repo);
-    let contents = match fs::read_to_string(&deferred_commits_path) {
-        Ok(contents) => contents,
-        Err(err) if err.kind() == io::ErrorKind::NotFound => Default::default(),
-        Err(err) => {
-            return Err(err)
-                .with_context(|| format!("Reading deferred commits at {deferred_commits_path:?}"))
-        }
-    };
-    let commit_oids = contents.lines().map(NonZeroOid::from_str).try_collect()?;
-    Ok(commit_oids)
-}
-
 #[instrument(skip(stream))]
 fn read_rewritten_list_entries(
     stream: &mut impl Read,
@@ -148,19 +133,6 @@ pub fn hook_post_rewrite(
     let event_log_db = EventLogDb::new(&conn)?;
     let event_tx_id = event_log_db.make_transaction_id(now, "hook-post-rewrite")?;
 
-    {
-        let deferred_commit_oids = read_deferred_commits(&repo)?;
-        let commit_events = deferred_commit_oids
-            .into_iter()
-            .map(|commit_oid| Event::CommitEvent {
-                timestamp,
-                event_tx_id,
-                commit_oid,
-            })
-            .collect_vec();
-        event_log_db.add_events(commit_events)?;
-    }
-
     let (rewritten_oids, rewrite_events) = {
         let rewritten_oids = read_rewritten_list_entries(&mut stdin().lock())?;
         let events = rewritten_oids

git-branchless-lib/src/testing.rs

@@ -536,14 +536,15 @@ then you can only run tests in the main `git-branchless` and \
     /// hash. The filename is always appended to the message prefix.
     #[track_caller]
     #[instrument]
-    pub fn commit_file_with_contents_and_message(
+    pub fn commit_file_with_contents_and_message_and_file_name(
         &self,
         name: &str,
         time: isize,
         contents: &str,
         message_prefix: &str,
+        file_name: &str,
     ) -> eyre::Result<NonZeroOid> {
-        let message = format!("{message_prefix} {name}.txt");
+        let message = format!("{message_prefix} {file_name}");
         self.write_file_txt(name, contents)?;
         self.run(&["add", "."])?;
         self.run_with_options(
@@ -562,6 +563,26 @@ then you can only run tests in the main `git-branchless` and \
         Ok(oid)
     }
 
+    /// Commit a file with given contents and message. The `time` argument is
+    /// used to set the commit timestamp, which is factored into the commit
+    /// hash. The filename is always appended to the message prefix.
+    #[instrument]
+    pub fn commit_file_with_contents_and_message(
+        &self,
+        name: &str,
+        time: isize,
+        contents: &str,
+        message_prefix: &str,
+    ) -> eyre::Result<NonZeroOid> {
+        self.commit_file_with_contents_and_message_and_file_name(
+            name,
+            time,
+            contents,
+            message_prefix,
+            format!("{name}.txt").as_str(),
+        )
+    }
+
     /// Commit a file with given contents and a default message. The `time`
     /// argument is used to set the commit timestamp, which is factored into the
     /// commit hash.
@@ -925,6 +946,18 @@ pub mod pty {
         branchless_subcommand: &str,
         args: &[&str],
         inputs: &[PtyAction],
+    ) -> eyre::Result<ExitStatus> {
+        // add "branchless" to subcommand list
+        run_in_pty_with_command(git, &["branchless", branchless_subcommand], args, inputs)
+    }
+
+    /// Run the provided script in the context of a virtual terminal.
+    #[track_caller]
+    pub fn run_in_pty_with_command(
+        git: &Git,
+        command: &[&str],
+        args: &[&str],
+        inputs: &[PtyAction],
     ) -> eyre::Result<ExitStatus> {
         // Use the native pty implementation for the system
         let pty_system = native_pty_system();
@@ -944,8 +977,7 @@ pub mod pty {
             cmd.env(k, v);
         }
         cmd.env("TERM", "xterm");
-        cmd.arg("branchless");
-        cmd.arg(branchless_subcommand);
+        cmd.args(command);
         cmd.args(args);
         cmd.cwd(&git.repo_path);
 

git-branchless/tests/test_hooks.rs

@@ -5,9 +5,12 @@ use lib::core::eventlog::{Event, EventLogDb, EventReplayer};
 use lib::core::formatting::Glyphs;
 use lib::git::GitVersion;
 use lib::testing::make_git;
+use lib::testing::pty::{run_in_pty_with_command, PtyAction};
 use lib::util::get_sh;
 use std::process::Command;
 
+const CARRIAGE_RETURN: &str = "\r";
+
 #[test]
 fn test_abandoned_commit_message() -> eyre::Result<()> {
     let git = make_git()?;
@@ -411,5 +414,57 @@ fn test_symbolic_transaction_ref() -> eyre::Result<()> {
         branchless: processing checkout
         "###);
     }
+
+    Ok(())
+}
+
+#[cfg(unix)]
+#[test]
+fn test_git_rebase_multiple_fixup_does_not_strand_commits() -> eyre::Result<()> {
+    let git = make_git()?;
+    git.init_repo()?;
+
+    git.detach_head()?;
+    git.commit_file_with_contents_and_message_and_file_name(
+        "test1",
+        1,
+        "bleh",
+        "create",
+        "test1.txt",
+    )?;
+    git.commit_file_with_contents_and_message_and_file_name(
+        "test2",
+        2,
+        "bleh",
+        "fixup! create",
+        "test1.txt",
+    )?;
+    git.commit_file_with_contents_and_message_and_file_name(
+        "test3",
+        3,
+        "bleh",
+        "fixup! create",
+        "test1.txt",
+    )?;
+
+    run_in_pty_with_command(
+        &git,
+        &["rebase"],
+        &["-i", "--autosquash", "master"],
+        &[
+            PtyAction::WaitUntilContains(" "),
+            PtyAction::Write(CARRIAGE_RETURN),
+        ],
+    )?;
+
+    {
+        let stdout = git.smartlog()?;
+        insta::assert_snapshot!(stdout, @r###"
+        O f777ecc (master) create initial.txt
+        |
+        @ 916a41f create test1.txt
+        "###);
+    }
+
     Ok(())
 }