-
-
Notifications
You must be signed in to change notification settings - Fork 89
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
(#1088) post-rewrite hook does not track intermediate commits from an interactive rebase #1098
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would rather keep the API smaller. Can you revert this change and call |
||
&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( | ||
|
@@ -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. | ||
|
@@ -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(); | ||
|
@@ -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); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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()?; | ||
|
@@ -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), | ||
], | ||
)?; | ||
Comment on lines
+421
to
+429
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we just write |
||
|
||
{ | ||
let stdout = git.smartlog()?; | ||
insta::assert_snapshot!(stdout, @r###" | ||
O f777ecc (master) create initial.txt | ||
| | ||
@ 916a41f create test1.txt | ||
"###); | ||
} | ||
|
||
Ok(()) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there code that also writes the deferred commits that is now also obsolete? If so, we should delete it too. Or, alternatively, keep all the code but comment out the
add_events
line with a note, as we might need similar code in the future to fix certain other bugs.