fix(record): support --create, advance branches, mark reachable

Fixes various defects encountered during testing.

Author: claytonrcarter

git-branchless-record/src/lib.rs

@@ -12,10 +12,10 @@
 use std::collections::HashSet;
 use std::ffi::OsString;
 use std::fmt::Write;
-use std::time::SystemTime;
+use std::time::{SystemTime, UNIX_EPOCH};
 
 use chrono::DateTime;
-use eyre::OptionExt;
+use eyre::{OptionExt, WrapErr};
 use git_branchless_invoke::CommandContext;
 use git_branchless_opts::{MessageArgs, RecordArgs, ResolveRevsetOptions, Revset};
 use git_branchless_reword::{ResolveFixupCommitError, edit_message, resolve_commit_to_fixup};
@@ -24,8 +24,9 @@ use lib::core::check_out::{CheckOutCommitOptions, CheckoutTarget, check_out_comm
 use lib::core::config::{get_commit_template, get_restack_preserve_timestamps};
 use lib::core::dag::{CommitSet, Dag};
 use lib::core::effects::{Effects, OperationType};
-use lib::core::eventlog::{EventLogDb, EventReplayer, EventTransactionId};
+use lib::core::eventlog::{Event as LogEvent, EventLogDb, EventReplayer, EventTransactionId};
 use lib::core::formatting::Pluralize;
+use lib::core::gc::mark_commit_reachable;
 use lib::core::repo_ext::RepoExt;
 use lib::core::rewrite::{
     BuildRebasePlanError, BuildRebasePlanOptions, ExecuteRebasePlanOptions,
@@ -34,10 +35,10 @@ use lib::core::rewrite::{
 };
 use lib::core::untracked_file_cache::{UntrackedFileStrategy, process_untracked_files};
 use lib::git::{
-    CategorizedReferenceName, ConfigRead, FileMode, GitRunInfo, MaybeZeroOid, NonZeroOid, Repo,
-    ResolvedReferenceInfo, Signature, Stage, UpdateIndexCommand, WorkingCopyChangesType,
-    WorkingCopySnapshot, process_diff_for_record, summarize_diff_for_temporary_commit,
-    update_index,
+    CategorizedReferenceName, ConfigRead, FileMode, GitRunInfo, MaybeZeroOid, NonZeroOid,
+    ReferenceName, Repo, ResolvedReferenceInfo, Signature, Stage, UpdateIndexCommand,
+    WorkingCopyChangesType, WorkingCopySnapshot, process_diff_for_record,
+    summarize_diff_for_temporary_commit, update_index,
 };
 use lib::try_exit_code;
 use lib::util::{ExitCode, EyreExitOr};
@@ -112,6 +113,7 @@ fn record(
             &event_log_db,
             &event_tx_id,
             messages,
+            branch_name,
         )?);
     } else {
         try_exit_code!(create_commit_with_changes(
@@ -454,13 +456,14 @@ fn create_new_commit(
     event_log_db: &EventLogDb,
     event_tx_id: &EventTransactionId,
     messages: Vec<String>,
+    branch_name: Option<String>,
 ) -> EyreExitOr<()> {
     let head_info = repo.get_head_info()?;
-    let current_commit_oid = match head_info {
+    let (current_commit_oid, existing_ref_name) = match head_info {
         ResolvedReferenceInfo {
             oid: Some(oid),
-            reference_name: _,
-        } => oid,
+            reference_name,
+        } => (oid, reference_name),
         ResolvedReferenceInfo {
             oid: None,
             reference_name: _,
@@ -510,22 +513,46 @@ fn create_new_commit(
         vec![&current_commit],
     )?;
 
-    // TODO: move branch if ref_name
-    // TODO: mark commit reachable (will show in sl if descendants, but not if detached)
-    // FIXME: --new --create <branch> doesn't seem to work
+    mark_commit_reachable(repo, new_oid)
+        .wrap_err("Marking commit as reachable for GC purposes.")?;
+    event_log_db.add_events(vec![LogEvent::CommitEvent {
+        timestamp: now.duration_since(UNIX_EPOCH)?.as_secs_f64(),
+        event_tx_id: *event_tx_id,
+        commit_oid: new_oid,
+    }])?;
+
+    let checkout_target = if let Some(name) = branch_name {
+        // --create <branch>: create the new branch at the new commit.
+        try_exit_code!(git_run_info.run(
+            effects,
+            Some(*event_tx_id),
+            &["branch", &name, &new_oid.to_string()],
+        )?);
+        let ref_name = ReferenceName::from(format!("refs/heads/{name}"));
+        CheckoutTarget::Reference(ref_name)
+    } else if let Some(ref_name) = existing_ref_name {
+        // On a named branch: advance it to the new commit, mirroring `git commit`.
+        try_exit_code!(git_run_info.run(
+            effects,
+            Some(*event_tx_id),
+            &["update-ref", ref_name.as_str(), &new_oid.to_string()],
+        )?);
+        CheckoutTarget::Reference(ref_name)
+    } else {
+        // Detached HEAD, no --create: check out by OID.
+        CheckoutTarget::Oid(new_oid)
+    };
 
     try_exit_code!(check_out_commit(
         effects,
         git_run_info,
         repo,
         event_log_db,
         *event_tx_id,
-        Some(CheckoutTarget::Oid(new_oid)),
+        Some(checkout_target),
         &CheckOutCommitOptions {
-            additional_args: vec![],
-            force_detach: false,
-            reset: false,
             render_smartlog: false,
+            ..Default::default()
         },
     )?);
 

git-branchless-record/tests/test_record.rs

@@ -526,7 +526,8 @@ fn test_record_new() -> eyre::Result<()> {
             },
         )?;
         insta::assert_snapshot!(stdout, @r###"
-        branchless: running command: <git-executable> checkout f25fe40ff47319c8b8c61a32c03f2c3558aacadd --
+        branchless: running command: <git-executable> update-ref refs/heads/master f25fe40ff47319c8b8c61a32c03f2c3558aacadd
+        branchless: running command: <git-executable> checkout master --
         M	test1.txt
         "###);
 
@@ -542,9 +543,7 @@ fn test_record_new() -> eyre::Result<()> {
         let stdout = git.smartlog()?;
         insta::assert_snapshot!(stdout, @r###"
         :
-        O 62fc20d (master) create test1.txt
-        |
-        @ f25fe40 empty commit 1
+        @ f25fe40 (> master) empty commit 1
         "###);
     }
 
@@ -561,7 +560,8 @@ fn test_record_new() -> eyre::Result<()> {
             },
         )?;
         insta::assert_snapshot!(stdout, @r###"
-        branchless: running command: <git-executable> checkout 46ba0c4efce07dc705498e7b79f7116d4e9ef7a3 --
+        branchless: running command: <git-executable> update-ref refs/heads/master 46ba0c4efce07dc705498e7b79f7116d4e9ef7a3
+        branchless: running command: <git-executable> checkout master --
         M	test1.txt
         "###);
 
@@ -577,11 +577,7 @@ fn test_record_new() -> eyre::Result<()> {
         let stdout = git.smartlog()?;
         insta::assert_snapshot!(stdout, @r###"
         :
-        O 62fc20d (master) create test1.txt
-        |
-        o f25fe40 empty commit 1
-        |
-        @ 46ba0c4 empty commit 2
+        @ 46ba0c4 (> master) empty commit 2
         "###);
     }
 
@@ -624,7 +620,8 @@ fn test_record_new_uses_user_name_and_email() -> eyre::Result<()> {
             },
         )?;
         insta::assert_snapshot!(stdout, @r###"
-        branchless: running command: <git-executable> checkout d4aea9dcdb2ad9ddedc964de9e782aad5a97b864 --
+        branchless: running command: <git-executable> update-ref refs/heads/master d4aea9dcdb2ad9ddedc964de9e782aad5a97b864
+        branchless: running command: <git-executable> checkout master --
         M	test1.txt
         "###);
 
@@ -1363,3 +1360,97 @@ fn test_record_fixup() -> eyre::Result<()> {
 
     Ok(())
 }
+
+#[test]
+fn test_record_new_moves_branch() -> eyre::Result<()> {
+    let git = make_git()?;
+    if !git.supports_reference_transactions()? {
+        return Ok(());
+    }
+    git.init_repo()?;
+
+    {
+        // Switch to a new branch and then create a new commit. The branch
+        // pointer should advance with the new commit.
+        git.run(&["switch", "--create", "test"])?;
+        git.branchless("record", &["-m", "empty commit 1", "--new"])?;
+
+        let stdout = git.smartlog()?;
+        insta::assert_snapshot!(stdout, @r###"
+        O f777ecc (master) create initial.txt
+        |
+        @ 1fa4693 (> test) empty commit 1
+        "###);
+    }
+
+    Ok(())
+}
+
+#[test]
+fn test_record_new_marks_commit_reachable() -> eyre::Result<()> {
+    let git = make_git()?;
+    if !git.supports_reference_transactions()? {
+        return Ok(());
+    }
+    git.init_repo()?;
+    git.commit_file("test1", 1)?;
+    git.detach_head()?;
+
+    let stdout = git.smartlog()?;
+    insta::assert_snapshot!(stdout, @r###"
+        :
+        @ 62fc20d (master) create test1.txt
+    "###);
+
+    {
+        // Create new commit then switch back to master.
+        git.branchless("record", &["-m", "empty commit 1", "--new"])?;
+        git.run(&["switch", "master"])?;
+
+        // The new commit should still appear in the smartlog even though no
+        // branch points to it.
+        let stdout = git.smartlog()?;
+        insta::assert_snapshot!(stdout, @r###"
+        :
+        @ 62fc20d (> master) create test1.txt
+        |
+        o dc158fa empty commit 1
+        "###);
+    }
+
+    Ok(())
+}
+
+#[test]
+fn test_record_new_with_create() -> eyre::Result<()> {
+    let git = make_git()?;
+    if !git.supports_reference_transactions()? {
+        return Ok(());
+    }
+    git.init_repo()?;
+    git.commit_file("test1", 1)?;
+
+    let stdout = git.smartlog()?;
+    insta::assert_snapshot!(stdout, @r###"
+        :
+        @ 62fc20d (> master) create test1.txt
+    "###);
+
+    {
+        // Create a new commit and a new branch, which should be checked out.
+        git.branchless(
+            "record",
+            &["-m", "empty commit 1", "--new", "--create", "foo"],
+        )?;
+
+        let stdout = git.smartlog()?;
+        insta::assert_snapshot!(stdout, @r###"
+        :
+        O 62fc20d (master) create test1.txt
+        |
+        @ dc158fa (> foo) empty commit 1
+        "###);
+    }
+
+    Ok(())
+}