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.

Author: Byron

Cargo.lock

@@ -44,6 +44,7 @@ legacy = [
     "dep:but-rules",
     "dep:but-gerrit",
     "dep:but-worktrees",
+    "dep:but-cherry-apply",
 ]
 
 
@@ -60,7 +61,6 @@ but-workspace.workspace = true
 but-settings.workspace = true
 but-secret.workspace = true
 but-rebase.workspace = true
-but-cherry-apply.workspace = true
 but-db.workspace = true
 but-forge.workspace = true
 but-forge-storage.workspace = true
@@ -74,6 +74,7 @@ but-oplog = { workspace = true, features = ["legacy"] }
 
 # What follows is *basically* newly written crates that are built on top of legacy,
 # so need some refactoring/rethinking to become usable.
+but-cherry-apply = { workspace = true, optional = true }
 but-action = { workspace = true, optional = true }
 but-hunk-assignment = { workspace = true, optional = true }
 but-hunk-dependency = { workspace = true, optional = true }

crates/but-api/Cargo.toml

@@ -3,8 +3,8 @@ use but_oplog::legacy::{OperationKind, SnapshotDetails, Trailer};
 use but_workspace::branch::OnWorkspaceMergeConflict;
 use but_workspace::branch::apply::{WorkspaceMerge, WorkspaceReferenceNaming};
 
