Skip to content

Commit 38504c1

Browse files
committed
Create a mixed-mode non-legacy apply.
In legacy mode, we can use non-legacy code, but not the other way around. So let's split it on the highest level. Also, let's not use feature toggles for the same reason, it's now not possible anymore to call into old code as it's not available anymore.
1 parent ddb24fd commit 38504c1

File tree

70 files changed

+390
-866
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

70 files changed

+390
-866
lines changed

Cargo.lock

Lines changed: 3 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/but-api/Cargo.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ legacy = [
4444
"dep:but-rules",
4545
"dep:but-gerrit",
4646
"dep:but-worktrees",
47+
"dep:but-cherry-apply",
4748
]
4849

4950

@@ -60,7 +61,6 @@ but-workspace.workspace = true
6061
but-settings.workspace = true
6162
but-secret.workspace = true
6263
but-rebase.workspace = true
63-
but-cherry-apply.workspace = true
6464
but-db.workspace = true
6565
but-forge.workspace = true
6666
but-forge-storage.workspace = true
@@ -74,6 +74,7 @@ but-oplog = { workspace = true, features = ["legacy"] }
7474

7575
# What follows is *basically* newly written crates that are built on top of legacy,
7676
# so need some refactoring/rethinking to become usable.
77+
but-cherry-apply = { workspace = true, optional = true }
7778
but-action = { workspace = true, optional = true }
7879
but-hunk-assignment = { workspace = true, optional = true }
7980
but-hunk-dependency = { workspace = true, optional = true }

crates/but-api/src/branch.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@ use but_oplog::legacy::{OperationKind, SnapshotDetails, Trailer};
33
use but_workspace::branch::OnWorkspaceMergeConflict;
44
use but_workspace::branch::apply::{WorkspaceMerge, WorkspaceReferenceNaming};
55

