Merge pull request #7876 from gitbutlerapp/make-refs-authoritative

make-refs-authoritative

Author: krlvi

Cargo.lock

@@ -43,7 +43,7 @@ pub fn locks(current_dir: &Path) -> anyhow::Result<()> {
     let worktree_changes = but_core::diff::worktree_changes(&repo)?;
     let input_stacks = but_hunk_dependency::workspace_stacks_to_input_stacks(
         &repo,
-        &but_workspace::stacks(&project.gb_dir())?,
+        &but_workspace::stacks(&project.gb_dir(), &repo)?,
         but_workspace::common_merge_base_with_target_branch(&project.gb_dir())?,
     )?;
     let ranges = but_hunk_dependency::WorkspaceRanges::try_from_stacks(input_stacks)?;

crates/but-cli/src/command/diff.rs

@@ -116,7 +116,9 @@ pub mod stacks {
 
     pub fn list(current_dir: &Path) -> anyhow::Result<()> {
         let project = project_from_path(current_dir)?;
-        debug_print(but_workspace::stacks(&project.gb_dir()))
+        let ctx = CommandContext::open(&project, AppSettings::default())?;
+        let repo = ctx.gix_repository()?;
+        debug_print(but_workspace::stacks(&project.gb_dir(), &repo))
     }
 
     pub fn branches(id: &str, current_dir: &Path) -> anyhow::Result<()> {

crates/but-cli/src/command/mod.rs

@@ -14,7 +14,7 @@ pub fn hunk_dependencies_for_workspace_changes_by_worktree_dir(
 ) -> anyhow::Result<HunkDependencies> {
     let repo = gix::open(worktree_dir).map_err(anyhow::Error::from)?;
     let worktree_changes = but_core::diff::worktree_changes(&repo)?;
-    let stacks = but_workspace::stacks(gitbutler_dir)?;
+    let stacks = but_workspace::stacks(gitbutler_dir, &repo)?;
     let common_merge_base = gitbutler_stack::VirtualBranchesHandle::new(gitbutler_dir)
         .get_default_target()?
         .sha;

crates/but-hunk-dependency/src/ui.rs

@@ -241,7 +241,7 @@ mod util {
 
     fn test_ctx_at(script_name: &str, name: &str) -> anyhow::Result<TestContext> {
         let ctx = gitbutler_testsupport::read_only::fixture(script_name, name)?;
-        let stacks = but_workspace::stacks(&ctx.project().gb_dir())?;
+        let stacks = but_workspace::stacks(&ctx.project().gb_dir(), &ctx.gix_repository()?)?;
 
         Ok(TestContext {
             repo: gix::open_opts(&ctx.project().path, gix::open::Options::isolated())?,

crates/but-hunk-dependency/tests/hunk_dependency/ui.rs

@@ -901,7 +901,7 @@ mod util {
 
     fn test_ctx_at(script_name: &str, name: &str) -> anyhow::Result<TestContext> {
         let ctx = gitbutler_testsupport::read_only::fixture(script_name, name)?;
-        let stacks = but_workspace::stacks(&ctx.project().gb_dir())?;
+        let stacks = but_workspace::stacks(&ctx.project().gb_dir(), &ctx.gix_repository()?)?;
         let handle = VirtualBranchesHandle::new(ctx.project().gb_dir());
 
         Ok(TestContext {

crates/but-hunk-dependency/tests/hunk_dependency/workspace_dependencies.rs

@@ -381,7 +381,6 @@ pub fn create_commit_and_update_refs(
             refs::rewrite(
                 repo,
                 vb,
-                None,
                 all_refs_by_id,
                 [(commit_in_graph, new_commit)],
                 &mut out.references,
@@ -442,7 +441,6 @@ pub fn create_commit_and_update_refs(
             refs::rewrite(
                 repo,
                 vb,
-                workspace_tip,
                 all_refs_by_id,
                 rebase
                     .commit_mapping

crates/but-workspace/src/commit_engine/mod.rs

@@ -56,12 +56,12 @@ impl ReferenceFrame {
                     .context("Didn't find stack - was it deleted just now?")?;
                 Ok(ReferenceFrame {
                     workspace_tip: Some(head_id.detach()),
-                    branch_tip: Some(stack.head.to_gix()),
+                    branch_tip: Some(stack.head(repo)?.to_gix()),
                 })
             }
             InferenceMode::CommitIdInStack(commit_id) => {
                 for stack in vb.branches.values() {
-                    let stack_tip = stack.head.to_gix();
+                    let stack_tip = stack.head(repo)?.to_gix();
                     if stack_tip
                         .attach(repo)
                         .ancestors()

crates/but-workspace/src/commit_engine/reference_frame.rs

@@ -4,8 +4,6 @@ use gitbutler_oxidize::{ObjectIdExt, OidExt};
 use gitbutler_stack::{CommitOrChangeId, VirtualBranchesState};
 use gix::prelude::ObjectIdExt as _;
 use gix::refs::transaction::PreviousValue;
-use gix::revision::walk::Sorting;
-use std::collections::BTreeMap;
 
 use super::StackSegmentId;
 
@@ -18,16 +16,18 @@ use super::StackSegmentId;
 pub fn rewrite(
     repo: &gix::Repository,
     state: &mut VirtualBranchesState,
-    workspace_tip: Option<gix::ObjectId>,
     mut refs_by_commit_id: gix::hashtable::HashMap<gix::ObjectId, Vec<gix::refs::FullName>>,
     changed_commits: impl IntoIterator<Item = (gix::ObjectId, gix::ObjectId)>,
     updated_refs: &mut Vec<UpdatedReference>,
     stack_segment: Option<&StackSegmentId>,
 ) -> anyhow::Result<()> {
     let mut ref_edits = Vec::new();
     let changed_commits: Vec<_> = changed_commits.into_iter().collect();
-    let change_id_to_id_map = generate_change_ids_to_commit_mapping(repo, &*state, workspace_tip)?;
-    let mut stacks_ordered: Vec<_> = state.branches.values_mut().collect();
+    let mut stacks_ordered: Vec<_> = state
+        .branches
+        .values_mut()
+        .filter(|stack| stack.in_workspace)
+        .collect();
     stacks_ordered.sort_by(|a, b| a.name.cmp(&b.name));
     for (old, new) in changed_commits {
         let old_git2 = old.to_git2();
@@ -38,8 +38,9 @@ pub fn rewrite(
                     continue; // Dont rewrite refs for other stacks
                 }
             }
-            if stack.head == old_git2 {
-                stack.head = new.to_git2();
+            if stack.head(repo)? == old_git2 {
+                // Perhaps skip this - the head will be updated later in this call
+                // stack.set_stack_head_without_persisting(repo, new.to_git2(), None)?;
                 stack.tree = new
                     .attach(repo)
                     .object()?
@@ -65,21 +66,7 @@ pub fn rewrite(
                             .find_map(|(idx, h)| (h.name == short_name).then_some(idx))
                     });
             for (idx, branch) in stack.heads.iter_mut().rev().enumerate() {
-                let id = match &mut branch.head() {
-                    CommitOrChangeId::CommitId(id_hex) => {
-                        let Some(id) = gix::ObjectId::from_hex(id_hex.as_bytes()).ok() else {
-                            continue;
-                        };
-                        id
-                    }
-                    #[allow(deprecated)]
-                    CommitOrChangeId::ChangeId(change_id) => {
-                        let Some(id) = change_id_to_id_map.get(change_id) else {
-                            continue;
-                        };
-                        *id
-                    }
-                };
+                let id = branch.head_oid(repo)?.to_gix();
                 if id == old {
                     if update_up_to_idx.is_some() && Some(idx) > update_up_to_idx {
                         // Make sure the actual refs also don't update (later)
@@ -135,63 +122,3 @@ pub fn rewrite(
     repo.edit_references(ref_edits)?;
     Ok(())
 }
-
-fn generate_change_ids_to_commit_mapping(
-    repo: &gix::Repository,
-    vb: &VirtualBranchesState,
-    workspace_tip: Option<gix::ObjectId>,
-) -> anyhow::Result<BTreeMap<String, gix::ObjectId>> {
-    let cache = repo.commit_graph_if_enabled()?;
-    let mut graph = repo.revision_graph(cache.as_ref());
-    let default_target_tip = vb
-        .default_target
-        .as_ref()
-        .map(|target| -> anyhow::Result<_> {
-            let r = repo.find_reference(&target.branch.to_string())?;
-            Ok(r.try_id())
-        })
-        .and_then(Result::ok)
-        .flatten();
-
-    let mut out = BTreeMap::new();
-    let merge_base = if default_target_tip.is_none() {
-        let Some(workspace_tip) = workspace_tip else {
-            return Ok(out);
-        };
-        let workspace_commit = workspace_tip
-            .attach(repo)
-            .object()?
-            .into_commit()
-            .decode()?
-            .to_owned();
-        if workspace_commit.parents.len() < 2 {
-            None
-        } else {
-            Some(repo.merge_base_octopus(workspace_commit.parents)?)
-        }
-    } else {
-        None
-    };
-    for stack in vb.branches.values().filter(|b| b.in_workspace) {
-        let stack_tip = stack.head.to_gix();
-        for info in stack_tip
-            .attach(repo)
-            .ancestors()
-            .with_boundary(match default_target_tip {
-                Some(target_tip) => {
-                    Some(repo.merge_base_with_graph(stack_tip, target_tip, &mut graph)?)
-                }
-                None => merge_base,
-            })
-            .sorting(Sorting::BreadthFirst)
-            .all()?
-            .filter_map(Result::ok)
-        {
-            let Some(headers) = but_core::Commit::from_id(info.id.attach(repo))?.headers() else {
-                continue;
-            };
-            out.insert(headers.change_id, info.id);
-        }
-    }
-    Ok(out)
-}

crates/but-workspace/src/commit_engine/refs.rs

@@ -127,18 +127,21 @@ impl StackEntry {
 /// If the GitButler state file in the provided path is missing or invalid, an error is returned.
 ///
 /// - `gb_dir`: The path to the GitButler state for the project. Normally this is `.git/gitbutler` in the project's repository.
-pub fn stacks(gb_dir: &Path) -> Result<Vec<StackEntry>> {
+pub fn stacks(gb_dir: &Path, repo: &gix::Repository) -> Result<Vec<StackEntry>> {
     let state = state_handle(gb_dir);
-    Ok(state
+
+    state
         .list_stacks_in_workspace()?
         .into_iter()
         .sorted_by_key(|s| s.order)
-        .map(|stack| StackEntry {
-            id: stack.id,
-            branch_names: stack.heads().into_iter().map(Into::into).collect(),
-            tip: stack.head().to_gix(),
+        .map(|stack| {
+            Ok(StackEntry {
+                id: stack.id,
+                branch_names: stack.heads().into_iter().map(Into::into).collect(),
+                tip: stack.head(repo).map(|h| h.to_gix())?,
+            })
         })
-        .collect())
+        .collect()
 }
 
 /// Represents the pushable status for the current stack.
@@ -213,12 +216,7 @@ pub struct StackDetails {
 }
 
 /// Determines if a force push is required to push a branch to its remote.
-fn requires_force(
-    ctx: &CommandContext,
-    stack: &Stack,
-    branch: &StackBranch,
-    remote: &str,
-) -> Result<bool> {
+fn requires_force(ctx: &CommandContext, branch: &StackBranch, remote: &str) -> Result<bool> {
     let upstream = branch.remote_reference(remote);
 
     let reference = match ctx.repo().refname_to_id(&upstream) {
@@ -232,7 +230,7 @@ fn requires_force(
         .find_commit(reference)
         .context("failed to find upstream commit")?;
 
-    let branch_head = branch.head_oid(&ctx.to_stack_context()?, stack)?;
+    let branch_head = branch.head_oid(&ctx.gix_repository()?)?;
     let merge_base = ctx.repo().merge_base(upstream_commit.id(), branch_head)?;
 
     Ok(merge_base != upstream_commit.id())
@@ -260,7 +258,7 @@ pub fn stack_info(gb_dir: &Path, stack_id: StackId, ctx: &CommandContext) -> Res
 
     for branch in branches {
         let mut branch_state = BranchState {
-            requires_force: requires_force(ctx, &stack, &branch, &remote)?,
+            requires_force: requires_force(ctx, &branch, &remote)?,
             ..Default::default()
         };
 
@@ -426,6 +424,7 @@ pub fn stack_branches(stack_id: String, ctx: &CommandContext) -> Result<Vec<Bran
     let mut stack = state.get_stack(Id::from_str(&stack_id)?)?;
     let stack_ctx = ctx.to_stack_context()?;
     let mut current_base = stack.merge_base(&stack_ctx)?.to_gix();
+    let repo = ctx.gix_repository()?;
     for internal in stack.branches() {
         let upstream_reference = ctx
             .repo()
@@ -441,7 +440,7 @@ pub fn stack_branches(stack_id: String, ctx: &CommandContext) -> Result<Vec<Bran
             archived: internal.archived,
             base_commit: current_base,
         };
-        current_base = internal.head_oid(&stack_ctx, &stack)?.to_gix();
+        current_base = internal.head_oid(&repo)?.to_gix();
         stack_branches.push(result);
     }
     stack.migrate_change_ids(ctx).ok(); // If it fails thats ok - best effort migration