Skip to content

Commit bf8d145

Browse files
committed
but-testing apply to test new implementation more easily in the real-world.
1 parent 62f98d6 commit bf8d145

File tree

10 files changed

+178
-14
lines changed

10 files changed

+178
-14
lines changed

crates/but-testing/src/args.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,14 @@ pub struct Args {
3636

3737
#[derive(Debug, clap::Subcommand)]
3838
pub enum Subcommands {
39+
/// Make an existing branch appear in the workspace.
40+
Apply {
41+
/// The 0-based place in the worktree into which the branch should be inserted.
42+
#[clap(long, short = 'o')]
43+
order: Option<usize>,
44+
/// The name of the branch to apply to the workspace, like `feature` or `origin/other`.
45+
branch_name: String,
46+
},
3947
/// Add the given Git repository as project for use with GitButler.
4048
AddProject {
4149
/// The long name of the remote reference to track, like `refs/remotes/origin/main`,

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

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@ use but_core::UnifiedDiff;
33
use but_db::poll::ItemKind;
44
use but_graph::VirtualBranchesTomlMetadata;
55
use but_settings::AppSettings;
6+
use but_workspace::branch::OnWorkspaceMergeConflict;
7+
use but_workspace::branch::apply::{IntegrationMode, WorkspaceReferenceNaming};
8+
use but_workspace::branch::checkout::UncommitedWorktreeChanges;
69
use but_workspace::branch::create_reference::{Anchor, Position};
710
use but_workspace::{DiffSpec, HunkHeader};
811
use gitbutler_project::{Project, ProjectId};
@@ -634,6 +637,32 @@ pub fn remove_reference(
634637
Ok(())
635638
}
636639

640+
pub fn apply(args: &super::Args, short_name: &str, order: Option<usize>) -> anyhow::Result<()> {
641+
let (repo, project, graph, mut meta) =
642+
repo_and_maybe_project_and_graph(args, RepositoryOpenMode::Merge)?;
643+
let branch = repo.find_reference(short_name)?;
644+
let ws = graph.to_workspace()?;
645+
_ = but_workspace::branch::apply(
646+
branch.name(),
647+
&ws,
648+
&repo,
649+
&mut *meta,
650+
but_workspace::branch::apply::Options {
651+
integration_mode: IntegrationMode::AlwaysMerge,
652+
on_workspace_conflict: OnWorkspaceMergeConflict::MaterializeAndReportConflictingStacks,
653+
workspace_reference_naming: WorkspaceReferenceNaming::Default,
654+
uncommitted_changes: UncommitedWorktreeChanges::KeepAndAbortOnConflict,
655+
order,
656+
},
657+
)?;
658+
659+
if project.is_some() {
660+
// write metadata if there are projects - this is a special case while we use vb.toml.
661+
ManuallyDrop::into_inner(meta);
662+
}
663+
Ok(())
664+
}
665+
637666
pub fn create_reference(
638667
args: &super::Args,
639668
short_name: &str,

crates/but-testing/src/main.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use command::parse_diff_spec;
77
use gix::bstr::BString;
88

99
mod args;
10+
use crate::args::Subcommands;
1011
use crate::command::{RepositoryOpenMode, repo_and_maybe_project};
1112
use args::Args;
1213

@@ -31,6 +32,7 @@ async fn main() -> Result<()> {
3132
}
3233

3334
match &args.cmd {
35+
Subcommands::Apply { branch_name, order } => command::apply(&args, branch_name, *order),
3436
args::Subcommands::AddProject {
3537
switch_to_workspace,
3638
path,
@@ -39,7 +41,6 @@ async fn main() -> Result<()> {
3941
path.to_owned(),
4042
switch_to_workspace.to_owned(),
4143
),
42-
4344
args::Subcommands::RemoveReference {
4445
permit_empty_stacks,
4546
keep_metadata,

crates/but-workspace/src/branch/apply.rs

Lines changed: 37 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ pub(crate) mod function {
157157
repo,
158158
checkout::Options {
159159
uncommitted_changes,
160+
skip_head_update: false,
160161
},
161162
)?;
162163
let graph = workspace.graph.redo_traversal_with_overlay(
@@ -353,31 +354,55 @@ pub(crate) mod function {
353354
});
354355
}
355356

356-
// All work is done, persist and exit.
357-
// Note that it could be that some stacks aren't merged in,
358-
// while being present in the workspace metadata.
359-
// This is OK for us. We also trust that the hero-branch was merged in, no matter what.
360-
if let Some(storage) = in_memory_repo.objects.take_object_memory() {
361-
storage.persist(repo)?;
362-
drop(in_memory_repo);
363-
}
364357
let prev_head_id = graph
365358
.entrypoint_commit()
366359
.context("BUG: how is it possible that there is no head commit?")?
367360
.id;
368361
let new_head_id = merge_result.workspace_commit_id;
369362
let graph = graph.redo_traversal_with_overlay(
370-
repo,
363+
&in_memory_repo,
371364
meta,
372365
overlay.with_entrypoint(new_head_id, Some(workspace_ref_name_to_update.clone())),
373366
)?;
367+
368+
let workspace = graph.to_workspace()?;
369+
let unapplied_branches: Vec<_> = branches_to_apply
370+
.iter()
371+
.filter_map(|rn| {
372+
if workspace.refname_is_segment(rn) {
373+
None
374+
} else {
375+
Some(rn)
376+
}
377+
})
378+
.collect();
379+
if !unapplied_branches.is_empty() {
380+
bail!(
381+
"Unexpectedly failed to apply {branches} which is/are still not in the workspace",
382+
branches = unapplied_branches
383+
.iter()
384+
.map(|rn| rn.shorten().to_string())
385+
.collect::<Vec<_>>()
386+
.join(", ")
387+
)
388+
}
389+
390+
// All work is done, persist and exit.
391+
// Note that it could be that some stacks aren't merged in,
392+
// while being present in the workspace metadata.
393+
// This is OK for us. We also trust that the hero-branch was merged in, no matter what.
394+
if let Some(storage) = in_memory_repo.objects.take_object_memory() {
395+
storage.persist(repo)?;
396+
drop(in_memory_repo);
397+
}
374398
persist_metadata(meta, &branches_to_apply, &ws_md)?;
375399
crate::branch::safe_checkout(
376400
prev_head_id,
377401
new_head_id,
378402
repo,
379403
checkout::Options {
380404
uncommitted_changes,
405+
skip_head_update: true,
381406
},
382407
)?;
383408

@@ -416,10 +441,9 @@ pub(crate) mod function {
416441
new_ref_target: gix::ObjectId,
417442
new_ref: Option<&gix::refs::FullNameRef>,
418443
) -> anyhow::Result<()> {
419-
// This also means we want HEAD to point to it.
420-
let head_message = "GitButler switch to workspace during apply-branch".into();
421444
let edits = match new_ref {
422445
None => {
446+
let head_message = "GitButler checkout workspace during apply-branch".into();
423447
vec![RefEdit {
424448
change: Change::Update {
425449
log: LogChange {
@@ -435,6 +459,8 @@ pub(crate) mod function {
435459
}]
436460
}
437461
Some(new_ref) => {
462+
// This also means we want HEAD to point to it.
463+
let head_message = "GitButler switch to workspace during apply-branch".into();
438464
vec![
439465
RefEdit {
440466
change: Change::Update {

crates/but-workspace/src/branch/checkout/function.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ pub fn safe_checkout(
5050
repo: &gix::Repository,
5151
Options {
5252
uncommitted_changes: conflicting_worktree_changes_opts,
53+
skip_head_update,
5354
}: Options,
5455
) -> anyhow::Result<Outcome> {
5556
let source_tree = current_head_id.attach(repo).object()?.peel_to_tree()?;
@@ -138,7 +139,7 @@ pub fn safe_checkout(
138139
}
139140

140141
let mut head_update = None;
141-
if new_object.kind.is_commit() {
142+
if new_object.kind.is_commit() && !skip_head_update {
142143
let needs_update = repo
143144
.head()?
144145
.id()

crates/but-workspace/src/branch/checkout/mod.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ pub enum UncommitedWorktreeChanges {
1717
pub struct Options {
1818
/// How to deal with uncommitted changes.
1919
pub uncommitted_changes: UncommitedWorktreeChanges,
20+
/// If `true`, do not change `HEAD` to the new commit.
21+
///
22+
/// This is typically to be avoided, but may be used if you want to change the HEAD location yourself.
23+
pub skip_head_update: bool,
2024
}
2125

2226
/// The successful outcome of [super::safe_checkout()] operation.
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
#!/usr/bin/env bash
2+
3+
set -eu -o pipefail
4+
5+
source "${BASH_SOURCE[0]%/*}/shared.sh"
6+
7+
### General Description
8+
9+
# A target branch along with a stack and a depdendent branch at the same tip, without workspace commit.
10+
git init
11+
commit M
12+
setup_target_to_match_main
13+
git checkout -b A
14+
commit A1
15+
git branch D
16+
git branch E
17+
commit A2
18+
git branch B
19+
git branch C
20+
21+
git checkout main

crates/but-workspace/tests/workspace/branch/apply_unapply_commit_uncommit.rs

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -521,6 +521,79 @@ fn detached_head_journey() -> anyhow::Result<()> {
521521
Ok(())
522522
}
523523

524+
#[test]
525+
fn apply_two_ambiguous_stacks_with_target() -> anyhow::Result<()> {
526+
let (_tmp, graph, repo, mut meta, _description) =
527+
named_writable_scenario_with_description_and_graph(
528+
"no-ws-ref-stack-and-dependent-branch",
529+
|_meta| {},
530+
)?;
531+
insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @r"
532+
* f084d61 (C, B, A) A2
533+
* 7076dee (E, D) A1
534+
* 85efbe4 (HEAD -> main, origin/main) M
535+
");
536+
537+
let ws = graph.to_workspace()?;
538+
insta::assert_snapshot!(graph_workspace(&ws), @r"
539+
⌂:0:main <> ✓!
540+
└── ≡:0:main
541+
└── :0:main
542+
└── ·85efbe4
543+
");
544+
545+
// Apply `A` first.
546+
let out =
547+
but_workspace::branch::apply(r("refs/heads/A"), &ws, &repo, &mut meta, default_options())?;
548+
insta::assert_debug_snapshot!(out, @r"
549+
Outcome {
550+
workspace_changed: true,
551+
workspace_ref_created: true,
552+
}
553+
");
554+
let graph = out.graph;
555+
let ws = graph.to_workspace()?;
556+
insta::assert_snapshot!(graph_workspace(&ws), @r"
557+
📕🏘️:0:gitbutler/workspace <> ✓refs/remotes/origin/main on 85efbe4
558+
└── ≡📙:3:A on 85efbe4
559+
└── 📙:3:A
560+
├── ·f084d61 (🏘️) ►B, ►C
561+
└── ·7076dee (🏘️) ►D, ►E
562+
");
563+
insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @r"
564+
* 6a706b7 (HEAD -> gitbutler/workspace) GitButler Workspace Commit
565+
* f084d61 (C, B, A) A2
566+
* 7076dee (E, D) A1
567+
* 85efbe4 (origin/main, main) M
568+
");
569+
570+
// Apply `B` - the only sane way is to make it its own stack, but allow it to diverge.
571+
let out =
572+
but_workspace::branch::apply(r("refs/heads/B"), &ws, &repo, &mut meta, default_options())
573+
.expect("apply actually works");
574+
insta::assert_debug_snapshot!(out, @r"
575+
Outcome {
576+
workspace_changed: true,
577+
workspace_ref_created: false,
578+
}
579+
");
580+
581+
// TODO: Make it work as dependent branch
582+
let graph = out.graph;
583+
let ws = graph.to_workspace()?;
584+
insta::assert_snapshot!(graph_workspace(&ws), @r"");
585+
insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @r"
586+
* ccbee14 (HEAD -> gitbutler/workspace) GitButler Workspace Commit
587+
|\
588+
| * 6a706b7 (main) GitButler Workspace Commit
589+
|/
590+
* f084d61 (B, A) A2
591+
* 7076dee A1
592+
* 85efbe4 (origin/main) M
593+
");
594+
Ok(())
595+
}
596+
524597
#[test]
525598
fn auto_checkout_of_enclosing_workspace_flat() -> anyhow::Result<()> {
526599
let (_tmp, graph, repo, mut meta, _description) =

crates/but-workspace/tests/workspace/branch/checkout.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -827,6 +827,7 @@ fn unrelated_additions_do_not_affect_worktree_changes() -> anyhow::Result<()> {
827827
fn overwrite_options() -> checkout::Options {
828828
checkout::Options {
829829
uncommitted_changes: UncommitedWorktreeChanges::KeepConflictingInSnapshotAndOverwrite,
830+
skip_head_update: false,
830831
}
831832
}
832833

crates/gitbutler-branch-actions/src/branch_manager/branch_creation.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ impl BranchManager<'_> {
168168
let applied_branch_stack_id = ws
169169
.find_segment_and_stack_by_refname(branch_to_apply)
170170
.with_context(||
171-
format!("Failed to apply branch and we probably have to handle more of the outcome\n{out:?}")
171+
format!("BUG: Can't find the branch to apply in workspace, but the 'apply' function should have failed instead \n{out:?}")
172172
)?
173173
.0
174174
.id

0 commit comments

Comments
 (0)