repo: forbid checking out the root commit

Prevents `jj edit root` from succeeding, which would otherwise place
the repo in a state where every operation panics.
This commit is contained in:
Benjamin Saunders 2022-10-20 16:33:14 -07:00
parent 986fced69e
commit 037eaaf36c
10 changed files with 126 additions and 52 deletions

View file

@ -14,6 +14,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* `jj git push` will search `@-` for branches to push if `@` has none.
### Fixed bugs
* `jj edit root` now fails gracefully.
## [0.5.1] - 2022-10-17
No changes (just trying to get automated GitHub release to work).

View file

@ -606,8 +606,16 @@ impl MutableRepo {
Ok(rebaser.rebased().len())
}
pub fn set_wc_commit(&mut self, workspace_id: WorkspaceId, commit_id: CommitId) {
pub fn set_wc_commit(
&mut self,
workspace_id: WorkspaceId,
commit_id: CommitId,
) -> Result<(), RewriteRootCommit> {
if &commit_id == self.store().root_commit_id() {
return Err(RewriteRootCommit);
}
self.view_mut().set_wc_commit(workspace_id, commit_id);
Ok(())
}
pub fn remove_wc_commit(&mut self, workspace_id: &WorkspaceId) {
@ -624,13 +632,18 @@ impl MutableRepo {
let open_commit =
CommitBuilder::for_open_commit(settings, commit.id().clone(), commit.tree_id().clone())
.write_to_repo(self);
self.set_wc_commit(workspace_id, open_commit.id().clone());
self.set_wc_commit(workspace_id, open_commit.id().clone())
.unwrap();
open_commit
}
pub fn edit(&mut self, workspace_id: WorkspaceId, commit: &Commit) {
pub fn edit(
&mut self,
workspace_id: WorkspaceId,
commit: &Commit,
) -> Result<(), RewriteRootCommit> {
self.leave_commit(&workspace_id);
self.set_wc_commit(workspace_id, commit.id().clone());
self.set_wc_commit(workspace_id, commit.id().clone())
}
fn leave_commit(&mut self, workspace_id: &WorkspaceId) {
@ -965,3 +978,8 @@ impl MutableRepo {
);
}
}
/// Error from attempts to check out the root commit for editing
#[derive(Debug, Copy, Clone, Error)]
#[error("Cannot rewrite the root commit")]
pub struct RewriteRootCommit;

View file

@ -356,7 +356,7 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> {
.write_to_repo(self.mut_repo)
};
for workspace_id in workspaces_to_update.into_iter() {
self.mut_repo.edit(workspace_id, &new_wc_commit);
self.mut_repo.edit(workspace_id, &new_wc_commit).unwrap();
}
Ok(())
}

View file

@ -38,7 +38,7 @@ fn test_edit(use_git: bool) {
let mut tx = repo.start_transaction("test");
let ws_id = WorkspaceId::default();
tx.mut_repo().edit(ws_id.clone(), &wc_commit);
tx.mut_repo().edit(ws_id.clone(), &wc_commit).unwrap();
let repo = tx.commit();
assert_eq!(repo.view().get_wc_commit_id(&ws_id), Some(wc_commit.id()));
}
@ -88,7 +88,7 @@ fn test_checkout_previous_not_empty(use_git: bool) {
.set_open(true)
.write_to_repo(mut_repo);
let ws_id = WorkspaceId::default();
mut_repo.edit(ws_id.clone(), &old_checkout);
mut_repo.edit(ws_id.clone(), &old_checkout).unwrap();
let repo = tx.commit();
let mut tx = repo.start_transaction("test");
@ -96,7 +96,7 @@ fn test_checkout_previous_not_empty(use_git: bool) {
let new_checkout = testutils::create_random_commit(&settings, &repo)
.set_open(true)
.write_to_repo(mut_repo);
mut_repo.edit(ws_id, &new_checkout);
mut_repo.edit(ws_id, &new_checkout).unwrap();
mut_repo.rebase_descendants(&settings).unwrap();
assert!(mut_repo.view().heads().contains(old_checkout.id()));
}
@ -119,7 +119,7 @@ fn test_checkout_previous_empty(use_git: bool) {
)
.write_to_repo(mut_repo);
let ws_id = WorkspaceId::default();
mut_repo.edit(ws_id.clone(), &old_checkout);
mut_repo.edit(ws_id.clone(), &old_checkout).unwrap();
let repo = tx.commit();
let mut tx = repo.start_transaction("test");
@ -127,7 +127,7 @@ fn test_checkout_previous_empty(use_git: bool) {
let new_wc_commit = testutils::create_random_commit(&settings, &repo)
.set_open(true)
.write_to_repo(mut_repo);
mut_repo.edit(ws_id, &new_wc_commit);
mut_repo.edit(ws_id, &new_wc_commit).unwrap();
mut_repo.rebase_descendants(&settings).unwrap();
assert!(!mut_repo.view().heads().contains(old_checkout.id()));
}
@ -151,7 +151,7 @@ fn test_checkout_previous_empty_with_description(use_git: bool) {
.set_description("not empty".to_string())
.write_to_repo(mut_repo);
let ws_id = WorkspaceId::default();
mut_repo.edit(ws_id.clone(), &old_checkout);
mut_repo.edit(ws_id.clone(), &old_checkout).unwrap();
let repo = tx.commit();
let mut tx = repo.start_transaction("test");
@ -159,7 +159,7 @@ fn test_checkout_previous_empty_with_description(use_git: bool) {
let new_checkout = testutils::create_random_commit(&settings, &repo)
.set_open(true)
.write_to_repo(mut_repo);
mut_repo.edit(ws_id, &new_checkout);
mut_repo.edit(ws_id, &new_checkout).unwrap();
mut_repo.rebase_descendants(&settings).unwrap();
assert!(mut_repo.view().heads().contains(old_checkout.id()));
}
@ -188,7 +188,7 @@ fn test_checkout_previous_empty_non_head(use_git: bool) {
)
.write_to_repo(mut_repo);
let ws_id = WorkspaceId::default();
mut_repo.edit(ws_id.clone(), &old_checkout);
mut_repo.edit(ws_id.clone(), &old_checkout).unwrap();
let repo = tx.commit();
let mut tx = repo.start_transaction("test");
@ -196,7 +196,7 @@ fn test_checkout_previous_empty_non_head(use_git: bool) {
let new_checkout = testutils::create_random_commit(&settings, &repo)
.set_open(true)
.write_to_repo(mut_repo);
mut_repo.edit(ws_id, &new_checkout);
mut_repo.edit(ws_id, &new_checkout).unwrap();
mut_repo.rebase_descendants(&settings).unwrap();
assert_eq!(
*mut_repo.view().heads(),
@ -221,7 +221,7 @@ fn test_edit_initial(use_git: bool) {
let mut tx = repo.start_transaction("test");
let workspace_id = WorkspaceId::new("new-workspace".to_string());
tx.mut_repo().edit(workspace_id.clone(), &checkout);
tx.mut_repo().edit(workspace_id.clone(), &checkout).unwrap();
let repo = tx.commit();
assert_eq!(
repo.view().get_wc_commit_id(&workspace_id),
@ -468,7 +468,9 @@ fn test_has_changed(use_git: bool) {
mut_repo.remove_head(commit2.id());
mut_repo.add_public_head(&commit1);
let ws_id = WorkspaceId::default();
mut_repo.set_wc_commit(ws_id.clone(), commit1.id().clone());
mut_repo
.set_wc_commit(ws_id.clone(), commit1.id().clone())
.unwrap();
mut_repo.set_local_branch("main".to_string(), RefTarget::Normal(commit1.id().clone()));
mut_repo.set_remote_branch(
"main".to_string(),
@ -485,7 +487,9 @@ fn test_has_changed(use_git: bool) {
mut_repo.add_public_head(&commit1);
mut_repo.add_head(&commit1);
mut_repo.set_wc_commit(ws_id.clone(), commit1.id().clone());
mut_repo
.set_wc_commit(ws_id.clone(), commit1.id().clone())
.unwrap();
mut_repo.set_local_branch("main".to_string(), RefTarget::Normal(commit1.id().clone()));
mut_repo.set_remote_branch(
"main".to_string(),
@ -515,9 +519,11 @@ fn test_has_changed(use_git: bool) {
mut_repo.remove_head(commit2.id());
assert!(!mut_repo.has_changes());
mut_repo.set_wc_commit(ws_id.clone(), commit2.id().clone());
mut_repo
.set_wc_commit(ws_id.clone(), commit2.id().clone())
.unwrap();
assert!(mut_repo.has_changes());
mut_repo.set_wc_commit(ws_id, commit1.id().clone());
mut_repo.set_wc_commit(ws_id, commit1.id().clone()).unwrap();
assert!(!mut_repo.has_changes());
mut_repo.set_local_branch("main".to_string(), RefTarget::Normal(commit2.id().clone()));

View file

@ -266,8 +266,10 @@ fn test_resolve_symbol_checkout(use_git: bool) {
);
// Add some workspaces
mut_repo.set_wc_commit(ws1.clone(), commit1.id().clone());
mut_repo.set_wc_commit(ws2, commit2.id().clone());
mut_repo
.set_wc_commit(ws1.clone(), commit1.id().clone())
.unwrap();
mut_repo.set_wc_commit(ws2, commit2.id().clone()).unwrap();
// @ cannot be resolved without a default workspace ID
assert_eq!(
resolve_symbol(mut_repo.as_repo_ref(), "@", None),
@ -390,7 +392,9 @@ fn test_resolve_symbol_git_refs() {
// Cannot shadow checkout ("@") or root symbols
let ws_id = WorkspaceId::default();
mut_repo.set_wc_commit(ws_id.clone(), commit1.id().clone());
mut_repo
.set_wc_commit(ws_id.clone(), commit1.id().clone())
.unwrap();
mut_repo.set_git_ref("@".to_string(), RefTarget::Normal(commit2.id().clone()));
mut_repo.set_git_ref("root".to_string(), RefTarget::Normal(commit3.id().clone()));
assert_eq!(
@ -457,7 +461,9 @@ fn test_evaluate_expression_root_and_checkout(use_git: bool) {
);
// Can find the current checkout
mut_repo.set_wc_commit(WorkspaceId::default(), commit1.id().clone());
mut_repo
.set_wc_commit(WorkspaceId::default(), commit1.id().clone())
.unwrap();
assert_eq!(
resolve_commit_ids_in_workspace(mut_repo.as_repo_ref(), "@", &WorkspaceId::default()),
vec![commit1.id().clone()]
@ -609,7 +615,9 @@ fn test_evaluate_expression_parents(use_git: bool) {
assert_eq!(resolve_commit_ids(mut_repo.as_repo_ref(), "root-"), vec![]);
// Can find parents of the current checkout
mut_repo.set_wc_commit(WorkspaceId::default(), commit2.id().clone());
mut_repo
.set_wc_commit(WorkspaceId::default(), commit2.id().clone())
.unwrap();
assert_eq!(
resolve_commit_ids_in_workspace(mut_repo.as_repo_ref(), "@-", &WorkspaceId::default()),
vec![commit1.id().clone()]

View file

@ -1281,11 +1281,14 @@ fn test_rebase_descendants_update_checkout(use_git: bool) {
let ws2_id = WorkspaceId::new("ws2".to_string());
let ws3_id = WorkspaceId::new("ws3".to_string());
tx.mut_repo()
.set_wc_commit(ws1_id.clone(), commit_b.id().clone());
.set_wc_commit(ws1_id.clone(), commit_b.id().clone())
.unwrap();
tx.mut_repo()
.set_wc_commit(ws2_id.clone(), commit_b.id().clone());
.set_wc_commit(ws2_id.clone(), commit_b.id().clone())
.unwrap();
tx.mut_repo()
.set_wc_commit(ws3_id.clone(), commit_a.id().clone());
.set_wc_commit(ws3_id.clone(), commit_a.id().clone())
.unwrap();
let repo = tx.commit();
let mut tx = repo.start_transaction("test");
@ -1325,11 +1328,14 @@ fn test_rebase_descendants_update_checkout_abandoned(use_git: bool) {
let ws2_id = WorkspaceId::new("ws2".to_string());
let ws3_id = WorkspaceId::new("ws3".to_string());
tx.mut_repo()
.set_wc_commit(ws1_id.clone(), commit_b.id().clone());
.set_wc_commit(ws1_id.clone(), commit_b.id().clone())
.unwrap();
tx.mut_repo()
.set_wc_commit(ws2_id.clone(), commit_b.id().clone());
.set_wc_commit(ws2_id.clone(), commit_b.id().clone())
.unwrap();
tx.mut_repo()
.set_wc_commit(ws3_id.clone(), commit_a.id().clone());
.set_wc_commit(ws3_id.clone(), commit_a.id().clone())
.unwrap();
let repo = tx.commit();
let mut tx = repo.start_transaction("test");
@ -1381,7 +1387,8 @@ fn test_rebase_descendants_update_checkout_abandoned_merge(use_git: bool) {
.write_to_repo(tx.mut_repo());
let workspace_id = WorkspaceId::default();
tx.mut_repo()
.set_wc_commit(workspace_id.clone(), commit_d.id().clone());
.set_wc_commit(workspace_id.clone(), commit_d.id().clone())
.unwrap();
let repo = tx.commit();
let mut tx = repo.start_transaction("test");

View file

@ -178,43 +178,56 @@ fn test_merge_views_checkout() {
let ws7_id = WorkspaceId::new("ws7".to_string());
initial_tx
.mut_repo()
.set_wc_commit(ws1_id.clone(), commit1.id().clone());
.set_wc_commit(ws1_id.clone(), commit1.id().clone())
.unwrap();
initial_tx
.mut_repo()
.set_wc_commit(ws2_id.clone(), commit1.id().clone());
.set_wc_commit(ws2_id.clone(), commit1.id().clone())
.unwrap();
initial_tx
.mut_repo()
.set_wc_commit(ws3_id.clone(), commit1.id().clone());
.set_wc_commit(ws3_id.clone(), commit1.id().clone())
.unwrap();
initial_tx
.mut_repo()
.set_wc_commit(ws4_id.clone(), commit1.id().clone());
.set_wc_commit(ws4_id.clone(), commit1.id().clone())
.unwrap();
initial_tx
.mut_repo()
.set_wc_commit(ws5_id.clone(), commit1.id().clone());
.set_wc_commit(ws5_id.clone(), commit1.id().clone())
.unwrap();
let repo = initial_tx.commit();
let mut tx1 = repo.start_transaction("test");
tx1.mut_repo()
.set_wc_commit(ws1_id.clone(), commit2.id().clone());
.set_wc_commit(ws1_id.clone(), commit2.id().clone())
.unwrap();
tx1.mut_repo()
.set_wc_commit(ws2_id.clone(), commit2.id().clone());
.set_wc_commit(ws2_id.clone(), commit2.id().clone())
.unwrap();
tx1.mut_repo().remove_wc_commit(&ws4_id);
tx1.mut_repo()
.set_wc_commit(ws5_id.clone(), commit2.id().clone());
.set_wc_commit(ws5_id.clone(), commit2.id().clone())
.unwrap();
tx1.mut_repo()
.set_wc_commit(ws6_id.clone(), commit2.id().clone());
.set_wc_commit(ws6_id.clone(), commit2.id().clone())
.unwrap();
tx1.commit();
let mut tx2 = repo.start_transaction("test");
tx2.mut_repo()
.set_wc_commit(ws1_id.clone(), commit3.id().clone());
.set_wc_commit(ws1_id.clone(), commit3.id().clone())
.unwrap();
tx2.mut_repo()
.set_wc_commit(ws3_id.clone(), commit3.id().clone());
.set_wc_commit(ws3_id.clone(), commit3.id().clone())
.unwrap();
tx2.mut_repo()
.set_wc_commit(ws4_id.clone(), commit3.id().clone());
.set_wc_commit(ws4_id.clone(), commit3.id().clone())
.unwrap();
tx2.mut_repo().remove_wc_commit(&ws5_id);
tx2.mut_repo()
.set_wc_commit(ws7_id.clone(), commit3.id().clone());
.set_wc_commit(ws7_id.clone(), commit3.id().clone())
.unwrap();
// Make sure the end time different, assuming the clock has sub-millisecond
// precision.
std::thread::sleep(std::time::Duration::from_millis(1));

View file

@ -31,7 +31,7 @@ use jujutsu_lib::matchers::{EverythingMatcher, Matcher, PrefixMatcher, Visit};
use jujutsu_lib::op_heads_store::{OpHeadResolutionError, OpHeads, OpHeadsStore};
use jujutsu_lib::op_store::{OpStore, OpStoreError, OperationId, WorkspaceId};
use jujutsu_lib::operation::Operation;
use jujutsu_lib::repo::{BackendFactories, MutableRepo, ReadonlyRepo, RepoRef};
use jujutsu_lib::repo::{BackendFactories, MutableRepo, ReadonlyRepo, RepoRef, RewriteRootCommit};
use jujutsu_lib::repo_path::RepoPath;
use jujutsu_lib::revset::{RevsetError, RevsetParseError};
use jujutsu_lib::settings::UserSettings;
@ -76,6 +76,12 @@ impl From<config::ConfigError> for CommandError {
}
}
impl From<RewriteRootCommit> for CommandError {
fn from(err: RewriteRootCommit) -> Self {
CommandError::UserError(err.to_string())
}
}
impl From<BackendError> for CommandError {
fn from(err: BackendError) -> Self {
CommandError::UserError(format!("Unexpected error from store: {err}"))
@ -665,7 +671,9 @@ impl WorkspaceCommandHelper {
let commit = CommitBuilder::for_rewrite_from(&self.settings, &wc_commit)
.set_tree(new_tree_id)
.write_to_repo(mut_repo);
mut_repo.set_wc_commit(workspace_id, commit.id().clone());
mut_repo
.set_wc_commit(workspace_id, commit.id().clone())
.unwrap();
// Rebase descendants
let num_rebased = mut_repo.rebase_descendants(&self.settings)?;

View file

@ -1147,7 +1147,8 @@ fn cmd_checkout(
let mut tx = workspace_command
.start_transaction(&format!("check out commit {}", target.id().hex()));
if target.is_open() {
tx.mut_repo().edit(workspace_id, &target);
// Root is never open
tx.mut_repo().edit(workspace_id, &target).unwrap();
} else {
let commit_builder = CommitBuilder::for_open_commit(
ui.settings(),
@ -1156,7 +1157,7 @@ fn cmd_checkout(
)
.set_description(args.message.clone());
let new_commit = commit_builder.write_to_repo(tx.mut_repo());
tx.mut_repo().edit(workspace_id, &new_commit);
tx.mut_repo().edit(workspace_id, &new_commit).unwrap();
}
workspace_command.finish_transaction(ui, tx)?;
}
@ -1170,7 +1171,7 @@ fn cmd_checkout(
)
.set_description(args.message.clone());
let new_commit = commit_builder.write_to_repo(tx.mut_repo());
tx.mut_repo().edit(workspace_id, &new_commit);
tx.mut_repo().edit(workspace_id, &new_commit).unwrap();
workspace_command.finish_transaction(ui, tx)?;
}
Ok(())
@ -2479,7 +2480,7 @@ fn cmd_close(ui: &mut Ui, command: &CommandHelper, args: &CloseArgs) -> Result<(
)
.write_to_repo(tx.mut_repo());
for workspace_id in workspace_ids {
tx.mut_repo().edit(workspace_id, &new_checkout);
tx.mut_repo().edit(workspace_id, &new_checkout).unwrap();
}
}
workspace_command.finish_transaction(ui, tx)?;
@ -2569,7 +2570,7 @@ fn cmd_edit(ui: &mut Ui, command: &CommandHelper, args: &EditArgs) -> Result<(),
} else {
let mut tx =
workspace_command.start_transaction(&format!("edit commit {}", new_commit.id().hex()));
tx.mut_repo().edit(workspace_id, &new_commit);
tx.mut_repo().edit(workspace_id, &new_commit)?;
workspace_command.finish_transaction(ui, tx)?;
}
Ok(())
@ -2591,7 +2592,7 @@ fn cmd_new(ui: &mut Ui, command: &CommandHelper, args: &NewArgs) -> Result<(), C
.set_open(true)
.write_to_repo(tx.mut_repo());
let workspace_id = workspace_command.workspace_id();
tx.mut_repo().edit(workspace_id, &new_commit);
tx.mut_repo().edit(workspace_id, &new_commit).unwrap();
workspace_command.finish_transaction(ui, tx)?;
Ok(())
}

View file

@ -68,3 +68,12 @@ fn get_log_output(test_env: &TestEnvironment, cwd: &Path) -> String {
],
)
}
#[test]
fn test_edit_root() {
let test_env = TestEnvironment::default();
test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]);
let repo_path = test_env.env_root().join("repo");
let stderr = test_env.jj_cmd_failure(&repo_path, &["edit", "root"]);
insta::assert_snapshot!(stderr, @"Error: Cannot rewrite the root commit");
}