cli: fix crash when initializing workspace colocated with Git repo

When initializing a workspace that shares its working copy with a Git
repo (i.e. `jj init --git-repo=.`), we import refs and HEAD when
creating the `WorkspaceCommandHelper` (as we do for all commands when
the working copy is shared). That makes the explicit import we do in
`cmd_init()` unnecessary. It also makes the checkout of HEAD I added
for the fix of #102 unnecessary. More importantly, as @yuja reported
in #177, it makes the command crash (at least if the repo is small
enough that the two checkouts happen within a second). I think the
problem is that the second checkout tries to create the same commit
except that the Change ID is different (the problem is not the
predecessors as I speculated in the issue tracker). The fix is to
simply avoid doing the redundant work. We still need a proper fix for
#27 eventually.

Closes #177.
This commit is contained in:
Martin von Zweigbergk 2022-03-28 06:46:21 -07:00 committed by Martin von Zweigbergk
parent a9d1b16937
commit 034fbb47e3
4 changed files with 56 additions and 39 deletions

View file

@ -43,6 +43,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed bugs
* Fixed crash on `jj init --git-repo=.` (it almost always crashed).
* When sharing the working copy with a Git repo, the automatic importing and
exporting (sometimes?) didn't happen on Windows.

View file

@ -1741,19 +1741,20 @@ fn cmd_init(ui: &mut Ui, command: &CommandHelper, args: &InitArgs) -> Result<(),
let mut workspace_command = command.for_loaded_repo(ui, workspace, repo)?;
if workspace_command.working_copy_shared_with_git() {
add_to_git_exclude(ui, &git_repo)?;
}
let mut tx = workspace_command.start_transaction("import git refs");
git::import_refs(tx.mut_repo(), &git_repo)?;
if let Some(git_head_id) = tx.mut_repo().view().git_head() {
let git_head_commit = tx.mut_repo().store().get_commit(&git_head_id)?;
tx.mut_repo().check_out(
workspace_command.workspace_id(),
ui.settings(),
&git_head_commit,
);
}
if tx.mut_repo().has_changes() {
workspace_command.finish_transaction(ui, tx)?;
} else {
let mut tx = workspace_command.start_transaction("import git refs");
git::import_refs(tx.mut_repo(), &git_repo)?;
if let Some(git_head_id) = tx.mut_repo().view().git_head() {
let git_head_commit = tx.mut_repo().store().get_commit(&git_head_id)?;
tx.mut_repo().check_out(
workspace_command.workspace_id(),
ui.settings(),
&git_head_commit,
);
}
if tx.mut_repo().has_changes() {
workspace_command.finish_transaction(ui, tx)?;
}
}
} else if args.git {
Workspace::init_internal_git(ui.settings(), wc_path.clone())?;

View file

@ -24,6 +24,7 @@ fn test_git_colocated() {
// Create a commit from jj and check that it's reflected in git
std::fs::write(workspace_root.join("new-file"), "contents").unwrap();
test_env.jj_cmd_success(&workspace_root, &["close", "-m", "add a file"]);
test_env.jj_cmd_success(&workspace_root, &["git", "import"]);
let stdout =
test_env.jj_cmd_success(&workspace_root, &["log", "-T", "commit_id \" \" branches"]);
insta::assert_snapshot!(stdout, @r###"

View file

@ -12,8 +12,38 @@
// See the License for the specific language governing permissions and
// limitations under the License.
use std::path::PathBuf;
use jujutsu::testutils::TestEnvironment;
fn init_git_repo(git_repo_path: &PathBuf) {
let git_repo = git2::Repository::init(&git_repo_path).unwrap();
let git_blob_oid = git_repo.blob(b"some content").unwrap();
let mut git_tree_builder = git_repo.treebuilder(None).unwrap();
git_tree_builder
.insert("some-file", git_blob_oid, 0o100644)
.unwrap();
let git_tree_id = git_tree_builder.write().unwrap();
let git_tree = git_repo.find_tree(git_tree_id).unwrap();
let git_signature = git2::Signature::new(
"Git User",
"git.user@example.com",
&git2::Time::new(123, 60),
)
.unwrap();
git_repo
.commit(
Some("refs/heads/my-branch"),
&git_signature,
&git_signature,
"My commit message",
&git_tree,
&[],
)
.unwrap();
git_repo.set_head("refs/heads/my-branch").unwrap();
}
#[test]
fn test_init_git_internal() {
let test_env = TestEnvironment::default();
@ -40,31 +70,7 @@ fn test_init_git_internal() {
fn test_init_git_external() {
let test_env = TestEnvironment::default();
let git_repo_path = test_env.env_root().join("git-repo");
let git_repo = git2::Repository::init(&git_repo_path).unwrap();
let git_blob_oid = git_repo.blob(b"some content").unwrap();
let mut git_tree_builder = git_repo.treebuilder(None).unwrap();
git_tree_builder
.insert("some-file", git_blob_oid, 0o100644)
.unwrap();
let git_tree_id = git_tree_builder.write().unwrap();
let git_tree = git_repo.find_tree(git_tree_id).unwrap();
let git_signature = git2::Signature::new(
"Git User",
"git.user@example.com",
&git2::Time::new(123, 60),
)
.unwrap();
git_repo
.commit(
Some("refs/heads/my-branch"),
&git_signature,
&git_signature,
"My commit message",
&git_tree,
&[],
)
.unwrap();
git_repo.set_head("refs/heads/my-branch").unwrap();
init_git_repo(&git_repo_path);
let stdout = test_env.jj_cmd_success(
test_env.env_root(),
@ -107,7 +113,7 @@ fn test_init_git_external() {
fn test_init_git_colocated() {
let test_env = TestEnvironment::default();
let workspace_root = test_env.env_root().join("repo");
git2::Repository::init(&workspace_root).unwrap();
init_git_repo(&workspace_root);
let stdout = test_env.jj_cmd_success(&workspace_root, &["init", "--git-repo", "."]);
// TODO: We should say "." instead of "" here
insta::assert_snapshot!(stdout, @r###"Initialized repo in ""
@ -125,6 +131,13 @@ fn test_init_git_colocated() {
assert!(git_target_file_contents
.replace('\\', "/")
.ends_with("../../../.git"));
// Check that the Git repo's HEAD got checked out
let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-r", "@-"]);
insta::assert_snapshot!(stdout, @r###"
o 8d698d4a8ee1 d3866db7e30a git.user@example.com 1970-01-01 01:02:03.000 +01:00 my-branch HEAD@git
~ My commit message
"###);
}
#[test]