Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions apps/desktop/src/lib/stacks/stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,18 @@ function prettyNamedListIfPossible(expectedNames: number, names: string[]): stri
}

export function handleApplyOutcome(outcome: ApplyOutcome) {
if (outcome.status === "conflictsWithTarget") {
const files = outcome.targetConflicts;
const fileList =
files.length > 0 ? `\n\nConflicting files:\n\n${files.map((f) => `- ${f}`).join("\n")}` : "";
showWarning(
"Couldn't apply branch due to conflicts",
`It is behind the workspace target and conflicts with it. Update the branch with the latest target changes, then try applying again.${fileList}`,
undefined,
TestId.BranchApplyConflictToast,
);
return;
}
if (outcome.status !== "conflictAborted") return;
const names = outcome.conflictingStacks.map((stack) => stack.shortName);
const single = names.length === 1;
Expand Down
14 changes: 13 additions & 1 deletion apps/lite/ui/src/api/mutations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,19 @@ export const useApply = () => {
return useMutation({
mutationFn: window.lite.apply,
onSuccess: async (response, input, _context, mutation) => {
if (response.conflictingStacks.length > 0) {
if (response.status === "conflictsWithTarget") {
const files = response.targetConflicts;
const fileList =
files.length > 0
? `\n\nConflicting files:\n${files.map((f) => `- ${f}`).join("\n")}`
: "";
toastManager.add({
type: "error",
title: "Failed to apply branch",
description: `'${input.existingBranch}' is behind the workspace target and conflicts with it. Update the branch with the latest target changes, then try applying again.${fileList}`,
priority: "high",
});
} else if (response.conflictingStacks.length > 0) {
const toastId = toastManager.add({
type: "error",
title: "Failed to apply branch",
Expand Down
5 changes: 5 additions & 0 deletions apps/lite/ui/src/components/Toasts.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@
gap: 12px;
}

.description {
/* Descriptions may carry line-separated lists, e.g. conflicting files. */
white-space: pre-line;
}

.actions {
display: flex;
flex-wrap: wrap;
Expand Down
2 changes: 1 addition & 1 deletion apps/lite/ui/src/components/Toasts.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export const Toasts: FC = () => {
// Default is `p` which restricts content elements.
<div />
}
className="text-13"
className={classes("text-13", styles.description)}
/>
<div className={styles.actions}>
{toast.actionProps && <Toast.Action className={getButtonClassName({})} />}
Expand Down
9 changes: 8 additions & 1 deletion crates/but-api/src/branch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,12 +98,17 @@ pub mod json {
pub workspace_changed: bool,
/// Branches activated or recorded by the operation.
///
/// This is empty for `alreadyApplied` and `conflictAborted`, and populated for `applied`.
/// This is empty for `alreadyApplied`, `conflictAborted` and `conflictsWithTarget`, and populated for `applied`.
pub applied_branches: Vec<crate::json::FullRefName>,
/// Whether the workspace reference had to be created.
pub workspace_ref_created: bool,
/// Stacks that conflicted while applying the branch.
pub conflicting_stacks: Vec<ConflictingStack>,
/// Worktree-relative paths at which the branch conflicts with the workspace target.
///
/// Only populated for `conflictsWithTarget`.
#[cfg_attr(feature = "export-schema", schemars(with = "Vec<String>"))]
pub target_conflicts: Vec<but_serde::BStringForFrontend>,
}

/// A stack that conflicted while applying a branch.
Expand Down Expand Up @@ -131,13 +136,15 @@ pub mod json {
workspace_ref_created,
workspace_merge: _,
conflicting_stacks,
target_conflicts,
} = value;

ApplyOutcome {
status,
workspace_changed,
applied_branches: applied_branches.into_iter().map(Into::into).collect(),
workspace_ref_created,
target_conflicts: target_conflicts.into_iter().map(Into::into).collect(),
conflicting_stacks: conflicting_stacks
.into_iter()
.map(|stack| {
Expand Down
93 changes: 91 additions & 2 deletions crates/but-workspace/src/branch/apply.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ pub enum OutcomeStatus {
Applied,
/// A workspace merge was attempted and conflicts prevented persistence.
ConflictAborted,
/// The branch conflicts with the workspace target it is based behind, so no stack is to blame.
/// The branch itself needs updating with the target changes before it can be applied.
ConflictsWithTarget,
}
#[cfg(feature = "export-schema")]
but_schemars::register_sdk_type!(OutcomeStatus);
Expand All @@ -47,6 +50,7 @@ impl OutcomeStatus {
OutcomeStatus::AlreadyApplied => "alreadyApplied",
OutcomeStatus::Applied => "applied",
OutcomeStatus::ConflictAborted => "conflictAborted",
OutcomeStatus::ConflictsWithTarget => "conflictsWithTarget",
}
}

Expand Down Expand Up @@ -86,6 +90,10 @@ pub struct Outcome {
/// Each entry includes the stable stack id and its tip ref name, so callers don't have to
/// recover names from the returned workspace graph.
pub conflicting_stacks: Vec<ConflictingStack>,
/// Worktree-relative paths at which the branch conflicts with the workspace target.
///
/// Only populated when [Outcome::status] is [OutcomeStatus::ConflictsWithTarget].
pub target_conflicts: Vec<bstr::BString>,
}

impl Outcome {
Expand All @@ -104,6 +112,7 @@ impl std::fmt::Debug for Outcome {
workspace_ref_created,
workspace_merge: _,
conflicting_stacks,
target_conflicts,
applied_branches,
} = self;
let mut f = f.debug_struct("Outcome");
Expand All @@ -123,6 +132,19 @@ impl std::fmt::Debug for Outcome {
if !conflicting_stacks.is_empty() {
f.field("conflicting_stacks", conflicting_stacks);
}
if !target_conflicts.is_empty() {
f.field(
"target_conflicts",
&format!(
"[{}]",
target_conflicts
.iter()
.map(|path| path.to_string())
.collect::<Vec<_>>()
.join(", ")
),
);
}
f.finish()
}
}
Expand Down Expand Up @@ -191,7 +213,7 @@ use tracing::instrument;
use crate::{
WorkspaceCommit,
branch::{anon_stacks, ensure_no_missing_stacks},
commit::merge::Tip,
commit::merge::{Tip, peel_to_tree},
ref_info::WorkspaceExt,
};

Expand Down Expand Up @@ -269,6 +291,7 @@ pub fn apply(
workspace_ref_created,
workspace_merge: None,
conflicting_stacks: Vec::new(),
target_conflicts: Vec::new(),
applied_branches: Vec::new(),
});
} else if !branch_has_applied_metadata && ws.refname_is_segment(branch.as_ref()) {
Expand Down Expand Up @@ -316,6 +339,7 @@ pub fn apply(
workspace_ref_created: false,
workspace_merge: None,
conflicting_stacks: Vec::new(),
target_conflicts: Vec::new(),
applied_branches,
});
};
Expand Down Expand Up @@ -551,6 +575,7 @@ pub fn apply(
workspace_ref_created: needs_ws_ref_creation,
workspace_merge: None,
conflicting_stacks: Vec::new(),
target_conflicts: Vec::new(),
applied_branches,
});
}
Expand All @@ -562,13 +587,30 @@ pub fn apply(
);
}

let mut in_memory_repo = repo.clone().for_tree_diffing()?.with_object_memory();
// The workspace merge only knows stack tips, so a branch that conflicts with the target
// it is based behind would blame whatever stacks stand between it and the target delta -
// even empty ones. Detect that case upfront and attribute it to the target instead.
if on_workspace_conflict.should_abort()
&& let Some(target_conflicts) =
branch_conflicts_with_target(&ws, branch.as_ref(), &in_memory_repo)?
{
return Ok(Outcome {
workspace: ws,
status: OutcomeStatus::ConflictsWithTarget,
workspace_ref_created: false,
workspace_merge: None,
conflicting_stacks: Vec::new(),
target_conflicts,
applied_branches: Vec::new(),
});
}
let existing_stacks_superseded_by_branch =
find_superseded_stacks(branch.as_ref(), &ws, &mut ws_md);
// At this point, the workspace-metadata already knows the new branch(es), but the workspace itself
// doesn't see one or more of to-be-applied branches (to become stacks).
// These are, however, part of the graph by now, and we want to try to create a workspace
// merge.
let mut in_memory_repo = repo.clone().for_tree_diffing()?.with_object_memory();
let mut merge_result = WorkspaceCommit::from_new_merge_with_metadata(
filter_superseded_metadata_stacks(
ws_md.stacks.iter(),
Expand All @@ -594,6 +636,7 @@ pub fn apply(
workspace_ref_created: false,
workspace_merge: Some(merge_result),
conflicting_stacks,
target_conflicts: Vec::new(),
applied_branches: Vec::new(),
});
}
Expand Down Expand Up @@ -708,6 +751,7 @@ pub fn apply(
workspace_ref_created: false,
workspace_merge: Some(merge_result),
conflicting_stacks,
target_conflicts: Vec::new(),
applied_branches: Vec::new(),
});
}
Expand Down Expand Up @@ -775,10 +819,55 @@ pub fn apply(
workspace_ref_created: needs_ws_ref_creation,
workspace_merge: Some(merge_result),
conflicting_stacks,
target_conflicts: Vec::new(),
applied_branches,
})
}

/// Return the worktree-relative paths at which the tree of `branch` conflicts with the
/// workspace's integration frame - the target commit if set, or the workspace lower bound
/// otherwise - when merged from their common ancestor.
///
/// This is `None` whenever there is no conflict or the question cannot be answered, e.g.
/// without a frame or with `branch` missing from the graph, so callers fall back to the
/// ordinary stack merge.
fn branch_conflicts_with_target(
ws: &but_graph::Workspace,
branch: &FullNameRef,
repo: &gix::Repository,
) -> anyhow::Result<Option<Vec<bstr::BString>>> {
let frame = (ws.target_commit.as_ref())
.map(|target| (target.segment_index, target.commit_id))
.or(ws.lower_bound_segment_id.zip(ws.lower_bound));
let branch_tip = ws.graph.segment_and_commit_by_ref_name(branch);
let (Some((frame_sidx, frame_id)), Some((branch_segment, branch_commit))) = (frame, branch_tip)
else {
return Ok(None);
};
let Some(base) = (ws.graph.find_merge_base(frame_sidx, branch_segment.id))
.and_then(|base_sidx| ws.graph.tip_skip_empty(base_sidx))
else {
return Ok(None);
};

// No fail-fast here - the conflicting paths are reported, so all of them are wanted.
let merge = repo.merge_trees(
peel_to_tree(base.id.attach(repo))?,
peel_to_tree(frame_id.attach(repo))?,
peel_to_tree(branch_commit.id.attach(repo))?,
repo.default_merge_labels(),
repo.tree_merge_options()?,
)?;
let conflict_kind = gix::merge::tree::TreatAsUnresolved::git();
let conflicting_paths: Vec<_> = merge
.conflicts
.iter()
.filter(|conflict| conflict.is_unresolved(conflict_kind))
.map(|conflict| conflict.ours.location().to_owned())
.collect();
Ok((!conflicting_paths.is_empty()).then_some(conflicting_paths))
Comment on lines +853 to +868
}

/// Map conflicting merge tips back to workspace stack metadata.
///
/// Merge conflicts report the tip ref names that could not be merged. This function resolves each
Expand Down
2 changes: 1 addition & 1 deletion crates/but-workspace/src/commit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,7 @@ pub mod merge {
Ok((peel_to_tree(base_commit_id)?, base_sidx))
}

fn peel_to_tree(commit: gix::Id) -> anyhow::Result<gix::ObjectId> {
pub(crate) fn peel_to_tree(commit: gix::Id) -> anyhow::Result<gix::ObjectId> {
let commit = but_core::Commit::from_id(commit)?;
Ok(commit.tree_id_or_auto_resolution()?.detach())
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
#!/usr/bin/env bash

set -eu -o pipefail
source "${BASH_SOURCE[0]%/*}/shared.sh"

### Description
# The workspace is an empty commit on an advanced target, with `lane-1` and `lane-2` parked
# on the base without content of their own. `hero` and `hero-clean` are based on the previous
# target position; `hero` changes the same file the target advanced with, `hero-clean` only
# adds an unrelated file.
git init
tick
echo original >shared.txt && echo original >shared2.txt && git add shared.txt shared2.txt && git commit -m M1
setup_target_to_match_main

git checkout -b hero main
tick
echo hero-change >shared.txt && echo hero-change >shared2.txt && git commit -am "hero: change shared files"

git checkout -b hero-clean main
tick
commit-file unrelated.txt

git checkout main
tick
echo target-advance >shared.txt && echo target-advance >shared2.txt && git commit -am "target: change shared files"
git update-ref refs/remotes/origin/main main

git branch lane-1 main
git branch lane-2 main

tick
create_workspace_commit_once
Loading
Loading