Skip to content

Commit 62053e0

Browse files
committed
Add tests for head_info() for common managed workspace scenarios.
1 parent a32664d commit 62053e0

File tree

12 files changed

+339
-102
lines changed

12 files changed

+339
-102
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/but-workspace/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,3 +46,4 @@ but-core = { workspace = true, features = ["testing"] }
4646
# TODO: remove once `gitbutler-repo` isn't needed anymore.
4747
gitbutler-commit = { workspace = true, features = ["testing"] }
4848
regex = "1.11.1"
49+
gitbutler-reference.workspace = true

crates/but-workspace/src/branch.rs

Lines changed: 50 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -508,6 +508,7 @@ pub struct Stack {
508508
/// If there is an integration branch, we know a base commit shared with the integration branch from
509509
/// which we branched off.
510510
/// Otherwise, it's the merge-base of all stacks in the current workspace.
511+
/// It is `None` if this is a stack derived from a branch without relation to any other branch.
511512
pub base: Option<gix::ObjectId>,
512513
/// The branch-name denoted segments of the stack from its tip to the point of reference, typically a merge-base.
513514
/// This array is never empty.
@@ -518,6 +519,7 @@ pub struct Stack {
518519
/// using Git commands.
519520
///
520521
/// The backend auto-applies floating stashes, but if that didn't happen, the frontend may guide the user.
522+
// TODO: refactor/remove this in favor of special stash commits.
521523
pub stash_status: Option<StashStatus>,
522524
}
523525

@@ -737,14 +739,17 @@ pub enum RefLocation {
737739
}
738740

739741
/// A list of all commits in a stack segment of a [`Stack`].
740-
#[derive(Default, Debug, Clone)]
742+
#[derive(Default, Clone)]
741743
pub struct StackSegment {
742744
/// The name of the branch at the tip of it, and the starting point of the walk.
743745
///
744746
/// It is `None` if this branch is the top-most stack segment and the `ref_name` wasn't pointing to
745747
/// a commit anymore that was reached by our rev-walk.
746748
/// This can happen if the ref is deleted, or if it was advanced by other means.
747749
pub ref_name: Option<gix::refs::FullName>,
750+
/// The name of the remote tracking branch of this segment, if present, i.e. `refs/remotes/origin/main`.
751+
/// Its presence means that a remote is configured and that the stack content
752+
pub remote_tracking_ref_name: Option<gix::refs::FullName>,
748753
/// Specify where the `ref_name` is specifically, or `None` if there is no ref-name.
749754
pub ref_location: Option<RefLocation>,
750755
/// The portion of commits that can be reached from the tip of the *branch* downwards, so that they are unique
@@ -760,15 +765,56 @@ pub struct StackSegment {
760765
/// no derived value to make this visible explicitly.
761766
// TODO: review this - should branch divergence be a thing? Rare, but not impossible.
762767
pub commits_unique_in_remote_tracking_branch: Vec<RemoteCommit>,
763-
/// The name of the remote tracking branch of this segment, if present, i.e. `refs/remotes/origin/main`.
764-
/// Its presence means that a remote is configured and that the stack content
765-
pub remote_tracking_ref_name: Option<gix::refs::FullName>,
766768
/// Metadata with additional information, or `None` if nothing was present.
767769
///
768770
/// Primary use for this is the consumer, as edits are forced to be made on 'connected' data, so refetching is necessary.
769771
pub metadata: Option<but_core::ref_metadata::Branch>,
770772
}
771773

774+
impl std::fmt::Debug for StackSegment {
775+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
776+
let StackSegment {
777+
ref_name,
778+
ref_location,
779+
commits_unique_from_tip,
780+
commits_unique_in_remote_tracking_branch,
781+
remote_tracking_ref_name,
782+
metadata,
783+
} = self;
784+
f.debug_struct("StackSegment")
785+
.field(
786+
"ref_name",
787+
&match ref_name.as_ref() {
788+
None => "None".to_string(),
789+
Some(name) => name.to_string(),
790+
},
791+
)
792+
.field(
793+
"remote_tracking_ref_name",
794+
&match remote_tracking_ref_name.as_ref() {
795+
None => "None".to_string(),
796+
Some(name) => name.to_string(),
797+
},
798+
)
799+
.field(
800+
"ref_location",
801+
&match ref_location {
802+
None => "None".to_string(),
803+
Some(location) => {
804+
format!("{:?}", location)
805+
}
806+
},
807+
)
808+
.field("commits_unique_from_tip", &commits_unique_from_tip)
809+
.field(
810+
"commits_unique_in_remote_tracking_branch",
811+
&commits_unique_in_remote_tracking_branch,
812+
)
813+
.field("metadata", &metadata)
814+
.finish()
815+
}
816+
}
817+
772818
/// Return all stack segments within the given `stack`.
773819
pub fn stack_segments(stack: Stack) -> anyhow::Result<Vec<StackSegment>> {
774820
todo!()

crates/but-workspace/src/commit.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,9 @@ impl WorkspaceCommit<'_> {
9393
/// once another branch is added to the workspace.
9494
pub fn is_managed(&self) -> bool {
9595
let message = gix::objs::commit::MessageRef::from_bytes(&self.message);
96-
message.title == Self::GITBUTLER_INTEGRATION_COMMIT_TITLE
97-
|| message.title == Self::GITBUTLER_WORKSPACE_COMMIT_TITLE
96+
let title = message.title.trim().as_bstr();
97+
title == Self::GITBUTLER_INTEGRATION_COMMIT_TITLE
98+
|| title == Self::GITBUTLER_WORKSPACE_COMMIT_TITLE
9899
}
99100
}
100101

crates/but-workspace/src/head_info.rs

Lines changed: 98 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/// Options for the [`head_info()`](crate::head_info) call.
2-
#[derive(Debug, Copy, Clone)]
2+
#[derive(Default, Debug, Copy, Clone)]
33
pub struct Options {
44
/// The maximum amount of commits to list *per stack*. Note that a [`StackSegment`](crate::branch::StackSegment) will always have a single commit, if available,
55
/// even if this exhausts the commit limit in that stack.
@@ -17,7 +17,7 @@ pub(crate) mod function {
1717
use crate::HeadInfo;
1818
use crate::branch::{LocalCommit, RefLocation, Stack, StackSegment};
1919
use but_core::ref_metadata::ValueInfo;
20-
use gix::prelude::ReferenceExt;
20+
use gix::prelude::{ObjectIdExt, ReferenceExt};
2121
use gix::revision::walk::Sorting;
2222
use tracing::instrument;
2323

@@ -66,10 +66,10 @@ pub(crate) mod function {
6666
};
6767

6868
let ws_data = workspace_data_of_workspace_branch(meta, existing_ref.name())?;
69-
let target_ref = if let Some(_data) = ws_data {
70-
todo!(
71-
"figure out what to do with workspace information, consolidate it with what's there as well"
72-
);
69+
let target_ref = if let Some(data) = ws_data {
70+
// TODO: figure out what to do with workspace information, consolidate it with what's there as well
71+
// to know which branch is where.
72+
data.target_ref
7373
} else {
7474
None
7575
};
@@ -79,33 +79,104 @@ pub(crate) mod function {
7979
id: head_commit.id(),
8080
inner: head_commit.decode()?.to_owned(),
8181
};
82-
if head_commit.is_managed() {
83-
todo!("deal with managed commits");
82+
let refs_by_id = collect_refs_by_commit_id(repo)?;
83+
let target_ref_id = target_ref
84+
.as_ref()
85+
.and_then(|r| repo.try_find_reference(r).transpose())
86+
.transpose()?
87+
.map(|mut r| r.peel_to_id_in_place())
88+
.transpose()?;
89+
let cache = repo.commit_graph_if_enabled()?;
90+
let mut graph = repo.revision_graph(cache.as_ref());
91+
let base =
92+
if target_ref_id.is_none() {
93+
Some(repo.merge_base_octopus_with_graph(
94+
head_commit.parents.iter().cloned(),
95+
&mut graph,
96+
)?)
97+
} else {
98+
None
99+
};
100+
let stacks = if head_commit.is_managed() {
101+
// The commits we have already associated with a stack segment.
102+
let mut associated_commits = gix::hashtable::HashSet::default();
103+
let mut stacks = Vec::new();
104+
for (index, commit_id) in head_commit.parents.iter().enumerate() {
105+
let tip = *commit_id;
106+
let base = base
107+
.map(Ok)
108+
.or_else(|| {
109+
target_ref_id
110+
.map(|target_id| repo.merge_base_with_graph(target_id, tip, &mut graph))
111+
})
112+
.transpose()?
113+
.map(|base| base.detach());
114+
associated_commits.extend(base);
115+
let segments = collect_stack_segments(
116+
tip.attach(repo),
117+
refs_by_id
118+
.get(&tip)
119+
.and_then(|refs| refs.first().map(|r| r.as_ref())),
120+
Some(RefLocation::ReachableFromWorkspaceCommit),
121+
&associated_commits, /* boundary commits */
122+
// TODO: get from workspace information maybe?
123+
&[], /* preferred refs */
124+
options.stack_commit_limit,
125+
&refs_by_id,
126+
)?;
127+
128+
associated_commits.extend(segments.iter().flat_map(|segment| {
129+
segment.commits_unique_from_tip.iter().map(|c| c.id).chain(
130+
segment
131+
.commits_unique_in_remote_tracking_branch
132+
.iter()
133+
.map(|c| c.id),
134+
)
135+
}));
136+
137+
stacks.push(Stack {
138+
index,
139+
tip: Some(tip),
140+
segments,
141+
base,
142+
// TODO: but as part of the commits.
143+
stash_status: None,
144+
})
145+
}
146+
stacks
84147
} else {
85148
// Discover all references that actually point to the reachable graph.
86-
let refs_by_id = collect_refs_by_commit_id(repo)?;
149+
let boundary = {
150+
let mut hs = gix::hashtable::HashSet::default();
151+
hs.extend(target_ref_id.map(|id| id.detach()));
152+
hs
153+
};
154+
let tip = head_commit.id;
87155
let segments = collect_stack_segments(
88-
head_commit.id,
156+
tip,
89157
Some(existing_ref.name()),
90158
Some(RefLocation::AtHead),
91-
&[], /* boundary commits */
159+
&boundary, /* boundary commits */
92160
&[existing_ref.name().to_owned()], /* preferred refs */
93161
options.stack_commit_limit,
94162
&refs_by_id,
95163
)?;
96-
Ok(HeadInfo {
97-
stacks: vec![Stack {
98-
index: 0,
99-
tip: segments
100-
.first()
101-
.and_then(|stack| Some(stack.commits_unique_from_tip.first()?.id)),
102-
base: None,
103-
segments,
104-
stash_status: None,
105-
}],
106-
target_ref,
107-
})
108-
}
164+
165+
let base = target_ref_id
166+
.map(|target_id| repo.merge_base_with_graph(target_id, tip, &mut graph))
167+
.transpose()?
168+
.map(|base| base.detach());
169+
vec![Stack {
170+
index: 0,
171+
tip: Some(tip.detach()),
172+
// TODO: compute base if target-ref is available, but only if this isn't the target ref!
173+
base,
174+
segments,
175+
stash_status: None,
176+
}]
177+
};
178+
179+
Ok(HeadInfo { stacks, target_ref })
109180
}
110181

111182
/// Walk down the commit-graph from `tip` until a `boundary_commits` is encountered, excluding it, or to the graph root if there is no boundary.
@@ -116,11 +187,12 @@ pub(crate) mod function {
116187
/// *and* there are more than one ref pointing to it.
117188
///
118189
/// Note that `boundary_commits` are sorted so binary-search can be used to quickly check membership.
190+
/// TODO: also add `hidden` commits, for a list of special commits like the merge-base where all parents should be hidden as well.
119191
fn collect_stack_segments(
120192
tip: gix::Id<'_>,
121193
tip_ref: Option<&gix::refs::FullNameRef>,
122194
ref_location: Option<RefLocation>,
123-
boundary_commits: &[gix::ObjectId],
195+
boundary_commits: &gix::hashtable::HashSet,
124196
preferred_refs: &[gix::refs::FullName],
125197
mut limit: usize,
126198
refs_by_id: &RefsById,
@@ -148,7 +220,7 @@ pub(crate) mod function {
148220
}
149221
}
150222
let info = info?;
151-
if boundary_commits.binary_search(&info.id).is_ok() {
223+
if boundary_commits.contains(&info.id) {
152224
out.extend(segment.take());
153225
break;
154226
}

crates/but-workspace/src/virtual_branches_metadata.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,14 @@ impl VirtualBranchesTomlMetadata {
8686
}
8787
}
8888

89+
/// Mostly used in testing, and it's fine as it's intermediate, and we are very practical here.
90+
impl VirtualBranchesTomlMetadata {
91+
/// Return a mutable snapshot of the underlying data. Useful for testing mainly.
92+
pub fn data_mut(&mut self) -> &mut VirtualBranchesState {
93+
&mut self.snapshot.content
94+
}
95+
}
96+
8997
// Emergency-behaviour in case the application winds down, we don't want data-loss (at least a chance).
9098
impl Drop for VirtualBranchesTomlMetadata {
9199
fn drop(&mut self) {

crates/but-workspace/tests/fixtures/generated-archives/.gitignore

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,4 +25,5 @@
2525
/mixed-hunk-modifications.tar
2626
/plain-modifications.tar
2727
/three-commits-with-line-offset-and-workspace-commit.tar
28-
/with_remotes_no_workspace.tar
28+
/with-remotes-no-workspace.tar
29+
/with-remotes-and-workspace.tar
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
#!/usr/bin/env bash
2+
3+
### General Description
4+
5+
# Various directories with different scenarios for testing stack information *with* a workspace commit,
6+
# and of course with a remote and a branch to integrate with.
7+
set -eu -o pipefail
8+
9+
function set_author() {
10+
local author=${1:?Author}
11+
12+
unset GIT_AUTHOR_NAME
13+
unset GIT_AUTHOR_EMAIL
14+
15+
git config user.name $author
16+
git config user.email $author@example.com
17+
}
18+
19+
20+
# can only be called once per test setup
21+
function create_workspace_commit_once() {
22+
local workspace_commit_subject="GitButler Workspace Commit"
23+
24+
if [ $# == 1 ]; then
25+
local current_branch=$(git rev-parse --abbrev-ref HEAD)
26+
if [[ "$current_branch" != "$1" ]]; then
27+
echo "BUG: Must assure the current branch is the branch passed as argument: $current_branch != $1"
28+
return 42
29+
fi
30+
fi
31+
32+
git checkout -b gitbutler/workspace
33+
if [ $# == 1 ] || [ $# == 0 ]; then
34+
git commit --allow-empty -m "$workspace_commit_subject"
35+
else
36+
git merge -m "$workspace_commit_subject" "${@}"
37+
fi
38+
}
39+
40+
git init remote
41+
(cd remote
42+
touch file
43+
git add . && git commit -m init-integration
44+
45+
git checkout -b A
46+
touch file-in-A && git add . && git commit -m "new file in A"
47+
echo change >file-in-A && git commit -am "change in A"
48+
49+
git checkout main
50+
)
51+
52+
# The remote has a new commit, but is fast-forwardable
53+
git clone remote remote-advanced-ff
54+
(cd remote-advanced-ff
55+
git checkout -b A origin/A
56+
git reset --hard @~1
57+
58+
create_workspace_commit_once A
59+
)

crates/but-workspace/tests/fixtures/scenario/with_remotes_no_workspace.sh renamed to crates/but-workspace/tests/fixtures/scenario/with-remotes-no-workspace.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,14 @@ git init remote
2727
git checkout main
2828
)
2929

30-
# The remote has a new commit, but is fast-forwardable
30+
# The remote has a new commit, but is fast-forwardable.
3131
git clone remote remote-tracking-advanced-ff
3232
(cd remote-tracking-advanced-ff
3333
git checkout -b A origin/A
3434
git reset --hard @~1
3535
)
3636

37-
# The remote has a new commit, but is fast-forwardable
37+
# The remote has a new commit, and local has a new commit in an unreconcilable way.
3838
cp -R remote-tracking-advanced-ff remote-diverged
3939
(cd remote-diverged
4040
set_author local-user

0 commit comments

Comments
 (0)