repo: extract a Repo trait for Arc<ReadonlyRepo> and MutableRepo

This will soon replace the `RepoRef` enum, just like how the `Index`
trait replaced the `IndexRef` enum.
This commit is contained in:
Martin von Zweigbergk 2023-02-14 13:51:55 -08:00 committed by Martin von Zweigbergk
parent b7dad291df
commit f6a4cb57da
31 changed files with 147 additions and 104 deletions

View file

@ -16,7 +16,7 @@ use std::sync::Arc;
use crate::backend::{self, BackendResult, ChangeId, CommitId, Signature, TreeId};
use crate::commit::Commit;
use crate::repo::MutableRepo;
use crate::repo::{MutableRepo, Repo};
use crate::settings::{JJRng, UserSettings};
#[must_use]

View file

@ -23,9 +23,8 @@ use thiserror::Error;
use crate::backend::{CommitId, ObjectId};
use crate::commit::Commit;
use crate::git_backend::NO_GC_REF_NAMESPACE;
use crate::index::Index;
use crate::op_store::RefTarget;
use crate::repo::MutableRepo;
use crate::repo::{MutableRepo, Repo};
use crate::settings::GitSettings;
use crate::view::RefName;

View file

@ -49,6 +49,32 @@ use crate::transaction::Transaction;
use crate::view::{RefName, View};
use crate::{backend, op_store};
pub trait Repo {
fn base_repo(&self) -> &Arc<ReadonlyRepo>;
fn store(&self) -> &Arc<Store>;
fn op_store(&self) -> &Arc<dyn OpStore>;
fn index(&self) -> &dyn Index;
fn view(&self) -> &View;
fn resolve_change_id(&self, change_id: &ChangeId) -> Option<Vec<IndexEntry>> {
// Replace this if we added more efficient lookup method.
let prefix = HexPrefix::from_bytes(change_id.as_bytes());
match self.resolve_change_id_prefix(&prefix) {
PrefixResolution::NoMatch => None,
PrefixResolution::SingleMatch(entries) => Some(entries),
PrefixResolution::AmbiguousMatch => panic!("complete change_id should be unambiguous"),
}
}
fn resolve_change_id_prefix(&self, prefix: &HexPrefix) -> PrefixResolution<Vec<IndexEntry>>;
fn shortest_unique_change_id_prefix_len(&self, target_id_bytes: &ChangeId) -> usize;
}
// TODO: Should we implement From<&ReadonlyRepo> and From<&MutableRepo> for
// RepoRef?
#[derive(Clone, Copy)]
@ -116,7 +142,7 @@ impl<'a> RepoRef<'a> {
pub fn shortest_unique_change_id_prefix_len(&self, target_id: &ChangeId) -> usize {
match self {
RepoRef::Readonly(repo) => repo.shortest_unique_change_id_prefix_len(target_id),
RepoRef::Mutable(_) => target_id.as_bytes().len() * 2, // TODO
RepoRef::Mutable(repo) => repo.shortest_unique_change_id_prefix_len(target_id),
}
}
}
@ -272,14 +298,10 @@ impl ReadonlyRepo {
})
}
pub fn index(&self) -> &dyn Index {
self.readonly_index().as_ref()
}
fn change_id_index(&self) -> &ChangeIdIndex {
self.change_id_index.get_or_init(|| {
let heads = self.view().heads().iter().cloned().collect_vec();
let walk = self.index().walk_revs(&heads, &[]);
let walk = self.readonly_index().walk_revs(&heads, &[]);
IdIndex::from_vec(
walk.map(|entry| (entry.change_id(), entry.position()))
.collect(),
@ -287,27 +309,6 @@ impl ReadonlyRepo {
})
}
pub fn resolve_change_id_prefix(
&self,
prefix: &HexPrefix,
) -> PrefixResolution<Vec<IndexEntry>> {
let index = self.index();
self.change_id_index()
.resolve_prefix_with(prefix, |&pos| index.entry_by_pos(pos))
}
pub fn shortest_unique_change_id_prefix_len(&self, target_id: &ChangeId) -> usize {
self.change_id_index().shortest_unique_prefix_len(target_id)
}
pub fn store(&self) -> &Arc<Store> {
&self.store
}
pub fn op_store(&self) -> &Arc<dyn OpStore> {
&self.op_store
}
pub fn op_heads_store(&self) -> &Arc<dyn OpHeadsStore> {
&self.op_heads_store
}
@ -341,6 +342,38 @@ impl ReadonlyRepo {
}
}
impl Repo for Arc<ReadonlyRepo> {
fn base_repo(&self) -> &Arc<ReadonlyRepo> {
self
}
fn store(&self) -> &Arc<Store> {
&self.store
}
fn op_store(&self) -> &Arc<dyn OpStore> {
&self.op_store
}
fn index(&self) -> &dyn Index {
self.readonly_index().as_ref()
}
fn view(&self) -> &View {
&self.view
}
fn resolve_change_id_prefix(&self, prefix: &HexPrefix) -> PrefixResolution<Vec<IndexEntry>> {
let index = self.index();
self.change_id_index()
.resolve_prefix_with(prefix, |&pos| index.entry_by_pos(pos))
}
fn shortest_unique_change_id_prefix_len(&self, target_id: &ChangeId) -> usize {
self.change_id_index().shortest_unique_prefix_len(target_id)
}
}
type BackendFactory = Box<dyn Fn(&Path) -> Box<dyn Backend>>;
type OpStoreFactory = Box<dyn Fn(&Path) -> Box<dyn OpStore>>;
type OpHeadsStoreFactory = Box<dyn Fn(&Path) -> Box<dyn OpHeadsStore>>;
@ -625,27 +658,6 @@ impl MutableRepo {
RepoRef::Mutable(self)
}
pub fn base_repo(&self) -> &Arc<ReadonlyRepo> {
&self.base_repo
}
pub fn store(&self) -> &Arc<Store> {
self.base_repo.store()
}
pub fn op_store(&self) -> &Arc<dyn OpStore> {
self.base_repo.op_store()
}
pub fn index(&self) -> &MutableIndex {
&self.index
}
pub fn view(&self) -> &View {
self.view
.get_or_ensure_clean(|v| self.enforce_view_invariants(v))
}
fn view_mut(&mut self) -> &mut View {
self.view.get_mut()
}
@ -661,31 +673,6 @@ impl MutableRepo {
(self.index, self.view.into_inner())
}
pub fn resolve_change_id_prefix<'a>(
&'a self,
prefix: &HexPrefix,
) -> PrefixResolution<Vec<IndexEntry<'a>>> {
// TODO: Create a persistent lookup from change id to (visible?) commit ids.
let mut found_change_id = None;
let mut found_entries = vec![];
let heads = self.view().heads().iter().cloned().collect_vec();
for entry in self.index().walk_revs(&heads, &[]) {
let change_id = entry.change_id();
if prefix.matches(&change_id) {
if let Some(previous_change_id) = found_change_id.replace(change_id.clone()) {
if previous_change_id != change_id {
return PrefixResolution::AmbiguousMatch;
}
}
found_entries.push(entry);
}
}
if found_change_id.is_none() {
return PrefixResolution::NoMatch;
}
PrefixResolution::SingleMatch(found_entries)
}
pub fn new_commit(
&mut self,
settings: &UserSettings,
@ -1157,6 +1144,55 @@ impl MutableRepo {
}
}
impl Repo for MutableRepo {
fn base_repo(&self) -> &Arc<ReadonlyRepo> {
&self.base_repo
}
fn store(&self) -> &Arc<Store> {
self.base_repo.store()
}
fn op_store(&self) -> &Arc<dyn OpStore> {
self.base_repo.op_store()
}
fn index(&self) -> &dyn Index {
&self.index
}
fn view(&self) -> &View {
self.view
.get_or_ensure_clean(|v| self.enforce_view_invariants(v))
}
fn resolve_change_id_prefix(&self, prefix: &HexPrefix) -> PrefixResolution<Vec<IndexEntry>> {
// TODO: Create a persistent lookup from change id to (visible?) commit ids.
let mut found_change_id = None;
let mut found_entries = vec![];
let heads = self.view().heads().iter().cloned().collect_vec();
for entry in self.index().walk_revs(&heads, &[]) {
let change_id = entry.change_id();
if prefix.matches(&change_id) {
if let Some(previous_change_id) = found_change_id.replace(change_id.clone()) {
if previous_change_id != change_id {
return PrefixResolution::AmbiguousMatch;
}
}
found_entries.push(entry);
}
}
if found_change_id.is_none() {
return PrefixResolution::NoMatch;
}
PrefixResolution::SingleMatch(found_entries)
}
fn shortest_unique_change_id_prefix_len(&self, target_id: &ChangeId) -> usize {
target_id.as_bytes().len() * 2 // TODO
}
}
/// Error from attempts to check out the root commit for editing
#[derive(Debug, Error)]
#[error("Cannot rewrite the root commit")]

