From 052b82e9a51670d87f1f624f16e88d86ca37979c Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 19 Aug 2025 10:51:37 +0200 Subject: [PATCH 1/4] Avoid `branch::create_reference` types to be available from multiple locations. --- crates/but-api/src/commands/stack.rs | 40 +++---- crates/but-api/tests/api/commands/stack.rs | 6 +- crates/but-testing/src/command/mod.rs | 31 +++-- .../src/branch/create_reference.rs | 54 +++++---- crates/but-workspace/src/branch/mod.rs | 4 +- .../workspace/branch/create_reference.rs | 107 +++++++++--------- 6 files changed, 115 insertions(+), 127 deletions(-) diff --git a/crates/but-api/src/commands/stack.rs b/crates/but-api/src/commands/stack.rs index dabc1c6e61..fd63a78b21 100644 --- a/crates/but-api/src/commands/stack.rs +++ b/crates/but-api/src/commands/stack.rs @@ -1,7 +1,6 @@ use crate::commands::stack::create_reference::Anchor; use crate::{App, error::Error}; use anyhow::{Context, anyhow}; -use but_settings::AppSettings; use but_workspace::branch::{ReferenceAnchor, ReferencePosition}; use gitbutler_branch_actions::internal::PushResult; use gitbutler_branch_actions::stack::CreateSeriesRequest; @@ -57,9 +56,9 @@ pub mod create_reference { } } -pub fn create_reference(_app: &App, params: create_reference::Params) -> Result<(), Error> { +pub fn create_reference(app: &App, params: create_reference::Params) -> Result<(), Error> { let project = gitbutler_project::get(params.project_id)?; - let ctx = CommandContext::open(&project, AppSettings::load_from_default_path_creating()?)?; + let ctx = CommandContext::open(&project, app.app_settings.get()?.clone())?; let create_reference::Request { new_name, anchor } = params.request; let new_ref = Category::LocalBranch .to_full_name(new_name.as_str()) @@ -101,10 +100,10 @@ pub fn create_reference(_app: &App, params: create_reference::Params) -> Result< Ok(()) } -pub fn create_branch(_app: &App, params: CreateBranchParams) -> Result<(), Error> { +pub fn create_branch(app: &App, params: CreateBranchParams) -> Result<(), Error> { let project = gitbutler_project::get(params.project_id)?; - let ctx = CommandContext::open(&project, AppSettings::load_from_default_path_creating()?)?; - if ctx.app_settings().feature_flags.ws3 { + let ctx = CommandContext::open(&project, app.app_settings.get()?.clone())?; + if app.app_settings.get()?.feature_flags.ws3 { use ReferencePosition::Above; let mut guard = project.exclusive_worktree_access(); let (repo, mut meta, graph) = ctx.graph_and_meta_mut_and_repo(guard.write_permission())?; @@ -165,11 +164,11 @@ pub struct RemoveBranchParams { pub branch_name: String, } -pub fn remove_branch(_app: &App, params: RemoveBranchParams) -> Result<(), Error> { +pub fn remove_branch(app: &App, params: RemoveBranchParams) -> Result<(), Error> { let project = gitbutler_project::get(params.project_id)?; - let ctx = CommandContext::open(&project, AppSettings::load_from_default_path_creating()?)?; + let ctx = CommandContext::open(&project, app.app_settings.get()?.clone())?; let mut guard = project.exclusive_worktree_access(); - if ctx.app_settings().feature_flags.ws3 { + if app.app_settings.get()?.feature_flags.ws3 { let (repo, mut meta, graph) = ctx.graph_and_meta_mut_and_repo(guard.write_permission())?; let ws = graph.to_workspace()?; let ref_name = Category::LocalBranch @@ -205,9 +204,9 @@ pub struct UpdateBranchNameParams { pub new_name: String, } -pub fn update_branch_name(_app: &App, params: UpdateBranchNameParams) -> Result<(), Error> { +pub fn update_branch_name(app: &App, params: UpdateBranchNameParams) -> Result<(), Error> { let project = gitbutler_project::get(params.project_id)?; - let ctx = CommandContext::open(&project, AppSettings::load_from_default_path_creating()?)?; + let ctx = CommandContext::open(&project, app.app_settings.get()?.clone())?; gitbutler_branch_actions::stack::update_branch_name( &ctx, params.stack_id, @@ -227,11 +226,11 @@ pub struct UpdateBranchDescriptionParams { } pub fn update_branch_description( - _app: &App, + app: &App, params: UpdateBranchDescriptionParams, ) -> Result<(), Error> { let project = gitbutler_project::get(params.project_id)?; - let ctx = CommandContext::open(&project, AppSettings::load_from_default_path_creating()?)?; + let ctx = CommandContext::open(&project, app.app_settings.get()?.clone())?; gitbutler_branch_actions::stack::update_branch_description( &ctx, params.stack_id, @@ -250,12 +249,9 @@ pub struct UpdateBranchPrNumberParams { pub pr_number: Option, } -pub fn update_branch_pr_number( - _app: &App, - params: UpdateBranchPrNumberParams, -) -> Result<(), Error> { +pub fn update_branch_pr_number(app: &App, params: UpdateBranchPrNumberParams) -> Result<(), Error> { let project = gitbutler_project::get(params.project_id)?; - let ctx = CommandContext::open(&project, AppSettings::load_from_default_path_creating()?)?; + let ctx = CommandContext::open(&project, app.app_settings.get()?.clone())?; gitbutler_branch_actions::stack::update_branch_pr_number( &ctx, params.stack_id, @@ -275,9 +271,9 @@ pub struct PushStackParams { pub branch: String, } -pub fn push_stack(_app: &App, params: PushStackParams) -> Result { +pub fn push_stack(app: &App, params: PushStackParams) -> Result { let project = gitbutler_project::get(params.project_id)?; - let ctx = CommandContext::open(&project, AppSettings::load_from_default_path_creating()?)?; + let ctx = CommandContext::open(&project, app.app_settings.get()?.clone())?; gitbutler_branch_actions::stack::push_stack( &ctx, params.stack_id, @@ -297,9 +293,9 @@ pub struct PushStackToReviewParams { pub user: User, } -pub fn push_stack_to_review(_app: &App, params: PushStackToReviewParams) -> Result { +pub fn push_stack_to_review(app: &App, params: PushStackToReviewParams) -> Result { let project = gitbutler_project::get(params.project_id)?; - let ctx = CommandContext::open(&project, AppSettings::load_from_default_path_creating()?)?; + let ctx = CommandContext::open(&project, app.app_settings.get()?.clone())?; let review_id = gitbutler_sync::stack_upload::push_stack_to_review( &ctx, ¶ms.user, diff --git a/crates/but-api/tests/api/commands/stack.rs b/crates/but-api/tests/api/commands/stack.rs index 19bb077d17..ef20d45637 100644 --- a/crates/but-api/tests/api/commands/stack.rs +++ b/crates/but-api/tests/api/commands/stack.rs @@ -2,7 +2,7 @@ mod create_reference { use but_api::commands::stack::create_reference; use but_api::commands::stack::create_reference::{Params, Request}; use but_api::hex_hash::HexHash; - use but_workspace::branch::ReferencePosition; + use but_workspace::branch::create_reference::Position; use gitbutler_project::ProjectId; use std::str::FromStr; @@ -34,7 +34,7 @@ mod create_reference { gix::ObjectId::from_str("5c69907b1244089142905dba380371728e2e8160") .unwrap(), ), - position: ReferencePosition::Above, + position: Position::Above, }), }, }; @@ -60,7 +60,7 @@ mod create_reference { new_name: "new-branch".to_string(), anchor: Some(create_reference::Anchor::AtReference { short_name: "anchor-ref".into(), - position: ReferencePosition::Above, + position: Position::Above, }), }, }; diff --git a/crates/but-testing/src/command/mod.rs b/crates/but-testing/src/command/mod.rs index 3792347bab..e92246bd0c 100644 --- a/crates/but-testing/src/command/mod.rs +++ b/crates/but-testing/src/command/mod.rs @@ -3,7 +3,7 @@ use but_core::UnifiedDiff; use but_db::poll::ItemKind; use but_graph::VirtualBranchesTomlMetadata; use but_settings::AppSettings; -use but_workspace::branch::{ReferenceAnchor, ReferencePosition}; +use but_workspace::branch::create_reference::{Anchor, Position}; use but_workspace::{DiffSpec, HunkHeader}; use gitbutler_project::{Project, ProjectId}; use gix::bstr::{BString, ByteSlice}; @@ -601,22 +601,21 @@ pub fn create_reference( ) -> anyhow::Result<()> { let (repo, project, graph, mut meta) = repo_and_maybe_project_and_graph(args, RepositoryOpenMode::General)?; - let resolve = - |spec: &str, position: ReferencePosition| -> anyhow::Result> { - Ok(match repo.try_find_reference(spec)? { - None => ReferenceAnchor::AtCommit { - commit_id: repo.rev_parse_single(spec)?.detach(), - position, - }, - Some(rn) => ReferenceAnchor::AtSegment { - ref_name: Cow::Owned(rn.inner.name), - position, - }, - }) - }; + let resolve = |spec: &str, position: Position| -> anyhow::Result> { + Ok(match repo.try_find_reference(spec)? { + None => Anchor::AtCommit { + commit_id: repo.rev_parse_single(spec)?.detach(), + position, + }, + Some(rn) => Anchor::AtSegment { + ref_name: Cow::Owned(rn.inner.name), + position, + }, + }) + }; let anchor = above - .map(|spec| resolve(spec, ReferencePosition::Above)) - .or_else(|| below.map(|spec| resolve(spec, ReferencePosition::Below))) + .map(|spec| resolve(spec, Position::Above)) + .or_else(|| below.map(|spec| resolve(spec, Position::Below))) .transpose()?; let new_ref = Category::LocalBranch.to_full_name(short_name)?; diff --git a/crates/but-workspace/src/branch/create_reference.rs b/crates/but-workspace/src/branch/create_reference.rs index d09aaa6661..c46a568e66 100644 --- a/crates/but-workspace/src/branch/create_reference.rs +++ b/crates/but-workspace/src/branch/create_reference.rs @@ -1,9 +1,9 @@ use anyhow::Context; use std::borrow::Cow; -/// For use in [`ReferenceAnchor`]. +/// For use in [`Anchor`]. #[derive(Debug, Clone, Copy, serde::Serialize, serde::Deserialize)] -pub enum ReferencePosition { +pub enum Position { /// The new dependent branch will appear above its anchor. Above, /// The new dependent branch will appear below its anchor. @@ -33,7 +33,7 @@ impl<'a> From<&'a but_graph::projection::StackCommit> for MinimalCommit<'a> { } } -impl ReferencePosition { +impl Position { fn resolve_commit( &self, commit: MinimalCommit<'_>, @@ -43,15 +43,13 @@ impl ReferencePosition { return Ok(commit.id); } Ok(match self { - ReferencePosition::Above => commit.id, - ReferencePosition::Below => { - commit.parent_ids.iter().cloned().next().with_context(|| { - format!( - "Commit {id} is the first in history and no branch can point below it", - id = commit.id - ) - })? - } + Position::Above => commit.id, + Position::Below => commit.parent_ids.iter().cloned().next().with_context(|| { + format!( + "Commit {id} is the first in history and no branch can point below it", + id = commit.id + ) + })?, }) } } @@ -63,7 +61,7 @@ impl ReferencePosition { /// go just by commit. We must be specifying it in terms of above/below ref-name when possible, /// or else they will always go on top. #[derive(Debug, Clone)] -pub enum ReferenceAnchor<'a> { +pub enum Anchor<'a> { /// Use a commit as position, which means we always need unambiguous placement /// without a way to stack references on top of other references - only on top /// of commits their segments may own. @@ -72,7 +70,7 @@ pub enum ReferenceAnchor<'a> { commit_id: gix::ObjectId, /// `Above` means the reference will point at `commit_id`, `Below` means it points at its /// parent if possible. - position: ReferencePosition, + position: Position, }, /// Use a segment as reference for positioning the new reference. /// Without a workspace, this is the same as saying 'the commit that the segment points to'. @@ -83,22 +81,22 @@ pub enum ReferenceAnchor<'a> { /// if it points to the same commit. /// `Below` means the reference will be right below the segment with `ref_name` even /// if it points to the same commit. - position: ReferencePosition, + position: Position, }, } -impl<'a> ReferenceAnchor<'a> { +impl<'a> Anchor<'a> { /// Create a new instance with an object ID as anchor. - pub fn at_id(commit_id: impl Into, position: ReferencePosition) -> Self { - ReferenceAnchor::AtCommit { + pub fn at_id(commit_id: impl Into, position: Position) -> Self { + Anchor::AtCommit { commit_id: commit_id.into(), position, } } /// Create a new instance with a segment name as anchor. - pub fn at_segment(ref_name: &'a gix::refs::FullNameRef, position: ReferencePosition) -> Self { - ReferenceAnchor::AtSegment { + pub fn at_segment(ref_name: &'a gix::refs::FullNameRef, position: Position) -> Self { + Anchor::AtSegment { ref_name: Cow::Borrowed(ref_name), position, } @@ -108,7 +106,7 @@ impl<'a> ReferenceAnchor<'a> { pub(super) mod function { #![expect(clippy::indexing_slicing)] - use crate::branch::{ReferenceAnchor, ReferencePosition}; + use crate::branch::create_reference::{Anchor, Position}; use anyhow::{Context, bail}; use but_core::ref_metadata::{StackId, WorkspaceStack, WorkspaceStackBranch}; use but_core::{RefMetadata, ref_metadata}; @@ -130,7 +128,7 @@ pub(super) mod function { /// Return a regenerated Graph that contains the new reference, and from which a new workspace can be derived. pub fn create_reference<'name, T: RefMetadata>( ref_name: impl Borrow, - anchor: impl Into>>, + anchor: impl Into>>, repo: &gix::Repository, workspace: &but_graph::projection::Workspace<'_>, meta: &mut T, @@ -172,7 +170,7 @@ pub(super) mod function { Some(Instruction::Independent), ) } - Some(ReferenceAnchor::AtCommit { + Some(Anchor::AtCommit { commit_id, position, }) => { @@ -200,7 +198,7 @@ pub(super) mod function { (validate_id, ref_target_id, instruction) } - Some(ReferenceAnchor::AtSegment { ref_name, position }) => { + Some(Anchor::AtSegment { ref_name, position }) => { let mut validate_id = true; let ref_target_id = if workspace.has_metadata() { let (stack_idx, seg_idx) = workspace @@ -396,8 +394,8 @@ pub(super) mod function { let branches = &mut ws_meta.stacks[stack_idx].branches; branches.insert( match position { - ReferencePosition::Above => branch_idx, - ReferencePosition::Below => branch_idx + 1, + Position::Above => branch_idx, + Position::Below => branch_idx + 1, }, WorkspaceStackBranch { ref_name: new_ref.to_owned(), @@ -417,7 +415,7 @@ pub(super) mod function { ws: &but_graph::projection::Workspace<'_>, anchor_id: gix::ObjectId, ) -> anyhow::Result> { - use ReferencePosition::*; + use Position::*; let (anchor_stack_idx, anchor_seg_idx, _anchor_commit_idx) = ws .find_owner_indexes_by_commit_id(anchor_id) .with_context(|| { @@ -468,7 +466,7 @@ pub(super) mod function { DependentInStack(StackId), Dependent { ref_name: Cow<'a, gix::refs::FullNameRef>, - position: ReferencePosition, + position: Position, }, } } diff --git a/crates/but-workspace/src/branch/mod.rs b/crates/but-workspace/src/branch/mod.rs index ce991acaa9..996234d824 100644 --- a/crates/but-workspace/src/branch/mod.rs +++ b/crates/but-workspace/src/branch/mod.rs @@ -460,6 +460,6 @@ impl Stack { pub mod remove_reference; pub use remove_reference::function::remove_reference; -mod create_reference; +/// related types for creating a workspace reference. +pub mod create_reference; pub use create_reference::function::create_reference; -pub use create_reference::*; diff --git a/crates/but-workspace/tests/workspace/branch/create_reference.rs b/crates/but-workspace/tests/workspace/branch/create_reference.rs index 00ee3eb3d1..fd48ef5123 100644 --- a/crates/but-workspace/tests/workspace/branch/create_reference.rs +++ b/crates/but-workspace/tests/workspace/branch/create_reference.rs @@ -2,12 +2,11 @@ use crate::ref_info::with_workspace_commit::utils::{ named_read_only_in_memory_scenario, named_writable_scenario, }; use crate::utils::{r, rc}; -use ReferencePosition::*; use but_core::RefMetadata; use but_core::ref_metadata::ValueInfo; use but_graph::init::Options; use but_testsupport::{graph_workspace, id_at, id_by_rev, visualize_commit_graph_all}; -use but_workspace::branch::{ReferenceAnchor, ReferencePosition}; +use but_workspace::branch::create_reference::{Anchor, Position::*}; mod with_workspace { use crate::ref_info::with_workspace_commit::utils::{ @@ -20,8 +19,7 @@ mod with_workspace { use but_graph::VirtualBranchesTomlMetadata; use but_graph::init::Options; use but_testsupport::{graph_workspace, id_at, id_by_rev, visualize_commit_graph_all}; - use but_workspace::branch::ReferenceAnchor; - use but_workspace::branch::ReferencePosition::*; + use but_workspace::branch::create_reference::{Anchor, Position::*}; use std::borrow::Cow; #[test] @@ -134,7 +132,7 @@ mod with_workspace { let above_a = rc("refs/heads/above-A"); let graph = but_workspace::branch::create_reference( above_a, - ReferenceAnchor::AtSegment { + Anchor::AtSegment { ref_name: Cow::Borrowed(a_ref), position: Above, }, @@ -155,7 +153,7 @@ mod with_workspace { let below_b = rc("refs/heads/below-B"); let graph = but_workspace::branch::create_reference( below_b, - ReferenceAnchor::AtSegment { + Anchor::AtSegment { ref_name: Cow::Borrowed(b_ref), position: Below, }, @@ -221,7 +219,7 @@ mod with_workspace { let bottom_id = id_by_rev(&repo, ":/A1"); let graph = but_workspace::branch::create_reference( above_bottom_ref, - ReferenceAnchor::AtCommit { + Anchor::AtCommit { commit_id: bottom_id.detach(), position: Above, }, @@ -244,7 +242,7 @@ mod with_workspace { let bottom_ref = rc("refs/heads/bottom"); let graph = but_workspace::branch::create_reference( bottom_ref, - ReferenceAnchor::AtSegment { + Anchor::AtSegment { ref_name: Cow::Borrowed(above_bottom_ref), position: Below, }, @@ -268,7 +266,7 @@ mod with_workspace { let a_id = id_by_rev(&repo, ":/A"); let graph = but_workspace::branch::create_reference( above_a_commit_ref, - ReferenceAnchor::AtCommit { + Anchor::AtCommit { commit_id: a_id.detach(), position: Above, }, @@ -295,7 +293,7 @@ mod with_workspace { let a_ref = rc("refs/heads/A"); let graph = but_workspace::branch::create_reference( a_ref, - ReferenceAnchor::AtCommit { + Anchor::AtCommit { commit_id: a_id.detach(), position: Above, }, @@ -321,7 +319,7 @@ mod with_workspace { let a_ref = rc("refs/heads/A"); let graph = but_workspace::branch::create_reference( above_a_ref, - ReferenceAnchor::AtSegment { + Anchor::AtSegment { ref_name: a_ref, position: Above, }, @@ -347,7 +345,7 @@ mod with_workspace { let below_a_commit_ref = rc("refs/heads/below-A-commit"); let graph = but_workspace::branch::create_reference( below_a_commit_ref, - ReferenceAnchor::AtCommit { + Anchor::AtCommit { commit_id: a_id.detach(), position: Below, }, @@ -373,7 +371,7 @@ mod with_workspace { let below_a_ref = rc("refs/heads/below-A"); let graph = but_workspace::branch::create_reference( below_a_ref, - ReferenceAnchor::AtSegment { + Anchor::AtSegment { ref_name: Cow::Borrowed(above_a_commit_ref), position: Below, }, @@ -420,7 +418,7 @@ mod with_workspace { let above_b_ref = rc("refs/heads/above-B"); let graph = but_workspace::branch::create_reference( above_b_ref, - ReferenceAnchor::AtSegment { + Anchor::AtSegment { ref_name: Cow::Borrowed(b_ref), position: Above, }, @@ -452,7 +450,7 @@ mod with_workspace { let below_b_ref = rc("refs/heads/below-B"); let graph = but_workspace::branch::create_reference( below_b_ref, - ReferenceAnchor::AtSegment { + Anchor::AtSegment { ref_name: Cow::Borrowed(b_ref), position: Below, }, @@ -540,7 +538,7 @@ mod with_workspace { let bottom_id = id_by_rev(&repo, ":/A1"); let graph = but_workspace::branch::create_reference( above_bottom_ref, - ReferenceAnchor::AtCommit { + Anchor::AtCommit { commit_id: bottom_id.detach(), position: Above, }, @@ -561,7 +559,7 @@ mod with_workspace { let bottom_ref = rc("refs/heads/bottom"); let graph = but_workspace::branch::create_reference( bottom_ref, - ReferenceAnchor::AtSegment { + Anchor::AtSegment { ref_name: Cow::Borrowed(above_bottom_ref), position: Below, }, @@ -587,7 +585,7 @@ mod with_workspace { let a_id = id_by_rev(&repo, ":/A"); let graph = but_workspace::branch::create_reference( above_a_commit_ref, - ReferenceAnchor::AtCommit { + Anchor::AtCommit { commit_id: a_id.detach(), position: Above, }, @@ -613,7 +611,7 @@ mod with_workspace { let a_ref = rc("refs/heads/A"); let graph = but_workspace::branch::create_reference( above_a_ref, - ReferenceAnchor::AtSegment { + Anchor::AtSegment { ref_name: a_ref, position: Above, }, @@ -641,7 +639,7 @@ mod with_workspace { let a_ref = rc("refs/heads/A"); let graph = but_workspace::branch::create_reference( above_a_ref, - ReferenceAnchor::AtSegment { + Anchor::AtSegment { ref_name: a_ref, position: Above, }, @@ -666,7 +664,7 @@ mod with_workspace { let below_a_commit_ref = rc("refs/heads/below-A-commit"); let graph = but_workspace::branch::create_reference( below_a_commit_ref, - ReferenceAnchor::AtCommit { + Anchor::AtCommit { commit_id: a_id.detach(), position: Below, }, @@ -692,7 +690,7 @@ mod with_workspace { let below_a_ref = rc("refs/heads/below-A"); let graph = but_workspace::branch::create_reference( below_a_ref, - ReferenceAnchor::AtSegment { + Anchor::AtSegment { ref_name: Cow::Borrowed(above_a_commit_ref), position: Below, }, @@ -739,7 +737,7 @@ mod with_workspace { let above_b_ref = rc("refs/heads/above-B"); let graph = but_workspace::branch::create_reference( above_b_ref, - ReferenceAnchor::AtSegment { + Anchor::AtSegment { ref_name: Cow::Borrowed(b_ref), position: Above, }, @@ -771,7 +769,7 @@ mod with_workspace { let below_b_ref = rc("refs/heads/below-B"); let graph = but_workspace::branch::create_reference( below_b_ref, - ReferenceAnchor::AtSegment { + Anchor::AtSegment { ref_name: Cow::Borrowed(b_ref), position: Below, }, @@ -857,7 +855,7 @@ mod with_workspace { let bottom_id = id_by_rev(&repo, ":/A1"); let graph = but_workspace::branch::create_reference( bottom_ref, - ReferenceAnchor::AtCommit { + Anchor::AtCommit { commit_id: bottom_id.detach(), position: Below, }, @@ -910,7 +908,7 @@ mod with_workspace { let bottom_a_id = id_by_rev(&repo, ":/A1"); let graph = but_workspace::branch::create_reference( bottom_ref_a, - ReferenceAnchor::AtCommit { + Anchor::AtCommit { commit_id: bottom_a_id.detach(), position: Below, }, @@ -934,7 +932,7 @@ mod with_workspace { let bottom_b_id = id_by_rev(&repo, ":/B1"); let graph = but_workspace::branch::create_reference( bottom_ref_b, - ReferenceAnchor::AtCommit { + Anchor::AtCommit { commit_id: bottom_b_id.detach(), position: Below, }, @@ -977,8 +975,8 @@ mod with_workspace { let (ws_id, ws_ref_name) = id_at(&repo, "gitbutler/workspace"); let main_remote_id = id_by_rev(&repo, "@~1"); for anchor in [ - (ReferenceAnchor::at_id(main_remote_id, Above)), - (ReferenceAnchor::at_segment(r("refs/remotes/origin/main"), Above)), + (Anchor::at_id(main_remote_id, Above)), + (Anchor::at_segment(r("refs/remotes/origin/main"), Above)), ] { let err = but_workspace::branch::create_reference( ws_ref_name.as_ref(), @@ -989,7 +987,7 @@ mod with_workspace { ) .unwrap_err(); - let expected_err = if matches!(anchor, ReferenceAnchor::AtCommit { .. }) { + let expected_err = if matches!(anchor, Anchor::AtCommit { .. }) { "Commit 3183e43ff482a2c4c8ff531d595453b64f58d90b isn't part of the workspace" } else { "Couldn't find any stack that contained the branch named 'origin/main'" @@ -1042,8 +1040,8 @@ mod with_workspace { // Try to set gitbutler/workspace to a position in the workspace, but one below its current position let (a_id, a_ref_name) = id_at(&repo, "A"); for anchor in [ - (ReferenceAnchor::at_id(a_id, Below)), - (ReferenceAnchor::at_segment(a_ref_name.as_ref(), Below)), + (Anchor::at_id(a_id, Below)), + (Anchor::at_segment(a_ref_name.as_ref(), Below)), ] { let err = but_workspace::branch::create_reference( ws_ref_name.as_ref(), @@ -1073,8 +1071,8 @@ mod with_workspace { // Try to set gitbutler/workspace to the same position, which technically is in the workspace // and is where it's currently pointing to so it seems like nothing changes. for anchor in [ - (ReferenceAnchor::at_id(a_id, Above)), - (ReferenceAnchor::at_segment(a_ref_name.as_ref(), Above)), + (Anchor::at_id(a_id, Above)), + (Anchor::at_segment(a_ref_name.as_ref(), Above)), ] { let err = but_workspace::branch::create_reference( ws_ref_name.as_ref(), @@ -1129,7 +1127,7 @@ mod with_workspace { let new_name = rc("refs/heads/new"); let err = but_workspace::branch::create_reference( new_name, - ReferenceAnchor::AtSegment { + Anchor::AtSegment { ref_name: rc("refs/heads/bogus"), position: Below, }, @@ -1171,8 +1169,8 @@ fn errors() -> anyhow::Result<()> { let (id, ref_name) = id_at(&repo, "main"); let new_name = r("refs/heads/does-not-matter"); for anchor in [ - ReferenceAnchor::at_id(id, Below), - ReferenceAnchor::at_segment(ref_name.as_ref(), Below), + Anchor::at_id(id, Below), + Anchor::at_segment(ref_name.as_ref(), Below), ] { // Below first in history let err = but_workspace::branch::create_reference(new_name, anchor, &repo, &ws, &mut *meta) @@ -1194,8 +1192,8 @@ fn errors() -> anyhow::Result<()> { // Ambiguity (multiple refs in one spot). for anchor in [ - ReferenceAnchor::at_id(id, Above), - ReferenceAnchor::at_segment(ref_name.as_ref(), Above), + Anchor::at_id(id, Above), + Anchor::at_segment(ref_name.as_ref(), Above), ] { assert!(repo.try_find_reference(new_name)?.is_none()); let err = but_workspace::branch::create_reference(new_name, anchor, &repo, &ws, &mut *meta) @@ -1217,10 +1215,7 @@ fn errors() -> anyhow::Result<()> { // Misaligned workspace - commit not included. let (id, ref_name) = id_at(&repo, "A"); - for anchor in [ - ReferenceAnchor::at_id(id, Below), - ReferenceAnchor::at_id(id, Above), - ] { + for anchor in [Anchor::at_id(id, Below), Anchor::at_id(id, Above)] { let err = but_workspace::branch::create_reference(new_name, anchor, &repo, &ws, &mut *meta) .unwrap_err(); assert_eq!( @@ -1242,8 +1237,8 @@ fn errors() -> anyhow::Result<()> { // Misaligned workspace - segment not included. let (a_id, a_ref) = id_at(&repo, "A"); for anchor in [ - (ReferenceAnchor::at_segment(a_ref.as_ref(), Below)), - (ReferenceAnchor::at_segment(a_ref.as_ref(), Above)), + (Anchor::at_segment(a_ref.as_ref(), Below)), + (Anchor::at_segment(a_ref.as_ref(), Above)), ] { let err = but_workspace::branch::create_reference(new_name, anchor, &repo, &ws, &mut *meta) .unwrap_err(); @@ -1278,8 +1273,8 @@ fn errors() -> anyhow::Result<()> { let a_ref = r("refs/heads/A"); let (main_id, main_ref) = id_at(&repo, "main"); for anchor in [ - (ReferenceAnchor::at_segment(main_ref.as_ref(), Above)), - (ReferenceAnchor::at_id(main_id, Above)), + (Anchor::at_segment(main_ref.as_ref(), Above)), + (Anchor::at_id(main_id, Above)), ] { let err = but_workspace::branch::create_reference(a_ref, anchor, &repo, &ws, &mut *meta) .unwrap_err(); @@ -1317,8 +1312,8 @@ fn errors() -> anyhow::Result<()> { let (a_id, _a_ref_owned) = id_at(&repo, "A"); for anchor in [ - (ReferenceAnchor::at_segment(a_ref, Below)), - (ReferenceAnchor::at_id(a_id, Below)), + (Anchor::at_segment(a_ref, Below)), + (Anchor::at_id(a_id, Below)), ] { let err = but_workspace::branch::create_reference(new_name, anchor, &repo, &ws, &mut *meta) .unwrap_err(); @@ -1363,7 +1358,7 @@ fn journey() -> anyhow::Result<()> { let new_name = r("refs/heads/below-main"); let graph = but_workspace::branch::create_reference( new_name, - ReferenceAnchor::at_segment(main_ref.as_ref(), Below), + Anchor::at_segment(main_ref.as_ref(), Below), &repo, &ws, &mut meta, @@ -1396,7 +1391,7 @@ fn journey() -> anyhow::Result<()> { // Creating the same reference again is idempotent. let graph = but_workspace::branch::create_reference( new_name, - ReferenceAnchor::at_id(main_id, Below), + Anchor::at_id(main_id, Below), &repo, &ws, &mut meta, @@ -1415,7 +1410,7 @@ fn journey() -> anyhow::Result<()> { // the last possible branch without a workspace. let graph = but_workspace::branch::create_reference( rc("refs/heads/two-below-main"), - ReferenceAnchor::at_segment(r("refs/heads/below-main"), Below), + Anchor::at_segment(r("refs/heads/below-main"), Below), &repo, &ws, &mut meta, @@ -1436,7 +1431,7 @@ fn journey() -> anyhow::Result<()> { // the last possible branch without a workspace. let err = but_workspace::branch::create_reference( rc("refs/heads/another-below-main"), - ReferenceAnchor::at_segment(main_ref.as_ref(), Below), + Anchor::at_segment(main_ref.as_ref(), Below), &repo, &ws, &mut meta, @@ -1470,7 +1465,7 @@ fn journey() -> anyhow::Result<()> { // However, creating a dependent branch creates metadata as well. let graph = but_workspace::branch::create_reference( main_ref, - ReferenceAnchor::AtCommit { + Anchor::AtCommit { commit_id: main_id.detach(), position: Above, }, @@ -1523,7 +1518,7 @@ fn journey_anon_workspace() -> anyhow::Result<()> { let first_id = id_by_rev(&repo, "@~2"); let graph = but_workspace::branch::create_reference( first_ref, - ReferenceAnchor::AtCommit { + Anchor::AtCommit { commit_id: first_id.detach(), position: Above, }, @@ -1555,7 +1550,7 @@ fn journey_anon_workspace() -> anyhow::Result<()> { let second_id = id_by_rev(&repo, "@~1"); let graph = but_workspace::branch::create_reference( second_ref, - ReferenceAnchor::AtCommit { + Anchor::AtCommit { commit_id: second_id.detach(), position: Above, }, From 04ae3d0ec6ebdc12d2286c3be5548b1b2c2e9b92 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 23 Aug 2025 18:10:55 +0200 Subject: [PATCH 2/4] Assure that an entrypoint doesn't alter the stack setup. I.e. if stacks are separate when looking at the workspace, they should remain separtate when one of these is checked out directly. --- crates/but-graph/tests/fixtures/scenarios.sh | 8 +++ .../tests/graph/init/with_workspace.rs | 52 +++++++++++++++++++ 2 files changed, 60 insertions(+) diff --git a/crates/but-graph/tests/fixtures/scenarios.sh b/crates/but-graph/tests/fixtures/scenarios.sh index 38a5f40706..21fa42c293 100644 --- a/crates/but-graph/tests/fixtures/scenarios.sh +++ b/crates/but-graph/tests/fixtures/scenarios.sh @@ -281,6 +281,14 @@ mkdir ws create_workspace_commit_once main ) + git init just-init-with-two-branches + (cd just-init-with-two-branches + commit init + git branch A + git branch B + git checkout -b gitbutler/workspace + ) + git init just-init-with-branches (cd just-init-with-branches commit init && setup_target_to_match_main diff --git a/crates/but-graph/tests/graph/init/with_workspace.rs b/crates/but-graph/tests/graph/init/with_workspace.rs index 361cd8b425..286136d796 100644 --- a/crates/but-graph/tests/graph/init/with_workspace.rs +++ b/crates/but-graph/tests/graph/init/with_workspace.rs @@ -634,6 +634,58 @@ fn minimal_merge() -> anyhow::Result<()> { Ok(()) } +#[test] +fn stack_configuration_is_respected_if_one_of_them_is_an_entrypoint() -> anyhow::Result<()> { + let (repo, mut meta) = read_only_in_memory_scenario("ws/just-init-with-two-branches")?; + insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @"* fafd9d0 (HEAD -> gitbutler/workspace, main, B, A) init"); + + add_stack_with_segments(&mut meta, 1, "A", StackState::InWorkspace, &[]); + add_stack_with_segments(&mut meta, 2, "B", StackState::InWorkspace, &[]); + + let graph = Graph::from_head( + &repo, + &*meta, + standard_options_with_extra_target(&repo, "main"), + )? + .validated()?; + insta::assert_snapshot!(graph_tree(&graph), @r" + └── πŸ‘‰πŸ“•β–Ίβ–Ίβ–Ί:0[0]:gitbutler/workspace + β”œβ”€β”€ πŸ“™β–Ί:2[1]:A + β”‚ └── β–Ί:1[2]:anon: + β”‚ └── Β·fafd9d0 (βŒ‚|🏘️|1) β–Ίmain + └── πŸ“™β–Ί:3[1]:B + └── β†’:1: + "); + insta::assert_snapshot!(graph_workspace(&graph.to_workspace()?), @r" + πŸ“•πŸ˜οΈβš οΈ:0:gitbutler/workspace <> βœ“! on fafd9d0 + β”œβ”€β”€ β‰‘πŸ“™:3:B on fafd9d0 + β”‚ └── πŸ“™:3:B + └── β‰‘πŸ“™:2:A on fafd9d0 + └── πŸ“™:2:A + "); + + let (id, ref_name) = id_at(&repo, "B"); + let graph = Graph::from_commit_traversal(id, ref_name.clone(), &*meta, standard_options())? + .validated()?; + // TODO: it shouldn't create a dependent branch here, but instead see A as a stack. + // problem is that for stack creation, there is no candidate. + insta::assert_snapshot!(graph_tree(&graph), @r" + └── πŸ“•β–Ίβ–Ίβ–Ί:1[0]:gitbutler/workspace + └── πŸ‘‰πŸ“™β–Ί:0[1]:B + └── πŸ“™β–Ί:2[2]:A + └── Β·fafd9d0 (βŒ‚|🏘️|1) β–Ίmain + "); + insta::assert_snapshot!(graph_workspace(&graph.to_workspace()?), @r" + πŸ“•πŸ˜οΈβš οΈ:1:gitbutler/workspace <> βœ“! + └── β‰‘πŸ‘‰πŸ“™:0:B + β”œβ”€β”€ πŸ‘‰πŸ“™:0:B + └── πŸ“™:2:A + └── Β·fafd9d0 (🏘️) β–Ίmain + "); + + Ok(()) +} + #[test] fn just_init_with_branches() -> anyhow::Result<()> { let (repo, mut meta) = read_only_in_memory_scenario("ws/just-init-with-branches")?; From 52ae78c9eb707d9fe39b766e68599459beb30cef Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 18 Aug 2025 14:49:14 +0200 Subject: [PATCH 3/4] Basic apply and unapply The idea is to develop both at the same time so it's easier to test that 'unapply' truly goes back to the (mostly) original state. --- crates/but-graph/src/init/mod.rs | 33 +- crates/but-graph/src/init/overlay.rs | 17 +- crates/but-graph/src/projection/workspace.rs | 11 +- crates/but-workspace/src/branch/apply.rs | 222 ++++++++++++++ .../src/branch/create_reference.rs | 11 +- crates/but-workspace/src/branch/mod.rs | 4 + crates/but-workspace/src/ref_info.rs | 99 +++--- .../detached-with-multiple-branches.sh | 19 ++ .../scenario/unborn-empty-detached-remote.sh | 19 ++ .../scenario/with-remotes-and-workspace.sh | 8 +- ...s-ref-no-ws-commit-one-stack-one-branch.sh | 14 + .../ws-ref-ws-commit-one-stack-ws-advanced.sh | 17 ++ .../branch/apply_unapply_commit_uncommit.rs | 281 ++++++++++++++++++ .../workspace/branch/create_reference.rs | 28 +- .../tests/workspace/branch/mod.rs | 2 + 15 files changed, 722 insertions(+), 63 deletions(-) create mode 100644 crates/but-workspace/src/branch/apply.rs create mode 100644 crates/but-workspace/tests/fixtures/scenario/detached-with-multiple-branches.sh create mode 100644 crates/but-workspace/tests/fixtures/scenario/unborn-empty-detached-remote.sh create mode 100644 crates/but-workspace/tests/fixtures/scenario/ws-ref-no-ws-commit-one-stack-one-branch.sh create mode 100644 crates/but-workspace/tests/fixtures/scenario/ws-ref-ws-commit-one-stack-ws-advanced.sh create mode 100644 crates/but-workspace/tests/workspace/branch/apply_unapply_commit_uncommit.rs diff --git a/crates/but-graph/src/init/mod.rs b/crates/but-graph/src/init/mod.rs index 3d3d4b57f9..b67e6ac3b1 100644 --- a/crates/but-graph/src/init/mod.rs +++ b/crates/but-graph/src/init/mod.rs @@ -21,10 +21,13 @@ mod remotes; mod overlay; mod post; +pub(crate) type Entrypoint = Option<(gix::ObjectId, Option)>; + /// A way to define information to be served from memory, instead of from the underlying data source, when /// [initializing](Graph::from_commit_traversal()) the graph. #[derive(Debug, Default)] pub struct Overlay { + entrypoint: Entrypoint, nonoverriding_references: Vec, meta_branches: Vec<(gix::refs::FullName, ref_metadata::Branch)>, workspace: Option<(gix::refs::FullName, ref_metadata::Workspace)>, @@ -141,7 +144,7 @@ impl Graph { let mut graph = Graph::default(); // It's OK to default-initialise this here as overlays are only used when redoing // the traversal. - let (_repo, meta) = Overlay::default().into_parts(repo, meta); + let (_repo, meta, _entrypoint) = Overlay::default().into_parts(repo, meta); graph.insert_root(branch_segment_from_name_and_meta( Some((ref_name, None)), &meta, @@ -228,7 +231,7 @@ impl Graph { meta: &impl RefMetadata, options: Options, ) -> anyhow::Result { - let (repo, meta) = Overlay::default().into_parts(tip.repo, meta); + let (repo, meta, _entrypoint) = Overlay::default().into_parts(tip.repo, meta); Graph::from_commit_traversal_inner(tip.detach(), &repo, ref_name, &meta, options) } @@ -657,16 +660,22 @@ impl Graph { meta: &impl RefMetadata, overlay: Overlay, ) -> anyhow::Result { - let (repo, meta) = overlay.into_parts(repo, meta); - let tip_sidx = self - .entrypoint - .context("BUG: entrypoint must always be set")? - .0; - let tip = self - .tip_skip_empty(tip_sidx) - .context("BUG: entrypoint must eventually point to a commit")? - .id; - let ref_name = self[tip_sidx].ref_name.clone(); + let (repo, meta, entrypoint) = overlay.into_parts(repo, meta); + let (tip, ref_name) = match entrypoint { + Some(t) => t, + None => { + let tip_sidx = self + .entrypoint + .context("BUG: entrypoint must always be set")? + .0; + let tip = self + .tip_skip_empty(tip_sidx) + .context("BUG: entrypoint must eventually point to a commit")? + .id; + let ref_name = self[tip_sidx].ref_name.clone(); + (tip, ref_name) + } + }; Graph::from_commit_traversal_inner(tip, &repo, ref_name, &meta, self.options.clone()) } diff --git a/crates/but-graph/src/init/overlay.rs b/crates/but-graph/src/init/overlay.rs index f69587eb93..f6a037a24a 100644 --- a/crates/but-graph/src/init/overlay.rs +++ b/crates/but-graph/src/init/overlay.rs @@ -1,5 +1,5 @@ -use crate::init::Overlay; use crate::init::walk::RefsById; +use crate::init::{Entrypoint, Overlay}; use crate::is_workspace_ref_name; use but_core::{RefMetadata, ref_metadata}; use gix::prelude::ReferenceExt; @@ -17,6 +17,17 @@ impl Overlay { self } + /// Override the starting position of the traversal by setting it to `id`, + /// and optionally, by providing the `ref_name` that points to `id`. + pub fn with_entrypoint( + mut self, + id: gix::ObjectId, + ref_name: Option, + ) -> Self { + self.entrypoint = Some((id, ref_name)); + self + } + /// Serve the given `branches` metadata from memory, as if they would exist, /// possibly overriding metadata of a ref that already exists. pub fn with_branch_metadata_override( @@ -43,7 +54,7 @@ impl Overlay { self, repo: &'repo gix::Repository, meta: &'meta T, - ) -> (OverlayRepo<'repo>, OverlayMetadata<'meta, T>) + ) -> (OverlayRepo<'repo>, OverlayMetadata<'meta, T>, Entrypoint) where T: RefMetadata, { @@ -51,6 +62,7 @@ impl Overlay { nonoverriding_references, meta_branches, workspace, + entrypoint, } = self; ( OverlayRepo { @@ -62,6 +74,7 @@ impl Overlay { meta_branches, workspace, }, + entrypoint, ) } } diff --git a/crates/but-graph/src/projection/workspace.rs b/crates/but-graph/src/projection/workspace.rs index 7f53d0a23d..863db7d7d3 100644 --- a/crates/but-graph/src/projection/workspace.rs +++ b/crates/but-graph/src/projection/workspace.rs @@ -149,6 +149,11 @@ impl Workspace<'_> { }) } + /// Return `true` if `name` is contained in the workspace as segment. + pub fn refname_is_segment(&self, name: &gix::refs::FullNameRef) -> bool { + self.find_segment_and_stack_by_refname(name).is_some() + } + /// Try to find `name` in any named [`StackSegment`] and return it along with the stack containing it. pub fn find_segment_and_stack_by_refname( &self, @@ -189,9 +194,9 @@ pub enum WorkspaceKind { /// The name of the reference pointing to the workspace commit. Useful for deriving the workspace name. ref_name: gix::refs::FullName, }, - /// Information for when a workspace reference was advanced by hand and does not point to a - /// managed workspace commit anymore. - /// That commit, however, is reachable by following the first parent from the workspace reference. + /// Information for when a workspace reference was *possibly* advanced by hand and does not point to a + /// managed workspace commit (anymore). + /// That workspace commit, may be reachable by following the first parent from the workspace reference. /// /// Note that the stacks that follow *will* be in unusable if the workspace commit is in a segment below, /// but typically is usable if there is just a single real stack, or any amount of virtual stacks below diff --git a/crates/but-workspace/src/branch/apply.rs b/crates/but-workspace/src/branch/apply.rs new file mode 100644 index 0000000000..9486bf87e2 --- /dev/null +++ b/crates/but-workspace/src/branch/apply.rs @@ -0,0 +1,222 @@ +use std::borrow::Cow; + +/// Returned by [function::apply()]. +pub struct Outcome<'graph> { + /// The newly created graph, if owned, useful to project a workspace and see how the workspace looks like with the branch applied. + /// If borrowed, the graph already contains the desired branch and nothing had to be applied. + pub graph: Cow<'graph, but_graph::Graph>, + /// `true` if we created the given workspace ref as it didn't exist yet. + pub workspace_ref_created: bool, +} + +impl Outcome<'_> { + /// Return `true` if a new graph traversal was performed, which always is a sign for an operation which changed the workspace. + /// This is `false` if the to be applied branch was already contained in the current workspace. + pub fn workspace_changed(&self) -> bool { + matches!(self.graph, Cow::Owned(_)) + } +} + +impl std::fmt::Debug for Outcome<'_> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("Outcome") + .field("workspace_changed", &self.workspace_changed()) + .field("workspace_ref_created", &self.workspace_ref_created) + .finish() + } +} + +/// How the newly applied branch should be integrated into the workspace. +#[derive(Default, Debug, Copy, Clone)] +pub enum IntegrationMode { + /// Do nothing but to merge it into the workspace commit, *even* if it's not needed as the workspace reference + /// can connect directly with the *one* workspace base. + #[default] + AlwaysMerge, + /// Only create a merge commit if a new commit is effectively merged in. This avoids *unnecessary* merge commits, + /// but also requires support for this when creating commits (which may then have to create a merge-commit themselves). + // TODO: make this the default + MergeIfNeeded, +} + +/// What to do if the applied branch conflicts? +#[derive(Default, Debug, Copy, Clone)] +pub enum OnWorkspaceConflict { + /// Provide additional information about the stack that conflicted and the files involved in it, + /// and don't perform the operation. + #[default] + AbortAndReportConflictingStack, +} + +/// Decide how a newly created workspace reference should be named. +#[derive(Default, Debug, Clone)] +pub enum WorkspaceReferenceNaming { + /// Create a default workspace branch + #[default] + Default, + /// Create a workspace with the given name instead. + Given(gix::refs::FullName), +} + +/// Options for [function::apply()]. +#[derive(Default, Debug, Clone)] +pub struct Options { + /// how the branch should be brought into the workspace. + pub integration_mode: IntegrationMode, + /// Decide how to deal with conflicts. + pub on_workspace_conflict: OnWorkspaceConflict, + /// How the workspace reference should be named should it be created. + /// The creation is always needed if there are more than one branch applied. + pub workspace_reference_naming: WorkspaceReferenceNaming, +} + +pub(crate) mod function { + use super::{Options, Outcome, WorkspaceReferenceNaming}; + use crate::ref_info::WorkspaceExt; + use anyhow::{Context, bail}; + use but_core::ref_metadata::{StackId, ValueInfo, WorkspaceStack, WorkspaceStackBranch}; + use but_core::{RefMetadata, ref_metadata}; + use but_graph::init::Overlay; + use but_graph::projection::WorkspaceKind; + use std::borrow::Cow; + + /// Apply `branch` to the given `workspace`, and possibly create the workspace reference in `repo` + /// along with its `meta`-data if it doesn't exist yet. + /// Otherwise, add it to the existing `workspace`, and update its metadata accordingly. + /// **This means that the contents of `branch` is observable from the new state of `repo`**. + /// + /// Note that `workspace` is expected to match the state in `repo` as it's used instead of querying `repo` directly + /// where possible. + /// + /// Also note that we will create a managed workspace reference as needed if necessary, and a workspace commit if there is more than + /// one reference in the workspace afterward. + /// + /// On `error`, neither `repo` nor `meta` will have been changed, but `repo` may contain in-memory objects. + /// Otherwise, objects will have been persisted, and references and metadata will have been updated. + pub fn apply<'graph, T: RefMetadata>( + branch: &gix::refs::FullNameRef, + workspace: &but_graph::projection::Workspace<'graph>, + repo: &mut gix::Repository, + meta: &mut T, + Options { + integration_mode: _, + on_workspace_conflict: _, + workspace_reference_naming, + }: Options, + ) -> anyhow::Result> { + if workspace.refname_is_segment(branch) { + return Ok(Outcome { + graph: Cow::Borrowed(workspace.graph), + workspace_ref_created: false, + }); + } + + if let Some(ws_ref_name) = workspace.ref_name() { + if repo.try_find_reference(ws_ref_name)?.is_none() { + // The workspace is the probably ad-hoc, and doesn't exist, *assume* unborn. + bail!( + "Cannot create reference on unborn branch '{}'", + ws_ref_name.shorten() + ); + } + } + + if workspace.has_workspace_commit_in_ancestry(repo) { + bail!("Refusing to work on workspace whose workspace commit isn't at the top"); + } + + let (workspace_ref_name_to_update, ws_ref_metadata, branch_to_apply_metadata, graph) = + match &workspace.kind { + WorkspaceKind::Managed { ref_name } + | WorkspaceKind::ManagedMissingWorkspaceCommit { ref_name } => ( + ref_name.clone(), + meta.workspace(ref_name.as_ref())?, + None, + Cow::Borrowed(workspace.graph), + ), + WorkspaceKind::AdHoc => { + // We need to switch over to a possibly existing workspace. + // We know that the current branch is *not* reachable from the workspace or isn't naturally included, + // so it needs to be added as well. + let next_ws_ref_name = match workspace_reference_naming { + WorkspaceReferenceNaming::Default => { + gix::refs::FullName::try_from("refs/heads/gitbutler/workspace") + .expect("known statically") + } + WorkspaceReferenceNaming::Given(name) => name, + }; + let ws_ref_id = match repo.try_find_reference(next_ws_ref_name.as_ref())? { + None => { + // Create a workspace reference later at the current AdHoc workspace id + let ws_id = workspace + .stacks + .first() + .and_then(|s| s.segments.first()) + .and_then(|s| s.commits.first().map(|c| c.id)) + .context("BUG: how can an empty ad-hoc workspace exist? Should have at least one stack-segment with commit")?; + ws_id + } + Some(mut existing_workspace_reference) => { + let id = existing_workspace_reference.peel_to_id_in_place()?; + id.detach() + } + }; + + // Get as close as possible to what would happen if the branch was already part of it (without merging), + // and branch-metadata can disambiguate. Having this data also isn't harmful. + // We want to see if the branch is naturally included in workspace already. + let branch_md = meta.branch(branch)?; + let (branch_md_override, branch_md_to_set) = if branch_md.is_default() { + ( + Some((branch.to_owned(), (*branch_md).clone())), + Some(branch_md), + ) + } else { + (None, None) + }; + let mut ws_md = meta.workspace(next_ws_ref_name.as_ref())?; + { + let ws_mut: &mut ref_metadata::Workspace = &mut *ws_md; + ws_mut.stacks.push(WorkspaceStack { + id: StackId::generate(), + branches: vec![WorkspaceStackBranch { + ref_name: branch.to_owned(), + archived: false, + }], + }) + } + let ws_md_override = Some((next_ws_ref_name.clone(), (*ws_md).clone())); + + let graph = workspace.graph.redo_traversal_with_overlay( + repo, + meta, + Overlay::default() + .with_entrypoint(ws_ref_id, Some(next_ws_ref_name)) + .with_branch_metadata_override(branch_md_override) + .with_workspace_metadata_override(ws_md_override), + )?; + + let ws = graph.to_workspace()?; + if ws.refname_is_segment(branch) { + todo!( + "no need to add this branch, but we need to add the current ad-hoc branch instead" + ); + } + + todo!("put current ad-hoc stack into ") + } + }; + + // Everything worked? Assure the ref exists now that (nearly nothing) can go wrong anymore. + let workspace_ref_created = false; // TODO: use rval of reference update to know if it existed. + + if let Some(branch_md) = branch_to_apply_metadata { + meta.set_branch(branch_md)?; + } + + Ok(Outcome { + graph, + workspace_ref_created, + }) + } +} diff --git a/crates/but-workspace/src/branch/create_reference.rs b/crates/but-workspace/src/branch/create_reference.rs index c46a568e66..bc9ffdec71 100644 --- a/crates/but-workspace/src/branch/create_reference.rs +++ b/crates/but-workspace/src/branch/create_reference.rs @@ -225,9 +225,14 @@ pub(super) mod function { ref_name.shorten() ); }; - position.resolve_commit(segment.commits.first().context( - "BUG: empty segments aren't possible without workspace metadata", - )?.into(), ws_base)? + position.resolve_commit( + segment + .commits + .first() + .context("Cannot create reference on unborn branch")? + .into(), + ws_base, + )? }; ( validate_id, diff --git a/crates/but-workspace/src/branch/mod.rs b/crates/but-workspace/src/branch/mod.rs index 996234d824..c008bcab67 100644 --- a/crates/but-workspace/src/branch/mod.rs +++ b/crates/but-workspace/src/branch/mod.rs @@ -456,6 +456,10 @@ impl Stack { } } +/// Functions and types related to applying a workspace branch. +pub mod apply; +pub use apply::function::apply; + /// related types for removing a workspace reference. pub mod remove_reference; pub use remove_reference::function::remove_reference; diff --git a/crates/but-workspace/src/ref_info.rs b/crates/but-workspace/src/ref_info.rs index c455133b1a..b3a4f2e5a0 100644 --- a/crates/but-workspace/src/ref_info.rs +++ b/crates/but-workspace/src/ref_info.rs @@ -2,6 +2,26 @@ // TODO: rename this module to `workspace`, make it private, and pub-use all content in the top-level, as we now literally // get the workspace, while possibly processing it for use in the UI. +use gix::Repository; + +/// Additional workspace functionality that can't easily be implemented in `but-graph`. +pub trait WorkspaceExt { + /// Return `true` if this workspace has a workspace commit that the workspace reference isn't directly pointing to. + fn has_workspace_commit_in_ancestry(&self, repo: &gix::Repository) -> bool; +} + +impl WorkspaceExt for but_graph::projection::Workspace<'_> { + fn has_workspace_commit_in_ancestry(&self, repo: &Repository) -> bool { + function::find_ancestor_workspace_commit( + self.graph, + repo, + self.id, + self.lower_bound_segment_id, + ) + .is_some() + } +} + /// Options for the [`ref_info()`](crate::ref_info) call. #[derive(Default, Debug, Clone)] pub struct Options { @@ -327,7 +347,7 @@ pub(crate) mod function { use but_core::ref_metadata::ValueInfo; use but_graph::petgraph::Direction; use but_graph::{ - Graph, is_workspace_ref_name, + Graph, SegmentIndex, is_workspace_ref_name, projection::{StackCommit, WorkspaceKind}, }; use gix::prelude::ObjectIdExt; @@ -371,6 +391,44 @@ pub(crate) mod function { graph_to_ref_info(graph, repo, opts) } + pub(crate) fn find_ancestor_workspace_commit( + graph: &Graph, + repo: &gix::Repository, + workspace_id: SegmentIndex, + lower_bound_segment_id: Option, + ) -> Option { + let lower_bound_generation = lower_bound_segment_id.map(|sidx| graph[sidx].generation); + + let mut commits_outside = Vec::new(); + let mut sidx_and_cidx = None; + graph.visit_all_segments_excluding_start_until(workspace_id, Direction::Outgoing, |s| { + if sidx_and_cidx.is_some() + || lower_bound_generation.is_some_and(|max_gen| s.generation > max_gen) + { + return true; + } + for (cidx, graph_commit) in s.commits.iter().enumerate() { + let Ok(commit) = WorkspaceCommit::from_id(graph_commit.id.attach(repo)) else { + continue; + }; + if commit.is_managed() { + sidx_and_cidx = Some((s.id, cidx)); + return true; + } + commits_outside.push(ui::Commit::from_commit_ahead_of_workspace_commit( + commit.inner, + graph_commit, + )); + } + false + }); + sidx_and_cidx.map(|(sidx, cidx)| AncestorWorkspaceCommit { + commits_outside, + segment_with_managed_commit: sidx, + commit_index_of_managed_commit: cidx, + }) + } + fn graph_to_ref_info( graph: Graph, repo: &gix::Repository, @@ -391,42 +449,9 @@ pub(crate) mod function { let (workspace_ref_name, is_managed_commit, ancestor_workspace_commit) = match kind { WorkspaceKind::Managed { ref_name } => (Some(ref_name), true, None), WorkspaceKind::ManagedMissingWorkspaceCommit { ref_name } => { - let lower_bound_generation = - lower_bound_segment_id.map(|sidx| graph[sidx].generation); - - let mut commits_outside = Vec::new(); - let mut sidx_and_cidx = None; - graph.visit_all_segments_excluding_start_until(id, Direction::Outgoing, |s| { - if sidx_and_cidx.is_some() - || lower_bound_generation.is_some_and(|max_gen| s.generation > max_gen) - { - return true; - } - for (cidx, graph_commit) in s.commits.iter().enumerate() { - let Ok(commit) = WorkspaceCommit::from_id(graph_commit.id.attach(repo)) - else { - continue; - }; - if commit.is_managed() { - sidx_and_cidx = Some((s.id, cidx)); - return true; - } - commits_outside.push(ui::Commit::from_commit_ahead_of_workspace_commit( - commit.inner, - graph_commit, - )); - } - false - }); - ( - Some(ref_name), - false, - sidx_and_cidx.map(|(sidx, cidx)| AncestorWorkspaceCommit { - commits_outside, - segment_with_managed_commit: sidx, - commit_index_of_managed_commit: cidx, - }), - ) + let maybe_ancestor_workspace_commit = + find_ancestor_workspace_commit(graph, repo, id, lower_bound_segment_id); + (Some(ref_name), false, maybe_ancestor_workspace_commit) } WorkspaceKind::AdHoc => (graph[id].ref_name.clone(), false, None), }; diff --git a/crates/but-workspace/tests/fixtures/scenario/detached-with-multiple-branches.sh b/crates/but-workspace/tests/fixtures/scenario/detached-with-multiple-branches.sh new file mode 100644 index 0000000000..da19574bcc --- /dev/null +++ b/crates/but-workspace/tests/fixtures/scenario/detached-with-multiple-branches.sh @@ -0,0 +1,19 @@ +#!/usr/bin/env bash + +set -eu -o pipefail +source "${BASH_SOURCE[0]%/*}/shared.sh" + +### Description +# The HEAD is detached, and there are multiple branches. + +git init +commit M1 +git branch B +git branch C +git checkout -b A + commit A1 +git checkout B + commit B1 +git checkout C + commit C1 +git checkout --detach HEAD diff --git a/crates/but-workspace/tests/fixtures/scenario/unborn-empty-detached-remote.sh b/crates/but-workspace/tests/fixtures/scenario/unborn-empty-detached-remote.sh new file mode 100644 index 0000000000..61344d21fd --- /dev/null +++ b/crates/but-workspace/tests/fixtures/scenario/unborn-empty-detached-remote.sh @@ -0,0 +1,19 @@ +#!/usr/bin/env bash + +set -eu -o pipefail + +source "${BASH_SOURCE[0]%/*}/shared.sh" + +### Description +# A newly initialized git repository, but with a known remote that has an object. + +git init remote +(cd remote + commit "M1" +) + +git init unborn +(cd unborn + git remote add orphan ../remote + git fetch orphan +) diff --git a/crates/but-workspace/tests/fixtures/scenario/with-remotes-and-workspace.sh b/crates/but-workspace/tests/fixtures/scenario/with-remotes-and-workspace.sh index 24cf000792..1dc1105f47 100644 --- a/crates/but-workspace/tests/fixtures/scenario/with-remotes-and-workspace.sh +++ b/crates/but-workspace/tests/fixtures/scenario/with-remotes-and-workspace.sh @@ -1,13 +1,13 @@ #!/usr/bin/env bash -### General Description - -# Various directories with different scenarios for testing stack information *with* a workspace commit, -# and of course with a remote and a branch to integrate with. set -eu -o pipefail source "${BASH_SOURCE[0]%/*}/shared.sh" +### General Description + +# Various directories with different scenarios for testing stack information *with* a workspace commit, +# and of course with a remote and a branch to integrate with. git init remote (cd remote diff --git a/crates/but-workspace/tests/fixtures/scenario/ws-ref-no-ws-commit-one-stack-one-branch.sh b/crates/but-workspace/tests/fixtures/scenario/ws-ref-no-ws-commit-one-stack-one-branch.sh new file mode 100644 index 0000000000..69c9af8eb0 --- /dev/null +++ b/crates/but-workspace/tests/fixtures/scenario/ws-ref-no-ws-commit-one-stack-one-branch.sh @@ -0,0 +1,14 @@ +#!/usr/bin/env bash + +set -eu -o pipefail + +source "${BASH_SOURCE[0]%/*}/shared.sh" + +### General Description + +# A ws-ref points to a commit, which is also owned by a stack. There is another branch outside of the workspace pointing to the same commit. +git init +commit A +git branch A +git branch B +git checkout -b gitbutler/workspace diff --git a/crates/but-workspace/tests/fixtures/scenario/ws-ref-ws-commit-one-stack-ws-advanced.sh b/crates/but-workspace/tests/fixtures/scenario/ws-ref-ws-commit-one-stack-ws-advanced.sh new file mode 100644 index 0000000000..e4a1b1f765 --- /dev/null +++ b/crates/but-workspace/tests/fixtures/scenario/ws-ref-ws-commit-one-stack-ws-advanced.sh @@ -0,0 +1,17 @@ +#!/usr/bin/env bash + +set -eu -o pipefail + +source "${BASH_SOURCE[0]%/*}/shared.sh" + +### General Description + +# A ws-ref points to a ws-commit, with an empty stack inside, but the workspace ref is advanced by an outside commit. +git init +commit M1 + +git branch B +git checkout -b A + +create_workspace_commit_once A +commit O1 diff --git a/crates/but-workspace/tests/workspace/branch/apply_unapply_commit_uncommit.rs b/crates/but-workspace/tests/workspace/branch/apply_unapply_commit_uncommit.rs new file mode 100644 index 0000000000..f705e180f9 --- /dev/null +++ b/crates/but-workspace/tests/workspace/branch/apply_unapply_commit_uncommit.rs @@ -0,0 +1,281 @@ +use crate::ref_info::with_workspace_commit::utils::{ + named_read_only_in_memory_scenario, named_writable_scenario_with_description_and_graph, +}; +use crate::utils::r; +use but_graph::init::Options; +use but_testsupport::{graph_workspace, id_at, visualize_commit_graph_all}; +use but_workspace::branch::apply::{ + IntegrationMode, OnWorkspaceConflict, WorkspaceReferenceNaming, +}; + +#[test] +#[ignore = "TBD: idempotent"] +fn unapply_branch_not_in_workspace() -> anyhow::Result<()> { + Ok(()) +} + +#[test] +fn operation_denied_on_improper_workspace() -> anyhow::Result<()> { + let (_tmp, graph, mut repo, mut meta, _description) = + named_writable_scenario_with_description_and_graph( + "ws-ref-ws-commit-one-stack-ws-advanced", + |_meta| {}, + )?; + insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @r" + * 0d01196 (HEAD -> gitbutler/workspace) O1 + * 4979833 GitButler Workspace Commit + * 3183e43 (main, B, A) M1 + "); + let ws = &graph.to_workspace()?; + insta::assert_snapshot!(graph_workspace(&ws), @r" + πŸ“•πŸ˜οΈβš οΈ:0:gitbutler/workspace <> βœ“! on 3183e43 + └── ≑:2:anon: on 3183e43 + └── :2:anon: + β”œβ”€β”€ Β·0d01196 (🏘️) + └── Β·4979833 (🏘️) + "); + + let err = but_workspace::branch::apply( + r("refs/heads/B"), + &ws, + &mut repo, + &mut meta, + default_options(), + ) + .unwrap_err(); + assert_eq!( + err.to_string(), + "Refusing to work on workspace whose workspace commit isn't at the top", + "cannot apply on a workspace that isn't proper" + ); + + // TODO: unapply, commit, uncommit + Ok(()) +} + +#[test] +fn ws_ref_no_ws_commit_two_stacks_on_same_commit() -> anyhow::Result<()> { + let (_tmp, graph, mut repo, mut meta, _description) = + named_writable_scenario_with_description_and_graph( + "ws-ref-no-ws-commit-one-stack-one-branch", + |_meta| {}, + )?; + insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @"* e5d0542 (HEAD -> gitbutler/workspace, main, B, A) A"); + let ws = &graph.to_workspace()?; + insta::assert_snapshot!(graph_workspace(&ws), @"πŸ“•πŸ˜οΈβš οΈ:0:gitbutler/workspace <> βœ“! on e5d0542"); + + // Put "A" into the workspace, yielding a single branch. + let out = but_workspace::branch::apply( + r("refs/heads/A"), + &ws, + &mut repo, + &mut meta, + default_options(), + )?; + insta::assert_debug_snapshot!(out, @r" + Outcome { + workspace_ref_created: false, + } + "); + insta::assert_snapshot!(graph_workspace(&out.graph.to_workspace()?), @"πŸ“•πŸ˜οΈβš οΈ:0:gitbutler/workspace <> βœ“! on e5d0542"); + // A ws commit was created as it's needed for the current commit implementation. + insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @"* e5d0542 (HEAD -> gitbutler/workspace, main, B, A) A"); + + let out = but_workspace::branch::apply( + r("refs/heads/B"), + &ws, + &mut repo, + &mut meta, + default_options(), + )?; + insta::assert_debug_snapshot!(out, @r" + Outcome { + workspace_ref_created: false, + } + "); + insta::assert_snapshot!(graph_workspace(&out.graph.to_workspace()?), @"πŸ“•πŸ˜οΈβš οΈ:0:gitbutler/workspace <> βœ“! on e5d0542"); + + // TODO: commit/uncommit + // TODO: unapply + + Ok(()) +} + +#[test] +fn new_workspace_exists_elsewhere_and_to_be_applied_branch_exists_there() -> anyhow::Result<()> { + let (_tmp, ws_graph, mut repo, mut meta, _description) = + named_writable_scenario_with_description_and_graph( + "ws-ref-no-ws-commit-one-stack-one-branch", + |_meta| {}, + )?; + insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @"* e5d0542 (HEAD -> gitbutler/workspace, main, B, A) A"); + // The default workspace, it's empty as target is set to `main`. + insta::assert_snapshot!(graph_workspace(&ws_graph.to_workspace()?), @"πŸ“•πŸ˜οΈβš οΈ:0:gitbutler/workspace <> βœ“! on e5d0542"); + + // Pretend "B" is checked out (it's at the right state independently of that) + let (b_id, b_ref) = id_at(&repo, "B"); + let graph = but_graph::Graph::from_commit_traversal(b_id, b_ref, &meta, Default::default())?; + let ws = graph.to_workspace()?; + insta::assert_snapshot!(graph_workspace(&ws), @r" + βŒ‚:0:B <> βœ“! + └── ≑:0:B + └── :0:B + └── Β·e5d0542 β–ΊA, β–Ίmain + "); + + // Put "A" into the workspace, hence we want "A" and "B" in it. + let out = but_workspace::branch::apply( + r("refs/heads/A"), + &ws, + &mut repo, + &mut meta, + default_options(), + )?; + insta::assert_debug_snapshot!(out, @r" + Outcome { + workspace_ref_created: false, + } + "); + + // HEAD must now point to the workspace (that already existed) + insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @"* e5d0542 (HEAD -> gitbutler/workspace, B, A) A"); + + Ok(()) +} + +#[test] +fn detached_head_journey() -> anyhow::Result<()> { + let (_tmp, graph, mut repo, mut meta, _description) = + named_writable_scenario_with_description_and_graph( + "detached-with-multiple-branches", + |_meta| {}, + )?; + insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @r" + * 49d4b34 (A) A1 + | * f57c528 (B) B1 + |/ + | * aaa195b (HEAD, C) C1 + |/ + * 3183e43 (main) M1 + "); + let ws = graph.to_workspace()?; + insta::assert_snapshot!(graph_workspace(&ws), @r" + βŒ‚:0:DETACHED <> βœ“! + └── ≑:0:anon: + β”œβ”€β”€ :0:anon: + β”‚ └── Β·aaa195b β–ΊC + └── :1:main + └── Β·3183e43 (βœ“) + "); + + let out = but_workspace::branch::apply( + r("refs/heads/A"), + &ws, + &mut repo, + &mut meta, + default_options(), + )?; + insta::assert_debug_snapshot!(out, @r" + Outcome { + workspace_ref_created: false, + } + "); + Ok(()) +} + +#[test] +fn apply_nonexisting_branch_failure() -> anyhow::Result<()> { + let (mut repo, mut meta) = + named_read_only_in_memory_scenario("ws-ref-no-ws-commit-one-stack-one-branch", "")?; + insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @"* e5d0542 (HEAD -> gitbutler/workspace, main, B, A) A"); + + let graph = but_graph::Graph::from_head(&repo, &*meta, Options::limited())?; + let ws = graph.to_workspace()?; + insta::assert_snapshot!(graph_workspace(&ws), @r" + πŸ“•πŸ˜οΈβš οΈ:0:gitbutler/workspace <> βœ“! + └── ≑:1:anon: + └── :1:anon: + └── Β·e5d0542 (🏘️) β–ΊA, β–ΊB, β–Ίmain + "); + + // Idempotency + let err = but_workspace::branch::apply( + r("refs/heads/does-not-exist"), + &ws, + &mut repo, + &mut *meta, + default_options(), + ) + .unwrap_err(); + assert_eq!(err.to_string(), "TBD"); + + // This fails without any effective change. + insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @"* e5d0542 (HEAD -> gitbutler/workspace, main, B, A) A"); + Ok(()) +} + +#[test] +#[ignore = "TBD"] +fn unapply_nonexisting_branch_failure() -> anyhow::Result<()> { + Ok(()) +} + +#[test] +fn unborn_apply_needs_base() -> anyhow::Result<()> { + let (mut repo, mut meta) = + named_read_only_in_memory_scenario("unborn-empty-detached-remote", "unborn")?; + insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @"* 3183e43 (orphan/main) M1"); + + let graph = but_graph::Graph::from_head(&repo, &*meta, Options::limited())?; + let ws = graph.to_workspace()?; + insta::assert_snapshot!(graph_workspace(&ws), @r" + βŒ‚:0:main <> βœ“! + └── ≑:0:main + └── :0:main + "); + + // Idempotency + let out = but_workspace::branch::apply( + r("refs/heads/main"), + &ws, + &mut repo, + &mut *meta, + default_options(), + )?; + insta::assert_debug_snapshot!(out, @r" + Outcome { + workspace_changed: false, + workspace_ref_created: false, + } + "); + + // Cannot apply branch without a base. + let err = but_workspace::branch::apply( + r("refs/remotes/orphan/main"), + &ws, + &mut repo, + &mut *meta, + default_options(), + ) + .unwrap_err(); + assert_eq!( + err.to_string(), + "Cannot create reference on unborn branch 'main'" + ); + Ok(()) +} + +fn default_options() -> but_workspace::branch::apply::Options { + but_workspace::branch::apply::Options { + integration_mode: IntegrationMode::MergeIfNeeded, + on_workspace_conflict: OnWorkspaceConflict::AbortAndReportConflictingStack, + workspace_reference_naming: WorkspaceReferenceNaming::Default, + } +} + +#[test] +#[ignore = "TBD"] +fn apply_branch_resting_on_base() -> anyhow::Result<()> { + // THis can't work, but should fail gracefully. + Ok(()) +} diff --git a/crates/but-workspace/tests/workspace/branch/create_reference.rs b/crates/but-workspace/tests/workspace/branch/create_reference.rs index fd48ef5123..5e96cd8a50 100644 --- a/crates/but-workspace/tests/workspace/branch/create_reference.rs +++ b/crates/but-workspace/tests/workspace/branch/create_reference.rs @@ -7,6 +7,7 @@ use but_core::ref_metadata::ValueInfo; use but_graph::init::Options; use but_testsupport::{graph_workspace, id_at, id_by_rev, visualize_commit_graph_all}; use but_workspace::branch::create_reference::{Anchor, Position::*}; +use std::borrow::Cow; mod with_workspace { use crate::ref_info::with_workspace_commit::utils::{ @@ -1148,6 +1149,30 @@ mod with_workspace { #[test] fn errors() -> anyhow::Result<()> { + let (repo, mut meta) = named_read_only_in_memory_scenario("unborn-empty", "")?; + let graph = but_graph::Graph::from_head(&repo, &*meta, Options::limited())?; + let ws = graph.to_workspace()?; + insta::assert_snapshot!(graph_workspace(&ws), @r" + βŒ‚:0:main <> βœ“! + └── ≑:0:main + └── :0:main + "); + + // Below first in history + let new_name = r("refs/heads/does-not-matter"); + let err = but_workspace::branch::create_reference( + new_name, + Anchor::AtSegment { + ref_name: Cow::Borrowed(r("refs/heads/main")), + position: Above, + }, + &repo, + &ws, + &mut *meta, + ) + .unwrap_err(); + assert_eq!(err.to_string(), "Cannot create reference on unborn branch"); + let (repo, mut meta) = named_read_only_in_memory_scenario("with-remotes-no-workspace", "remote")?; insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @r" @@ -1167,7 +1192,6 @@ fn errors() -> anyhow::Result<()> { "); let (id, ref_name) = id_at(&repo, "main"); - let new_name = r("refs/heads/does-not-matter"); for anchor in [ Anchor::at_id(id, Below), Anchor::at_segment(ref_name.as_ref(), Below), @@ -1334,7 +1358,7 @@ fn errors() -> anyhow::Result<()> { } #[test] -fn journey() -> anyhow::Result<()> { +fn journey_with_commits() -> anyhow::Result<()> { let (_tmp, repo, mut meta) = named_writable_scenario("single-branch-with-3-commits")?; insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @r" * 281da94 (HEAD -> main) 3 diff --git a/crates/but-workspace/tests/workspace/branch/mod.rs b/crates/but-workspace/tests/workspace/branch/mod.rs index c07ca58875..9dee6cb027 100644 --- a/crates/but-workspace/tests/workspace/branch/mod.rs +++ b/crates/but-workspace/tests/workspace/branch/mod.rs @@ -1,2 +1,4 @@ +/// Various journeys with apply, unapply and commit operations. +mod apply_unapply_commit_uncommit; mod create_reference; mod remove_reference; From a9724b4d081b7277208ffe9c02f483fe6fcbe60a Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 23 Aug 2025 17:17:07 +0200 Subject: [PATCH 4/4] SQUASH ME --- crates/but-graph/src/projection/workspace.rs | 33 +++ crates/but-workspace/src/branch/apply.rs | 194 +++++++++--------- .../branch/apply_unapply_commit_uncommit.rs | 77 ++++++- 3 files changed, 200 insertions(+), 104 deletions(-) diff --git a/crates/but-graph/src/projection/workspace.rs b/crates/but-graph/src/projection/workspace.rs index 863db7d7d3..ad7ae41810 100644 --- a/crates/but-graph/src/projection/workspace.rs +++ b/crates/but-graph/src/projection/workspace.rs @@ -61,6 +61,13 @@ pub type CommitOwnerIndexes = (usize, usize, CommitIndex); /// Utilities impl Workspace<'_> { + /// Return `true` if the workspace itself is where `HEAD` is pointing to. + /// If `false`, one of the stack-segments is checked out instead. + pub fn is_entrypoint(&self) -> bool { + self.stacks + .iter() + .all(|s| s.segments.iter().all(|s| !s.is_entrypoint)) + } /// Lookup a triple obtained by [`Self::find_owner_indexes_by_commit_id()`] or panic. pub fn lookup_commit(&self, (stack_idx, seg_idx, cidx): CommitOwnerIndexes) -> &StackCommit { &self.stacks[stack_idx].segments[seg_idx].commits[cidx] @@ -154,6 +161,32 @@ impl Workspace<'_> { self.find_segment_and_stack_by_refname(name).is_some() } + /// Return `true` if the entrypoint. + pub fn is_reachable_from_entrypoint(&self, name: &gix::refs::FullNameRef) -> bool { + if self.is_entrypoint() { + self.refname_is_segment(name) + } else { + let Some((stack, segment_idx)) = self.stacks.iter().find_map(|stack| { + stack + .segments + .iter() + .enumerate() + .find_map(|(idx, segment)| segment.is_entrypoint.then_some((stack, idx))) + }) else { + return false; + }; + stack + .segments + .get(segment_idx..) + .into_iter() + .any(|segments| { + segments + .iter() + .any(|s| s.ref_name.as_ref().is_some_and(|rn| rn.as_ref() == name)) + }) + } + } + /// Try to find `name` in any named [`StackSegment`] and return it along with the stack containing it. pub fn find_segment_and_stack_by_refname( &self, diff --git a/crates/but-workspace/src/branch/apply.rs b/crates/but-workspace/src/branch/apply.rs index 9486bf87e2..c95dc85bd1 100644 --- a/crates/but-workspace/src/branch/apply.rs +++ b/crates/but-workspace/src/branch/apply.rs @@ -73,10 +73,8 @@ pub struct Options { pub(crate) mod function { use super::{Options, Outcome, WorkspaceReferenceNaming}; use crate::ref_info::WorkspaceExt; - use anyhow::{Context, bail}; - use but_core::ref_metadata::{StackId, ValueInfo, WorkspaceStack, WorkspaceStackBranch}; - use but_core::{RefMetadata, ref_metadata}; - use but_graph::init::Overlay; + use anyhow::bail; + use but_core::RefMetadata; use but_graph::projection::WorkspaceKind; use std::borrow::Cow; @@ -104,11 +102,21 @@ pub(crate) mod function { workspace_reference_naming, }: Options, ) -> anyhow::Result> { - if workspace.refname_is_segment(branch) { - return Ok(Outcome { - graph: Cow::Borrowed(workspace.graph), - workspace_ref_created: false, - }); + if workspace.is_reachable_from_entrypoint(branch) { + if workspace.is_entrypoint() + || workspace + .stacks + .iter() + .flat_map(|s| s.segments.iter().filter_map(|s| s.ref_name.as_ref())) + .any(|rn| rn.as_ref() == branch) + { + return Ok(Outcome { + graph: Cow::Borrowed(workspace.graph), + workspace_ref_created: false, + }); + } + } else if workspace.refname_is_segment(branch) { + todo!("checkout workspace so the to-be-applied branch becomes visible") } if let Some(ws_ref_name) = workspace.ref_name() { @@ -125,98 +133,86 @@ pub(crate) mod function { bail!("Refusing to work on workspace whose workspace commit isn't at the top"); } - let (workspace_ref_name_to_update, ws_ref_metadata, branch_to_apply_metadata, graph) = - match &workspace.kind { - WorkspaceKind::Managed { ref_name } - | WorkspaceKind::ManagedMissingWorkspaceCommit { ref_name } => ( - ref_name.clone(), - meta.workspace(ref_name.as_ref())?, - None, - Cow::Borrowed(workspace.graph), - ), - WorkspaceKind::AdHoc => { - // We need to switch over to a possibly existing workspace. - // We know that the current branch is *not* reachable from the workspace or isn't naturally included, - // so it needs to be added as well. - let next_ws_ref_name = match workspace_reference_naming { - WorkspaceReferenceNaming::Default => { - gix::refs::FullName::try_from("refs/heads/gitbutler/workspace") - .expect("known statically") - } - WorkspaceReferenceNaming::Given(name) => name, - }; - let ws_ref_id = match repo.try_find_reference(next_ws_ref_name.as_ref())? { - None => { - // Create a workspace reference later at the current AdHoc workspace id - let ws_id = workspace - .stacks - .first() - .and_then(|s| s.segments.first()) - .and_then(|s| s.commits.first().map(|c| c.id)) - .context("BUG: how can an empty ad-hoc workspace exist? Should have at least one stack-segment with commit")?; - ws_id - } - Some(mut existing_workspace_reference) => { - let id = existing_workspace_reference.peel_to_id_in_place()?; - id.detach() - } - }; - - // Get as close as possible to what would happen if the branch was already part of it (without merging), - // and branch-metadata can disambiguate. Having this data also isn't harmful. - // We want to see if the branch is naturally included in workspace already. - let branch_md = meta.branch(branch)?; - let (branch_md_override, branch_md_to_set) = if branch_md.is_default() { - ( - Some((branch.to_owned(), (*branch_md).clone())), - Some(branch_md), - ) - } else { - (None, None) - }; - let mut ws_md = meta.workspace(next_ws_ref_name.as_ref())?; - { - let ws_mut: &mut ref_metadata::Workspace = &mut *ws_md; - ws_mut.stacks.push(WorkspaceStack { - id: StackId::generate(), - branches: vec![WorkspaceStackBranch { - ref_name: branch.to_owned(), - archived: false, - }], - }) - } - let ws_md_override = Some((next_ws_ref_name.clone(), (*ws_md).clone())); - - let graph = workspace.graph.redo_traversal_with_overlay( - repo, - meta, - Overlay::default() - .with_entrypoint(ws_ref_id, Some(next_ws_ref_name)) - .with_branch_metadata_override(branch_md_override) - .with_workspace_metadata_override(ws_md_override), - )?; - - let ws = graph.to_workspace()?; - if ws.refname_is_segment(branch) { - todo!( - "no need to add this branch, but we need to add the current ad-hoc branch instead" - ); + // In general, we only have to deal with one branch to apply. But when we are on an adhoc workspace, + // we need to assure both branches go into the existing or the new workspace. + let (workspace_ref_name_to_update, branches_to_apply) = match &workspace.kind { + WorkspaceKind::Managed { ref_name } + | WorkspaceKind::ManagedMissingWorkspaceCommit { ref_name } => { + (ref_name.clone(), vec![branch]) + } + WorkspaceKind::AdHoc => { + // We need to switch over to a possibly existing workspace. + // We know that the current branch is *not* reachable from the workspace or isn't naturally included, + // so it needs to be added as well. + let next_ws_ref_name = match workspace_reference_naming { + WorkspaceReferenceNaming::Default => { + gix::refs::FullName::try_from("refs/heads/gitbutler/workspace") + .expect("known statically") } - - todo!("put current ad-hoc stack into ") - } - }; + WorkspaceReferenceNaming::Given(name) => name, + }; + ( + next_ws_ref_name, + workspace + .ref_name() + .into_iter() + .chain(Some(branch)) + .collect(), + ) + // let ws_ref_id = match repo.try_find_reference(next_ws_ref_name.as_ref())? { + // None => { + // // Create a workspace reference later at the current AdHoc workspace id + // let ws_id = workspace + // .stacks + // .first() + // .and_then(|s| s.segments.first()) + // .and_then(|s| s.commits.first().map(|c| c.id)) + // .context("BUG: how can an empty ad-hoc workspace exist? Should have at least one stack-segment with commit")?; + // ws_id + // } + // Some(mut existing_workspace_reference) => { + // let id = existing_workspace_reference.peel_to_id_in_place()?; + // id.detach() + // } + // }; + + // let mut ws_md = meta.workspace(next_ws_ref_name.as_ref())?; + // { + // let ws_mut: &mut ref_metadata::Workspace = &mut *ws_md; + // ws_mut.stacks.push(WorkspaceStack { + // id: StackId::generate(), + // branches: vec![WorkspaceStackBranch { + // ref_name: branch.to_owned(), + // archived: false, + // }], + // }) + // } + // let ws_md_override = Some((next_ws_ref_name.clone(), (*ws_md).clone())); + + // let graph = workspace.graph.redo_traversal_with_overlay( + // repo, + // meta, + // Overlay::default() + // .with_entrypoint(ws_ref_id, Some(next_ws_ref_name)) + // .with_branch_metadata_override(branch_md_override) + // .with_workspace_metadata_override(ws_md_override), + // )?; + } + }; // Everything worked? Assure the ref exists now that (nearly nothing) can go wrong anymore. - let workspace_ref_created = false; // TODO: use rval of reference update to know if it existed. - - if let Some(branch_md) = branch_to_apply_metadata { - meta.set_branch(branch_md)?; - } - - Ok(Outcome { - graph, - workspace_ref_created, - }) + let _workspace_ref_created = false; // TODO: use rval of reference update to know if it existed. + + // if let Some(branch_md) = branch_to_apply_metadata { + // meta.set_branch(branch_md)?; + // } + + todo!( + "prepare outcome once all values were written out and the graph was regenerated - the simulation is now reality" + ); + // Ok(Outcome { + // graph: Cow::Borrowed(workspace.graph), + // workspace_ref_created, + // }) } } diff --git a/crates/but-workspace/tests/workspace/branch/apply_unapply_commit_uncommit.rs b/crates/but-workspace/tests/workspace/branch/apply_unapply_commit_uncommit.rs index f705e180f9..58f8a043a2 100644 --- a/crates/but-workspace/tests/workspace/branch/apply_unapply_commit_uncommit.rs +++ b/crates/but-workspace/tests/workspace/branch/apply_unapply_commit_uncommit.rs @@ -1,5 +1,6 @@ use crate::ref_info::with_workspace_commit::utils::{ - named_read_only_in_memory_scenario, named_writable_scenario_with_description_and_graph, + StackState, add_stack_with_segments, named_read_only_in_memory_scenario, + named_writable_scenario_with_description_and_graph, }; use crate::utils::r; use but_graph::init::Options; @@ -74,6 +75,7 @@ fn ws_ref_no_ws_commit_two_stacks_on_same_commit() -> anyhow::Result<()> { )?; insta::assert_debug_snapshot!(out, @r" Outcome { + workspace_changed: false, workspace_ref_created: false, } "); @@ -90,6 +92,7 @@ fn ws_ref_no_ws_commit_two_stacks_on_same_commit() -> anyhow::Result<()> { )?; insta::assert_debug_snapshot!(out, @r" Outcome { + workspace_changed: false, workspace_ref_created: false, } "); @@ -133,12 +136,13 @@ fn new_workspace_exists_elsewhere_and_to_be_applied_branch_exists_there() -> any )?; insta::assert_debug_snapshot!(out, @r" Outcome { + workspace_changed: false, workspace_ref_created: false, } "); // HEAD must now point to the workspace (that already existed) - insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @"* e5d0542 (HEAD -> gitbutler/workspace, B, A) A"); + insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @"* e5d0542 (HEAD -> gitbutler/workspace, main, B, A) A"); Ok(()) } @@ -177,6 +181,70 @@ fn detached_head_journey() -> anyhow::Result<()> { )?; insta::assert_debug_snapshot!(out, @r" Outcome { + workspace_changed: false, + workspace_ref_created: false, + } + "); + Ok(()) +} + +#[test] +fn auto_checkout_of_enclosing_workspace() -> anyhow::Result<()> { + let (_tmp, graph, mut repo, mut meta, _description) = + named_writable_scenario_with_description_and_graph( + "ws-ref-no-ws-commit-one-stack-one-branch", + |meta| { + add_stack_with_segments(meta, 1, "A", StackState::InWorkspace, &[]); + add_stack_with_segments(meta, 2, "B", StackState::InWorkspace, &[]); + }, + )?; + + insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @"* e5d0542 (HEAD -> gitbutler/workspace, main, B, A) A"); + + let ws = graph.to_workspace()?; + insta::assert_snapshot!(graph_workspace(&ws), @r" + πŸ“•πŸ˜οΈβš οΈ:0:gitbutler/workspace <> βœ“! on e5d0542 + β”œβ”€β”€ β‰‘πŸ“™:3:B on e5d0542 + β”‚ └── πŸ“™:3:B + └── β‰‘πŸ“™:2:A on e5d0542 + └── πŸ“™:2:A + "); + + let (b_id, b_ref) = id_at(&repo, "B"); + let graph = + but_graph::Graph::from_commit_traversal(b_id, b_ref.clone(), &meta, Default::default())?; + let ws = graph.to_workspace()?; + // TODO: fix this - the entrypoint shouldn't alter the stack setup. + insta::assert_snapshot!(graph_workspace(&ws), @r" + πŸ“•πŸ˜οΈβš οΈ:1:gitbutler/workspace <> βœ“! + └── β‰‘πŸ‘‰πŸ“™:0:B + β”œβ”€β”€ πŸ‘‰πŸ“™:0:B + └── πŸ“™:2:A + └── Β·e5d0542 (🏘️) β–Ίmain + "); + + // Already applied (the HEAD points to it, it literally IS the workspace). + let out = + but_workspace::branch::apply(b_ref.as_ref(), &ws, &mut repo, &mut meta, default_options())?; + insta::assert_debug_snapshot!(out, @r" + Outcome { + workspace_changed: false, + workspace_ref_created: false, + } + "); + + // To apply, we just checkout the surrounding workspace. + // TODO: doesn't work because A isn't a separate stack like it should. + let out = but_workspace::branch::apply( + r("refs/heads/A"), + &ws, + &mut repo, + &mut meta, + default_options(), + )?; + insta::assert_debug_snapshot!(out, @r" + Outcome { + workspace_changed: false, workspace_ref_created: false, } "); @@ -198,7 +266,6 @@ fn apply_nonexisting_branch_failure() -> anyhow::Result<()> { └── Β·e5d0542 (🏘️) β–ΊA, β–ΊB, β–Ίmain "); - // Idempotency let err = but_workspace::branch::apply( r("refs/heads/does-not-exist"), &ws, @@ -209,7 +276,7 @@ fn apply_nonexisting_branch_failure() -> anyhow::Result<()> { .unwrap_err(); assert_eq!(err.to_string(), "TBD"); - // This fails without any effective change. + // Nothing should be changed insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @"* e5d0542 (HEAD -> gitbutler/workspace, main, B, A) A"); Ok(()) } @@ -234,7 +301,7 @@ fn unborn_apply_needs_base() -> anyhow::Result<()> { └── :0:main "); - // Idempotency + // Idempotency in ad-hoc workspace let out = but_workspace::branch::apply( r("refs/heads/main"), &ws,