-/// Just like [apply_without_oplog()] but runs application-specific side effects, typically undo-queue and oplog settings.
-pub fn apply_without_oplog(
+/// Just like [apply()], but without updating the oplog.
+pub fn apply_only(
     ctx: &but_ctx::Context,
     existing_branch: &gix::refs::FullNameRef,
 ) -> anyhow::Result<but_workspace::branch::apply::Outcome<'static>> {
@@ -48,7 +48,7 @@ pub fn apply(
     )
     .ok();
 
-    let res = apply_without_oplog(ctx, existing_branch);
+    let res = apply_only(ctx, existing_branch);
     if let Some(snapshot) = maybe_oplog_entry.filter(|_| res.is_ok()) {
         snapshot.commit(ctx).ok();
     }

crates/but-api/src/branch.rs

@@ -327,7 +327,7 @@ pub fn amend_commit_from_worktree_changes(
     let app_settings = AppSettings::load_from_default_path_creating()?;
     let outcome = but_workspace::legacy::commit_engine::create_commit_and_update_refs_with_project(
         &repo,
-        &project,
+        &project.gb_dir(),
         Some(stack_id),
         commit_engine::Destination::AmendCommit {
             commit_id: commit_id.into(),
@@ -620,7 +620,7 @@ pub fn stash_into_branch(
 
     let outcome = but_workspace::legacy::commit_engine::create_commit_and_update_refs_with_project(
         &repo,
-        &ctx.legacy_project,
+        &ctx.project_data_dir(),
         Some(stack.id),
         commit_engine::Destination::NewCommit {
             parent_commit_id: Some(parent_commit_id),

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

@@ -18,6 +18,7 @@ legacy = ["dep:gitbutler-project", "dep:tracing"]
 [dependencies]
 but-core.workspace = true
 but-settings.workspace = true
+# Legacy is needed here for our only RefMetadata implementation that adapts `virtual-branches.toml`.
 but-meta = { workspace = true, features = ["legacy"] }
 but-db.workspace = true
 but-graph.workspace = true

crates/but-ctx/Cargo.toml

@@ -14,9 +14,6 @@ pub(crate) mod types {
     pub type LegacyProject = gitbutler_project::Project;
 }
 
-mod repository_ext;
-pub use repository_ext::RepositoryExtLite;
-
 /// Legacy Lifecycle
 impl Context {
     /// Open the repository identified by `legacy_project` and `settings`.
@@ -97,48 +94,6 @@ impl Context {
         )?;
         Ok((repo.clone(), meta, graph))
     }
-
-    /// Open the repository with standard options and create a new Graph traversal from the current HEAD,
-    /// along with a new metadata instance, and the graph itself.
-    ///
-    /// The write-permission is required to obtain a mutable metadata instance. Note that it must be held
-    /// for until the end of the operation for the protection to be effective.
-    ///
-    /// Use [`Self::graph_and_meta_and_repo_from_head()`] if control over the repository configuration is needed.
-    // TODO: make this non-legacy once we don't need the legacy refmetadata implementation anymore.
-    pub fn graph_and_meta_mut_and_repo_from_head(
-        &self,
-        _write: &mut WorktreeWritePermission,
-    ) -> anyhow::Result<(
-        gix::Repository,
-        impl but_core::RefMetadata + 'static,
-        but_graph::Graph,
-    )> {
-        let repo = self.repo.get()?;
-        let meta = self.meta_inner()?;
-        let graph = but_graph::Graph::from_head(&repo, &meta, but_graph::init::Options::limited())?;
-        Ok((repo.clone(), meta, graph))
-    }
-
-    /// Create a new Graph traversal from the current HEAD, using (and returning) the given `repo` (configured by the caller),
-    /// along with a new metadata instance, and the graph itself.
-    ///
-    /// The read-permission is required to obtain a shared metadata instance. Note that it must be held
-    /// for until the end of the operation for the protection to be effective.
-    // TODO: make this non-legacy once we don't need the legacy refmetadata implementation anymore.
-    pub fn graph_and_meta_and_repo_from_head(
-        &self,
-        repo: gix::Repository,
-        _read_only: &WorktreeReadPermission,
-    ) -> anyhow::Result<(
-        gix::Repository,
-        impl but_core::RefMetadata + 'static,
-        but_graph::Graph,
-    )> {
-        let meta = self.meta_inner()?;
-        let graph = but_graph::Graph::from_head(&repo, &meta, but_graph::init::Options::limited())?;
-        Ok((repo, meta, graph))
-    }
 }
 
 /// Legacy - none of this should be kept.
@@ -168,10 +123,4 @@ impl Context {
     ) -> anyhow::Result<but_meta::VirtualBranchesTomlMetadata> {
         self.meta_inner()
     }
-
-    pub(super) fn meta_inner(&self) -> anyhow::Result<but_meta::VirtualBranchesTomlMetadata> {
-        but_meta::VirtualBranchesTomlMetadata::from_path(
-            self.project_data_dir().join("virtual_branches.toml"),
-        )
-    }
 }

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

@@ -2,9 +2,9 @@
 #![deny(missing_docs)]
 #![forbid(unsafe_code)]
 
-use std::path::{Path, PathBuf};
-
+use but_core::sync::{WorktreeReadPermission, WorktreeWritePermission};
 use but_settings::AppSettings;
+use std::path::{Path, PathBuf};
 
 /// Legacy types that shouldn't be used.
 #[cfg(feature = "legacy")]
@@ -193,6 +193,55 @@ impl Context {
     }
 }
 
+/// Trampolines that create new uncached instances of major types.
+impl Context {
+    /// Open the repository with standard options and create a new Graph traversal from the current HEAD,
+    /// along with a new metadata instance, and the graph itself.
+    ///
+    /// The write-permission is required to obtain a mutable metadata instance. Note that it must be held
+    /// for until the end of the operation for the protection to be effective.
+    ///
+    /// Use [`Self::graph_and_meta_and_repo_from_head()`] if control over the repository configuration is needed.
+    pub fn graph_and_meta_mut_and_repo_from_head(
+        &self,
+        _write: &mut WorktreeWritePermission,
+    ) -> anyhow::Result<(
+        gix::Repository,
+        impl but_core::RefMetadata + 'static,
+        but_graph::Graph,
+    )> {
+        let repo = self.repo.get()?;
+        let meta = self.meta_inner()?;
+        let graph = but_graph::Graph::from_head(&repo, &meta, but_graph::init::Options::limited())?;
+        Ok((repo.clone(), meta, graph))
+    }
+
+    /// Create a new Graph traversal from the current HEAD, using (and returning) the given `repo` (configured by the caller),
+    /// along with a new metadata instance, and the graph itself.
+    ///
+    /// The read-permission is required to obtain a shared metadata instance. Note that it must be held
+    /// for until the end of the operation for the protection to be effective.
+    pub fn graph_and_meta_and_repo_from_head(
+        &self,
+        repo: gix::Repository,
+        _read_only: &WorktreeReadPermission,
+    ) -> anyhow::Result<(
+        gix::Repository,
+        impl but_core::RefMetadata + 'static,
+        but_graph::Graph,
+    )> {
+        let meta = self.meta_inner()?;
+        let graph = but_graph::Graph::from_head(&repo, &meta, but_graph::init::Options::limited())?;
+        Ok((repo, meta, graph))
+    }
+
+    fn meta_inner(&self) -> anyhow::Result<but_meta::VirtualBranchesTomlMetadata> {
+        but_meta::VirtualBranchesTomlMetadata::from_path(
+            self.project_data_dir().join("virtual_branches.toml"),
+        )
+    }
+}
+
 /// Utilities
 impl Context {
     /// Return a wrapper for metadata that only supports read-only access when presented with the project wide permission

crates/but-ctx/src/lib.rs

@@ -37,15 +37,15 @@
 //!
 //! ### Mode of operation: Side Effect
 //!
-//! When applying a mutation the repository, the oplog runs as side . The *effect* itself changes state, and is expected to either fully succeed,
+//! When applying a mutation to the repository, the oplog runs as a side effect. The *effect* itself changes state, and is expected to either fully succeed,
 //! or leave no trace of ever running.
 //!
 //! This means that snapshots have to be recorded in such a way that they can't be observed until they are committed, which happens only when the
 //! *effect* succeeds. On failure, there is no entry in the oplog.
 //!
 //! ### Status of the implementation
 //!
-//! Right now it's merely a letter of intend with an API sketch that is sufficient to 'record but not persist unless command is successful'.
+//! Right now it's merely a letter of intent with an API sketch that is sufficient to 'record but not persist unless command is successful'.
 //! Note that the `gitbutler-oplog` is still the backbone of this implementation, but modified to the extent necessary.
 //! Legacy commands will always see their restore point created as their failure might leave partial changes that might still be undoable
 //! with the restore point.

crates/but-oplog/src/lib.rs

@@ -57,11 +57,11 @@ impl ObjectIdExt for gix::Id<'_> {
 }
 
 pub trait RepoExt {
-    fn to_gix(&self) -> anyhow::Result<gix::Repository>;
+    fn to_gix_repo(&self) -> anyhow::Result<gix::Repository>;
 }
 
 impl RepoExt for &git2::Repository {
-    fn to_gix(&self) -> anyhow::Result<gix::Repository> {
+    fn to_gix_repo(&self) -> anyhow::Result<gix::Repository> {
         let repo = gix::open(self.path())?;
         Ok(repo)
     }

crates/but-oxidize/src/lib.rs

@@ -81,9 +81,8 @@ fn handle_amend(
     change_id: String,
 ) -> anyhow::Result<()> {
     let changes: Vec<DiffSpec> = assignments.into_iter().map(|a| a.into()).collect();
-    let project = &ctx.legacy_project;
     let mut guard = ctx.exclusive_worktree_access();
-    let repo = project.open_repo_for_merging()?;
+    let repo = ctx.open_repo_for_merging()?;
 
     let meta = VirtualBranchesTomlMetadata::from_path(
         ctx.project_data_dir().join("virtual_branches.toml"),
@@ -114,7 +113,7 @@ fn handle_amend(
 
     commit_engine::create_commit_and_update_refs_with_project(
         &repo,
-        project,
+        &ctx.project_data_dir(),
         None,
         but_workspace::commit_engine::Destination::AmendCommit {
             commit_id,