View file

@ -19,9 +19,8 @@ use itertools::{process_results, Itertools};
use crate::backend::{BackendError, BackendResult, CommitId, ObjectId};
use crate::commit::Commit;
use crate::dag_walk;
use crate::index::Index;
use crate::op_store::RefTarget;
use crate::repo::{MutableRepo, RepoRef};
use crate::repo::{MutableRepo, Repo, RepoRef};
use crate::repo_path::RepoPath;
use crate::revset::RevsetExpression;
use crate::settings::UserSettings;

View file

@ -20,7 +20,7 @@ use crate::index::ReadonlyIndex;
use crate::op_store;
use crate::op_store::OperationMetadata;
use crate::operation::Operation;
use crate::repo::{MutableRepo, ReadonlyRepo, RepoLoader};
use crate::repo::{MutableRepo, ReadonlyRepo, Repo, RepoLoader};
use crate::settings::UserSettings;
use crate::view::View;

View file

@ -26,7 +26,7 @@ use crate::op_heads_store::OpHeadsStore;
use crate::op_store::{self, OpStore, OperationMetadata, WorkspaceId};
use crate::operation::Operation;
use crate::repo::{
CheckOutCommitError, IoResultExt, PathError, ReadonlyRepo, RepoLoader, StoreFactories,
CheckOutCommitError, IoResultExt, PathError, ReadonlyRepo, Repo, RepoLoader, StoreFactories,
};
use crate::settings::UserSettings;
use crate::working_copy::WorkingCopy;

