Skip to content

Commit

Permalink
drop "support" for legacy tree config
Browse files Browse the repository at this point in the history
The tree-level conflicts have worked well in practice and we don't
want to allow users to use legacy trees for new commits. We don't
really support legacy trees very well since 0590f8b anyway.
  • Loading branch information
martinvonz committed Aug 18, 2024
1 parent e2a5c83 commit aa0fbd9
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 55 deletions.
3 changes: 0 additions & 3 deletions cli/src/config/misc.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,6 @@ amend = ["squash"]
co = ["checkout"]
unamend = ["unsquash"]

[format]
tree-level-conflicts = true

[ui]
# TODO: delete ui.allow-filesets in jj 0.26+
allow-filesets = true
Expand Down
51 changes: 19 additions & 32 deletions lib/src/git_backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,20 +124,14 @@ pub struct GitBackend {
empty_tree_id: TreeId,
extra_metadata_store: TableStore,
cached_extra_metadata: Mutex<Option<Arc<ReadonlyTable>>>,
/// Whether tree of imported commit should be promoted to non-legacy format.
imported_commit_uses_tree_conflict_format: bool,
}

impl GitBackend {
pub fn name() -> &'static str {
"git"
}

fn new(
base_repo: gix::ThreadSafeRepository,
extra_metadata_store: TableStore,
imported_commit_uses_tree_conflict_format: bool,
) -> Self {
fn new(base_repo: gix::ThreadSafeRepository, extra_metadata_store: TableStore) -> Self {
let repo = Mutex::new(base_repo.to_thread_local());
let root_commit_id = CommitId::from_bytes(&[0; HASH_LENGTH]);
let root_change_id = ChangeId::from_bytes(&[0; CHANGE_ID_LENGTH]);
Expand All @@ -150,7 +144,6 @@ impl GitBackend {
empty_tree_id,
extra_metadata_store,
cached_extra_metadata: Mutex::new(None),
imported_commit_uses_tree_conflict_format,
}
}

Expand All @@ -166,7 +159,7 @@ impl GitBackend {
gix_open_opts_from_settings(settings),
)
.map_err(GitBackendInitError::InitRepository)?;
Self::init_with_repo(settings, store_path, git_repo_path, git_repo)
Self::init_with_repo(store_path, git_repo_path, git_repo)
}

/// Initializes backend by creating a new Git repo at the specified
Expand All @@ -190,7 +183,7 @@ impl GitBackend {
)
.map_err(GitBackendInitError::InitRepository)?;
let git_repo_path = workspace_root.join(".git");
Self::init_with_repo(settings, store_path, &git_repo_path, git_repo)
Self::init_with_repo(store_path, &git_repo_path, git_repo)
}

/// Initializes backend with an existing Git repo at the specified path.
Expand All @@ -210,11 +203,10 @@ impl GitBackend {
gix_open_opts_from_settings(settings),
)
.map_err(GitBackendInitError::OpenRepository)?;
Self::init_with_repo(settings, store_path, git_repo_path, git_repo)
Self::init_with_repo(store_path, git_repo_path, git_repo)
}

fn init_with_repo(
settings: &UserSettings,
store_path: &Path,
git_repo_path: &Path,
git_repo: gix::ThreadSafeRepository,
Expand Down Expand Up @@ -244,11 +236,7 @@ impl GitBackend {
.map_err(GitBackendInitError::Path)?;
};
let extra_metadata_store = TableStore::init(extra_path, HASH_LENGTH);
Ok(GitBackend::new(
git_repo,
extra_metadata_store,
settings.use_tree_conflict_format(),
))
Ok(GitBackend::new(git_repo, extra_metadata_store))
}

pub fn load(
Expand All @@ -271,11 +259,7 @@ impl GitBackend {
)
.map_err(GitBackendLoadError::OpenRepository)?;
let extra_metadata_store = TableStore::load(store_path.join("extra"), HASH_LENGTH);
Ok(GitBackend::new(
repo,
extra_metadata_store,
settings.use_tree_conflict_format(),
))
Ok(GitBackend::new(repo, extra_metadata_store))
}

fn lock_git_repo(&self) -> MutexGuard<'_, gix::Repository> {
Expand Down Expand Up @@ -348,6 +332,14 @@ impl GitBackend {
pub fn import_head_commits<'a>(
&self,
head_ids: impl IntoIterator<Item = &'a CommitId>,
) -> BackendResult<()> {
self.import_head_commits_with_tree_conflicts(head_ids, true)
}

