Skip to content

Commit

Permalink
(#1088) post-rewrite hook does not track intermediate commits from a …
Browse files Browse the repository at this point in the history
…rebase
  • Loading branch information
cshinaver committed Nov 14, 2023
1 parent 1f9e583 commit cedfd12
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 37 deletions.
3 changes: 0 additions & 3 deletions git-branchless-hook/tests/test_hook.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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###"
Expand Down Expand Up @@ -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
|
Expand Down
32 changes: 2 additions & 30 deletions git-branchless-lib/src/core/rewrite/rewrite_hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
40 changes: 36 additions & 4 deletions git-branchless-lib/src/testing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -498,14 +498,15 @@ then you can only run tests in the main `git-branchless` and \
/// 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(
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(
Expand All @@ -524,6 +525,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.
Expand Down Expand Up @@ -877,6 +898,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();
Expand All @@ -896,8 +929,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);

Expand Down
54 changes: 54 additions & 0 deletions git-branchless/tests/test_hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()?;
Expand Down Expand Up @@ -385,3 +388,54 @@ fn test_git_am_recorded() -> eyre::Result<()> {

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(())
}

0 comments on commit cedfd12

Please sign in to comment.