From 4cd29a21303cba55c3cc4466a1ffd2fc758a02d9 Mon Sep 17 00:00:00 2001 From: Jun Wu Date: Wed, 10 Mar 2021 22:13:52 -0800 Subject: [PATCH 1/6] working_copy: avoid std::os::unix on Windows std::os::unix::fs::PermissionsExt::mode() does not exist on Windows. Treat files on Windows as regular files. --- lib/src/working_copy.rs | 3 +++ lib/tests/test_working_copy.rs | 2 ++ 2 files changed, 5 insertions(+) diff --git a/lib/src/working_copy.rs b/lib/src/working_copy.rs index 94eab7e3b..bf05e1416 100644 --- a/lib/src/working_copy.rs +++ b/lib/src/working_copy.rs @@ -254,7 +254,10 @@ impl TreeState { } else if metadata_file_type.is_symlink() { FileType::Symlink } else { + #[cfg(unix)] let mode = metadata.permissions().mode(); + #[cfg(windows)] + let mode = 0; if mode & 0o111 != 0 { FileType::Executable } else { diff --git a/lib/tests/test_working_copy.rs b/lib/tests/test_working_copy.rs index 7aec37290..844e23335 100644 --- a/lib/tests/test_working_copy.rs +++ b/lib/tests/test_working_copy.rs @@ -195,6 +195,7 @@ fn test_checkout_file_transitions(use_git: bool) { assert_eq!(maybe_metadata.is_ok(), true, "{:?} should exist", path); let metadata = maybe_metadata.unwrap(); assert_eq!(metadata.is_file(), true, "{:?} should be a file", path); + #[cfg(unix)] assert_eq!( metadata.permissions().mode() & 0o111, 0, @@ -206,6 +207,7 @@ fn test_checkout_file_transitions(use_git: bool) { assert_eq!(maybe_metadata.is_ok(), true, "{:?} should exist", path); let metadata = maybe_metadata.unwrap(); assert_eq!(metadata.is_file(), true, "{:?} should be a file", path); + #[cfg(unix)] assert_ne!( metadata.permissions().mode() & 0o111, 0, From eacab648b0715dc5eda1fac19bbdd204d61dad74 Mon Sep 17 00:00:00 2001 From: Jun Wu Date: Wed, 10 Mar 2021 22:28:39 -0800 Subject: [PATCH 2/6] working_copy: clean up ".git" automatically TreeState::write_tree leaves a ".git" file in the working copy. This is undesirable but more problematic on Windows - The second time TreeState::write_tree would panic because Repository::init_opts will fail with a Permission Denied error. This seems to be a libgit2 defect. But for now let's just remove ".git" automatically. This makes `cargo test --test smoke_test` pass on Windows. --- lib/src/working_copy.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/lib/src/working_copy.rs b/lib/src/working_copy.rs index bf05e1416..970a917d2 100644 --- a/lib/src/working_copy.rs +++ b/lib/src/working_copy.rs @@ -292,6 +292,19 @@ impl TreeState { let git_repo_dir = tempfile::tempdir().unwrap(); let mut git_repo_options = RepositoryInitOptions::new(); git_repo_options.workdir_path(&self.working_copy_path); + // Repository::init_opts creates a ".git" file in the working copy, + // which is undesired. On Windows it's worse because that ".git" makes + // the next Repository::init_opts fail with "Permission Denied". + // Automatically remove it. + let _cleanup_dot_git = { + struct Cleanup(PathBuf); + impl Drop for Cleanup { + fn drop(&mut self) { + let _ = fs::remove_file(&self.0); + } + } + Cleanup(self.working_copy_path.join(".git")) + }; let git_repo = Repository::init_opts(git_repo_dir.path(), &git_repo_options).unwrap(); let mut work = vec![(DirRepoPath::root(), self.working_copy_path.clone())]; From 2f93ebd42c84cd4f17f07a31cfe45b416acd16b3 Mon Sep 17 00:00:00 2001 From: Jun Wu Date: Wed, 10 Mar 2021 22:35:16 -0800 Subject: [PATCH 3/6] commands: do not use debug print for path "{:?}" escapes `\` to `\\` for Windows paths. That breaks tests checking paths without using "{:?}". Use PathBuf::display() in both commands and tests to get consistent output. This fixes test_init_local, test_init_git_internal, and test_init_git_external on Windows. --- src/commands.rs | 6 +++++- tests/test_init_command.rs | 6 ++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/commands.rs b/src/commands.rs index f81d19c50..6c2dc8c15 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -615,7 +615,11 @@ fn cmd_init( } else { repo = ReadonlyRepo::init_local(ui.settings(), wc_path); } - writeln!(ui, "Initialized repo in {:?}", repo.working_copy_path()); + writeln!( + ui, + "Initialized repo in \"{}\"", + repo.working_copy_path().display() + ); Ok(()) } diff --git a/tests/test_init_command.rs b/tests/test_init_command.rs index b21d23e0a..ec77cd251 100644 --- a/tests/test_init_command.rs +++ b/tests/test_init_command.rs @@ -51,10 +51,12 @@ fn test_init_git_external() { assert!(repo_path.join(".jj").is_dir()); let store_file_contents = std::fs::read_to_string(repo_path.join(".jj").join("store")).unwrap(); assert!(store_file_contents.starts_with("git: ")); - assert!(store_file_contents.ends_with("/git-repo")); + assert!(store_file_contents + .replace('\\', "/") + .ends_with("/git-repo")); assert_eq!( output.stdout_string(), - format!("Initialized repo in \"{}\"\n", repo_path.to_str().unwrap()) + format!("Initialized repo in \"{}\"\n", repo_path.display()) ); } From 935da3e13f0d76033b6427b383d924b4747b89eb Mon Sep 17 00:00:00 2001 From: Jun Wu Date: Wed, 10 Mar 2021 22:59:52 -0800 Subject: [PATCH 4/6] lock: treat PermissionDenied on Windows as transient error On Windows it can be PermissionDenied when creating the new file exclusively. This change makes lock_concurrent test pass on Windows. --- lib/src/lock.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/src/lock.rs b/lib/src/lock.rs index 193e5725f..1ac400312 100644 --- a/lib/src/lock.rs +++ b/lib/src/lock.rs @@ -36,6 +36,9 @@ impl FileLock { Err(err) if err.kind() == std::io::ErrorKind::AlreadyExists => { Err(backoff::Error::Transient(err)) } + Err(err) if cfg!(windows) && err.kind() == std::io::ErrorKind::PermissionDenied => { + Err(backoff::Error::Transient(err)) + } Err(err) => Err(backoff::Error::Permanent(err)), }; let mut backoff = ExponentialBackoff { From d1d502c062b8ea6a9bce20dd1937b4ee41e208b1 Mon Sep 17 00:00:00 2001 From: Jun Wu Date: Wed, 10 Mar 2021 23:08:22 -0800 Subject: [PATCH 5/6] tests: disable tests failing on Windows This unblocks enabling GitHub CI. I took a quick look at some failures but the causes do not seem obvious to me. --- lib/tests/test_commit_concurrent.rs | 2 ++ lib/tests/test_working_copy.rs | 1 + 2 files changed, 3 insertions(+) diff --git a/lib/tests/test_commit_concurrent.rs b/lib/tests/test_commit_concurrent.rs index c30404c01..e90c590f6 100644 --- a/lib/tests/test_commit_concurrent.rs +++ b/lib/tests/test_commit_concurrent.rs @@ -36,6 +36,7 @@ fn count_non_merge_operations(repo: &ReadonlyRepo) -> u32 { num_ops } +#[cfg(unix)] #[test_case(false ; "local store")] #[test_case(true ; "git store")] fn test_commit_parallel(use_git: bool) { @@ -68,6 +69,7 @@ fn test_commit_parallel(use_git: bool) { assert_eq!(count_non_merge_operations(&repo), 101); } +#[cfg(unix)] #[test_case(false ; "local store")] #[test_case(true ; "git store")] fn test_commit_parallel_instances(use_git: bool) { diff --git a/lib/tests/test_working_copy.rs b/lib/tests/test_working_copy.rs index 844e23335..1bfa527e2 100644 --- a/lib/tests/test_working_copy.rs +++ b/lib/tests/test_working_copy.rs @@ -50,6 +50,7 @@ fn test_root(use_git: bool) { assert_eq!(wc_commit.committer().email, settings.user_email()); } +#[cfg(unix)] #[test_case(false ; "local store")] #[test_case(true ; "git store")] fn test_checkout_file_transitions(use_git: bool) { From 0fb59a5155ad4ea083edbb9e078fd53a922d31e7 Mon Sep 17 00:00:00 2001 From: Jun Wu Date: Wed, 10 Mar 2021 23:02:24 -0800 Subject: [PATCH 6/6] github: setup CI Run tests on major platforms using GitHub actions. --- .github/workflows/build.yml | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 .github/workflows/build.yml diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml new file mode 100644 index 000000000..85d0cfb6e --- /dev/null +++ b/.github/workflows/build.yml @@ -0,0 +1,29 @@ +name: build + +on: + push: + branches: [ main ] + pull_request: + branches: [ main ] + +jobs: + build: + runs-on: ${{ matrix.operating-system }} + strategy: + matrix: + operating-system: [ ubuntu-latest, windows-latest, macos-latest ] + + steps: + - uses: actions/checkout@v2 + - name: Install Rust nightly + uses: actions-rs/toolchain@v1 + with: + toolchain: nightly + override: true + profile: minimal + - name: Build + run: | + cargo build --workspace --verbose + - name: Test + run: | + cargo test --workspace --verbose