Skip to content

Commit 954f74d

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

File tree

7 files changed

+177
-13
lines changed

7 files changed

+177
-13
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: 35 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -353,24 +353,47 @@ pub(crate) mod function {
353353
});
354354
}
355355

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-
}
364356
let prev_head_id = graph
365357
.entrypoint_commit()
366358
.context("BUG: how is it possible that there is no head commit?")?
367359
.id;
368360
let new_head_id = merge_result.workspace_commit_id;
369361
let graph = graph.redo_traversal_with_overlay(
370-
repo,
362+
&in_memory_repo,
371363
meta,
372364
overlay.with_entrypoint(new_head_id, Some(workspace_ref_name_to_update.clone())),
373365
)?;
366+
367+
let workspace = graph.to_workspace()?;
368+
let unapplied_branches: Vec<_> = branches_to_apply
369+
.iter()
370+
.filter_map(|rn| {
371+
if workspace.refname_is_segment(rn) {
372+
None
373+
} else {
374+
Some(rn)
375+
}
376+
})
377+
.collect();
378+
if !unapplied_branches.is_empty() {
379+
bail!(
380+
"Unexpectedly failed to apply {branches} which is/are still not in the workspace",
381+
branches = unapplied_branches
382+
.iter()
383+
.map(|rn| rn.shorten().to_string())
384+
.collect::<Vec<_>>()
385+
.join(", ")
386+
)
387+
}
388+
389+
// All work is done, persist and exit.
390+
// Note that it could be that some stacks aren't merged in,
391+
// while being present in the workspace metadata.
392+
// This is OK for us. We also trust that the hero-branch was merged in, no matter what.
393+
if let Some(storage) = in_memory_repo.objects.take_object_memory() {
394+
storage.persist(repo)?;
395+
drop(in_memory_repo);
396+
}
374397
persist_metadata(meta, &branches_to_apply, &ws_md)?;
375398
crate::branch::safe_checkout(
376399
prev_head_id,
@@ -416,10 +439,9 @@ pub(crate) mod function {
416439
new_ref_target: gix::ObjectId,
417440
new_ref: Option<&gix::refs::FullNameRef>,
418441
) -> 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();
421442
let edits = match new_ref {
422443
None => {
444+
let head_message = "GitButler checkout workspace during apply-branch".into();
423445
vec![RefEdit {
424446
change: Change::Update {
425447
log: LogChange {
@@ -435,6 +457,8 @@ pub(crate) mod function {
435457
}]
436458
}
437459
Some(new_ref) => {
460+
// This also means we want HEAD to point to it.
461+
let head_message = "GitButler switch to workspace during apply-branch".into();
438462
vec![
439463
RefEdit {
440464
change: Change::Update {
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
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+
commit A2
16+
git branch B
17+
18+
git checkout main

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

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -521,6 +521,90 @@ 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 (B, A) A2
533+
* 7076dee 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
561+
└── ·7076dee (🏘️)
562+
");
563+
// TODO: main should stay on 85ef!
564+
insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @r"
565+
* 6a706b7 (HEAD -> gitbutler/workspace, main) GitButler Workspace Commit
566+
* f084d61 (B, A) A2
567+
* 7076dee A1
568+
* 85efbe4 (origin/main) M
569+
");
570+
571+
// Apply `B` - the only sane way is to make it its own stack, but allow it to diverge.
572+
let out =
573+
but_workspace::branch::apply(r("refs/heads/B"), &ws, &repo, &mut meta, default_options())
574+
.expect("apply actually works");
575+
insta::assert_debug_snapshot!(out, @r"
576+
Outcome {
577+
workspace_changed: true,
578+
workspace_ref_created: false,
579+
}
580+
");
581+
582+
// TODO: Why does it fail to do that, but still claims it's present?
583+
let graph = out.graph;
584+
let ws = graph.to_workspace()?;
585+
insta::assert_snapshot!(graph_workspace(&ws), @r"
586+
📕🏘️:0:gitbutler/workspace <> ✓refs/remotes/origin/main on 85efbe4
587+
├── ≡📙:4:A on 85efbe4
588+
│ └── 📙:4:A
589+
│ ├── ·f084d61 (🏘️) ►B
590+
│ └── ·7076dee (🏘️)
591+
└── ≡📙:4:A on 85efbe4
592+
└── 📙:4:A
593+
├── ·f084d61 (🏘️) ►B
594+
└── ·7076dee (🏘️)
595+
");
596+
insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @r"
597+
* ccbee14 (HEAD -> gitbutler/workspace) GitButler Workspace Commit
598+
|\
599+
| * 6a706b7 (main) GitButler Workspace Commit
600+
|/
601+
* f084d61 (B, A) A2
602+
* 7076dee A1
603+
* 85efbe4 (origin/main) M
604+
");
605+
Ok(())
606+
}
607+
524608
#[test]
525609
fn auto_checkout_of_enclosing_workspace_flat() -> anyhow::Result<()> {
526610
let (_tmp, graph, repo, mut meta, _description) =

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)