fn import_head_commits_with_tree_conflicts<'a>(
&self,
head_ids: impl IntoIterator<Item = &'a CommitId>,
uses_tree_conflict_format: bool,
) -> BackendResult<()> {
let head_ids: HashSet<&CommitId> = head_ids
.into_iter()
Expand Down Expand Up @@ -377,7 +369,7 @@ impl GitBackend {
&mut mut_table,
&table_lock,
&head_ids,
self.imported_commit_uses_tree_conflict_format,
uses_tree_conflict_format,
)?;
self.save_extra_metadata_table(mut_table, &table_lock)
}
Expand Down Expand Up @@ -1491,14 +1483,7 @@ mod tests {
#[test_case(false; "legacy tree format")]
#[test_case(true; "tree-level conflict format")]
fn read_plain_git_commit(uses_tree_conflict_format: bool) {
let settings = {
let config = config::Config::builder()
.set_override("format.tree-level-conflicts", uses_tree_conflict_format)
.unwrap()
.build()
.unwrap();
UserSettings::from_config(config)
};
let settings = user_settings();
let temp_dir = testutils::new_temp_dir();
let store_path = temp_dir.path();
let git_repo_path = temp_dir.path().join("git");
Expand Down Expand Up @@ -1561,7 +1546,9 @@ mod tests {
let backend = GitBackend::init_external(&settings, store_path, git_repo.path()).unwrap();

// Import the head commit and its ancestors
backend.import_head_commits([&commit_id2]).unwrap();
backend
.import_head_commits_with_tree_conflicts([&commit_id2], uses_tree_conflict_format)
.unwrap();
// Ref should be created only for the head commit
let git_refs = backend
.open_git_repo()
Expand Down
3 changes: 1 addition & 2 deletions lib/src/repo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ impl ReadonlyRepo {
let backend = backend_initializer(user_settings, &store_path)?;
let backend_path = store_path.join("type");
fs::write(&backend_path, backend.name()).context(&backend_path)?;
let store = Store::new(backend, signer, user_settings.use_tree_conflict_format());
let store = Store::new(backend, signer);
let repo_settings = user_settings.with_repo(&repo_path).unwrap();

let op_store_path = repo_path.join("op_store");
Expand Down Expand Up @@ -632,7 +632,6 @@ impl RepoLoader {
let store = Store::new(
store_factories.load_backend(user_settings, &repo_path.join("store"))?,
Signer::from_settings(user_settings)?,
user_settings.use_tree_conflict_format(),
);
let repo_settings = user_settings.with_repo(repo_path).unwrap();
let op_store =
Expand Down
6 changes: 0 additions & 6 deletions lib/src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,12 +147,6 @@ impl UserSettings {
self.rng.clone()
}

pub fn use_tree_conflict_format(&self) -> bool {
self.config
.get_bool("format.tree-level-conflicts")
.unwrap_or(false)
}

pub fn user_name(&self) -> String {
self.config.get_string("user.name").unwrap_or_default()
}
Expand Down
13 changes: 1 addition & 12 deletions lib/src/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ pub struct Store {
signer: Signer,
commit_cache: RwLock<HashMap<CommitId, Arc<backend::Commit>>>,
tree_cache: RwLock<HashMap<(RepoPathBuf, TreeId), Arc<backend::Tree>>>,
use_tree_conflict_format: bool,
}

impl Debug for Store {
Expand All @@ -56,17 +55,12 @@ impl Debug for Store {
}

impl Store {
pub fn new(
backend: Box<dyn Backend>,
signer: Signer,
use_tree_conflict_format: bool,
) -> Arc<Self> {
pub fn new(backend: Box<dyn Backend>, signer: Signer) -> Arc<Self> {
Arc::new(Store {
backend,
signer,
commit_cache: Default::default(),
tree_cache: Default::default(),
use_tree_conflict_format,
})
}

Expand All @@ -78,11 +72,6 @@ impl Store {
&self.signer
}

/// Whether new tree should be written using the tree-level format.
pub fn use_tree_conflict_format(&self) -> bool {
self.use_tree_conflict_format
}

pub fn get_copy_records(
&self,
paths: Option<&[RepoPathBuf]>,
Expand Down

0 comments on commit aa0fbd9

Please sign in to comment.