View file

@ -14,7 +14,7 @@
use std::path::Path;
use jujutsu_lib::repo::{ReadonlyRepo, StoreFactories};
use jujutsu_lib::repo::{ReadonlyRepo, Repo, StoreFactories};
use jujutsu_lib::workspace::Workspace;
use test_case::test_case;
use testutils::{create_random_commit, TestWorkspace};

View file

@ -13,6 +13,7 @@
// limitations under the License.
use jujutsu_lib::matchers::EverythingMatcher;
use jujutsu_lib::repo::Repo;
use jujutsu_lib::repo_path::RepoPath;
use jujutsu_lib::settings::UserSettings;
use jujutsu_lib::tree::DiffSummary;

View file

@ -13,14 +13,15 @@
// limitations under the License.
use std::cmp::max;
use std::sync::Arc;
use std::thread;
use jujutsu_lib::dag_walk;
use jujutsu_lib::repo::{ReadonlyRepo, StoreFactories};
use jujutsu_lib::repo::{ReadonlyRepo, Repo, StoreFactories};
use test_case::test_case;
use testutils::{write_random_commit, TestWorkspace};
fn count_non_merge_operations(repo: &ReadonlyRepo) -> usize {
fn count_non_merge_operations(repo: &Arc<ReadonlyRepo>) -> usize {
let op_store = repo.op_store();
let op_id = repo.op_id().clone();
let mut num_ops = 0;

View file

@ -15,6 +15,7 @@
use jujutsu_lib::backend::{Conflict, ConflictPart, TreeValue};
use jujutsu_lib::conflicts::{materialize_conflict, parse_conflict, update_conflict_from_content};
use jujutsu_lib::files::{ConflictHunk, MergeHunk};
use jujutsu_lib::repo::Repo;
use jujutsu_lib::repo_path::RepoPath;
use jujutsu_lib::store::Store;
use testutils::TestRepo;

View file

@ -23,7 +23,7 @@ use jujutsu_lib::git;
use jujutsu_lib::git::{GitFetchError, GitPushError, GitRefUpdate};
use jujutsu_lib::git_backend::GitBackend;
use jujutsu_lib::op_store::{BranchTarget, RefTarget};
use jujutsu_lib::repo::ReadonlyRepo;
use jujutsu_lib::repo::{ReadonlyRepo, Repo};
use jujutsu_lib::settings::{GitSettings, UserSettings};
use maplit::{btreemap, hashset};
use tempfile::TempDir;

View file

@ -17,8 +17,8 @@ use std::sync::Arc;
use jujutsu_lib::backend::CommitId;
use jujutsu_lib::commit::Commit;
use jujutsu_lib::commit_builder::CommitBuilder;
use jujutsu_lib::index::{Index};
use jujutsu_lib::repo::{MutableRepo, ReadonlyRepo, StoreFactories};
use jujutsu_lib::index::Index;
use jujutsu_lib::repo::{MutableRepo, ReadonlyRepo, Repo, StoreFactories};
use jujutsu_lib::settings::UserSettings;
use test_case::test_case;
use testutils::{create_random_commit, write_random_commit, CommitGraphBuilder, TestRepo};
@ -446,7 +446,7 @@ fn create_n_commits(
tx.commit()
}
fn commits_by_level(repo: &ReadonlyRepo) -> Vec<u32> {
fn commits_by_level(repo: &Arc<ReadonlyRepo>) -> Vec<u32> {
repo.index()
.stats()
.levels

View file

@ -15,6 +15,7 @@
use std::path::{Path, PathBuf};
use jujutsu_lib::op_store::WorkspaceId;
use jujutsu_lib::repo::Repo;
use jujutsu_lib::settings::UserSettings;
use jujutsu_lib::workspace::Workspace;
use test_case::test_case;

View file

@ -15,6 +15,7 @@
use assert_matches::assert_matches;
use itertools::Itertools;
use jujutsu_lib::backend::{ConflictPart, TreeValue};
use jujutsu_lib::repo::Repo;
use jujutsu_lib::repo_path::{RepoPath, RepoPathComponent};
use jujutsu_lib::rewrite::rebase_commit;
use jujutsu_lib::tree;

View file

@ -12,8 +12,8 @@
// See the License for the specific language governing permissions and
// limitations under the License.
use jujutsu_lib::index::Index;
use jujutsu_lib::op_store::{RefTarget, WorkspaceId};
use jujutsu_lib::repo::Repo;
use maplit::hashset;
use test_case::test_case;
use testutils::{

View file

@ -15,7 +15,7 @@
use std::path::Path;
use jujutsu_lib::backend::CommitId;
use jujutsu_lib::repo::RepoRef;
use jujutsu_lib::repo::{Repo, RepoRef};
use test_case::test_case;
use testutils::{create_random_commit, write_random_commit, TestRepo};

View file

@ -14,6 +14,7 @@
use jujutsu_lib::op_store::RefTarget;
use jujutsu_lib::refs::merge_ref_targets;
use jujutsu_lib::repo::Repo;
use testutils::{CommitGraphBuilder, TestWorkspace};
#[test]

View file

@ -19,7 +19,7 @@ use jujutsu_lib::backend::{CommitId, MillisSinceEpoch, ObjectId, Signature, Time
use jujutsu_lib::git;
use jujutsu_lib::matchers::{FilesMatcher, Matcher};
use jujutsu_lib::op_store::{RefTarget, WorkspaceId};
use jujutsu_lib::repo::RepoRef;
use jujutsu_lib::repo::{Repo, RepoRef};
use jujutsu_lib::repo_path::RepoPath;
use jujutsu_lib::revset::{
self, optimize, parse, resolve_symbol, RevsetAliasesMap, RevsetError, RevsetExpression,

View file

@ -13,6 +13,7 @@
// limitations under the License.
use itertools::Itertools;
use jujutsu_lib::repo::Repo;
use jujutsu_lib::revset::revset_for_commits;
use jujutsu_lib::revset_graph_iterator::RevsetGraphEdge;
use test_case::test_case;

View file

@ -13,6 +13,7 @@
// limitations under the License.
use jujutsu_lib::op_store::{RefTarget, WorkspaceId};
use jujutsu_lib::repo::Repo;
use jujutsu_lib::repo_path::RepoPath;
use jujutsu_lib::rewrite::DescendantRebaser;
use maplit::{hashmap, hashset};

View file

@ -15,7 +15,7 @@
use std::sync::Arc;
use jujutsu_lib::op_store::{BranchTarget, RefTarget, WorkspaceId};
use jujutsu_lib::repo::ReadonlyRepo;
use jujutsu_lib::repo::{ReadonlyRepo, Repo};
use jujutsu_lib::settings::UserSettings;
use jujutsu_lib::transaction::Transaction;
use maplit::{btreemap, hashset};

View file

@ -26,7 +26,7 @@ use jujutsu_lib::gitignore::GitIgnoreFile;
#[cfg(unix)]
use jujutsu_lib::op_store::OperationId;
use jujutsu_lib::op_store::WorkspaceId;
use jujutsu_lib::repo::ReadonlyRepo;
use jujutsu_lib::repo::{ReadonlyRepo, Repo};
use jujutsu_lib::repo_path::{RepoPath, RepoPathComponent, RepoPathJoin};
use jujutsu_lib::settings::UserSettings;
use jujutsu_lib::tree_builder::TreeBuilder;

View file

@ -17,7 +17,7 @@ use std::thread;
use assert_matches::assert_matches;
use jujutsu_lib::gitignore::GitIgnoreFile;
use jujutsu_lib::repo::StoreFactories;
use jujutsu_lib::repo::{Repo, StoreFactories};
use jujutsu_lib::repo_path::RepoPath;
use jujutsu_lib::working_copy::CheckoutError;
use jujutsu_lib::workspace::Workspace;

View file

@ -15,6 +15,7 @@
use itertools::Itertools;
use jujutsu_lib::gitignore::GitIgnoreFile;
use jujutsu_lib::matchers::EverythingMatcher;
use jujutsu_lib::repo::Repo;
use jujutsu_lib::repo_path::RepoPath;
use jujutsu_lib::working_copy::{CheckoutStats, WorkingCopy};
use testutils::TestWorkspace;

View file

@ -14,7 +14,7 @@
use assert_matches::assert_matches;
use jujutsu_lib::op_store::WorkspaceId;
use jujutsu_lib::repo::StoreFactories;
use jujutsu_lib::repo::{Repo, StoreFactories};
use jujutsu_lib::workspace::{Workspace, WorkspaceLoadError};
use test_case::test_case;
use testutils::TestWorkspace;

View file

@ -24,7 +24,7 @@ use jujutsu_lib::commit::Commit;
use jujutsu_lib::commit_builder::CommitBuilder;
use jujutsu_lib::git_backend::GitBackend;
use jujutsu_lib::local_backend::LocalBackend;
use jujutsu_lib::repo::{MutableRepo, ReadonlyRepo};
use jujutsu_lib::repo::{MutableRepo, ReadonlyRepo, Repo};
use jujutsu_lib::repo_path::RepoPath;
use jujutsu_lib::rewrite::RebasedDescendant;
use jujutsu_lib::settings::UserSettings;
@ -189,7 +189,7 @@ pub fn write_symlink(tree_builder: &mut TreeBuilder, path: &RepoPath, target: &s
tree_builder.set(path.clone(), TreeValue::Symlink(id));
}
pub fn create_tree(repo: &ReadonlyRepo, path_contents: &[(&RepoPath, &str)]) -> Tree {
pub fn create_tree(repo: &Arc<ReadonlyRepo>, path_contents: &[(&RepoPath, &str)]) -> Tree {
let store = repo.store();
let mut tree_builder = store.tree_builder(store.empty_tree_id().clone());
for (path, contents) in path_contents {
@ -200,7 +200,7 @@ pub fn create_tree(repo: &ReadonlyRepo, path_contents: &[(&RepoPath, &str)]) ->
}
#[must_use]
pub fn create_random_tree(repo: &ReadonlyRepo) -> TreeId {
pub fn create_random_tree(repo: &Arc<ReadonlyRepo>) -> TreeId {
let mut tree_builder = repo
.store()
.tree_builder(repo.store().empty_tree_id().clone());

View file

@ -33,13 +33,12 @@ use jujutsu_lib::commit::Commit;
use jujutsu_lib::git::{GitExportError, GitImportError};
use jujutsu_lib::gitignore::GitIgnoreFile;
use jujutsu_lib::hex_util::to_reverse_hex;
use jujutsu_lib::index::Index;
use jujutsu_lib::matchers::{EverythingMatcher, Matcher, PrefixMatcher, Visit};
use jujutsu_lib::op_heads_store::{self, OpHeadResolutionError, OpHeadsStore};
use jujutsu_lib::op_store::{OpStore, OpStoreError, OperationId, RefTarget, WorkspaceId};
use jujutsu_lib::operation::Operation;
use jujutsu_lib::repo::{
CheckOutCommitError, EditCommitError, MutableRepo, ReadonlyRepo, RepoLoader, RepoRef,
CheckOutCommitError, EditCommitError, MutableRepo, ReadonlyRepo, Repo, RepoLoader, RepoRef,
RewriteRootCommit, StoreFactories,
};
use jujutsu_lib::repo_path::{FsPathParseError, RepoPath};

View file

@ -4,7 +4,7 @@ use clap::builder::NonEmptyStringValueParser;
use itertools::Itertools;
use jujutsu_lib::backend::{CommitId, ObjectId};
use jujutsu_lib::op_store::RefTarget;
use jujutsu_lib::repo::RepoRef;
use jujutsu_lib::repo::{Repo, RepoRef};
use jujutsu_lib::view::View;
use crate::cli_util::{user_error, user_error_with_hint, CommandError, CommandHelper, RevisionArg};

View file

@ -12,7 +12,7 @@ use jujutsu_lib::backend::ObjectId;
use jujutsu_lib::git::{self, GitFetchError, GitRefUpdate};
use jujutsu_lib::op_store::{BranchTarget, RefTarget};
use jujutsu_lib::refs::{classify_branch_push_action, BranchPushAction, BranchPushUpdate};
use jujutsu_lib::repo::RepoRef;
use jujutsu_lib::repo::{Repo, RepoRef};
use jujutsu_lib::settings::UserSettings;
use jujutsu_lib::store::Store;
use jujutsu_lib::view::View;

View file

@ -20,6 +20,7 @@ use std::collections::{BTreeMap, HashSet};
use std::fmt::Debug;
use std::io::{Read, Seek, SeekFrom, Write};
use std::path::PathBuf;
use std::sync::Arc;
use std::{fs, io};
use clap::builder::NonEmptyStringValueParser;
@ -32,7 +33,7 @@ use jujutsu_lib::dag_walk::topo_order_reverse;
use jujutsu_lib::index::{Index, IndexEntry};
use jujutsu_lib::matchers::EverythingMatcher;
use jujutsu_lib::op_store::{RefTarget, WorkspaceId};
use jujutsu_lib::repo::ReadonlyRepo;
use jujutsu_lib::repo::{ReadonlyRepo, Repo};
use jujutsu_lib::repo_path::RepoPath;
use jujutsu_lib::revset::{RevsetAliasesMap, RevsetExpression};
use jujutsu_lib::revset_graph_iterator::{RevsetGraphEdge, RevsetGraphEdgeType};
@ -2917,7 +2918,7 @@ fn rebase_revision(
}
fn check_rebase_destinations(
repo: &ReadonlyRepo,
repo: &Arc<ReadonlyRepo>,
new_parents: &[Commit],
commit: &Commit,
) -> Result<(), CommandError> {

View file

@ -23,7 +23,7 @@ use jujutsu_lib::commit::Commit;
use jujutsu_lib::diff::{Diff, DiffHunk};
use jujutsu_lib::files::DiffLine;
use jujutsu_lib::matchers::Matcher;
use jujutsu_lib::repo::ReadonlyRepo;
use jujutsu_lib::repo::{ReadonlyRepo, Repo};
use jujutsu_lib::repo_path::RepoPath;
use jujutsu_lib::settings::UserSettings;
use jujutsu_lib::tree::{Tree, TreeDiffIterator};