Crash if a dir jj doesn't track is a file in another commit. Also happens if untracked file conflicts with a tracked file.

[ilyagr]

Here is a part of a test that crashes (add it to test_resolve_command.rs):

#[test]
fn test_crash() {
    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");

    create_commit(&test_env, &repo_path, "base", &[], &[("file", "base\n")]);
    create_commit(&test_env, &repo_path, "dir", &["base"], &[]);
    std::fs::remove_file(repo_path.join("file")).unwrap();
    std::fs::create_dir(repo_path.join("file")).unwrap();

    // Uncommenting the line below fixes the crash
    // std::fs::write(repo_path.join("file").join("placeholder"), "").unwrap();
    // Crash happens here
    create_commit(&test_env, &repo_path, "del", &["base"], &[]);
    std::fs::remove_file(repo_path.join("file")).unwrap();
}

The point is that file is a directory that jj doesn't track since it's empty. In another commit, it is a file. Updating to the second commit confuses jj.

The error is:

thread 'test_crazy_conflict_description' panicked at 'Unexpected failure.
code-255
stderr=```"BUG: Working copy lock was dropped without being closed.\nInternal error: Failed to check out commit 0e5547529a736970e82066471c16ff3bec6ae23b: Failed to open file /tmp/jj-test-aMpIzb/repo/file for writing: Os { code: 17, kind: AlreadyExists, message: \"File exists\" }\n"```
command=`"/home/ilyagr/dev/jj/target/debug/jj" "new" "-m" "del" "base"`
code=255
stdout=""
stderr="BUG: Working copy lock was dropped without being closed.\nInternal error: Failed to check out commit 0e5547529a736970e82066471c16ff3bec6ae23b: Failed to open file /tmp/jj-test-aMpIzb/repo/file for writing: Os { code: 17, kind: AlreadyExists, message: \"File exists\" }\n"
', /rustc/352eb59a4c33abf739914422f2ad975925750146/library/core/src/ops/function.rs:507:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

[ilyagr]

This error probably occurred at:

jj/lib/src/working_copy.rs

Lines 715 to 722 in af55d17

	let mut file = OpenOptions::new()
	.write(true)
	.create_new(true) // Don't overwrite un-ignored file. Don't follow symlink.
	.open(disk_path)
	.map_err(|err| CheckoutError::IoError {
	message: format!("Failed to open file {} for writing", disk_path.display()),
	err,
	})?;

There is also a very similar line in the write_conflict function:

jj/lib/src/working_copy.rs

Line 784 in af55d17

message: format!("Failed to open file {} for writing", disk_path.display()),

[ilyagr]

One solution would be to check if anything happens to be in the way around

jj/lib/src/working_copy.rs

Lines 896 to 900 in af55d17

	Diff::Added(after) => {
	let file_state = match after {
	TreeValue::File { id, executable } => {
	self.write_file(&disk_path, &path, &id, executable)?
	}

We could then remove it. I'm unsure if that's safe

In other news, when I reproduced this bug in my own repo, I got a bit stuck until I removed the offending directory:

$ rm Cargo.toml && mkdir Cargo.toml
$ RUST_BACKTRACE=1 target/debug/jj up @-
BUG: Working copy lock was dropped without being closed.
Internal error: Failed to check out commit 75eddb01c4150fd349a5ce1f4b5ece718940a281: Failed to open file /home/ilyagr/dev/jj/Cargo.toml for writing: Os { code: 17, kind: AlreadyExists, message: "File exists" }

$ JJ_BACKTRACE=1 target/debug/jj up @-
Error: The working copy is stale (not updated since operation 8508b3dece63).
Hint: Run `jj workspace update-stale` to update it.

$ jj workspace update-stale
BUG: Working copy lock was dropped without being closed.
Internal error: Failed to check out commit 75eddb01c4150fd349a5ce1f4b5ece718940a281: Failed to open file /home/ilyagr/dev/jj/Cargo.toml for writing: Os { code: 17, kind: AlreadyExists, message: "File exists" }

[martinvonz]

I suppose we could just delete the directory if it's empty in this scenario, but it can be more complicated if the directory was instead ignored. I think we should return a list of paths and errors we encountered, so we can tell the user that we failed to update some paths. Maybe we can just leave those paths as they are in the working copy. That would mean that they would instead appear modified in the working copy. I'm not sure if there's a better option.

The "Working copy lock was dropped without being closed." can probably be ignored. I think that just happened because of the other bug, and it only happens in debug builds.