From d59496aba8693a2307b3df6b8ee72019d29655ad Mon Sep 17 00:00:00 2001 From: Caleb Owens Date: Wed, 20 May 2026 16:01:04 +0200 Subject: [PATCH] Head info handles gerrit metadata This brings the old gerrit-mode logic that previously was only present on the super legacy `stacks_details` function, and brings in into the `RefInfo` construction. I realise that this is the first time bringing the database into the `but-workspace` crate, but in this instance, it felt reasonable to provide a specific `GerritMetadataHandle` directly rather than adding more abstraction layers over it. --- Cargo.lock | 2 + crates/but-api/src/legacy/workspace.rs | 9 + crates/but-api/src/workspace_state.rs | 1 + crates/but-db/src/lib.rs | 2 +- crates/but-workspace/Cargo.toml | 2 + crates/but-workspace/src/legacy/stacks.rs | 18 +- crates/but-workspace/src/ref_info.rs | 131 ++++++++++- crates/but-workspace/src/ui/mod.rs | 4 +- .../tests/workspace/ref_info/mod.rs | 3 +- .../with_workspace_commit/journey/mod.rs | 2 +- .../ref_info/with_workspace_commit/mod.rs | 220 ++++++++++++------ crates/gitbutler-branch-actions/src/branch.rs | 1 + .../src/upstream_integration.rs | 1 + 13 files changed, 303 insertions(+), 93 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3f9ba601cc6..03c01aaf5ae 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1711,6 +1711,7 @@ dependencies = [ "bstr", "but-core", "but-ctx", + "but-db", "but-error", "but-graph", "but-meta", @@ -1719,6 +1720,7 @@ dependencies = [ "but-serde", "but-testsupport", "but-workspace", + "chrono", "flume", "git2", "gitbutler-cherry-pick", diff --git a/crates/but-api/src/legacy/workspace.rs b/crates/but-api/src/legacy/workspace.rs index bbc1dbfe1aa..8ac0652aa00 100644 --- a/crates/but-api/src/legacy/workspace.rs +++ b/crates/but-api/src/legacy/workspace.rs @@ -30,12 +30,21 @@ use crate::json::HexHash; pub fn head_info(ctx: &but_ctx::Context) -> Result { let repo = ctx.clone_repo_for_merging_non_persisting()?; let meta = ctx.meta()?; + let gerrit_mode_enabled = repo.git_settings()?.gitbutler_gerrit_mode.unwrap_or(false); + let db = gerrit_mode_enabled + .then(|| ctx.db.get_cache()) + .transpose()?; + let gerrit_mode = match db.as_ref() { + Some(db) => but_workspace::ref_info::GerritMode::Enabled(db.gerrit_metadata()), + None => but_workspace::ref_info::GerritMode::Disabled, + }; but_workspace::head_info( &repo, &meta, but_workspace::ref_info::Options { traversal: but_graph::init::Options::limited(), expensive_commit_info: true, + gerrit_mode, }, ) .map(|info| info.pruned_to_entrypoint()) diff --git a/crates/but-api/src/workspace_state.rs b/crates/but-api/src/workspace_state.rs index 805c2dd2269..57c02ee5fc7 100644 --- a/crates/but-api/src/workspace_state.rs +++ b/crates/but-api/src/workspace_state.rs @@ -38,6 +38,7 @@ impl WorkspaceState { but_workspace::ref_info::Options { traversal: but_graph::init::Options::limited(), expensive_commit_info: true, + ..Default::default() }, )? .pruned_to_entrypoint(); diff --git a/crates/but-db/src/lib.rs b/crates/but-db/src/lib.rs index bcb498c7442..1b210d9094c 100644 --- a/crates/but-db/src/lib.rs +++ b/crates/but-db/src/lib.rs @@ -76,7 +76,7 @@ pub use table::{ claude::{ClaudeMessage, ClaudePermissionRequest, ClaudeSession}, file_write_locks::FileWriteLock, workspace_rules::WorkspaceRule, - gerrit_metadata::GerritMeta, + gerrit_metadata::{GerritMeta, GerritMetadataHandle}, forge_reviews::ForgeReview, ci_checks::CiCheck, virtual_branches::{VbBranchTarget, VbStack, VbStackHead, VbState, VirtualBranchesSnapshot, VirtualBranchesHandle, VirtualBranchesHandleMut}, diff --git a/crates/but-workspace/Cargo.toml b/crates/but-workspace/Cargo.toml index fb20dc9c507..4980ccccab4 100644 --- a/crates/but-workspace/Cargo.toml +++ b/crates/but-workspace/Cargo.toml @@ -29,6 +29,7 @@ export-schema = ["dep:schemars", "dep:but-schemars", "but-core/export-schema"] [dependencies] but-core.workspace = true +but-db.workspace = true but-graph.workspace = true but-rebase.workspace = true but-error.workspace = true @@ -68,6 +69,7 @@ insta = { version = "1.47.2", features = ["json"] } # TODO: remove once `gitbutler-repo` isn't needed anymore. gitbutler-commit = { workspace = true, features = ["testing"] } gitbutler-reference.workspace = true +chrono.workspace = true [lints] workspace = true diff --git a/crates/but-workspace/src/legacy/stacks.rs b/crates/but-workspace/src/legacy/stacks.rs index 8dba632f0ec..86f8cdafdfd 100644 --- a/crates/but-workspace/src/legacy/stacks.rs +++ b/crates/but-workspace/src/legacy/stacks.rs @@ -229,6 +229,7 @@ pub fn stacks_v3( let options = ref_info::Options { expensive_commit_info: false, traversal: but_graph::init::Options::limited(), + ..Default::default() }; let info = match ref_name_override { None => head_info(repo, meta, options), @@ -304,11 +305,14 @@ pub fn stack_details_v3( .into_iter() .find(|stack| stack.id == Some(stack_id)) } - let mut ref_info_options = ref_info::Options { - // TODO(perf): make this so it can be enabled for a specific stack-id. - expensive_commit_info: true, - traversal: but_graph::init::Options::limited(), - }; + fn new_ref_info_options() -> ref_info::Options<'static> { + ref_info::Options { + expensive_commit_info: true, + traversal: but_graph::init::Options::limited(), + ..Default::default() + } + } + let mut ref_info_options = new_ref_info_options(); let mut stack = match stack_id { None => { // assume single-branch mode. @@ -335,7 +339,7 @@ pub fn stack_details_v3( } Some(stack_id) => { if let Some(stack) = - stack_by_id(head_info(repo, meta, ref_info_options.clone())?, stack_id) + stack_by_id(head_info(repo, meta, new_ref_info_options())?, stack_id) { stack } else { @@ -349,7 +353,7 @@ pub fn stack_details_v3( .with_context(|| { format!("Couldn't find any refs for stack {stack_id} in the repository") })?; - let ref_info = ref_info(existing_ref, meta, ref_info_options)?; + let ref_info = ref_info(existing_ref, meta, new_ref_info_options())?; stack_by_id(ref_info, stack_id).with_context(|| { format!("Really couldn't find {stack_id} in the current workspace projection") })? diff --git a/crates/but-workspace/src/ref_info.rs b/crates/but-workspace/src/ref_info.rs index e073b39dea0..a611646a281 100644 --- a/crates/but-workspace/src/ref_info.rs +++ b/crates/but-workspace/src/ref_info.rs @@ -5,6 +5,7 @@ use std::{ borrow::Cow, ops::{Deref, DerefMut}, + str::FromStr, }; use bstr::BString; @@ -45,6 +46,8 @@ pub struct Commit { /// if it's not stored in the Commit. Use [`Self::change_id()`] to always get the change id, /// if necessary, by deriving it from the commit hash itself. pub change_id: Option, + /// Optional URL to the Gerrit review for this commit, if applicable. + pub gerrit_review_url: Option, } impl std::fmt::Debug for Commit { @@ -80,6 +83,7 @@ impl From> for Commit { change_id, refs: Vec::new(), flags: StackCommitFlags::empty(), + gerrit_review_url: None, } } } @@ -114,6 +118,7 @@ impl Commit { refs: graph_commit.refs.clone(), flags: graph_commit.flags.into(), change_id: hdr.and_then(|hdr| hdr.change_id), + gerrit_review_url: None, } } } @@ -222,15 +227,48 @@ impl WorkspaceExt for but_graph::Workspace { } } +/// Controls whether [`RefInfo`] should be interpreted with Gerrit push metadata. +/// +/// Standard `head_info()` derives commit relation and push status from the Git +/// graph: local branch tips, remote-tracking refs, target reachability, and +/// similarity checks. Gerrit mode has an extra source of truth: after a push, +/// GitButler records the Gerrit Change-Id, the patchset commit id accepted by +/// Gerrit, and the review URL in the cache database. +/// +/// When enabled, that recorded metadata is applied after the standard graph and +/// similarity pass. This lets `RefInfo` report commits as already present on +/// Gerrit even when there is no normal remote-tracking branch update for +/// `refs/for/*` pushes, and lets the UI link commits back to their Gerrit +/// reviews. +#[derive(Default)] +pub enum GerritMode<'db> { + /// Use only the standard graph-derived `head_info()` data. + #[default] + Disabled, + /// Apply Gerrit metadata from the cache database to commits and push status. + Enabled(but_db::GerritMetadataHandle<'db>), +} + +impl std::fmt::Debug for GerritMode<'_> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + GerritMode::Disabled => f.write_str("Disabled"), + GerritMode::Enabled(_) => f.write_str("Enabled(..)"), + } + } +} + /// Options for the [`ref_info()`](crate::ref_info()) call. -#[derive(Default, Debug, Clone)] -pub struct Options { +#[derive(Default, Debug)] +pub struct Options<'db> { /// Control how to traverse the commit-graph as the basis for the workspace conversion. pub traversal: but_graph::init::Options, /// Perform expensive computations on a per-commit basis. /// /// Note that less expensive checks are still performed. pub expensive_commit_info: bool, + /// Configure whether Gerrit metadata should augment the standard graph-derived result. + pub gerrit_mode: GerritMode<'db>, } /// A segment of a commit graph, representing a set of commits exclusively. @@ -358,7 +396,7 @@ impl std::fmt::Debug for Segment { } } -use anyhow::bail; +use anyhow::{Context as _, bail}; use but_core::{is_workspace_ref_name, ref_metadata::ValueInfo}; use but_graph::{ Graph, @@ -377,7 +415,7 @@ use crate::{AncestorWorkspaceCommit, RefInfo, WorkspaceCommit, branch, ui::PushS pub fn head_info( repo: &gix::Repository, meta: &impl but_core::RefMetadata, - opts: Options, + opts: Options<'_>, ) -> anyhow::Result { let graph = Graph::from_head(repo, meta, opts.traversal.clone())?; graph_to_ref_info(&graph.into_workspace()?, repo, opts) @@ -395,7 +433,7 @@ pub fn head_info( pub fn ref_info( mut existing_ref: gix::Reference<'_>, meta: &impl but_core::RefMetadata, - opts: Options, + opts: Options<'_>, ) -> anyhow::Result { let id = existing_ref.peel_to_id()?; let repo = id.repo; @@ -451,7 +489,7 @@ pub(crate) fn find_ancestor_workspace_commit( pub fn graph_to_ref_info( workspace: &but_graph::Workspace, repo: &gix::Repository, - opts: Options, + opts: Options<'_>, ) -> anyhow::Result { if workspace.graph.hard_limit_hit() { tracing::warn!(hard_limit=?opts.traversal.hard_limit, @@ -518,9 +556,90 @@ pub fn graph_to_ref_info( bail!("{msg}"); } info.compute_similarity(graph, repo, opts.expensive_commit_info)?; + if let GerritMode::Enabled(metadata) = opts.gerrit_mode { + info.apply_gerrit_metadata(metadata)?; + } Ok(info) } +impl RefInfo { + /// Enrich standard `RefInfo` output with Gerrit review metadata. + /// + /// The regular construction path has already computed stack shape, commit + /// similarity, integration state, and ordinary push status from refs and + /// graph reachability. Gerrit pushes do not update a branch remote-tracking + /// ref in the same way a normal Git push does, so those graph-only signals + /// are not enough to tell whether a local commit has already been accepted + /// by Gerrit. + /// + /// For each local commit, this pass looks up its GitButler Change-Id in the + /// Gerrit metadata table populated after successful pushes. A hit attaches + /// the review URL and, unless the commit is already integrated, marks the + /// commit as `LocalAndRemote(recorded_patchset_commit_id)`. If the recorded + /// patchset id differs from the local commit id, the commit is treated as a + /// rewritten patchset that would require another Gerrit push. + fn apply_gerrit_metadata( + &mut self, + metadata: but_db::GerritMetadataHandle<'_>, + ) -> anyhow::Result<()> { + for segment in self + .stacks + .iter_mut() + .flat_map(|stack| stack.segments.iter_mut()) + { + for commit in &mut segment.commits { + let Some(meta) = metadata.get(&commit.change_id().to_string())? else { + continue; + }; + commit.inner.gerrit_review_url = Some(meta.review_url); + if !matches!(commit.relation, LocalCommitRelation::Integrated(_)) { + let remote_id = + gix::ObjectId::from_str(&meta.commit_id).with_context(|| { + format!( + "Gerrit metadata for change-id {} has invalid commit id", + meta.change_id + ) + })?; + commit.relation = LocalCommitRelation::LocalAndRemote(remote_id); + } + } + segment.push_status = gerrit_push_status(segment); + } + Ok(()) + } +} + +/// Derive push status for a Gerrit-enriched segment. +/// +/// Standard push status compares local branches with their remote-tracking +/// branches. Gerrit mode instead compares local commits with the patchset +/// commit ids recorded in Gerrit metadata. A matching id means the current +/// local commit is already pushed as-is, a different recorded id means the +/// local commit has been rewritten since the last Gerrit patchset, and a +/// local-only commit still needs to be pushed. +fn gerrit_push_status(segment: &crate::ref_info::Segment) -> PushStatus { + let has_local_only = segment + .commits + .iter() + .any(|commit| matches!(commit.relation, LocalCommitRelation::LocalOnly)); + let has_diverged = segment.commits.iter().any(|commit| { + matches!(commit.relation, LocalCommitRelation::LocalAndRemote(remote_id) if commit.id != remote_id) + }); + let all_pushed = segment.commits.iter().all(|commit| { + matches!(commit.relation, LocalCommitRelation::LocalAndRemote(remote_id) if commit.id == remote_id) + }); + + if has_diverged { + PushStatus::UnpushedCommitsRequiringForce + } else if has_local_only { + PushStatus::UnpushedCommits + } else if all_pushed { + PushStatus::NothingToPush + } else { + segment.push_status + } +} + impl branch::Stack { fn try_from_graph_stack( stack: &but_graph::workspace::Stack, diff --git a/crates/but-workspace/src/ui/mod.rs b/crates/but-workspace/src/ui/mod.rs index 69650bf7ba7..d30c62db9d1 100644 --- a/crates/but-workspace/src/ui/mod.rs +++ b/crates/but-workspace/src/ui/mod.rs @@ -387,6 +387,7 @@ impl From<&crate::ref_info::Commit> for ui::UpstreamCommit { // TODO: Represent this in the UI (maybe) and/or deal with divergence of the local and remote tracking branch. has_conflicts: _, change_id, + gerrit_review_url: _, }: &crate::ref_info::Commit, ) -> Self { ui::UpstreamCommit { @@ -417,6 +418,7 @@ impl From<&LocalCommit> for ui::Commit { flags: _, has_conflicts, change_id, + gerrit_review_url, }, relation, }: &LocalCommit, @@ -437,7 +439,7 @@ impl From<&LocalCommit> for ui::Commit { .unwrap_or_else(|| { but_core::commit::Headers::synthetic_change_id_from_commit_id(*id).to_string() }), - gerrit_review_url: None, + gerrit_review_url: gerrit_review_url.clone(), } } } diff --git a/crates/but-workspace/tests/workspace/ref_info/mod.rs b/crates/but-workspace/tests/workspace/ref_info/mod.rs index 1b708e8e9e5..70c94cbd04f 100644 --- a/crates/but-workspace/tests/workspace/ref_info/mod.rs +++ b/crates/but-workspace/tests/workspace/ref_info/mod.rs @@ -869,10 +869,11 @@ mod utils { Ok((tmp, repo, meta)) } - pub fn standard_options() -> but_workspace::ref_info::Options { + pub fn standard_options() -> but_workspace::ref_info::Options<'static> { ref_info::Options { expensive_commit_info: true, traversal: Default::default(), + ..Default::default() } } } diff --git a/crates/but-workspace/tests/workspace/ref_info/with_workspace_commit/journey/mod.rs b/crates/but-workspace/tests/workspace/ref_info/with_workspace_commit/journey/mod.rs index 1597c00f9f7..f0fe47bbd01 100644 --- a/crates/but-workspace/tests/workspace/ref_info/with_workspace_commit/journey/mod.rs +++ b/crates/but-workspace/tests/workspace/ref_info/with_workspace_commit/journey/mod.rs @@ -7,7 +7,7 @@ mod utils { pub fn standard_options_with_extra_target( repo: &gix::Repository, revspec: &str, - ) -> but_workspace::ref_info::Options { + ) -> but_workspace::ref_info::Options<'static> { but_workspace::ref_info::Options { traversal: but_graph::init::Options { extra_target_commit_id: repo.rev_parse_single(revspec).unwrap().detach().into(), diff --git a/crates/but-workspace/tests/workspace/ref_info/with_workspace_commit/mod.rs b/crates/but-workspace/tests/workspace/ref_info/with_workspace_commit/mod.rs index d4cbae9f583..8d48d558179 100644 --- a/crates/but-workspace/tests/workspace/ref_info/with_workspace_commit/mod.rs +++ b/crates/but-workspace/tests/workspace/ref_info/with_workspace_commit/mod.rs @@ -24,6 +24,101 @@ pub fn ref_info( but_workspace::ref_info(existing_ref, meta, opts) } +#[test] +fn gerrit_mode_uses_metadata_for_commit_review_urls_and_push_status() -> anyhow::Result<()> { + let (repo, mut meta) = read_only_in_memory_scenario("remote-advanced-ff")?; + add_stack(&mut meta, 1, "A", StackState::InWorkspace); + let base_info = head_info(&repo, &meta, standard_options())?; + let local_commit = base_info.stacks[0].segments[0].commits[0].clone(); + let change_id = local_commit.change_id().to_string(); + let review_url = "https://gerrit.example.com/c/project/+/1"; + let mut db = but_db::DbHandle::new_at_path(":memory:")?; + db.gerrit_metadata_mut() + .insert(gerrit_meta(change_id, local_commit.id, review_url))?; + + let info = head_info( + &repo, + &meta, + but_workspace::ref_info::Options { + gerrit_mode: but_workspace::ref_info::GerritMode::Enabled(db.gerrit_metadata()), + ..standard_options() + }, + )?; + let segment = &info.stacks[0].segments[0]; + let commit = &segment.commits[0]; + + assert_eq!( + segment.push_status, + but_workspace::ui::PushStatus::NothingToPush + ); + assert!( + matches!(commit.relation, but_workspace::ref_info::LocalCommitRelation::LocalAndRemote(remote_id) if remote_id == local_commit.id) + ); + assert_eq!(commit.inner.gerrit_review_url.as_deref(), Some(review_url)); + + let ui_info: but_workspace::ui::RefInfo = info.try_into()?; + assert_eq!( + ui_info.stacks[0].segments[0].commits[0] + .gerrit_review_url + .as_deref(), + Some(review_url) + ); + + Ok(()) +} + +#[test] +fn gerrit_mode_treats_recorded_different_patchset_as_force_push() -> anyhow::Result<()> { + let (repo, mut meta) = read_only_in_memory_scenario("remote-advanced-ff")?; + add_stack(&mut meta, 1, "A", StackState::InWorkspace); + let base_info = head_info(&repo, &meta, standard_options())?; + let local_commit = base_info.stacks[0].segments[0].commits[0].clone(); + let remote_commit_id = repo.find_reference("origin/A")?.peel_to_id()?.detach(); + let mut db = but_db::DbHandle::new_at_path(":memory:")?; + db.gerrit_metadata_mut().insert(gerrit_meta( + local_commit.change_id().to_string(), + remote_commit_id, + "https://gerrit.example.com/c/project/+/2", + ))?; + + let info = head_info( + &repo, + &meta, + but_workspace::ref_info::Options { + gerrit_mode: but_workspace::ref_info::GerritMode::Enabled(db.gerrit_metadata()), + ..standard_options() + }, + )?; + let segment = &info.stacks[0].segments[0]; + + assert_eq!( + segment.push_status, + but_workspace::ui::PushStatus::UnpushedCommitsRequiringForce + ); + assert!( + matches!(segment.commits[0].relation, but_workspace::ref_info::LocalCommitRelation::LocalAndRemote(remote_id) if remote_id == remote_commit_id) + ); + + Ok(()) +} + +fn gerrit_meta( + change_id: String, + commit_id: gix::ObjectId, + review_url: &str, +) -> but_db::GerritMeta { + let timestamp = chrono::DateTime::from_timestamp(1_000_000, 0) + .expect("fixed timestamp is valid") + .naive_utc(); + but_db::GerritMeta { + change_id, + commit_id: commit_id.to_string(), + review_url: review_url.to_string(), + created_at: timestamp, + updated_at: timestamp, + } +} + #[test] fn remote_ahead_fast_forwardable() -> anyhow::Result<()> { let (mut repo, mut meta) = read_only_in_memory_scenario("remote-advanced-ff")?; @@ -38,8 +133,7 @@ fn remote_ahead_fast_forwardable() -> anyhow::Result<()> { // Needs a branch for workspace implied by a branch with metadata. add_stack(&mut meta, 1, "A", StackState::InWorkspace); // We can look at a workspace ref directly (via HEAD) - let opts = standard_options(); - let info = head_info(&repo, &meta, opts.clone())?; + let info = head_info(&repo, &meta, standard_options())?; insta::assert_debug_snapshot!(info, @r#" RefInfo { workspace_ref_info: Some( @@ -114,7 +208,7 @@ fn remote_ahead_fast_forwardable() -> anyhow::Result<()> { "#); let at = repo.find_reference("refs/heads/A")?; - let info = ref_info(at, &meta, opts.clone())?; + let info = ref_info(at, &meta, standard_options())?; // Information doesn't change just because the starting point is different. insta::assert_debug_snapshot!(info, @r#" RefInfo { @@ -195,7 +289,7 @@ fn remote_ahead_fast_forwardable() -> anyhow::Result<()> { .remove_section("branch", info.stacks[0].name().unwrap().shorten().as_bstr()); let at = repo.find_reference("refs/heads/A")?; - let info = ref_info(at, &meta, opts)?; + let info = ref_info(at, &meta, standard_options())?; insta::assert_debug_snapshot!(info, @r#" RefInfo { workspace_ref_info: Some( @@ -287,8 +381,7 @@ fn two_dependent_branches_rebased_with_remotes() -> anyhow::Result<()> { add_stack_with_segments(&mut meta, 0, "B-on-A", StackState::InWorkspace, &["A"]); - let opts = standard_options(); - let info = head_info(&repo, &meta, opts)?; + let info = head_info(&repo, &meta, standard_options())?; insta::assert_debug_snapshot!(info, @r#" RefInfo { workspace_ref_info: Some( @@ -393,8 +486,7 @@ fn stacked_bottom_remote_still_points_at_now_split_top() -> anyhow::Result<()> { add_stack_with_segments(&mut meta, 0, "top", StackState::InWorkspace, &["bottom"]); - let opts = standard_options(); - let info = head_info(&repo, &meta, opts)?; + let info = head_info(&repo, &meta, standard_options())?; let bottom = info .stacks .first() @@ -463,8 +555,7 @@ fn two_dependent_branches_rebased_explicit_remote_in_extra_segment() -> anyhow:: // and it comes with an official remote configuration. add_stack_with_segments(&mut meta, 0, "B-on-A", StackState::InWorkspace, &["A"]); - let opts = standard_options(); - let info = head_info(&repo, &meta, opts)?; + let info = head_info(&repo, &meta, standard_options())?; insta::assert_debug_snapshot!(info, @r#" RefInfo { workspace_ref_info: Some( @@ -582,8 +673,7 @@ fn two_dependent_branches_first_merged_no_ff() -> anyhow::Result<()> { add_stack_with_segments(&mut meta, 0, "B-on-A", StackState::InWorkspace, &["A"]); - let opts = standard_options(); - let info = head_info(&repo, &meta, opts)?; + let info = head_info(&repo, &meta, standard_options())?; insta::assert_debug_snapshot!(info, @r#" RefInfo { workspace_ref_info: Some( @@ -696,9 +786,8 @@ fn two_dependent_branches_first_merged_no_ff_second_merged_on_remote_into_base_b add_stack_with_segments(&mut meta, 0, "B-on-A", StackState::InWorkspace, &["A"]); - let opts = standard_options(); // With the standard targets, A is considered integrated. - let info = head_info(&repo, &meta, opts.clone())?; + let info = head_info(&repo, &meta, standard_options())?; insta::assert_debug_snapshot!(info, @r#" RefInfo { workspace_ref_info: Some( @@ -776,7 +865,7 @@ fn two_dependent_branches_first_merged_no_ff_second_merged_on_remote_into_base_b .as_mut() .expect("target setup") .sha = repo.rev_parse_single("fafd9d0")?.detach(); - let info = head_info(&repo, &meta, opts)?; + let info = head_info(&repo, &meta, standard_options())?; insta::assert_debug_snapshot!(info, @r#" RefInfo { workspace_ref_info: Some( @@ -880,8 +969,7 @@ fn two_dependent_branches_first_rebased_and_merged_into_target() -> anyhow::Resu add_workspace(&mut meta); - let opts = standard_options(); - let info = head_info(&repo, &meta, opts.clone())?; + let info = head_info(&repo, &meta, standard_options())?; insta::assert_debug_snapshot!(info, @r#" RefInfo { workspace_ref_info: Some( @@ -966,7 +1054,7 @@ fn two_dependent_branches_first_rebased_and_merged_into_target() -> anyhow::Resu repo.config_snapshot_mut() .remove_section("remote", Some("origin".into())); - let info = head_info(&repo, &meta, opts)?; + let info = head_info(&repo, &meta, standard_options())?; // Without remote setup, remotes can't be deducted. However, we still have a commits reachable from the target remote tracking // branch up to the workspace base, which we should consider. insta::assert_debug_snapshot!(info, @r#" @@ -1070,8 +1158,7 @@ fn target_ahead_remote_rewritten() -> anyhow::Result<()> { "); add_stack(&mut meta, 1, "A", StackState::InWorkspace); - let opts = standard_options(); - let info = ref_info(repo.find_reference("A")?, &meta, opts)?; + let info = ref_info(repo.find_reference("A")?, &meta, standard_options())?; insta::assert_debug_snapshot!(info, @r#" RefInfo { workspace_ref_info: Some( @@ -1169,9 +1256,7 @@ fn single_commit_but_two_branches_one_in_ws_commit() -> anyhow::Result<()> { { add_stack(&mut meta, idx as u128, name, StackState::InWorkspace); } - - let opts = standard_options(); - let info = head_info(&repo, &meta, opts)?; + let info = head_info(&repo, &meta, standard_options())?; insta::assert_debug_snapshot!(info, @r#" RefInfo { workspace_ref_info: Some( @@ -1316,8 +1401,7 @@ fn single_commit_but_two_branches_one_in_ws_commit_with_virtual_segments() -> an // The stacks should come out just like defined above, "lane" and then "lane2" with all the right segments. // The lane-segment01|02 bits are brought up as dependent branch as well. - let opts = standard_options(); - let info = ref_info(repo.find_reference("lane")?, &meta, opts)?; + let info = ref_info(repo.find_reference("lane")?, &meta, standard_options())?; insta::assert_debug_snapshot!(info, @r#" RefInfo { workspace_ref_info: Some( @@ -1470,9 +1554,7 @@ fn single_commit_but_two_branches_one_in_ws_commit_with_virtual_segments() -> an StackState::InWorkspace, &["lane-segment-01", "lane-segment-02"], ); - - let opts = standard_options(); - let info = ref_info(repo.find_reference("lane")?, &meta, opts)?; + let info = ref_info(repo.find_reference("lane")?, &meta, standard_options())?; insta::assert_debug_snapshot!(info, @r#" RefInfo { workspace_ref_info: Some( @@ -1626,9 +1708,7 @@ fn single_commit_but_two_branches_both_in_ws_commit() -> anyhow::Result<()> { for (idx, name) in ["advanced-lane", "lane"].into_iter().enumerate() { add_stack(&mut meta, idx as u128, name, StackState::InWorkspace); } - - let opts = standard_options(); - let info = head_info(&repo, &meta, opts)?; + let info = head_info(&repo, &meta, standard_options())?; insta::assert_debug_snapshot!(info, @r#" RefInfo { workspace_ref_info: Some( @@ -1738,8 +1818,7 @@ fn single_commit_pushed_but_two_branches_both_in_ws_commit() -> anyhow::Result<( // For complexity, we also don't set up any branch metadata, only 'something' to get the target ref. add_workspace(&mut meta); - let opts = standard_options(); - let info = head_info(&repo, &meta, opts)?; + let info = head_info(&repo, &meta, standard_options())?; insta::assert_debug_snapshot!(info, @r#" RefInfo { workspace_ref_info: Some( @@ -1832,8 +1911,7 @@ fn single_commit_pushed_but_two_branches_both_in_ws_commit_empty_dependent() -> &["advanced-lane"], ); - let opts = standard_options(); - let info = head_info(&repo, &meta, opts.clone())?; + let info = head_info(&repo, &meta, standard_options())?; insta::assert_debug_snapshot!(info, @r#" RefInfo { workspace_ref_info: Some( @@ -1927,7 +2005,7 @@ fn single_commit_pushed_but_two_branches_both_in_ws_commit_empty_dependent() -> // Even though we *could* special-case this to keep the commit in the branch that has a remote, // we just keep it below at all times. The frontend currently only creates them on top, for good reason. - let info = head_info(&repo, &meta, opts)?; + let info = head_info(&repo, &meta, standard_options())?; insta::assert_debug_snapshot!(info, @r#" RefInfo { workspace_ref_info: Some( @@ -2031,8 +2109,7 @@ fn single_commit_pushed_ws_commit_empty_dependent() -> anyhow::Result<()> { &["dependent", "advanced-lane"], ); - let opts = standard_options(); - let info = head_info(&repo, &meta, opts.clone())?; + let info = head_info(&repo, &meta, standard_options())?; insta::assert_debug_snapshot!(info, @r#" RefInfo { workspace_ref_info: Some( @@ -2135,7 +2212,7 @@ fn single_commit_pushed_ws_commit_empty_dependent() -> anyhow::Result<()> { &["on-top-of-dependent", "advanced-lane"], ); - let info = head_info(&repo, &meta, opts.clone())?; + let info = head_info(&repo, &meta, standard_options())?; insta::assert_debug_snapshot!(info, @r#" RefInfo { workspace_ref_info: Some( @@ -2249,9 +2326,7 @@ fn two_branches_stacked_with_remotes() -> anyhow::Result<()> { StackState::InWorkspace, &["lane"], ); - - let opts = standard_options(); - let info = head_info(&repo, &meta, opts)?; + let info = head_info(&repo, &meta, standard_options())?; insta::assert_debug_snapshot!(info, @r#" RefInfo { workspace_ref_info: Some( @@ -2355,9 +2430,7 @@ fn two_branches_stacked_with_interesting_remote_setup() -> anyhow::Result<()> { // Just a single explicit reference we want to know of. add_stack(&mut meta, 1, "A", StackState::InWorkspace); - - let opts = standard_options(); - let info = ref_info(repo.find_reference("A")?, &meta, opts).unwrap(); + let info = ref_info(repo.find_reference("A")?, &meta, standard_options()).unwrap(); insta::assert_debug_snapshot!(info, @r#" RefInfo { @@ -2459,9 +2532,7 @@ fn single_commit_but_two_branches_stack_on_top_of_ws_commit() -> anyhow::Result< for (idx, name) in ["advanced-lane", "lane"].into_iter().enumerate() { add_stack(&mut meta, idx as u128, name, StackState::InWorkspace); } - - let opts = standard_options(); - let info = head_info(&repo, &meta, opts.clone())?; + let info = head_info(&repo, &meta, standard_options())?; // It's fine to have no managed commit, but we have to deal with it - see flag is_managed. insta::assert_debug_snapshot!(info, @r#" RefInfo { @@ -2555,7 +2626,12 @@ fn single_commit_but_two_branches_stack_on_top_of_ws_commit() -> anyhow::Result< } "#); - let info = ref_info(repo.find_reference("advanced-lane")?, &meta, opts).unwrap(); + let info = ref_info( + repo.find_reference("advanced-lane")?, + &meta, + standard_options(), + ) + .unwrap(); insta::assert_debug_snapshot!(info, @r#" RefInfo { workspace_ref_info: Some( @@ -2669,9 +2745,7 @@ fn two_branches_one_advanced_two_parent_ws_commit_diverged_remote_tracking_branc for (idx, name) in ["lane", "advanced-lane"].into_iter().enumerate() { add_stack(&mut meta, idx as u128, name, StackState::InWorkspace); } - - let opts = standard_options(); - let info = head_info(&repo, &meta, opts.clone())?; + let info = head_info(&repo, &meta, standard_options())?; insta::assert_debug_snapshot!(info, @r#" RefInfo { workspace_ref_info: Some( @@ -2763,7 +2837,11 @@ fn two_branches_one_advanced_two_parent_ws_commit_diverged_remote_tracking_branc "#); // Everything is show so the workspace stays clear, the entrypoint says what to focus on. - let info = ref_info(repo.find_reference("advanced-lane")?, &meta, opts.clone())?; + let info = ref_info( + repo.find_reference("advanced-lane")?, + &meta, + standard_options(), + )?; insta::assert_debug_snapshot!(info, @r#" RefInfo { workspace_ref_info: Some( @@ -2854,7 +2932,7 @@ fn two_branches_one_advanced_two_parent_ws_commit_diverged_remote_tracking_branc } "#); - let info = ref_info(repo.find_reference("lane")?, &meta, opts.clone())?; + let info = ref_info(repo.find_reference("lane")?, &meta, standard_options())?; insta::assert_debug_snapshot!(info, @r#" RefInfo { workspace_ref_info: Some( @@ -2951,7 +3029,7 @@ fn two_branches_one_advanced_two_parent_ws_commit_diverged_remote_tracking_branc add_stack(&mut meta, idx as u128, name, StackState::InWorkspace); } - let info = head_info(&repo, &meta, opts)?; + let info = head_info(&repo, &meta, standard_options())?; insta::assert_debug_snapshot!(info, @r#" RefInfo { workspace_ref_info: Some( @@ -3053,9 +3131,7 @@ fn disjoint() -> anyhow::Result<()> { "); add_stack(&mut meta, 1, "disjoint", StackState::InWorkspace); - - let opts = standard_options(); - let info = head_info(&repo, &meta, opts)?; + let info = head_info(&repo, &meta, standard_options())?; // We see the commit in the branch as there is no base to hide it. insta::assert_debug_snapshot!(info, @r#" @@ -3135,9 +3211,7 @@ fn multiple_branches_with_shared_segment() -> anyhow::Result<()> { "); add_stack(&mut meta, 1, "C-on-A", StackState::InWorkspace); - - let opts = standard_options(); - let info = head_info(&repo, &meta, opts.clone())?; + let info = head_info(&repo, &meta, standard_options())?; // The shared "A" segment is used in both stacks, as it's reachable from both. // Stack A isn't listed, so it has no stack id. @@ -3263,7 +3337,7 @@ fn multiple_branches_with_shared_segment() -> anyhow::Result<()> { } "#); - let info = ref_info(repo.find_reference("C-on-A")?, &meta, opts.clone())?; + let info = ref_info(repo.find_reference("C-on-A")?, &meta, standard_options())?; // A partial workspace is provided, but the entire workspace is known. insta::assert_debug_snapshot!(info, @r#" @@ -3388,7 +3462,7 @@ fn multiple_branches_with_shared_segment() -> anyhow::Result<()> { } "#); - let b_info = ref_info(repo.find_reference("B-on-A")?, &meta, opts.clone())?; + let b_info = ref_info(repo.find_reference("B-on-A")?, &meta, standard_options())?; // It's like the stack is part of the workspace, the result is the same, with entrypoints changed. insta::assert_debug_snapshot!(b_info, @r#" @@ -3513,7 +3587,7 @@ fn multiple_branches_with_shared_segment() -> anyhow::Result<()> { } "#); - let a_info = ref_info(repo.find_reference("A")?, &meta, opts)?; + let a_info = ref_info(repo.find_reference("A")?, &meta, standard_options())?; // We can also show segments that are part of the stack (like homing in on them), as long as they are in a workspace. // It's notable how there are two entrypoints, so the UI has to assure both are visible. @@ -3650,9 +3724,7 @@ fn empty_workspace_with_branch_below() -> anyhow::Result<()> { "); add_stack(&mut meta, 1, "unrelated", StackState::InWorkspace); - - let opts = standard_options(); - let info = head_info(&repo, &meta, opts.clone())?; + let info = head_info(&repo, &meta, standard_options())?; // Active branches we should see, but only "unrelated", // not any other branch that happens to point at that commit. insta::assert_debug_snapshot!(info, @r#" @@ -3724,7 +3796,7 @@ fn empty_workspace_with_branch_below() -> anyhow::Result<()> { } "#); - let info = ref_info(repo.find_reference("unrelated")?, &meta, opts.clone())?; + let info = ref_info(repo.find_reference("unrelated")?, &meta, standard_options())?; // It can be checked out with the same effect, the parent workspace is still known. insta::assert_debug_snapshot!(info, @r#" RefInfo { @@ -3798,7 +3870,7 @@ fn empty_workspace_with_branch_below() -> anyhow::Result<()> { // Change the stack to be inactive, so it's not considered to be part of the workspace. add_stack(&mut meta, 1, "unrelated", StackState::Inactive); - let info = head_info(&repo, &meta, opts.clone())?; + let info = head_info(&repo, &meta, standard_options())?; // Now there should be no stack, it's an empty workspace. insta::assert_debug_snapshot!(info, @r#" RefInfo { @@ -3849,7 +3921,7 @@ fn empty_workspace_with_branch_below() -> anyhow::Result<()> { // The unrelated reference would be its own pseudo-workspace, single-branch mode effectively. // It's on the base and clearly outside the workspace. - let info = ref_info(repo.find_reference("unrelated")?, &meta, opts)?; + let info = ref_info(repo.find_reference("unrelated")?, &meta, standard_options())?; insta::assert_debug_snapshot!(info, @r#" RefInfo { workspace_ref_info: Some( @@ -3923,9 +3995,7 @@ fn advanced_workspace_multi_stack() -> anyhow::Result<()> { add_stack_with_segments(&mut meta, 0, "A", StackState::InWorkspace, &[]); add_stack_with_segments(&mut meta, 1, "B", StackState::InWorkspace, &[]); - - let opts = standard_options(); - let err = head_info(&repo, &meta, opts).unwrap_err(); + let err = head_info(&repo, &meta, standard_options()).unwrap_err(); insta::assert_snapshot!(err.to_string(), @" Found 5 commit(s) on top of the workspace commit. @@ -3955,9 +4025,7 @@ fn advanced_workspace_single_stack() -> anyhow::Result<()> { "); add_stack_with_segments(&mut meta, 0, "A", StackState::InWorkspace, &[]); - - let opts = standard_options(); - let err = head_info(&repo, &meta, opts).unwrap_err(); + let err = head_info(&repo, &meta, standard_options()).unwrap_err(); insta::assert_snapshot!(err.to_string(), @" Found 4 commit(s) on top of the workspace commit. diff --git a/crates/gitbutler-branch-actions/src/branch.rs b/crates/gitbutler-branch-actions/src/branch.rs index a40b006036c..d733074208f 100644 --- a/crates/gitbutler-branch-actions/src/branch.rs +++ b/crates/gitbutler-branch-actions/src/branch.rs @@ -74,6 +74,7 @@ pub fn list_branches( but_workspace::ref_info::Options { traversal: but_graph::init::Options::limited(), expensive_commit_info: false, + ..Default::default() }, )?; info.stacks diff --git a/crates/gitbutler-branch-actions/src/upstream_integration.rs b/crates/gitbutler-branch-actions/src/upstream_integration.rs index d307753d620..83a2bcdbc0a 100644 --- a/crates/gitbutler-branch-actions/src/upstream_integration.rs +++ b/crates/gitbutler-branch-actions/src/upstream_integration.rs @@ -232,6 +232,7 @@ impl<'a> UpstreamIntegrationContext<'a> { Options { expensive_commit_info: true, traversal: but_graph::init::Options::limited(), + ..Default::default() }, )?; }