6-
/// Just like [apply_without_oplog()] but runs application-specific side effects, typically undo-queue and oplog settings.
7-
pub fn apply_without_oplog(
6+
/// Just like [apply()], but update the oplog.
7+
pub fn apply_only(
88
ctx: &but_ctx::Context,
99
existing_branch: &gix::refs::FullNameRef,
1010
) -> anyhow::Result<but_workspace::branch::apply::Outcome<'static>> {
@@ -48,7 +48,7 @@ pub fn apply(
4848
)
4949
.ok();
5050

51-
let res = apply_without_oplog(ctx, existing_branch);
51+
let res = apply_only(ctx, existing_branch);
5252
if let Some(snapshot) = maybe_oplog_entry.filter(|_| res.is_ok()) {
5353
snapshot.commit(ctx).ok();
5454
}

crates/but-api/src/legacy/workspace.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,7 @@ pub fn amend_commit_from_worktree_changes(
327327
let app_settings = AppSettings::load_from_default_path_creating()?;
328328
let outcome = but_workspace::legacy::commit_engine::create_commit_and_update_refs_with_project(
329329
&repo,
330-
&project,
330+
&project.gb_dir(),
331331
Some(stack_id),
332332
commit_engine::Destination::AmendCommit {
333333
commit_id: commit_id.into(),
@@ -620,7 +620,7 @@ pub fn stash_into_branch(
620620

621621
let outcome = but_workspace::legacy::commit_engine::create_commit_and_update_refs_with_project(
622622
&repo,
623-
&ctx.legacy_project,
623+
&ctx.project_data_dir(),
624624
Some(stack.id),
625625
commit_engine::Destination::NewCommit {
626626
parent_commit_id: Some(parent_commit_id),

crates/but-ctx/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ legacy = ["dep:gitbutler-project", "dep:tracing"]
1818
[dependencies]
1919
but-core.workspace = true
2020
but-settings.workspace = true
21+
# Legacy is needed here for our only RefMetadata implementation that adapts `virtual-branches.toml`.
2122
but-meta = { workspace = true, features = ["legacy"] }
2223
but-db.workspace = true
2324
but-graph.workspace = true

crates/but-ctx/src/legacy/mod.rs renamed to crates/but-ctx/src/legacy.rs

Lines changed: 0 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,6 @@ pub(crate) mod types {
1414
pub type LegacyProject = gitbutler_project::Project;
1515
}
1616

17-
mod repository_ext;
18-
pub use repository_ext::RepositoryExtLite;
19-
2017
/// Legacy Lifecycle
2118
impl Context {
2219
/// Open the repository identified by `legacy_project` and `settings`.
@@ -97,48 +94,6 @@ impl Context {
9794
)?;
9895
Ok((repo.clone(), meta, graph))
9996
}
100-
101-
/// Open the repository with standard options and create a new Graph traversal from the current HEAD,
102-
/// along with a new metadata instance, and the graph itself.
103-
///
104-
/// The write-permission is required to obtain a mutable metadata instance. Note that it must be held
105-
/// for until the end of the operation for the protection to be effective.
106-
///
107-
/// Use [`Self::graph_and_meta_and_repo_from_head()`] if control over the repository configuration is needed.
108-
// TODO: make this non-legacy once we don't need the legacy refmetadata implementation anymore.
109-
pub fn graph_and_meta_mut_and_repo_from_head(
110-
&self,
111-
_write: &mut WorktreeWritePermission,
112-
) -> anyhow::Result<(
113-
gix::Repository,
114-
impl but_core::RefMetadata + 'static,
115-
but_graph::Graph,
116-
)> {
117-
let repo = self.repo.get()?;
118-
let meta = self.meta_inner()?;
119-
let graph = but_graph::Graph::from_head(&repo, &meta, but_graph::init::Options::limited())?;
120-
Ok((repo.clone(), meta, graph))
121-
}
122-
123-
/// Create a new Graph traversal from the current HEAD, using (and returning) the given `repo` (configured by the caller),
124-
/// along with a new metadata instance, and the graph itself.
125-
///
126-
/// The read-permission is required to obtain a shared metadata instance. Note that it must be held
127-
/// for until the end of the operation for the protection to be effective.
128-
// TODO: make this non-legacy once we don't need the legacy refmetadata implementation anymore.
129-
pub fn graph_and_meta_and_repo_from_head(
130-
&self,
131-
repo: gix::Repository,
132-
_read_only: &WorktreeReadPermission,
133-
) -> anyhow::Result<(
134-
gix::Repository,
135-
impl but_core::RefMetadata + 'static,
136-
but_graph::Graph,
137-
)> {
138-
let meta = self.meta_inner()?;
139-
let graph = but_graph::Graph::from_head(&repo, &meta, but_graph::init::Options::limited())?;
140-
Ok((repo, meta, graph))
141-
}
14297
}
14398

14499
/// Legacy - none of this should be kept.
@@ -168,10 +123,4 @@ impl Context {
168123
) -> anyhow::Result<but_meta::VirtualBranchesTomlMetadata> {
169124
self.meta_inner()
170125
}
171-
172-
pub(super) fn meta_inner(&self) -> anyhow::Result<but_meta::VirtualBranchesTomlMetadata> {
173-
but_meta::VirtualBranchesTomlMetadata::from_path(
174-
self.project_data_dir().join("virtual_branches.toml"),
175-
)
176-
}
177126
}

crates/but-ctx/src/lib.rs

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@
22
#![deny(missing_docs)]
33
#![forbid(unsafe_code)]
44

5-
use std::path::{Path, PathBuf};
6-
5+
use but_core::sync::{WorktreeReadPermission, WorktreeWritePermission};
76
use but_settings::AppSettings;
7+
use std::path::{Path, PathBuf};
88

99
/// Legacy types that shouldn't be used.
1010
#[cfg(feature = "legacy")]
@@ -193,6 +193,55 @@ impl Context {
193193
}
194194
}
195195

196+
/// Trampolines that create new uncached instances of major types.
197+
impl Context {
198+
/// Open the repository with standard options and create a new Graph traversal from the current HEAD,
199+
/// along with a new metadata instance, and the graph itself.
200+
///
201+
/// The write-permission is required to obtain a mutable metadata instance. Note that it must be held
202+
/// for until the end of the operation for the protection to be effective.
203+
///
204+
/// Use [`Self::graph_and_meta_and_repo_from_head()`] if control over the repository configuration is needed.
205+
pub fn graph_and_meta_mut_and_repo_from_head(
206+
&self,
207+
_write: &mut WorktreeWritePermission,
208+
) -> anyhow::Result<(
209+
gix::Repository,
210+
impl but_core::RefMetadata + 'static,
211+
but_graph::Graph,
212+
)> {
213+
let repo = self.repo.get()?;
214+
let meta = self.meta_inner()?;
215+
let graph = but_graph::Graph::from_head(&repo, &meta, but_graph::init::Options::limited())?;
216+
Ok((repo.clone(), meta, graph))
217+
}
218+
219+
/// Create a new Graph traversal from the current HEAD, using (and returning) the given `repo` (configured by the caller),
220+
/// along with a new metadata instance, and the graph itself.
221+
///
222+
/// The read-permission is required to obtain a shared metadata instance. Note that it must be held
223+
/// for until the end of the operation for the protection to be effective.
224+
pub fn graph_and_meta_and_repo_from_head(
225+
&self,
226+
repo: gix::Repository,
227+
_read_only: &WorktreeReadPermission,
228+
) -> anyhow::Result<(
229+
gix::Repository,
230+
impl but_core::RefMetadata + 'static,
231+
but_graph::Graph,
232+
)> {
233+
let meta = self.meta_inner()?;
234+
let graph = but_graph::Graph::from_head(&repo, &meta, but_graph::init::Options::limited())?;
235+
Ok((repo, meta, graph))
236+
}
237+
238+
fn meta_inner(&self) -> anyhow::Result<but_meta::VirtualBranchesTomlMetadata> {
239+
but_meta::VirtualBranchesTomlMetadata::from_path(
240+
self.project_data_dir().join("virtual_branches.toml"),
241+
)
242+
}
243+
}
244+
196245
/// Utilities
197246
impl Context {
198247
/// Return a wrapper for metadata that only supports read-only access when presented with the project wide permission

crates/but-oxidize/src/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,11 @@ impl ObjectIdExt for gix::Id<'_> {
5757
}
5858

5959
pub trait RepoExt {
60-
fn to_gix(&self) -> anyhow::Result<gix::Repository>;
60+
fn to_gix_repo(&self) -> anyhow::Result<gix::Repository>;
6161
}
6262

6363
impl RepoExt for &git2::Repository {
64-
fn to_gix(&self) -> anyhow::Result<gix::Repository> {
64+
fn to_gix_repo(&self) -> anyhow::Result<gix::Repository> {
6565
let repo = gix::open(self.path())?;
6666
Ok(repo)
6767
}

crates/but-rules/src/handler.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,9 +81,8 @@ fn handle_amend(
8181
change_id: String,
8282
) -> anyhow::Result<()> {
8383
let changes: Vec<DiffSpec> = assignments.into_iter().map(|a| a.into()).collect();
84-
let project = &ctx.legacy_project;
8584
let mut guard = ctx.exclusive_worktree_access();
86-
let repo = project.open_repo_for_merging()?;
85+
let repo = ctx.open_repo_for_merging()?;
8786

8887
let meta = VirtualBranchesTomlMetadata::from_path(
8988
ctx.project_data_dir().join("virtual_branches.toml"),
@@ -114,7 +113,7 @@ fn handle_amend(
114113

115114
commit_engine::create_commit_and_update_refs_with_project(
116115
&repo,
117-
project,
116+
&ctx.project_data_dir(),
118117
None,
119118
but_workspace::commit_engine::Destination::AmendCommit {
120119
commit_id,

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ fn commit_with_project(
145145
let mut guard = but_core::sync::exclusive_worktree_access(project.git_dir());
146146
let outcome = but_workspace::legacy::commit_engine::create_commit_and_update_refs_with_project(
147147
repo,
148-
project,
148+
&project.gb_dir(),
149149
None,
150150
destination,
151151
changes,

0 commit comments

Comments
 (0)