From ab8c1e614f36962e29f80da13fbb9ab15b7f6f10 Mon Sep 17 00:00:00 2001 From: bsdinis Date: Fri, 10 Jan 2025 23:29:57 +0100 Subject: [PATCH] git: spawn a separate git process for network operations Reasoning: `jj` fails to push/fetch over ssh depending on the system. Issue #4979 lists over 20 related issues on this and proposes spawning a `git` subprocess for tasks related to the network (in fact, just push/fetch are enough). This PR implements this. Implementation Details: This PR implements shelling out to `git` via `std::process::Command`. There are 2 sharp edges with the patch: - it relies on having to parse out git errors to match the error codes (and parsing git2's errors in one particular instance to match the error behaviour). This seems mostly unavoidable - to ensure matching behaviour with git2, the tests are maintained across the two implementations. This is done using test_case, as with the rest of the codebase Testing: Run the rust tests: ``` $ cargo test ``` Build: ``` $ cargo build ``` Clone a private repo: ``` $ path/to/jj git clone --config='git.subprocess=true' ``` Create new commit and push ``` $ echo "TEST" > this_is_a_test_file.txt $ path/to/jj describe -m 'test commit' $ path/to/jj git push --config='git.subprocess=true' -b ``` Issues Closed With a grain of salt, but most of these problems should be fixed (or at least checked if they are fixed). They are the ones listed in #4979 . SSH: - https://github.com/jj-vcs/jj/issues/63 - https://github.com/jj-vcs/jj/issues/440 - https://github.com/jj-vcs/jj/issues/1455 - https://github.com/jj-vcs/jj/issues/1507 - https://github.com/jj-vcs/jj/issues/2931 - https://github.com/jj-vcs/jj/issues/2958 - https://github.com/jj-vcs/jj/issues/3322 - https://github.com/jj-vcs/jj/issues/4101 - https://github.com/jj-vcs/jj/issues/4333 - https://github.com/jj-vcs/jj/issues/4386 - https://github.com/jj-vcs/jj/issues/4488 - https://github.com/jj-vcs/jj/issues/4591 - https://github.com/jj-vcs/jj/issues/4802 - https://github.com/jj-vcs/jj/issues/4870 - https://github.com/jj-vcs/jj/issues/4937 - https://github.com/jj-vcs/jj/issues/4978 - https://github.com/jj-vcs/jj/issues/5120 - https://github.com/jj-vcs/jj/issues/5166 Clone/fetch/push/pull: - https://github.com/jj-vcs/jj/issues/360 - https://github.com/jj-vcs/jj/issues/1278 - https://github.com/jj-vcs/jj/issues/1957 - https://github.com/jj-vcs/jj/issues/2295 - https://github.com/jj-vcs/jj/issues/3851 - https://github.com/jj-vcs/jj/issues/4177 - https://github.com/jj-vcs/jj/issues/4682 - https://github.com/jj-vcs/jj/issues/4719 - https://github.com/jj-vcs/jj/issues/4889 - https://github.com/jj-vcs/jj/discussions/5147 - https://github.com/jj-vcs/jj/issues/5238 Notable Holdouts: - Interactive HTTP authentication (https://github.com/jj-vcs/jj/issues/401, https://github.com/jj-vcs/jj/issues/469) - libssh2-sys dependency on windows problem (can only be removed if/when we get rid of libgit2): https://github.com/jj-vcs/jj/issues/3984 --- .github/workflows/build.yml | 13 +- CHANGELOG.md | 5 + cli/src/commands/git/clone.rs | 4 +- cli/src/commands/git/push.rs | 20 +- cli/src/config-schema.json | 10 + cli/src/git_util.rs | 54 +-- cli/tests/common/mod.rs | 13 + cli/tests/test_git_clone.rs | 367 ++++++++++++++++++--- cli/tests/test_git_fetch.rs | 473 +++++++++++++++++++++++--- cli/tests/test_git_push.rs | 543 ++++++++++++++++++++++++++---- docs/config.md | 22 ++ flake.nix | 4 + lib/src/config/misc.toml | 2 + lib/src/git.rs | 299 ++++++++++++++--- lib/src/git_subprocess.rs | 603 ++++++++++++++++++++++++++++++++++ lib/src/lib.rs | 2 + lib/src/settings.rs | 7 + lib/tests/test_git.rs | 192 +++++++---- 18 files changed, 2340 insertions(+), 293 deletions(-) create mode 100644 lib/src/git_subprocess.rs diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 255f6b1a06..14870e74ef 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -86,9 +86,20 @@ jobs: tool: nextest - name: Build run: cargo build --workspace --all-targets --verbose ${{ matrix.cargo_flags }} - - name: Test + - name: Test [non-windows] + if: ${{ matrix.os != 'windows-latest' }} + run: | + cargo nextest run --workspace --all-targets --verbose ${{ matrix.cargo_flags }} + env: + RUST_BACKTRACE: 1 + CARGO_TERM_COLOR: always + - name: Test [windows] + if: ${{ matrix.os == 'windows-latest' }} run: cargo nextest run --workspace --all-targets --verbose ${{ matrix.cargo_flags }} env: + # tests clear the PATH variable (but is only felt on windows), + # so we need to set the git executable + TEST_GIT_EXECUTABLE_PATH: 'C:\Program Files\Git\bin\git.exe' RUST_BACKTRACE: 1 CARGO_TERM_COLOR: always diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d71a42b34..f2191b954d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -68,6 +68,11 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### New features +* `jj git {push,clone,fetch}` can now spawn an external `git` subprocess, via + the `git.subprocess = true` config knob. This provides an alternative that, + when turned on, fixes SSH bugs when interacting with Git remotes due to + `libgit2`s limitations [#4979](https://github.com/jj-vcs/jj/issues/4979). + * `jj evolog` and `jj op log` now accept `--reversed`. * `jj restore` now supports `-i`/`--interactive` selection. diff --git a/cli/src/commands/git/clone.rs b/cli/src/commands/git/clone.rs index 17a74ef9db..49e2a843ce 100644 --- a/cli/src/commands/git/clone.rs +++ b/cli/src/commands/git/clone.rs @@ -204,8 +204,7 @@ fn fetch_new_remote( )?; let git_settings = workspace_command.settings().git_settings()?; let mut fetch_tx = workspace_command.start_transaction(); - - let stats = with_remote_git_callbacks(ui, None, |cb| { + let stats = with_remote_git_callbacks(ui, None, &git_settings, |cb| { git::fetch( fetch_tx.repo_mut(), &git_repo, @@ -222,6 +221,7 @@ fn fetch_new_remote( } GitFetchError::GitImportError(err) => CommandError::from(err), GitFetchError::InternalGitError(err) => map_git_error(err), + GitFetchError::Subprocess(err) => user_error(err), GitFetchError::InvalidBranchPattern => { unreachable!("we didn't provide any globs") } diff --git a/cli/src/commands/git/push.rs b/cli/src/commands/git/push.rs index 9111cdd217..3cb97ed0ed 100644 --- a/cli/src/commands/git/push.rs +++ b/cli/src/commands/git/push.rs @@ -357,9 +357,23 @@ pub fn cmd_git_push( let mut sideband_progress_callback = |progress_message: &[u8]| { _ = writer.write(ui, progress_message); }; - with_remote_git_callbacks(ui, Some(&mut sideband_progress_callback), |cb| { - git::push_branches(tx.repo_mut(), &git_repo, &remote, &targets, cb) - }) + + let git_settings = tx.settings().git_settings()?; + with_remote_git_callbacks( + ui, + Some(&mut sideband_progress_callback), + &git_settings, + |cb| { + git::push_branches( + tx.repo_mut(), + &git_repo, + &git_settings, + &remote, + &targets, + cb, + ) + }, + ) .map_err(|err| match err { GitPushError::InternalGitError(err) => map_git_error(err), GitPushError::RefInUnexpectedLocation(refs) => user_error_with_hint( diff --git a/cli/src/config-schema.json b/cli/src/config-schema.json index 69f4c01edd..84f8182dce 100644 --- a/cli/src/config-schema.json +++ b/cli/src/config-schema.json @@ -380,6 +380,16 @@ "type": "boolean", "description": "Whether jj should sign commits before pushing", "default": "false" + }, + "subprocess": { + "type": "boolean", + "description": "Whether jj spawns a git subprocess for network operations (push/fetch/clone)", + "default": false + }, + "executable-path": { + "type": "string", + "description": "Path to the git executable", + "default": "git" } } }, diff --git a/cli/src/git_util.rs b/cli/src/git_util.rs index 604c48259f..f1898c1213 100644 --- a/cli/src/git_util.rs +++ b/cli/src/git_util.rs @@ -41,6 +41,7 @@ use jj_lib::op_store::RefTarget; use jj_lib::op_store::RemoteRef; use jj_lib::repo::ReadonlyRepo; use jj_lib::repo::Repo; +use jj_lib::settings::GitSettings; use jj_lib::store::Store; use jj_lib::str_util::StringPattern; use jj_lib::workspace::Workspace; @@ -282,29 +283,40 @@ type SidebandProgressCallback<'a> = &'a mut dyn FnMut(&[u8]); pub fn with_remote_git_callbacks( ui: &Ui, sideband_progress_callback: Option>, + git_settings: &GitSettings, f: impl FnOnce(git::RemoteCallbacks<'_>) -> T, ) -> T { - let mut callbacks = git::RemoteCallbacks::default(); - let mut progress_callback = None; - if let Some(mut output) = ui.progress_output() { - let mut progress = Progress::new(Instant::now()); - progress_callback = Some(move |x: &git::Progress| { - _ = progress.update(Instant::now(), x, &mut output); - }); + if git_settings.subprocess { + // TODO: with git2, we are able to display progress from the data that is given + // With the git processes themselves, this is significantly harder, as it + // requires parsing the output directly + // + // In any case, this would be the place to add that funcionalty + f(git::RemoteCallbacks::default()) + } else { + let mut callbacks = git::RemoteCallbacks::default(); + let mut progress_callback = None; + if let Some(mut output) = ui.progress_output() { + let mut progress = Progress::new(Instant::now()); + progress_callback = Some(move |x: &git::Progress| { + _ = progress.update(Instant::now(), x, &mut output); + }); + } + callbacks.progress = progress_callback + .as_mut() + .map(|x| x as &mut dyn FnMut(&git::Progress)); + callbacks.sideband_progress = + sideband_progress_callback.map(|x| x as &mut dyn FnMut(&[u8])); + let mut get_ssh_keys = get_ssh_keys; // Coerce to unit fn type + callbacks.get_ssh_keys = Some(&mut get_ssh_keys); + let mut get_pw = + |url: &str, _username: &str| pinentry_get_pw(url).or_else(|| terminal_get_pw(ui, url)); + callbacks.get_password = Some(&mut get_pw); + let mut get_user_pw = + |url: &str| Some((terminal_get_username(ui, url)?, terminal_get_pw(ui, url)?)); + callbacks.get_username_password = Some(&mut get_user_pw); + f(callbacks) } - callbacks.progress = progress_callback - .as_mut() - .map(|x| x as &mut dyn FnMut(&git::Progress)); - callbacks.sideband_progress = sideband_progress_callback.map(|x| x as &mut dyn FnMut(&[u8])); - let mut get_ssh_keys = get_ssh_keys; // Coerce to unit fn type - callbacks.get_ssh_keys = Some(&mut get_ssh_keys); - let mut get_pw = - |url: &str, _username: &str| pinentry_get_pw(url).or_else(|| terminal_get_pw(ui, url)); - callbacks.get_password = Some(&mut get_pw); - let mut get_user_pw = - |url: &str| Some((terminal_get_username(ui, url)?, terminal_get_pw(ui, url)?)); - callbacks.get_username_password = Some(&mut get_user_pw); - f(callbacks) } pub fn print_git_import_stats( @@ -635,7 +647,7 @@ pub fn git_fetch( let git_settings = tx.settings().git_settings()?; for remote in remotes { - let stats = with_remote_git_callbacks(ui, None, |cb| { + let stats = with_remote_git_callbacks(ui, None, &git_settings, |cb| { git::fetch( tx.repo_mut(), git_repo, diff --git a/cli/tests/common/mod.rs b/cli/tests/common/mod.rs index e1a883934c..a295ebc665 100644 --- a/cli/tests/common/mod.rs +++ b/cli/tests/common/mod.rs @@ -80,6 +80,7 @@ impl Default for TestEnvironment { 'format_time_range(time_range)' = 'time_range.start() ++ " - " ++ time_range.end()' "#, ); + env } } @@ -280,6 +281,18 @@ impl TestEnvironment { self.env_vars.insert(key.into(), val.into()); } + pub fn set_up_git_subprocessing(&self) { + self.add_config("git.subprocess = true"); + + // add a git path from env: this is only used if git.subprocess = true + if let Ok(git_executable_path) = std::env::var("TEST_GIT_EXECUTABLE_PATH") { + self.add_config(format!( + "git.executable-path = {}", + to_toml_value(git_executable_path) + )); + } + } + pub fn current_operation_id(&self, repo_path: &Path) -> String { let id_and_newline = self.jj_cmd_success(repo_path, &["debug", "operation", "--display=id"]); diff --git a/cli/tests/test_git_clone.rs b/cli/tests/test_git_clone.rs index bbca8ab754..18fd4a640f 100644 --- a/cli/tests/test_git_clone.rs +++ b/cli/tests/test_git_clone.rs @@ -17,9 +17,11 @@ use std::path::Path; use std::path::PathBuf; use indoc::formatdoc; +use test_case::test_case; use crate::common::get_stderr_string; use crate::common::get_stdout_string; +use crate::common::strip_last_line; use crate::common::to_toml_value; use crate::common::TestEnvironment; @@ -50,28 +52,37 @@ fn set_up_git_repo_with_file(git_repo: &git2::Repository, filename: &str) { git_repo.set_head("refs/heads/main").unwrap(); } -#[test] -fn test_git_clone() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_clone(subprocess: bool) { let test_env = TestEnvironment::default(); test_env.add_config("git.auto-local-bookmark = true"); + if subprocess { + test_env.set_up_git_subprocessing(); + } let git_repo_path = test_env.env_root().join("source"); let git_repo = git2::Repository::init(git_repo_path).unwrap(); // Clone an empty repo let (stdout, stderr) = test_env.jj_cmd_ok(test_env.env_root(), &["git", "clone", "source", "empty"]); - insta::assert_snapshot!(stdout, @""); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Fetching into new repo in "$TEST_ENV/empty" Nothing changed. "###); + } set_up_non_empty_git_repo(&git_repo); // Clone with relative source path let (stdout, stderr) = test_env.jj_cmd_ok(test_env.env_root(), &["git", "clone", "source", "clone"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Fetching into new repo in "$TEST_ENV/clone" bookmark: main@origin [new] tracked @@ -80,15 +91,20 @@ fn test_git_clone() { Parent commit : mzyxwzks 9f01a0e0 main | message Added 1 files, modified 0 files, removed 0 files "###); + } assert!(test_env.env_root().join("clone").join("file").exists()); // Subsequent fetch should just work even if the source path was relative let (stdout, stderr) = test_env.jj_cmd_ok(&test_env.env_root().join("clone"), &["git", "fetch"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Nothing changed. "###); + } // Failed clone should clean up the destination directory std::fs::create_dir(test_env.env_root().join("bad")).unwrap(); @@ -98,11 +114,21 @@ fn test_git_clone() { .code(1); let stdout = test_env.normalize_output(&get_stdout_string(&assert)); let stderr = test_env.normalize_output(&get_stderr_string(&assert)); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); - insta::assert_snapshot!(stderr, @r###" - Fetching into new repo in "$TEST_ENV/failed" - Error: could not find repository at '$TEST_ENV/bad'; class=Repository (6) - "###); + } + // git2's internal error is slightly different + if subprocess { + insta::assert_snapshot!(stderr, @r#" + Fetching into new repo in "$TEST_ENV/failed" + Error: Could not find repository at '$TEST_ENV/bad' + "#); + } else { + insta::assert_snapshot!(stderr, @r#" + Fetching into new repo in "$TEST_ENV/failed" + Error: could not find repository at '$TEST_ENV/bad'; class=Repository (6) + "#); + } assert!(!test_env.env_root().join("failed").exists()); // Failed clone shouldn't remove the existing destination directory @@ -113,47 +139,68 @@ fn test_git_clone() { .code(1); let stdout = test_env.normalize_output(&get_stdout_string(&assert)); let stderr = test_env.normalize_output(&get_stderr_string(&assert)); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); - insta::assert_snapshot!(stderr, @r###" + } + // git2's internal error is slightly different + if subprocess { + insta::assert_snapshot!(stderr, @r#" Fetching into new repo in "$TEST_ENV/failed" - Error: could not find repository at '$TEST_ENV/bad'; class=Repository (6) - "###); + Error: Could not find repository at '$TEST_ENV/bad' + "#); + } else { + insta::assert_snapshot!(stderr, @r#" + Fetching into new repo in "$TEST_ENV/failed" + Error: could not find repository at '$TEST_ENV/bad'; class=Repository (6) + "#); + } assert!(test_env.env_root().join("failed").exists()); assert!(!test_env.env_root().join("failed").join(".jj").exists()); // Failed clone (if attempted) shouldn't remove the existing workspace let stderr = test_env.jj_cmd_failure(test_env.env_root(), &["git", "clone", "bad", "clone"]); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Error: Destination path exists and is not an empty directory "###); + } assert!(test_env.env_root().join("clone").join(".jj").exists()); // Try cloning into an existing workspace let stderr = test_env.jj_cmd_failure(test_env.env_root(), &["git", "clone", "source", "clone"]); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Error: Destination path exists and is not an empty directory "###); + } // Try cloning into an existing file std::fs::write(test_env.env_root().join("file"), "contents").unwrap(); let stderr = test_env.jj_cmd_failure(test_env.env_root(), &["git", "clone", "source", "file"]); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Error: Destination path exists and is not an empty directory "###); + } // Try cloning into non-empty, non-workspace directory std::fs::remove_dir_all(test_env.env_root().join("clone").join(".jj")).unwrap(); let stderr = test_env.jj_cmd_failure(test_env.env_root(), &["git", "clone", "source", "clone"]); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Error: Destination path exists and is not an empty directory "###); + } // Clone into a nested path let (stdout, stderr) = test_env.jj_cmd_ok( test_env.env_root(), &["git", "clone", "source", "nested/path/to/repo"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Fetching into new repo in "$TEST_ENV/nested/path/to/repo" bookmark: main@origin [new] tracked @@ -162,30 +209,43 @@ fn test_git_clone() { Parent commit : mzyxwzks 9f01a0e0 main | message Added 1 files, modified 0 files, removed 0 files "###); + } } -#[test] -fn test_git_clone_bad_source() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_clone_bad_source(subprocess: bool) { let test_env = TestEnvironment::default(); + if subprocess { + test_env.set_up_git_subprocessing(); + } let stderr = test_env.jj_cmd_cli_error(test_env.env_root(), &["git", "clone", "", "dest"]); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#"Error: local path "" does not specify a path to a repository"#); + } // Invalid port number let stderr = test_env.jj_cmd_cli_error( test_env.env_root(), &["git", "clone", "https://example.net:bad-port/bar", "dest"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Error: URL "https://example.net:bad-port/bar" can not be parsed as valid URL Caused by: invalid port number "#); + } } -#[test] -fn test_git_clone_colocate() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_clone_colocate(subprocess: bool) { let test_env = TestEnvironment::default(); test_env.add_config("git.auto-local-bookmark = true"); + if subprocess { + test_env.set_up_git_subprocessing(); + } let git_repo_path = test_env.env_root().join("source"); let git_repo = git2::Repository::init(git_repo_path).unwrap(); @@ -194,20 +254,26 @@ fn test_git_clone_colocate() { test_env.env_root(), &["git", "clone", "source", "empty", "--colocate"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Fetching into new repo in "$TEST_ENV/empty" Nothing changed. "###); + } // git_target path should be relative to the store let store_path = test_env .env_root() .join(PathBuf::from_iter(["empty", ".jj", "repo", "store"])); let git_target_file_contents = std::fs::read_to_string(store_path.join("git_target")).unwrap(); + insta::allow_duplicates! { insta::assert_snapshot!( git_target_file_contents.replace(path::MAIN_SEPARATOR, "/"), @"../../../.git"); + } set_up_non_empty_git_repo(&git_repo); @@ -216,7 +282,10 @@ fn test_git_clone_colocate() { test_env.env_root(), &["git", "clone", "source", "clone", "--colocate"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Fetching into new repo in "$TEST_ENV/clone" bookmark: main@origin [new] tracked @@ -225,6 +294,7 @@ fn test_git_clone_colocate() { Parent commit : mzyxwzks 9f01a0e0 main | message Added 1 files, modified 0 files, removed 0 files "###); + } assert!(test_env.env_root().join("clone").join("file").exists()); assert!(test_env.env_root().join("clone").join(".git").exists()); @@ -253,27 +323,35 @@ fn test_git_clone_colocate() { .iter() .map(|entry| format!("{:?} {}\n", entry.status(), entry.path().unwrap())) .collect(); + insta::allow_duplicates! { insta::assert_snapshot!(git_statuses, @r###" Status(IGNORED) .jj/.gitignore Status(IGNORED) .jj/repo/ Status(IGNORED) .jj/working_copy/ "###); + } // The old default bookmark "master" shouldn't exist. + insta::allow_duplicates! { insta::assert_snapshot!( get_bookmark_output(&test_env, &test_env.env_root().join("clone")), @r###" main: mzyxwzks 9f01a0e0 message @git: mzyxwzks 9f01a0e0 message @origin: mzyxwzks 9f01a0e0 message "###); + } // Subsequent fetch should just work even if the source path was relative let (stdout, stderr) = test_env.jj_cmd_ok(&test_env.env_root().join("clone"), &["git", "fetch"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Nothing changed. "###); + } // Failed clone should clean up the destination directory std::fs::create_dir(test_env.env_root().join("bad")).unwrap(); @@ -286,11 +364,21 @@ fn test_git_clone_colocate() { .code(1); let stdout = test_env.normalize_output(&get_stdout_string(&assert)); let stderr = test_env.normalize_output(&get_stderr_string(&assert)); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); - insta::assert_snapshot!(stderr, @r###" - Fetching into new repo in "$TEST_ENV/failed" - Error: could not find repository at '$TEST_ENV/bad'; class=Repository (6) - "###); + } + // git2's internal error is slightly different + if subprocess { + insta::assert_snapshot!(stderr, @r#" + Fetching into new repo in "$TEST_ENV/failed" + Error: Could not find repository at '$TEST_ENV/bad' + "#); + } else { + insta::assert_snapshot!(stderr, @r#" + Fetching into new repo in "$TEST_ENV/failed" + Error: could not find repository at '$TEST_ENV/bad'; class=Repository (6) + "#); + } assert!(!test_env.env_root().join("failed").exists()); // Failed clone shouldn't remove the existing destination directory @@ -304,11 +392,21 @@ fn test_git_clone_colocate() { .code(1); let stdout = test_env.normalize_output(&get_stdout_string(&assert)); let stderr = test_env.normalize_output(&get_stderr_string(&assert)); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); - insta::assert_snapshot!(stderr, @r###" + } + // git2's internal error is slightly different + if subprocess { + insta::assert_snapshot!(stderr, @r#" Fetching into new repo in "$TEST_ENV/failed" - Error: could not find repository at '$TEST_ENV/bad'; class=Repository (6) - "###); + Error: Could not find repository at '$TEST_ENV/bad' + "#); + } else { + insta::assert_snapshot!(stderr, @r#" + Fetching into new repo in "$TEST_ENV/failed" + Error: could not find repository at '$TEST_ENV/bad'; class=Repository (6) + "#); + } assert!(test_env.env_root().join("failed").exists()); assert!(!test_env.env_root().join("failed").join(".git").exists()); assert!(!test_env.env_root().join("failed").join(".jj").exists()); @@ -318,9 +416,11 @@ fn test_git_clone_colocate() { test_env.env_root(), &["git", "clone", "--colocate", "bad", "clone"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Error: Destination path exists and is not an empty directory "###); + } assert!(test_env.env_root().join("clone").join(".git").exists()); assert!(test_env.env_root().join("clone").join(".jj").exists()); @@ -329,9 +429,11 @@ fn test_git_clone_colocate() { test_env.env_root(), &["git", "clone", "source", "clone", "--colocate"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Error: Destination path exists and is not an empty directory "###); + } // Try cloning into an existing file std::fs::write(test_env.env_root().join("file"), "contents").unwrap(); @@ -339,9 +441,11 @@ fn test_git_clone_colocate() { test_env.env_root(), &["git", "clone", "source", "file", "--colocate"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Error: Destination path exists and is not an empty directory "###); + } // Try cloning into non-empty, non-workspace directory std::fs::remove_dir_all(test_env.env_root().join("clone").join(".jj")).unwrap(); @@ -349,9 +453,11 @@ fn test_git_clone_colocate() { test_env.env_root(), &["git", "clone", "source", "clone", "--colocate"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Error: Destination path exists and is not an empty directory "###); + } // Clone into a nested path let (stdout, stderr) = test_env.jj_cmd_ok( @@ -364,7 +470,10 @@ fn test_git_clone_colocate() { "--colocate", ], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Fetching into new repo in "$TEST_ENV/nested/path/to/repo" bookmark: main@origin [new] tracked @@ -373,11 +482,16 @@ fn test_git_clone_colocate() { Parent commit : mzyxwzks 9f01a0e0 main | message Added 1 files, modified 0 files, removed 0 files "###); + } } -#[test] -fn test_git_clone_remote_default_bookmark() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_clone_remote_default_bookmark(subprocess: bool) { let test_env = TestEnvironment::default(); + if subprocess { + test_env.set_up_git_subprocessing(); + } let git_repo_path = test_env.env_root().join("source"); let git_repo = git2::Repository::init(git_repo_path).unwrap(); set_up_non_empty_git_repo(&git_repo); @@ -395,6 +509,7 @@ fn test_git_clone_remote_default_bookmark() { test_env.add_config("git.auto-local-bookmark = true"); let (_stdout, stderr) = test_env.jj_cmd_ok(test_env.env_root(), &["git", "clone", "source", "clone1"]); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Fetching into new repo in "$TEST_ENV/clone1" bookmark: feature1@origin [new] tracked @@ -404,6 +519,8 @@ fn test_git_clone_remote_default_bookmark() { Parent commit : mzyxwzks 9f01a0e0 feature1 main | message Added 1 files, modified 0 files, removed 0 files "###); + } + insta::allow_duplicates! { insta::assert_snapshot!( get_bookmark_output(&test_env, &test_env.env_root().join("clone1")), @r###" feature1: mzyxwzks 9f01a0e0 message @@ -411,20 +528,24 @@ fn test_git_clone_remote_default_bookmark() { main: mzyxwzks 9f01a0e0 message @origin: mzyxwzks 9f01a0e0 message "###); + } // "trunk()" alias should be set to default bookmark "main" let stdout = test_env.jj_cmd_success( &test_env.env_root().join("clone1"), &["config", "list", "--repo", "revset-aliases.'trunk()'"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @r###" revset-aliases.'trunk()' = "main@origin" "###); + } // Only the default bookmark will be imported if auto-local-bookmark is off test_env.add_config("git.auto-local-bookmark = false"); let (_stdout, stderr) = test_env.jj_cmd_ok(test_env.env_root(), &["git", "clone", "source", "clone2"]); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Fetching into new repo in "$TEST_ENV/clone2" bookmark: feature1@origin [new] untracked @@ -434,17 +555,21 @@ fn test_git_clone_remote_default_bookmark() { Parent commit : mzyxwzks 9f01a0e0 feature1@origin main | message Added 1 files, modified 0 files, removed 0 files "###); + } + insta::allow_duplicates! { insta::assert_snapshot!( get_bookmark_output(&test_env, &test_env.env_root().join("clone2")), @r###" feature1@origin: mzyxwzks 9f01a0e0 message main: mzyxwzks 9f01a0e0 message @origin: mzyxwzks 9f01a0e0 message "###); + } // Change the default bookmark in remote git_repo.set_head("refs/heads/feature1").unwrap(); let (_stdout, stderr) = test_env.jj_cmd_ok(test_env.env_root(), &["git", "clone", "source", "clone3"]); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Fetching into new repo in "$TEST_ENV/clone3" bookmark: feature1@origin [new] untracked @@ -454,26 +579,35 @@ fn test_git_clone_remote_default_bookmark() { Parent commit : mzyxwzks 9f01a0e0 feature1 main@origin | message Added 1 files, modified 0 files, removed 0 files "###); + } + insta::allow_duplicates! { insta::assert_snapshot!( get_bookmark_output(&test_env, &test_env.env_root().join("clone2")), @r###" feature1@origin: mzyxwzks 9f01a0e0 message main: mzyxwzks 9f01a0e0 message @origin: mzyxwzks 9f01a0e0 message "###); + } // "trunk()" alias should be set to new default bookmark "feature1" let stdout = test_env.jj_cmd_success( &test_env.env_root().join("clone3"), &["config", "list", "--repo", "revset-aliases.'trunk()'"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @r###" revset-aliases.'trunk()' = "feature1@origin" "###); + } } -#[test] -fn test_git_clone_ignore_working_copy() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_clone_ignore_working_copy(subprocess: bool) { let test_env = TestEnvironment::default(); + if subprocess { + test_env.set_up_git_subprocessing(); + } let git_repo_path = test_env.env_root().join("source"); let git_repo = git2::Repository::init(git_repo_path).unwrap(); set_up_non_empty_git_repo(&git_repo); @@ -483,33 +617,45 @@ fn test_git_clone_ignore_working_copy() { test_env.env_root(), &["git", "clone", "--ignore-working-copy", "source", "clone"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Fetching into new repo in "$TEST_ENV/clone" bookmark: main@origin [new] untracked Setting the revset alias "trunk()" to "main@origin" "###); + } let clone_path = test_env.env_root().join("clone"); let (stdout, stderr) = test_env.jj_cmd_ok(&clone_path, &["status", "--ignore-working-copy"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @r###" The working copy is clean Working copy : sqpuoqvx cad212e1 (empty) (no description set) Parent commit: mzyxwzks 9f01a0e0 main | message "###); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @""); + } // TODO: Correct, but might be better to check out the root commit? let stderr = test_env.jj_cmd_failure(&clone_path, &["status"]); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r##" Error: The working copy is stale (not updated since operation eac759b9ab75). Hint: Run `jj workspace update-stale` to update it. See https://jj-vcs.github.io/jj/latest/working-copy/#stale-working-copy for more information. "##); + } } -#[test] -fn test_git_clone_at_operation() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_clone_at_operation(subprocess: bool) { let test_env = TestEnvironment::default(); + if subprocess { + test_env.set_up_git_subprocessing(); + } let git_repo_path = test_env.env_root().join("source"); let git_repo = git2::Repository::init(git_repo_path).unwrap(); set_up_non_empty_git_repo(&git_repo); @@ -518,15 +664,21 @@ fn test_git_clone_at_operation() { test_env.env_root(), &["git", "clone", "--at-op=@-", "source", "clone"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Error: --at-op is not respected "###); + } } -#[test] -fn test_git_clone_with_remote_name() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_clone_with_remote_name(subprocess: bool) { let test_env = TestEnvironment::default(); test_env.add_config("git.auto-local-bookmark = true"); + if subprocess { + test_env.set_up_git_subprocessing(); + } let git_repo_path = test_env.env_root().join("source"); let git_repo = git2::Repository::init(git_repo_path).unwrap(); set_up_non_empty_git_repo(&git_repo); @@ -536,7 +688,10 @@ fn test_git_clone_with_remote_name() { test_env.env_root(), &["git", "clone", "source", "clone", "--remote", "upstream"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Fetching into new repo in "$TEST_ENV/clone" bookmark: main@upstream [new] tracked @@ -545,11 +700,16 @@ fn test_git_clone_with_remote_name() { Parent commit : mzyxwzks 9f01a0e0 main | message Added 1 files, modified 0 files, removed 0 files "#); + } } -#[test] -fn test_git_clone_with_remote_named_git() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_clone_with_remote_named_git(subprocess: bool) { let test_env = TestEnvironment::default(); + if subprocess { + test_env.set_up_git_subprocessing(); + } let git_repo_path = test_env.env_root().join("source"); git2::Repository::init(git_repo_path).unwrap(); @@ -557,12 +717,18 @@ fn test_git_clone_with_remote_named_git() { test_env.env_root(), &["git", "clone", "--remote=git", "source", "dest"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @"Error: Git remote named 'git' is reserved for local Git repository"); + } } -#[test] -fn test_git_clone_trunk_deleted() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_clone_trunk_deleted(subprocess: bool) { let test_env = TestEnvironment::default(); + if subprocess { + test_env.set_up_git_subprocessing(); + } let git_repo_path = test_env.env_root().join("source"); let git_repo = git2::Repository::init(git_repo_path).unwrap(); set_up_non_empty_git_repo(&git_repo); @@ -570,7 +736,10 @@ fn test_git_clone_trunk_deleted() { let (stdout, stderr) = test_env.jj_cmd_ok(test_env.env_root(), &["git", "clone", "source", "clone"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Fetching into new repo in "$TEST_ENV/clone" bookmark: main@origin [new] untracked @@ -579,16 +748,22 @@ fn test_git_clone_trunk_deleted() { Parent commit : mzyxwzks 9f01a0e0 main | message Added 1 files, modified 0 files, removed 0 files "#); + } let (stdout, stderr) = test_env.jj_cmd_ok(&clone_path, &["bookmark", "forget", "main"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Forgot 1 bookmarks. Warning: Failed to resolve `revset-aliases.trunk()`: Revision "main@origin" doesn't exist Hint: Use `jj config edit --repo` to adjust the `trunk()` alias. "#); + } let (stdout, stderr) = test_env.jj_cmd_ok(&clone_path, &["log"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @r#" @ sqpuoqvx test.user@example.com 2001-02-03 08:05:07 cad212e1 │ (empty) (no description set) @@ -596,10 +771,13 @@ fn test_git_clone_trunk_deleted() { │ message ◆ zzzzzzzz root() 00000000 "#); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Warning: Failed to resolve `revset-aliases.trunk()`: Revision "main@origin" doesn't exist Hint: Use `jj config edit --repo` to adjust the `trunk()` alias. "#); + } } #[test] @@ -685,13 +863,15 @@ fn test_git_clone_conditional_config() { } #[test] -fn test_git_clone_with_depth() { +fn test_git_clone_with_depth_git2() { let test_env = TestEnvironment::default(); test_env.add_config("git.auto-local-bookmark = true"); let git_repo_path = test_env.env_root().join("source"); let git_repo = git2::Repository::init(git_repo_path).unwrap(); set_up_non_empty_git_repo(&git_repo); + // git does support shallow clones on the local transport, so it will work + // (we cannot replicate git2's erroneous behaviour wrt git) // local transport does not support shallow clones so we just test that the // depth arg is passed on here let stderr = test_env.jj_cmd_failure( @@ -705,8 +885,49 @@ fn test_git_clone_with_depth() { } #[test] -fn test_git_clone_invalid_immutable_heads() { +fn test_git_clone_with_depth_subprocess() { + let test_env = TestEnvironment::default(); + test_env.add_config("git.auto-local-bookmark = true"); + test_env.set_up_git_subprocessing(); + let clone_path = test_env.env_root().join("clone"); + let git_repo_path = test_env.env_root().join("source"); + let git_repo = git2::Repository::init(git_repo_path).unwrap(); + set_up_non_empty_git_repo(&git_repo); + + // git does support shallow clones on the local transport, so it will work + // (we cannot replicate git2's erroneous behaviour wrt git) + let (stdout, stderr) = test_env.jj_cmd_ok( + test_env.env_root(), + &["git", "clone", "--depth", "1", "source", "clone"], + ); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r#" + Fetching into new repo in "$TEST_ENV/clone" + bookmark: main@origin [new] tracked + Setting the revset alias "trunk()" to "main@origin" + Working copy now at: sqpuoqvx cad212e1 (empty) (no description set) + Parent commit : mzyxwzks 9f01a0e0 main | message + Added 1 files, modified 0 files, removed 0 files + "#); + + let (stdout, stderr) = test_env.jj_cmd_ok(&clone_path, &["log"]); + insta::assert_snapshot!(stdout, @r" + @ sqpuoqvx test.user@example.com 2001-02-03 08:05:07 cad212e1 + │ (empty) (no description set) + ◆ mzyxwzks some.one@example.com 1970-01-01 11:00:00 main 9f01a0e0 + │ message + ~ + "); + insta::assert_snapshot!(stderr, @""); +} + +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_clone_invalid_immutable_heads(subprocess: bool) { let test_env = TestEnvironment::default(); + if subprocess { + test_env.set_up_git_subprocessing(); + } let git_repo_path = test_env.env_root().join("source"); let git_repo = git2::Repository::init(git_repo_path).unwrap(); set_up_non_empty_git_repo(&git_repo); @@ -718,6 +939,7 @@ fn test_git_clone_invalid_immutable_heads() { // The error shouldn't be counted as an immutable working-copy commit. It // should be reported. let stderr = test_env.jj_cmd_failure(test_env.env_root(), &["git", "clone", "source", "clone"]); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Fetching into new repo in "$TEST_ENV/clone" bookmark: main@origin [new] untracked @@ -725,11 +947,16 @@ fn test_git_clone_invalid_immutable_heads() { Caused by: Revision "unknown" doesn't exist For help, see https://jj-vcs.github.io/jj/latest/config/. "#); + } } -#[test] -fn test_git_clone_malformed() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_clone_malformed(subprocess: bool) { let test_env = TestEnvironment::default(); + if subprocess { + test_env.set_up_git_subprocessing(); + } let git_repo_path = test_env.env_root().join("source"); let git_repo = git2::Repository::init(git_repo_path).unwrap(); let clone_path = test_env.env_root().join("clone"); @@ -740,6 +967,7 @@ fn test_git_clone_malformed() { // TODO: Perhaps, this should be a user error, not an internal error. let stderr = test_env.jj_cmd_internal_error(test_env.env_root(), &["git", "clone", "source", "clone"]); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Fetching into new repo in "$TEST_ENV/clone" bookmark: main@origin [new] untracked @@ -747,31 +975,96 @@ fn test_git_clone_malformed() { Internal error: Failed to check out commit 039a1eae03465fd3be0fbad87c9ca97303742677 Caused by: Reserved path component .jj in $TEST_ENV/clone/.jj "#); + } // The cloned workspace isn't usable. let stderr = test_env.jj_cmd_failure(&clone_path, &["status"]); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r##" Error: The working copy is stale (not updated since operation 4a8ddda0ff63). Hint: Run `jj workspace update-stale` to update it. See https://jj-vcs.github.io/jj/latest/working-copy/#stale-working-copy for more information. "##); + } // The error can be somehow recovered. // TODO: add an update-stale flag to reset the working-copy? let stderr = test_env.jj_cmd_internal_error(&clone_path, &["workspace", "update-stale"]); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Internal error: Failed to check out commit 039a1eae03465fd3be0fbad87c9ca97303742677 Caused by: Reserved path component .jj in $TEST_ENV/clone/.jj "#); + } let (_stdout, stderr) = test_env.jj_cmd_ok(&clone_path, &["new", "root()", "--ignore-working-copy"]); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @""); + } let stdout = test_env.jj_cmd_success(&clone_path, &["status"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @r#" The working copy is clean Working copy : zsuskuln f652c321 (empty) (no description set) Parent commit: zzzzzzzz 00000000 (empty) (no description set) "#); + } +} + +#[test] +fn test_git_clone_no_git_executable() { + let mut test_env = TestEnvironment::default(); + test_env.add_config("git.subprocess = true"); + test_env.add_env_var("PATH", ""); + let git_repo_path = test_env.env_root().join("source"); + let git_repo = git2::Repository::init(git_repo_path).unwrap(); + // libgit2 doesn't allow to create a malformed repo containing ".git", etc., + // but we can insert ".jj" entry. + set_up_git_repo_with_file(&git_repo, ".jj"); + + let stderr = test_env.jj_cmd_failure(test_env.env_root(), &["git", "clone", "source", "clone"]); + if cfg!(windows) { + insta::assert_snapshot!(stderr, @r#" + Fetching into new repo in "$TEST_ENV/clone" + Error: Could not execute the git process, found in the OS path 'git' + Caused by: program not found + "#); + } else { + insta::assert_snapshot!(stderr, @r#" + Fetching into new repo in "$TEST_ENV/clone" + Error: Could not execute the git process, found in the OS path 'git' + Caused by: No such file or directory (os error 2) + "#); + } +} + +#[test] +fn test_git_clone_no_git_executable_with_path() { + let mut test_env = TestEnvironment::default(); + let invalid_git_executable_path = test_env + .env_root() + .join("this") + .join("is") + .join("an") + .join("invalid") + .join("path"); + test_env.add_config("git.subprocess = true"); + test_env.add_config(format!( + "git.executable-path = {}", + to_toml_value(invalid_git_executable_path.to_str().unwrap()) + )); + test_env.add_env_var("PATH", ""); + let git_repo_path = test_env.env_root().join("source"); + let git_repo = git2::Repository::init(git_repo_path).unwrap(); + // libgit2 doesn't allow to create a malformed repo containing ".git", etc., + // but we can insert ".jj" entry. + set_up_git_repo_with_file(&git_repo, ".jj"); + + let stderr = test_env.jj_cmd_failure(test_env.env_root(), &["git", "clone", "source", "clone"]); + insta::assert_snapshot!(strip_last_line(&stderr), @r#" + Fetching into new repo in "$TEST_ENV/clone" + Error: Could not execute git process at specified path '$TEST_ENV/this/is/an/invalid/path' + "#); } fn get_bookmark_output(test_env: &TestEnvironment, repo_path: &Path) -> String { diff --git a/cli/tests/test_git_fetch.rs b/cli/tests/test_git_fetch.rs index 2ea6d24792..6545ad1c6a 100644 --- a/cli/tests/test_git_fetch.rs +++ b/cli/tests/test_git_fetch.rs @@ -13,6 +13,8 @@ // limitations under the License. use std::path::Path; +use test_case::test_case; + use crate::common::TestEnvironment; /// Creates a remote Git repo containing a bookmark with the same name @@ -72,56 +74,80 @@ fn get_log_output(test_env: &TestEnvironment, workspace_root: &Path) -> String { test_env.jj_cmd_success(workspace_root, &["log", "-T", template, "-r", "all()"]) } -#[test] -fn test_git_fetch_with_default_config() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_fetch_with_default_config(subprocess: bool) { let test_env = TestEnvironment::default(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); let repo_path = test_env.env_root().join("repo"); add_git_remote(&test_env, &repo_path, "origin"); test_env.jj_cmd_ok(&repo_path, &["git", "fetch"]); + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @r###" origin@origin: oputwtnw ffecd2d6 message "###); + } } -#[test] -fn test_git_fetch_default_remote() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_fetch_default_remote(subprocess: bool) { let test_env = TestEnvironment::default(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.add_config("git.auto-local-bookmark = true"); test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); let repo_path = test_env.env_root().join("repo"); add_git_remote(&test_env, &repo_path, "origin"); test_env.jj_cmd_ok(&repo_path, &["git", "fetch"]); + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @r###" origin: oputwtnw ffecd2d6 message @origin: oputwtnw ffecd2d6 message "###); + } } -#[test] -fn test_git_fetch_single_remote() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_fetch_single_remote(subprocess: bool) { let test_env = TestEnvironment::default(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.add_config("git.auto-local-bookmark = true"); test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); let repo_path = test_env.env_root().join("repo"); add_git_remote(&test_env, &repo_path, "rem1"); let (_stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["git", "fetch"]); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Hint: Fetching from the only existing remote: rem1 bookmark: rem1@rem1 [new] tracked "###); + } + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @r###" rem1: qxosxrvv 6a211027 message @rem1: qxosxrvv 6a211027 message "###); + } } -#[test] -fn test_git_fetch_single_remote_all_remotes_flag() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_fetch_single_remote_all_remotes_flag(subprocess: bool) { let test_env = TestEnvironment::default(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.add_config("git.auto-local-bookmark = true"); test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); let repo_path = test_env.env_root().join("repo"); @@ -131,30 +157,42 @@ fn test_git_fetch_single_remote_all_remotes_flag() { .jj_cmd(&repo_path, &["git", "fetch", "--all-remotes"]) .assert() .success(); + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @r###" rem1: qxosxrvv 6a211027 message @rem1: qxosxrvv 6a211027 message "###); + } } -#[test] -fn test_git_fetch_single_remote_from_arg() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_fetch_single_remote_from_arg(subprocess: bool) { let test_env = TestEnvironment::default(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.add_config("git.auto-local-bookmark = true"); test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); let repo_path = test_env.env_root().join("repo"); add_git_remote(&test_env, &repo_path, "rem1"); test_env.jj_cmd_ok(&repo_path, &["git", "fetch", "--remote", "rem1"]); + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @r###" rem1: qxosxrvv 6a211027 message @rem1: qxosxrvv 6a211027 message "###); + } } -#[test] -fn test_git_fetch_single_remote_from_config() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_fetch_single_remote_from_config(subprocess: bool) { let test_env = TestEnvironment::default(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.add_config("git.auto-local-bookmark = true"); test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); let repo_path = test_env.env_root().join("repo"); @@ -162,15 +200,21 @@ fn test_git_fetch_single_remote_from_config() { test_env.add_config(r#"git.fetch = "rem1""#); test_env.jj_cmd_ok(&repo_path, &["git", "fetch"]); + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @r###" rem1: qxosxrvv 6a211027 message @rem1: qxosxrvv 6a211027 message "###); + } } -#[test] -fn test_git_fetch_multiple_remotes() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_fetch_multiple_remotes(subprocess: bool) { let test_env = TestEnvironment::default(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.add_config("git.auto-local-bookmark = true"); test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); let repo_path = test_env.env_root().join("repo"); @@ -181,17 +225,23 @@ fn test_git_fetch_multiple_remotes() { &repo_path, &["git", "fetch", "--remote", "rem1", "--remote", "rem2"], ); + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @r###" rem1: qxosxrvv 6a211027 message @rem1: qxosxrvv 6a211027 message rem2: yszkquru 2497a8a0 message @rem2: yszkquru 2497a8a0 message "###); + } } -#[test] -fn test_git_fetch_all_remotes() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_fetch_all_remotes(subprocess: bool) { let test_env = TestEnvironment::default(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.add_config("git.auto-local-bookmark = true"); test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); let repo_path = test_env.env_root().join("repo"); @@ -199,17 +249,23 @@ fn test_git_fetch_all_remotes() { add_git_remote(&test_env, &repo_path, "rem2"); test_env.jj_cmd_ok(&repo_path, &["git", "fetch", "--all-remotes"]); + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @r###" rem1: qxosxrvv 6a211027 message @rem1: qxosxrvv 6a211027 message rem2: yszkquru 2497a8a0 message @rem2: yszkquru 2497a8a0 message "###); + } } -#[test] -fn test_git_fetch_multiple_remotes_from_config() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_fetch_multiple_remotes_from_config(subprocess: bool) { let test_env = TestEnvironment::default(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.add_config("git.auto-local-bookmark = true"); test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); let repo_path = test_env.env_root().join("repo"); @@ -218,17 +274,23 @@ fn test_git_fetch_multiple_remotes_from_config() { test_env.add_config(r#"git.fetch = ["rem1", "rem2"]"#); test_env.jj_cmd_ok(&repo_path, &["git", "fetch"]); + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @r###" rem1: qxosxrvv 6a211027 message @rem1: qxosxrvv 6a211027 message rem2: yszkquru 2497a8a0 message @rem2: yszkquru 2497a8a0 message "###); + } } -#[test] -fn test_git_fetch_nonexistent_remote() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_fetch_nonexistent_remote(subprocess: bool) { let test_env = TestEnvironment::default(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); let repo_path = test_env.env_root().join("repo"); add_git_remote(&test_env, &repo_path, "rem1"); @@ -237,37 +299,52 @@ fn test_git_fetch_nonexistent_remote() { &repo_path, &["git", "fetch", "--remote", "rem1", "--remote", "rem2"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" bookmark: rem1@rem1 [new] untracked Error: No git remote named 'rem2' "###); + } + insta::allow_duplicates! { // No remote should have been fetched as part of the failing transaction insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @""); + } } -#[test] -fn test_git_fetch_nonexistent_remote_from_config() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_fetch_nonexistent_remote_from_config(subprocess: bool) { let test_env = TestEnvironment::default(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); let repo_path = test_env.env_root().join("repo"); add_git_remote(&test_env, &repo_path, "rem1"); test_env.add_config(r#"git.fetch = ["rem1", "rem2"]"#); let stderr = &test_env.jj_cmd_failure(&repo_path, &["git", "fetch"]); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" bookmark: rem1@rem1 [new] untracked Error: No git remote named 'rem2' "###); // No remote should have been fetched as part of the failing transaction insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @""); + } } -#[test] -fn test_git_fetch_from_remote_named_git() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_fetch_from_remote_named_git(subprocess: bool) { let test_env = TestEnvironment::default(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.add_config("git.auto-local-bookmark = true"); let repo_path = test_env.env_root().join("repo"); init_git_remote(&test_env, "git"); + let git_repo = git2::Repository::init(&repo_path).unwrap(); git_repo.remote("git", "../git").unwrap(); @@ -276,15 +353,20 @@ fn test_git_fetch_from_remote_named_git() { // Try fetching from the remote named 'git'. let stderr = &test_env.jj_cmd_failure(&repo_path, &["git", "fetch", "--remote=git"]); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Error: Failed to import refs from underlying Git repo Caused by: Git remote named 'git' is reserved for local Git repository Hint: Run `jj git remote rename` to give different name. "###); + } // Implicit import shouldn't fail because of the remote ref. let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["bookmark", "list", "--all-remotes"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @""); // Explicit import is an error. @@ -294,32 +376,43 @@ fn test_git_fetch_from_remote_named_git() { Caused by: Git remote named 'git' is reserved for local Git repository Hint: Run `jj git remote rename` to give different name. "###); + } // The remote can be renamed, and the ref can be imported. test_env.jj_cmd_ok(&repo_path, &["git", "remote", "rename", "git", "bar"]); let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["bookmark", "list", "--all-remotes"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @r###" git: mrylzrtu 76fc7466 message @bar: mrylzrtu 76fc7466 message @git: mrylzrtu 76fc7466 message "###); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Done importing changes from the underlying Git repo. "###); + } } -#[test] -fn test_git_fetch_prune_before_updating_tips() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_fetch_prune_before_updating_tips(subprocess: bool) { let test_env = TestEnvironment::default(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.add_config("git.auto-local-bookmark = true"); test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); let repo_path = test_env.env_root().join("repo"); add_git_remote(&test_env, &repo_path, "origin"); test_env.jj_cmd_ok(&repo_path, &["git", "fetch"]); + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @r###" origin: oputwtnw ffecd2d6 message @origin: oputwtnw ffecd2d6 message "###); + } // Remove origin bookmark in git repo and create origin/subname let git_repo = git2::Repository::open(test_env.env_root().join("origin")).unwrap(); @@ -330,15 +423,21 @@ fn test_git_fetch_prune_before_updating_tips() { .unwrap(); test_env.jj_cmd_ok(&repo_path, &["git", "fetch"]); + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @r###" origin/subname: oputwtnw ffecd2d6 message @origin: oputwtnw ffecd2d6 message "###); + } } -#[test] -fn test_git_fetch_conflicting_bookmarks() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_fetch_conflicting_bookmarks(subprocess: bool) { let test_env = TestEnvironment::default(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.add_config("git.auto-local-bookmark = true"); test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); let repo_path = test_env.env_root().join("repo"); @@ -347,41 +446,53 @@ fn test_git_fetch_conflicting_bookmarks() { // Create a rem1 bookmark locally test_env.jj_cmd_ok(&repo_path, &["new", "root()"]); test_env.jj_cmd_ok(&repo_path, &["bookmark", "create", "rem1"]); + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @r###" rem1: kkmpptxz fcdbbd73 (empty) (no description set) "###); + } test_env.jj_cmd_ok( &repo_path, &["git", "fetch", "--remote", "rem1", "--branch", "glob:*"], ); // This should result in a CONFLICTED bookmark + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @r###" rem1 (conflicted): + kkmpptxz fcdbbd73 (empty) (no description set) + qxosxrvv 6a211027 message @rem1 (behind by 1 commits): qxosxrvv 6a211027 message "###); + } } -#[test] -fn test_git_fetch_conflicting_bookmarks_colocated() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_fetch_conflicting_bookmarks_colocated(subprocess: bool) { let test_env = TestEnvironment::default(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.add_config("git.auto-local-bookmark = true"); let repo_path = test_env.env_root().join("repo"); let _git_repo = git2::Repository::init(&repo_path).unwrap(); // create_colocated_repo_and_bookmarks_from_trunk1(&test_env, &repo_path); test_env.jj_cmd_ok(&repo_path, &["git", "init", "--git-repo", "."]); add_git_remote(&test_env, &repo_path, "rem1"); + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @""); + } // Create a rem1 bookmark locally test_env.jj_cmd_ok(&repo_path, &["new", "root()"]); test_env.jj_cmd_ok(&repo_path, &["bookmark", "create", "rem1"]); + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @r###" rem1: zsuskuln f652c321 (empty) (no description set) @git: zsuskuln f652c321 (empty) (no description set) "###); + } test_env.jj_cmd_ok( &repo_path, @@ -389,6 +500,7 @@ fn test_git_fetch_conflicting_bookmarks_colocated() { ); // This should result in a CONFLICTED bookmark // See https://github.com/jj-vcs/jj/pull/1146#discussion_r1112372340 for the bug this tests for. + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @r###" rem1 (conflicted): + zsuskuln f652c321 (empty) (no description set) @@ -396,6 +508,7 @@ fn test_git_fetch_conflicting_bookmarks_colocated() { @git (behind by 1 commits): zsuskuln f652c321 (empty) (no description set) @rem1 (behind by 1 commits): qxosxrvv 6a211027 message "###); + } } // Helper functions to test obtaining multiple bookmarks at once and changed @@ -427,9 +540,13 @@ fn create_trunk2_and_rebase_bookmarks(test_env: &TestEnvironment, repo_path: &Pa ) } -#[test] -fn test_git_fetch_all() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_fetch_all(subprocess: bool) { let test_env = TestEnvironment::default(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.add_config("git.auto-local-bookmark = true"); test_env.add_config(r#"revset-aliases."immutable_heads()" = "none()""#); let source_git_repo_path = test_env.env_root().join("source"); @@ -438,15 +555,20 @@ fn test_git_fetch_all() { // Clone an empty repo. The target repo is a normal `jj` repo, *not* colocated let (stdout, stderr) = test_env.jj_cmd_ok(test_env.env_root(), &["git", "clone", "source", "target"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Fetching into new repo in "$TEST_ENV/target" Nothing changed. "###); + } let target_jj_repo_path = test_env.env_root().join("target"); let source_log = create_colocated_repo_and_bookmarks_from_trunk1(&test_env, &source_git_repo_path); + insta::allow_duplicates! { insta::assert_snapshot!(source_log, @r###" ===== Source git repo contents ===== @ c7d4bdcbc215 descr_for_b b @@ -457,21 +579,31 @@ fn test_git_fetch_all() { ○ ff36dc55760e descr_for_trunk1 trunk1 ◆ 000000000000 "###); + } // Nothing in our repo before the fetch + insta::allow_duplicates! { insta::assert_snapshot!(get_log_output(&test_env, &target_jj_repo_path), @r###" @ 230dd059e1b0 ◆ 000000000000 "###); + } + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &target_jj_repo_path), @""); + } let (stdout, stderr) = test_env.jj_cmd_ok(&target_jj_repo_path, &["git", "fetch"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" bookmark: a1@origin [new] tracked bookmark: a2@origin [new] tracked bookmark: b@origin [new] tracked bookmark: trunk1@origin [new] tracked "###); + } + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &target_jj_repo_path), @r###" a1: nknoxmzm 359a9a02 descr_for_a1 @origin: nknoxmzm 359a9a02 descr_for_a1 @@ -482,6 +614,8 @@ fn test_git_fetch_all() { trunk1: zowqyktl ff36dc55 descr_for_trunk1 @origin: zowqyktl ff36dc55 descr_for_trunk1 "###); + } + insta::allow_duplicates! { insta::assert_snapshot!(get_log_output(&test_env, &target_jj_repo_path), @r#" @ 230dd059e1b0 │ ○ c7d4bdcbc215 descr_for_b b @@ -493,10 +627,12 @@ fn test_git_fetch_all() { ├─╯ ◆ 000000000000 "#); + } // ==== Change both repos ==== // First, change the target repo: let source_log = create_trunk2_and_rebase_bookmarks(&test_env, &source_git_repo_path); + insta::allow_duplicates! { insta::assert_snapshot!(source_log, @r###" ===== Source git repo contents ===== ○ babc49226c14 descr_for_b b @@ -508,6 +644,7 @@ fn test_git_fetch_all() { ○ ff36dc55760e descr_for_trunk1 trunk1 ◆ 000000000000 "###); + } // Change a bookmark in the source repo as well, so that it becomes conflicted. test_env.jj_cmd_ok( &target_jj_repo_path, @@ -515,6 +652,7 @@ fn test_git_fetch_all() { ); // Our repo before and after fetch + insta::allow_duplicates! { insta::assert_snapshot!(get_log_output(&test_env, &target_jj_repo_path), @r#" @ 230dd059e1b0 │ ○ 061eddbb43ab new_descr_for_b_to_create_conflict b* @@ -526,6 +664,8 @@ fn test_git_fetch_all() { ├─╯ ◆ 000000000000 "#); + } + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &target_jj_repo_path), @r###" a1: nknoxmzm 359a9a02 descr_for_a1 @origin: nknoxmzm 359a9a02 descr_for_a1 @@ -536,8 +676,12 @@ fn test_git_fetch_all() { trunk1: zowqyktl ff36dc55 descr_for_trunk1 @origin: zowqyktl ff36dc55 descr_for_trunk1 "###); + } let (stdout, stderr) = test_env.jj_cmd_ok(&target_jj_repo_path, &["git", "fetch"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" bookmark: a1@origin [updated] tracked bookmark: a2@origin [updated] tracked @@ -545,6 +689,8 @@ fn test_git_fetch_all() { bookmark: trunk2@origin [new] tracked Abandoned 2 commits that are no longer reachable. "###); + } + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &target_jj_repo_path), @r###" a1: quxllqov 0424f6df descr_for_a1 @origin: quxllqov 0424f6df descr_for_a1 @@ -560,6 +706,8 @@ fn test_git_fetch_all() { trunk2: umznmzko 8f1f14fb descr_for_trunk2 @origin: umznmzko 8f1f14fb descr_for_trunk2 "###); + } + insta::allow_duplicates! { insta::assert_snapshot!(get_log_output(&test_env, &target_jj_repo_path), @r#" @ 230dd059e1b0 │ ○ babc49226c14 descr_for_b b?? b@origin @@ -574,11 +722,16 @@ fn test_git_fetch_all() { ├─╯ ◆ 000000000000 "#); + } } -#[test] -fn test_git_fetch_some_of_many_bookmarks() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_fetch_some_of_many_bookmarks(subprocess: bool) { let test_env = TestEnvironment::default(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.add_config("git.auto-local-bookmark = true"); test_env.add_config(r#"revset-aliases."immutable_heads()" = "none()""#); let source_git_repo_path = test_env.env_root().join("source"); @@ -587,15 +740,20 @@ fn test_git_fetch_some_of_many_bookmarks() { // Clone an empty repo. The target repo is a normal `jj` repo, *not* colocated let (stdout, stderr) = test_env.jj_cmd_ok(test_env.env_root(), &["git", "clone", "source", "target"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Fetching into new repo in "$TEST_ENV/target" Nothing changed. "###); + } let target_jj_repo_path = test_env.env_root().join("target"); let source_log = create_colocated_repo_and_bookmarks_from_trunk1(&test_env, &source_git_repo_path); + insta::allow_duplicates! { insta::assert_snapshot!(source_log, @r###" ===== Source git repo contents ===== @ c7d4bdcbc215 descr_for_b b @@ -606,31 +764,43 @@ fn test_git_fetch_some_of_many_bookmarks() { ○ ff36dc55760e descr_for_trunk1 trunk1 ◆ 000000000000 "###); + } // Test an error message let stderr = test_env.jj_cmd_failure( &target_jj_repo_path, &["git", "fetch", "--branch", "glob:^:a*"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @"Error: Invalid branch pattern provided. When fetching, branch names and globs may not contain the characters `:`, `^`, `?`, `[`, `]`"); + } let stderr = test_env.jj_cmd_failure(&target_jj_repo_path, &["git", "fetch", "--branch", "a*"]); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r" Error: Branch names may not include `*`. Hint: Prefix the pattern with `glob:` to expand `*` as a glob "); + } // Nothing in our repo before the fetch + insta::allow_duplicates! { insta::assert_snapshot!(get_log_output(&test_env, &target_jj_repo_path), @r###" @ 230dd059e1b0 ◆ 000000000000 "###); + } // Fetch one bookmark... let (stdout, stderr) = test_env.jj_cmd_ok(&target_jj_repo_path, &["git", "fetch", "--branch", "b"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" bookmark: b@origin [new] tracked "###); + } + insta::allow_duplicates! { insta::assert_snapshot!(get_log_output(&test_env, &target_jj_repo_path), @r#" @ 230dd059e1b0 │ ○ c7d4bdcbc215 descr_for_b b @@ -638,21 +808,29 @@ fn test_git_fetch_some_of_many_bookmarks() { ├─╯ ◆ 000000000000 "#); + } // ...check what the intermediate state looks like... + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &target_jj_repo_path), @r###" b: vpupmnsl c7d4bdcb descr_for_b @origin: vpupmnsl c7d4bdcb descr_for_b "###); + } // ...then fetch two others with a glob. let (stdout, stderr) = test_env.jj_cmd_ok( &target_jj_repo_path, &["git", "fetch", "--branch", "glob:a*"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" bookmark: a1@origin [new] tracked bookmark: a2@origin [new] tracked "###); + } + insta::allow_duplicates! { insta::assert_snapshot!(get_log_output(&test_env, &target_jj_repo_path), @r#" @ 230dd059e1b0 │ ○ decaa3966c83 descr_for_a2 a2 @@ -664,10 +842,14 @@ fn test_git_fetch_some_of_many_bookmarks() { ├─╯ ◆ 000000000000 "#); + } // Fetching the same bookmark again let (stdout, stderr) = test_env.jj_cmd_ok(&target_jj_repo_path, &["git", "fetch", "--branch", "a1"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Nothing changed. "###); @@ -682,10 +864,12 @@ fn test_git_fetch_some_of_many_bookmarks() { ├─╯ ◆ 000000000000 "#); + } // ==== Change both repos ==== // First, change the target repo: let source_log = create_trunk2_and_rebase_bookmarks(&test_env, &source_git_repo_path); + insta::allow_duplicates! { insta::assert_snapshot!(source_log, @r###" ===== Source git repo contents ===== ○ 01d115196c39 descr_for_b b @@ -697,6 +881,7 @@ fn test_git_fetch_some_of_many_bookmarks() { ○ ff36dc55760e descr_for_trunk1 trunk1 ◆ 000000000000 "###); + } // Change a bookmark in the source repo as well, so that it becomes conflicted. test_env.jj_cmd_ok( &target_jj_repo_path, @@ -704,6 +889,7 @@ fn test_git_fetch_some_of_many_bookmarks() { ); // Our repo before and after fetch of two bookmarks + insta::allow_duplicates! { insta::assert_snapshot!(get_log_output(&test_env, &target_jj_repo_path), @r#" @ 230dd059e1b0 │ ○ 6ebd41dc4f13 new_descr_for_b_to_create_conflict b* @@ -715,16 +901,22 @@ fn test_git_fetch_some_of_many_bookmarks() { ├─╯ ◆ 000000000000 "#); + } let (stdout, stderr) = test_env.jj_cmd_ok( &target_jj_repo_path, &["git", "fetch", "--branch", "b", "--branch", "a1"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" bookmark: a1@origin [updated] tracked bookmark: b@origin [updated] tracked Abandoned 1 commits that are no longer reachable. "###); + } + insta::allow_duplicates! { insta::assert_snapshot!(get_log_output(&test_env, &target_jj_repo_path), @r#" @ 230dd059e1b0 │ ○ 01d115196c39 descr_for_b b?? b@origin @@ -739,8 +931,10 @@ fn test_git_fetch_some_of_many_bookmarks() { ├─╯ ◆ 000000000000 "#); + } // We left a2 where it was before, let's see how `jj bookmark list` sees this. + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &target_jj_repo_path), @r###" a1: ypowunwp 6df2d34c descr_for_a1 @origin: ypowunwp 6df2d34c descr_for_a1 @@ -752,17 +946,23 @@ fn test_git_fetch_some_of_many_bookmarks() { + nxrpswuq 01d11519 descr_for_b @origin (behind by 1 commits): nxrpswuq 01d11519 descr_for_b "###); + } // Now, let's fetch a2 and double-check that fetching a1 and b again doesn't do // anything. let (stdout, stderr) = test_env.jj_cmd_ok( &target_jj_repo_path, &["git", "fetch", "--branch", "b", "--branch", "glob:a*"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" bookmark: a2@origin [updated] tracked Abandoned 1 commits that are no longer reachable. "###); + } + insta::allow_duplicates! { insta::assert_snapshot!(get_log_output(&test_env, &target_jj_repo_path), @r#" @ 230dd059e1b0 │ ○ 31c7d94b1f29 descr_for_a2 a2 @@ -777,6 +977,8 @@ fn test_git_fetch_some_of_many_bookmarks() { ├─╯ ◆ 000000000000 "#); + } + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &target_jj_repo_path), @r###" a1: ypowunwp 6df2d34c descr_for_a1 @origin: ypowunwp 6df2d34c descr_for_a1 @@ -788,11 +990,16 @@ fn test_git_fetch_some_of_many_bookmarks() { + nxrpswuq 01d11519 descr_for_b @origin (behind by 1 commits): nxrpswuq 01d11519 descr_for_b "###); + } } -#[test] -fn test_git_fetch_bookmarks_some_missing() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_fetch_bookmarks_some_missing(subprocess: bool) { let test_env = TestEnvironment::default(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.add_config("git.auto-local-bookmark = true"); test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); let repo_path = test_env.env_root().join("repo"); @@ -804,11 +1011,15 @@ fn test_git_fetch_bookmarks_some_missing() { // single missing bookmark, implicit remotes (@origin) let (_stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["git", "fetch", "--branch", "noexist"]); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Warning: No branch matching `noexist` found on any specified/configured remote Nothing changed. "###); + } + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @""); + } // multiple missing bookmarks, implicit remotes (@origin) let (_stdout, stderr) = test_env.jj_cmd_ok( @@ -817,22 +1028,30 @@ fn test_git_fetch_bookmarks_some_missing() { "git", "fetch", "--branch", "noexist1", "--branch", "noexist2", ], ); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Warning: No branch matching `noexist1` found on any specified/configured remote Warning: No branch matching `noexist2` found on any specified/configured remote Nothing changed. "###); + } + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @""); + } // single existing bookmark, implicit remotes (@origin) let (_stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["git", "fetch", "--branch", "origin"]); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" bookmark: origin@origin [new] tracked "###); + } + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @r###" origin: oputwtnw ffecd2d6 message @origin: oputwtnw ffecd2d6 message "###); + } // multiple existing bookmark, explicit remotes, each bookmark is only in one // remote. @@ -843,12 +1062,15 @@ fn test_git_fetch_bookmarks_some_missing() { "rem1", "--remote", "rem2", "--remote", "rem3", ], ); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r" bookmark: rem1@rem1 [new] tracked bookmark: rem2@rem2 [new] tracked bookmark: rem3@rem3 [new] tracked "); - insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @r" + } + insta::allow_duplicates! { + insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @r" origin: oputwtnw ffecd2d6 message @origin: oputwtnw ffecd2d6 message rem1: qxosxrvv 6a211027 message @@ -858,6 +1080,7 @@ fn test_git_fetch_bookmarks_some_missing() { rem3: lvsrtwwm 4ffdff2b message @rem3: lvsrtwwm 4ffdff2b message "); + } // multiple bookmarks, one exists, one doesn't let (_stdout, stderr) = test_env.jj_cmd_ok( @@ -866,10 +1089,13 @@ fn test_git_fetch_bookmarks_some_missing() { "git", "fetch", "--branch", "rem1", "--branch", "notexist", "--remote", "rem1", ], ); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Warning: No branch matching `notexist` found on any specified/configured remote Nothing changed. "###); + } + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @r" origin: oputwtnw ffecd2d6 message @origin: oputwtnw ffecd2d6 message @@ -880,13 +1106,18 @@ fn test_git_fetch_bookmarks_some_missing() { rem3: lvsrtwwm 4ffdff2b message @rem3: lvsrtwwm 4ffdff2b message "); + } } // See `test_undo_restore_commands.rs` for fetch-undo-push and fetch-undo-fetch // of the same bookmarks for various kinds of undo. -#[test] -fn test_git_fetch_undo() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_fetch_undo(subprocess: bool) { let test_env = TestEnvironment::default(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.add_config("git.auto-local-bookmark = true"); let source_git_repo_path = test_env.env_root().join("source"); let _git_repo = git2::Repository::init(source_git_repo_path.clone()).unwrap(); @@ -894,15 +1125,20 @@ fn test_git_fetch_undo() { // Clone an empty repo. The target repo is a normal `jj` repo, *not* colocated let (stdout, stderr) = test_env.jj_cmd_ok(test_env.env_root(), &["git", "clone", "source", "target"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Fetching into new repo in "$TEST_ENV/target" Nothing changed. "###); + } let target_jj_repo_path = test_env.env_root().join("target"); let source_log = create_colocated_repo_and_bookmarks_from_trunk1(&test_env, &source_git_repo_path); + insta::allow_duplicates! { insta::assert_snapshot!(source_log, @r###" ===== Source git repo contents ===== @ c7d4bdcbc215 descr_for_b b @@ -913,13 +1149,17 @@ fn test_git_fetch_undo() { ○ ff36dc55760e descr_for_trunk1 trunk1 ◆ 000000000000 "###); + } // Fetch 2 bookmarks let (stdout, stderr) = test_env.jj_cmd_ok( &target_jj_repo_path, &["git", "fetch", "--branch", "b", "--branch", "a1"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" bookmark: a1@origin [new] tracked bookmark: b@origin [new] tracked @@ -933,8 +1173,12 @@ fn test_git_fetch_undo() { ├─╯ ◆ 000000000000 "#); + } let (stdout, stderr) = test_env.jj_cmd_ok(&target_jj_repo_path, &["undo"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Undid operation: eb2029853b02 (2001-02-03 08:05:18) fetch from git remote(s) origin "#); @@ -943,10 +1187,14 @@ fn test_git_fetch_undo() { @ 230dd059e1b0 ◆ 000000000000 "###); + } // Now try to fetch just one bookmark let (stdout, stderr) = test_env.jj_cmd_ok(&target_jj_repo_path, &["git", "fetch", "--branch", "b"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" bookmark: b@origin [new] tracked "###); @@ -957,13 +1205,18 @@ fn test_git_fetch_undo() { ├─╯ ◆ 000000000000 "#); + } } // Compare to `test_git_import_undo` in test_git_import_export // TODO: Explain why these behaviors are useful -#[test] -fn test_fetch_undo_what() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_fetch_undo_what(subprocess: bool) { let test_env = TestEnvironment::default(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.add_config("git.auto-local-bookmark = true"); let source_git_repo_path = test_env.env_root().join("source"); let _git_repo = git2::Repository::init(source_git_repo_path.clone()).unwrap(); @@ -971,15 +1224,20 @@ fn test_fetch_undo_what() { // Clone an empty repo. The target repo is a normal `jj` repo, *not* colocated let (stdout, stderr) = test_env.jj_cmd_ok(test_env.env_root(), &["git", "clone", "source", "target"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Fetching into new repo in "$TEST_ENV/target" Nothing changed. "###); + } let repo_path = test_env.env_root().join("target"); let source_log = create_colocated_repo_and_bookmarks_from_trunk1(&test_env, &source_git_repo_path); + insta::allow_duplicates! { insta::assert_snapshot!(source_log, @r###" ===== Source git repo contents ===== @ c7d4bdcbc215 descr_for_b b @@ -990,18 +1248,26 @@ fn test_fetch_undo_what() { ○ ff36dc55760e descr_for_trunk1 trunk1 ◆ 000000000000 "###); + } // Initial state we will try to return to after `op restore`. There are no // bookmarks. + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @""); + } let base_operation_id = test_env.current_operation_id(&repo_path); // Fetch a bookmark let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["git", "fetch", "--branch", "b"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" bookmark: b@origin [new] tracked "###); + } + insta::allow_duplicates! { insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r#" @ 230dd059e1b0 │ ○ c7d4bdcbc215 descr_for_b b @@ -1009,10 +1275,13 @@ fn test_fetch_undo_what() { ├─╯ ◆ 000000000000 "#); + } + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @r###" b: vpupmnsl c7d4bdcb descr_for_b @origin: vpupmnsl c7d4bdcb descr_for_b "###); + } // We can undo the change in the repo without moving the remote-tracking // bookmark @@ -1020,23 +1289,31 @@ fn test_fetch_undo_what() { &repo_path, &["op", "restore", "--what", "repo", &base_operation_id], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Restored to operation: eac759b9ab75 (2001-02-03 08:05:07) add workspace 'default' "#); + } + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @r###" b (deleted) @origin: vpupmnsl hidden c7d4bdcb descr_for_b "###); + } // Now, let's demo restoring just the remote-tracking bookmark. First, let's // change our local repo state... test_env.jj_cmd_ok(&repo_path, &["bookmark", "c", "newbookmark"]); + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @r###" b (deleted) @origin: vpupmnsl hidden c7d4bdcb descr_for_b newbookmark: qpvuntsm 230dd059 (empty) (no description set) "###); + } // Restoring just the remote-tracking state will not affect `newbookmark`, but // will eliminate `b@origin`. let (stdout, stderr) = test_env.jj_cmd_ok( @@ -1049,103 +1326,143 @@ fn test_fetch_undo_what() { &base_operation_id, ], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Restored to operation: eac759b9ab75 (2001-02-03 08:05:07) add workspace 'default' "#); + } + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @r###" newbookmark: qpvuntsm 230dd059 (empty) (no description set) "###); + } } -#[test] -fn test_git_fetch_remove_fetch() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_fetch_remove_fetch(subprocess: bool) { let test_env = TestEnvironment::default(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.add_config("git.auto-local-bookmark = true"); test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); let repo_path = test_env.env_root().join("repo"); add_git_remote(&test_env, &repo_path, "origin"); test_env.jj_cmd_ok(&repo_path, &["bookmark", "create", "origin"]); + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @r###" origin: qpvuntsm 230dd059 (empty) (no description set) "###); + } test_env.jj_cmd_ok(&repo_path, &["git", "fetch"]); + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @r###" origin (conflicted): + qpvuntsm 230dd059 (empty) (no description set) + oputwtnw ffecd2d6 message @origin (behind by 1 commits): oputwtnw ffecd2d6 message "###); + } test_env.jj_cmd_ok(&repo_path, &["git", "remote", "remove", "origin"]); + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @r###" origin (conflicted): + qpvuntsm 230dd059 (empty) (no description set) + oputwtnw ffecd2d6 message "###); + } test_env.jj_cmd_ok(&repo_path, &["git", "remote", "add", "origin", "../origin"]); // Check that origin@origin is properly recreated let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["git", "fetch"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" bookmark: origin@origin [new] tracked "###); + } + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @r###" origin (conflicted): + qpvuntsm 230dd059 (empty) (no description set) + oputwtnw ffecd2d6 message @origin (behind by 1 commits): oputwtnw ffecd2d6 message "###); + } } -#[test] -fn test_git_fetch_rename_fetch() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_fetch_rename_fetch(subprocess: bool) { let test_env = TestEnvironment::default(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.add_config("git.auto-local-bookmark = true"); test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); let repo_path = test_env.env_root().join("repo"); add_git_remote(&test_env, &repo_path, "origin"); test_env.jj_cmd_ok(&repo_path, &["bookmark", "create", "origin"]); + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @r###" origin: qpvuntsm 230dd059 (empty) (no description set) "###); + } test_env.jj_cmd_ok(&repo_path, &["git", "fetch"]); + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @r###" origin (conflicted): + qpvuntsm 230dd059 (empty) (no description set) + oputwtnw ffecd2d6 message @origin (behind by 1 commits): oputwtnw ffecd2d6 message "###); + } test_env.jj_cmd_ok( &repo_path, &["git", "remote", "rename", "origin", "upstream"], ); + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @r###" origin (conflicted): + qpvuntsm 230dd059 (empty) (no description set) + oputwtnw ffecd2d6 message @upstream (behind by 1 commits): oputwtnw ffecd2d6 message "###); + } // Check that jj indicates that nothing has changed let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["git", "fetch", "--remote", "upstream"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Nothing changed. "###); + } } -#[test] -fn test_git_fetch_removed_bookmark() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_fetch_removed_bookmark(subprocess: bool) { let test_env = TestEnvironment::default(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.add_config("git.auto-local-bookmark = true"); let source_git_repo_path = test_env.env_root().join("source"); let _git_repo = git2::Repository::init(source_git_repo_path.clone()).unwrap(); @@ -1153,15 +1470,20 @@ fn test_git_fetch_removed_bookmark() { // Clone an empty repo. The target repo is a normal `jj` repo, *not* colocated let (stdout, stderr) = test_env.jj_cmd_ok(test_env.env_root(), &["git", "clone", "source", "target"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Fetching into new repo in "$TEST_ENV/target" Nothing changed. "###); + } let target_jj_repo_path = test_env.env_root().join("target"); let source_log = create_colocated_repo_and_bookmarks_from_trunk1(&test_env, &source_git_repo_path); + insta::allow_duplicates! { insta::assert_snapshot!(source_log, @r###" ===== Source git repo contents ===== @ c7d4bdcbc215 descr_for_b b @@ -1172,16 +1494,22 @@ fn test_git_fetch_removed_bookmark() { ○ ff36dc55760e descr_for_trunk1 trunk1 ◆ 000000000000 "###); + } // Fetch all bookmarks let (stdout, stderr) = test_env.jj_cmd_ok(&target_jj_repo_path, &["git", "fetch"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" bookmark: a1@origin [new] tracked bookmark: a2@origin [new] tracked bookmark: b@origin [new] tracked bookmark: trunk1@origin [new] tracked "###); + } + insta::allow_duplicates! { insta::assert_snapshot!(get_log_output(&test_env, &target_jj_repo_path), @r#" @ 230dd059e1b0 │ ○ c7d4bdcbc215 descr_for_b b @@ -1193,6 +1521,7 @@ fn test_git_fetch_removed_bookmark() { ├─╯ ◆ 000000000000 "#); + } // Remove a2 bookmark in origin test_env.jj_cmd_ok(&source_git_repo_path, &["bookmark", "forget", "a2"]); @@ -1200,10 +1529,15 @@ fn test_git_fetch_removed_bookmark() { // Fetch bookmark a1 from origin and check that a2 is still there let (stdout, stderr) = test_env.jj_cmd_ok(&target_jj_repo_path, &["git", "fetch", "--branch", "a1"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Nothing changed. "###); + } + insta::allow_duplicates! { insta::assert_snapshot!(get_log_output(&test_env, &target_jj_repo_path), @r#" @ 230dd059e1b0 │ ○ c7d4bdcbc215 descr_for_b b @@ -1215,11 +1549,15 @@ fn test_git_fetch_removed_bookmark() { ├─╯ ◆ 000000000000 "#); + } // Fetch bookmarks a2 from origin, and check that it has been removed locally let (stdout, stderr) = test_env.jj_cmd_ok(&target_jj_repo_path, &["git", "fetch", "--branch", "a2"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" bookmark: a2@origin [deleted] untracked Abandoned 1 commits that are no longer reachable. @@ -1233,11 +1571,16 @@ fn test_git_fetch_removed_bookmark() { ├─╯ ◆ 000000000000 "#); + } } -#[test] -fn test_git_fetch_removed_parent_bookmark() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_fetch_removed_parent_bookmark(subprocess: bool) { let test_env = TestEnvironment::default(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.add_config("git.auto-local-bookmark = true"); let source_git_repo_path = test_env.env_root().join("source"); let _git_repo = git2::Repository::init(source_git_repo_path.clone()).unwrap(); @@ -1245,15 +1588,20 @@ fn test_git_fetch_removed_parent_bookmark() { // Clone an empty repo. The target repo is a normal `jj` repo, *not* colocated let (stdout, stderr) = test_env.jj_cmd_ok(test_env.env_root(), &["git", "clone", "source", "target"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Fetching into new repo in "$TEST_ENV/target" Nothing changed. "###); + } let target_jj_repo_path = test_env.env_root().join("target"); let source_log = create_colocated_repo_and_bookmarks_from_trunk1(&test_env, &source_git_repo_path); + insta::allow_duplicates! { insta::assert_snapshot!(source_log, @r###" ===== Source git repo contents ===== @ c7d4bdcbc215 descr_for_b b @@ -1264,16 +1612,22 @@ fn test_git_fetch_removed_parent_bookmark() { ○ ff36dc55760e descr_for_trunk1 trunk1 ◆ 000000000000 "###); + } // Fetch all bookmarks let (stdout, stderr) = test_env.jj_cmd_ok(&target_jj_repo_path, &["git", "fetch"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" bookmark: a1@origin [new] tracked bookmark: a2@origin [new] tracked bookmark: b@origin [new] tracked bookmark: trunk1@origin [new] tracked "###); + } + insta::allow_duplicates! { insta::assert_snapshot!(get_log_output(&test_env, &target_jj_repo_path), @r#" @ 230dd059e1b0 │ ○ c7d4bdcbc215 descr_for_b b @@ -1285,6 +1639,7 @@ fn test_git_fetch_removed_parent_bookmark() { ├─╯ ◆ 000000000000 "#); + } // Remove all bookmarks in origin. test_env.jj_cmd_ok(&source_git_repo_path, &["bookmark", "forget", "glob:*"]); @@ -1298,13 +1653,18 @@ fn test_git_fetch_removed_parent_bookmark() { "git", "fetch", "--branch", "master", "--branch", "trunk1", "--branch", "a1", ], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" bookmark: a1@origin [deleted] untracked bookmark: trunk1@origin [deleted] untracked Abandoned 1 commits that are no longer reachable. Warning: No branch matching `master` found on any specified/configured remote "###); + } + insta::allow_duplicates! { insta::assert_snapshot!(get_log_output(&test_env, &target_jj_repo_path), @r#" @ 230dd059e1b0 │ ○ c7d4bdcbc215 descr_for_b b @@ -1314,11 +1674,16 @@ fn test_git_fetch_removed_parent_bookmark() { ├─╯ ◆ 000000000000 "#); + } } -#[test] -fn test_git_fetch_remote_only_bookmark() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_fetch_remote_only_bookmark(subprocess: bool) { let test_env = TestEnvironment::default(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); let repo_path = test_env.env_root().join("repo"); @@ -1353,10 +1718,12 @@ fn test_git_fetch_remote_only_bookmark() { // Fetch using git.auto_local_bookmark = true test_env.add_config("git.auto-local-bookmark = true"); test_env.jj_cmd_ok(&repo_path, &["git", "fetch", "--remote=origin"]); + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @r###" feature1: mzyxwzks 9f01a0e0 message @origin: mzyxwzks 9f01a0e0 message "###); + } git_repo .commit( @@ -1372,15 +1739,19 @@ fn test_git_fetch_remote_only_bookmark() { // Fetch using git.auto_local_bookmark = false test_env.add_config("git.auto-local-bookmark = false"); test_env.jj_cmd_ok(&repo_path, &["git", "fetch", "--remote=origin"]); + insta::allow_duplicates! { insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r#" @ 230dd059e1b0 │ ◆ 9f01a0e04879 message feature1 feature2@origin ├─╯ ◆ 000000000000 "#); + } + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @r###" feature1: mzyxwzks 9f01a0e0 message @origin: mzyxwzks 9f01a0e0 message feature2@origin: mzyxwzks 9f01a0e0 message "###); + } } diff --git a/cli/tests/test_git_push.rs b/cli/tests/test_git_push.rs index 528ab4cbaa..1277d666f7 100644 --- a/cli/tests/test_git_push.rs +++ b/cli/tests/test_git_push.rs @@ -15,6 +15,8 @@ use std::path::Path; use std::path::PathBuf; +use test_case::test_case; + use crate::common::TestEnvironment; fn set_up() -> (TestEnvironment, PathBuf) { @@ -47,27 +49,41 @@ fn set_up() -> (TestEnvironment, PathBuf) { (test_env, workspace_root) } -#[test] -fn test_git_push_nothing() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_push_nothing(subprocess: bool) { let (test_env, workspace_root) = set_up(); + if subprocess { + test_env.set_up_git_subprocessing(); + } // Show the setup. `insta` has trouble if this is done inside `set_up()` + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &workspace_root), @r###" bookmark1: xtvrqkyv d13ecdbd (empty) description 1 @origin: xtvrqkyv d13ecdbd (empty) description 1 bookmark2: rlzusymt 8476341e (empty) description 2 @origin: rlzusymt 8476341e (empty) description 2 "###); + } // No bookmarks to push yet let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--all"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Nothing changed. "###); + } } -#[test] -fn test_git_push_current_bookmark() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_push_current_bookmark(subprocess: bool) { let (test_env, workspace_root) = set_up(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.add_config(r#"revset-aliases."immutable_heads()" = "none()""#); // Update some bookmarks. `bookmark1` is not a current bookmark, but // `bookmark2` and `my-bookmark` are. @@ -80,6 +96,7 @@ fn test_git_push_current_bookmark() { test_env.jj_cmd_ok(&workspace_root, &["bookmark", "create", "my-bookmark"]); test_env.jj_cmd_ok(&workspace_root, &["describe", "-m", "foo"]); // Check the setup + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &workspace_root), @r###" bookmark1: xtvrqkyv 0f8dc656 (empty) modified bookmark1 commit @origin (ahead by 1 commits, behind by 1 commits): xtvrqkyv hidden d13ecdbd (empty) description 1 @@ -87,20 +104,28 @@ fn test_git_push_current_bookmark() { @origin (behind by 1 commits): rlzusymt 8476341e (empty) description 2 my-bookmark: yostqsxw bc7610b6 (empty) foo "###); + } // First dry-run. `bookmark1` should not get pushed. let (stdout, stderr) = test_env.jj_cmd_ok( &workspace_root, &["git", "push", "--allow-new", "--dry-run"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Changes to push to origin: Move forward bookmark bookmark2 from 8476341eb395 to bc7610b65a91 Add bookmark my-bookmark to bc7610b65a91 Dry-run requested, not pushing. "#); + } let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--allow-new"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Changes to push to origin: Move forward bookmark bookmark2 from 8476341eb395 to bc7610b65a91 @@ -114,6 +139,7 @@ fn test_git_push_current_bookmark() { my-bookmark: yostqsxw bc7610b6 (empty) foo @origin: yostqsxw bc7610b6 (empty) foo "###); + } // Try pushing backwards test_env.jj_cmd_ok( @@ -129,23 +155,35 @@ fn test_git_push_current_bookmark() { // This behavior is a strangeness of our definition of the default push revset. // We could consider changing it. let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Warning: No bookmarks found in the default push revset: remote_bookmarks(remote=origin)..@ Nothing changed. "###); + } // We can move a bookmark backwards let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "-bbookmark2"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Changes to push to origin: Move backward bookmark bookmark2 from bc7610b65a91 to 8476341eb395 "#); + } } -#[test] -fn test_git_push_parent_bookmark() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_push_parent_bookmark(subprocess: bool) { let (test_env, workspace_root) = set_up(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.add_config(r#"revset-aliases."immutable_heads()" = "none()""#); test_env.jj_cmd_ok(&workspace_root, &["edit", "bookmark1"]); test_env.jj_cmd_ok( @@ -155,43 +193,67 @@ fn test_git_push_parent_bookmark() { test_env.jj_cmd_ok(&workspace_root, &["new", "-m", "non-empty description"]); std::fs::write(workspace_root.join("file"), "file").unwrap(); let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Changes to push to origin: Move sideways bookmark bookmark1 from d13ecdbda2a2 to e612d524a5c6 "#); + } } -#[test] -fn test_git_push_no_matching_bookmark() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_push_no_matching_bookmark(subprocess: bool) { let (test_env, workspace_root) = set_up(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.jj_cmd_ok(&workspace_root, &["new"]); let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Warning: No bookmarks found in the default push revset: remote_bookmarks(remote=origin)..@ Nothing changed. "###); + } } -#[test] -fn test_git_push_matching_bookmark_unchanged() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_push_matching_bookmark_unchanged(subprocess: bool) { let (test_env, workspace_root) = set_up(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.jj_cmd_ok(&workspace_root, &["new", "bookmark1"]); let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Warning: No bookmarks found in the default push revset: remote_bookmarks(remote=origin)..@ Nothing changed. "###); + } } /// Test that `jj git push` without arguments pushes a bookmark to the specified /// remote even if it's already up to date on another remote /// (`remote_bookmarks(remote=)..@` vs. `remote_bookmarks()..@`). -#[test] -fn test_git_push_other_remote_has_bookmark() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_push_other_remote_has_bookmark(subprocess: bool) { let (test_env, workspace_root) = set_up(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.add_config(r#"revset-aliases."immutable_heads()" = "none()""#); // Create another remote (but actually the same) let other_remote_path = test_env @@ -215,18 +277,26 @@ fn test_git_push_other_remote_has_bookmark() { test_env.jj_cmd_ok(&workspace_root, &["edit", "bookmark1"]); test_env.jj_cmd_ok(&workspace_root, &["describe", "-m=modified"]); let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Changes to push to origin: Move sideways bookmark bookmark1 from d13ecdbda2a2 to a657f1b61b94 "#); + } // Since it's already pushed to origin, nothing will happen if push again let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Warning: No bookmarks found in the default push revset: remote_bookmarks(remote=origin)..@ Nothing changed. "###); + } // The bookmark was moved on the "other" remote as well (since it's actually the // same remote), but `jj` is not aware of that since it thinks this is a // different remote. So, the push should fail. @@ -239,16 +309,24 @@ fn test_git_push_other_remote_has_bookmark() { &workspace_root, &["git", "push", "--allow-new", "--remote=other"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Changes to push to other: Add bookmark bookmark1 to a657f1b61b94 "#); + } } -#[test] -fn test_git_push_forward_unexpectedly_moved() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_push_forward_unexpectedly_moved(subprocess: bool) { let (test_env, workspace_root) = set_up(); + if subprocess { + test_env.set_up_git_subprocessing(); + } // Move bookmark1 forward on the remote let origin_path = test_env.env_root().join("origin"); @@ -264,29 +342,37 @@ fn test_git_push_forward_unexpectedly_moved() { // Pushing should fail let stderr = test_env.jj_cmd_failure(&workspace_root, &["git", "push"]); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Changes to push to origin: Move forward bookmark bookmark1 from d13ecdbda2a2 to 6750425ff51c Error: Refusing to push a bookmark that unexpectedly moved on the remote. Affected refs: refs/heads/bookmark1 Hint: Try fetching from the remote, then make the bookmark point to where you want it to be, and push again. "#); + } } -#[test] -fn test_git_push_sideways_unexpectedly_moved() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_push_sideways_unexpectedly_moved(subprocess: bool) { let (test_env, workspace_root) = set_up(); + if subprocess { + test_env.set_up_git_subprocessing(); + } // Move bookmark1 forward on the remote let origin_path = test_env.env_root().join("origin"); test_env.jj_cmd_ok(&origin_path, &["new", "bookmark1", "-m=remote"]); std::fs::write(origin_path.join("remote"), "remote").unwrap(); test_env.jj_cmd_ok(&origin_path, &["bookmark", "set", "bookmark1"]); + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &origin_path), @r###" bookmark1: vruxwmqv 80284bec remote @git (behind by 1 commits): qpvuntsm d13ecdbd (empty) description 1 bookmark2: zsuskuln 8476341e (empty) description 2 @git: zsuskuln 8476341e (empty) description 2 "###); + } test_env.jj_cmd_ok(&origin_path, &["git", "export"]); // Move bookmark1 sideways to another commit locally @@ -296,73 +382,93 @@ fn test_git_push_sideways_unexpectedly_moved() { &workspace_root, &["bookmark", "set", "bookmark1", "--allow-backwards"], ); + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &workspace_root), @r###" bookmark1: kmkuslsw 0f8bf988 local @origin (ahead by 1 commits, behind by 1 commits): xtvrqkyv d13ecdbd (empty) description 1 bookmark2: rlzusymt 8476341e (empty) description 2 @origin: rlzusymt 8476341e (empty) description 2 "###); + } let stderr = test_env.jj_cmd_failure(&workspace_root, &["git", "push"]); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Changes to push to origin: Move sideways bookmark bookmark1 from d13ecdbda2a2 to 0f8bf988588e Error: Refusing to push a bookmark that unexpectedly moved on the remote. Affected refs: refs/heads/bookmark1 Hint: Try fetching from the remote, then make the bookmark point to where you want it to be, and push again. "#); + } } // This tests whether the push checks that the remote bookmarks are in expected // positions. -#[test] -fn test_git_push_deletion_unexpectedly_moved() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_push_deletion_unexpectedly_moved(subprocess: bool) { let (test_env, workspace_root) = set_up(); + if subprocess { + test_env.set_up_git_subprocessing(); + } // Move bookmark1 forward on the remote let origin_path = test_env.env_root().join("origin"); test_env.jj_cmd_ok(&origin_path, &["new", "bookmark1", "-m=remote"]); std::fs::write(origin_path.join("remote"), "remote").unwrap(); test_env.jj_cmd_ok(&origin_path, &["bookmark", "set", "bookmark1"]); + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &origin_path), @r###" bookmark1: vruxwmqv 80284bec remote @git (behind by 1 commits): qpvuntsm d13ecdbd (empty) description 1 bookmark2: zsuskuln 8476341e (empty) description 2 @git: zsuskuln 8476341e (empty) description 2 "###); + } test_env.jj_cmd_ok(&origin_path, &["git", "export"]); // Delete bookmark1 locally test_env.jj_cmd_ok(&workspace_root, &["bookmark", "delete", "bookmark1"]); + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &workspace_root), @r###" bookmark1 (deleted) @origin: xtvrqkyv d13ecdbd (empty) description 1 bookmark2: rlzusymt 8476341e (empty) description 2 @origin: rlzusymt 8476341e (empty) description 2 "###); + } let stderr = test_env.jj_cmd_failure(&workspace_root, &["git", "push", "--bookmark", "bookmark1"]); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Changes to push to origin: Delete bookmark bookmark1 from d13ecdbda2a2 Error: Refusing to push a bookmark that unexpectedly moved on the remote. Affected refs: refs/heads/bookmark1 Hint: Try fetching from the remote, then make the bookmark point to where you want it to be, and push again. "#); + } } -#[test] -fn test_git_push_unexpectedly_deleted() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_push_unexpectedly_deleted(subprocess: bool) { let (test_env, workspace_root) = set_up(); + if subprocess { + test_env.set_up_git_subprocessing(); + } // Delete bookmark1 forward on the remote let origin_path = test_env.env_root().join("origin"); test_env.jj_cmd_ok(&origin_path, &["bookmark", "delete", "bookmark1"]); + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &origin_path), @r###" bookmark1 (deleted) @git: qpvuntsm d13ecdbd (empty) description 1 bookmark2: zsuskuln 8476341e (empty) description 2 @git: zsuskuln 8476341e (empty) description 2 "###); + } test_env.jj_cmd_ok(&origin_path, &["git", "export"]); // Move bookmark1 sideways to another commit locally @@ -372,42 +478,65 @@ fn test_git_push_unexpectedly_deleted() { &workspace_root, &["bookmark", "set", "bookmark1", "--allow-backwards"], ); + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &workspace_root), @r###" bookmark1: kpqxywon 1ebe27ba local @origin (ahead by 1 commits, behind by 1 commits): xtvrqkyv d13ecdbd (empty) description 1 bookmark2: rlzusymt 8476341e (empty) description 2 @origin: rlzusymt 8476341e (empty) description 2 "###); + } // Pushing a moved bookmark fails if deleted on remote let stderr = test_env.jj_cmd_failure(&workspace_root, &["git", "push"]); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Changes to push to origin: Move sideways bookmark bookmark1 from d13ecdbda2a2 to 1ebe27ba04bf Error: Refusing to push a bookmark that unexpectedly moved on the remote. Affected refs: refs/heads/bookmark1 Hint: Try fetching from the remote, then make the bookmark point to where you want it to be, and push again. "#); + } test_env.jj_cmd_ok(&workspace_root, &["bookmark", "delete", "bookmark1"]); + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &workspace_root), @r###" bookmark1 (deleted) @origin: xtvrqkyv d13ecdbd (empty) description 1 bookmark2: rlzusymt 8476341e (empty) description 2 @origin: rlzusymt 8476341e (empty) description 2 "###); - // Pushing a *deleted* bookmark succeeds if deleted on remote, even if we expect - // bookmark1@origin to exist and point somewhere. - let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "-bbookmark1"]); - insta::assert_snapshot!(stdout, @""); - insta::assert_snapshot!(stderr, @r#" - Changes to push to origin: - Delete bookmark bookmark1 from d13ecdbda2a2 - "#); + } + + if subprocess { + // git does not allow to push a deleted bookmark if we expect it to exist even + // though it was already deleted + let stderr = test_env.jj_cmd_failure(&workspace_root, &["git", "push", "-bbookmark1"]); + insta::assert_snapshot!(stderr, @r" + Changes to push to origin: + Delete bookmark bookmark1 from d13ecdbda2a2 + Error: Refusing to push a bookmark that unexpectedly moved on the remote. Affected refs: refs/heads/bookmark1 + Hint: Try fetching from the remote, then make the bookmark point to where you want it to be, and push again. + "); + } else { + // Pushing a *deleted* bookmark succeeds if deleted on remote, even if we expect + // bookmark1@origin to exist and point somewhere. + let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "-bbookmark1"]); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r#" + Changes to push to origin: + Delete bookmark bookmark1 from d13ecdbda2a2 + "#); + } } -#[test] -fn test_git_push_creation_unexpectedly_already_exists() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_push_creation_unexpectedly_already_exists(subprocess: bool) { let (test_env, workspace_root) = set_up(); + if subprocess { + test_env.set_up_git_subprocessing(); + } // Forget bookmark1 locally test_env.jj_cmd_ok(&workspace_root, &["bookmark", "forget", "bookmark1"]); @@ -416,24 +545,32 @@ fn test_git_push_creation_unexpectedly_already_exists() { test_env.jj_cmd_ok(&workspace_root, &["new", "root()", "-m=new bookmark1"]); std::fs::write(workspace_root.join("local"), "local").unwrap(); test_env.jj_cmd_ok(&workspace_root, &["bookmark", "create", "bookmark1"]); + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &workspace_root), @r###" bookmark1: yostqsxw cb17dcdc new bookmark1 bookmark2: rlzusymt 8476341e (empty) description 2 @origin: rlzusymt 8476341e (empty) description 2 "###); + } let stderr = test_env.jj_cmd_failure(&workspace_root, &["git", "push", "--allow-new"]); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Changes to push to origin: Add bookmark bookmark1 to cb17dcdc74d5 Error: Refusing to push a bookmark that unexpectedly moved on the remote. Affected refs: refs/heads/bookmark1 Hint: Try fetching from the remote, then make the bookmark point to where you want it to be, and push again. "#); + } } -#[test] -fn test_git_push_locally_created_and_rewritten() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_push_locally_created_and_rewritten(subprocess: bool) { let (test_env, workspace_root) = set_up(); + if subprocess { + test_env.set_up_git_subprocessing(); + } // Ensure that remote bookmarks aren't tracked automatically test_env.add_config("git.auto-local-bookmark = false"); @@ -441,33 +578,40 @@ fn test_git_push_locally_created_and_rewritten() { test_env.jj_cmd_ok(&workspace_root, &["new", "root()", "-mlocal 1"]); test_env.jj_cmd_ok(&workspace_root, &["bookmark", "create", "my"]); let (_stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push"]); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r" Warning: Refusing to create new remote bookmark my@origin Hint: Use --allow-new to push new bookmark. Use --remote to specify the remote to push to. Nothing changed. "); + } // Either --allow-new or git.push-new-bookmarks=true should work let (_stdout, stderr) = test_env.jj_cmd_ok( &workspace_root, &["git", "push", "--allow-new", "--dry-run"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r" Changes to push to origin: Add bookmark my to fcc999921ce9 Dry-run requested, not pushing. "); + } let (_stdout, stderr) = test_env.jj_cmd_ok( &workspace_root, &["git", "push", "--config=git.push-new-bookmarks=true"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Changes to push to origin: Add bookmark my to fcc999921ce9 "#); + } // Rewrite it and push again, which would fail if the pushed bookmark weren't // set to "tracking" test_env.jj_cmd_ok(&workspace_root, &["describe", "-mlocal 2"]); + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &workspace_root), @r" bookmark1: xtvrqkyv d13ecdbd (empty) description 1 @origin: xtvrqkyv d13ecdbd (empty) description 1 @@ -476,16 +620,23 @@ fn test_git_push_locally_created_and_rewritten() { my: vruxwmqv 423bb660 (empty) local 2 @origin (ahead by 1 commits, behind by 1 commits): vruxwmqv hidden fcc99992 (empty) local 1 "); + } let (_stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push"]); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r" Changes to push to origin: Move sideways bookmark my from fcc999921ce9 to 423bb66069e7 "); + } } -#[test] -fn test_git_push_multiple() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_push_multiple(subprocess: bool) { let (test_env, workspace_root) = set_up(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.jj_cmd_ok(&workspace_root, &["bookmark", "delete", "bookmark1"]); test_env.jj_cmd_ok( &workspace_root, @@ -494,6 +645,7 @@ fn test_git_push_multiple() { test_env.jj_cmd_ok(&workspace_root, &["bookmark", "create", "my-bookmark"]); test_env.jj_cmd_ok(&workspace_root, &["describe", "-m", "foo"]); // Check the setup + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &workspace_root), @r###" bookmark1 (deleted) @origin: xtvrqkyv d13ecdbd (empty) description 1 @@ -501,10 +653,14 @@ fn test_git_push_multiple() { @origin (ahead by 1 commits, behind by 1 commits): rlzusymt 8476341e (empty) description 2 my-bookmark: yqosqzyt c4a3c310 (empty) foo "###); + } // First dry-run let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--all", "--dry-run"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Changes to push to origin: Delete bookmark bookmark1 from d13ecdbda2a2 @@ -512,6 +668,7 @@ fn test_git_push_multiple() { Add bookmark my-bookmark to c4a3c3105d92 Dry-run requested, not pushing. "#); + } // Dry run requesting two specific bookmarks let (stdout, stderr) = test_env.jj_cmd_ok( &workspace_root, @@ -524,13 +681,17 @@ fn test_git_push_multiple() { "--dry-run", ], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Changes to push to origin: Delete bookmark bookmark1 from d13ecdbda2a2 Add bookmark my-bookmark to c4a3c3105d92 Dry-run requested, not pushing. "#); + } // Dry run requesting two specific bookmarks twice let (stdout, stderr) = test_env.jj_cmd_ok( &workspace_root, @@ -545,41 +706,56 @@ fn test_git_push_multiple() { "--dry-run", ], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Changes to push to origin: Delete bookmark bookmark1 from d13ecdbda2a2 Add bookmark my-bookmark to c4a3c3105d92 Dry-run requested, not pushing. "#); + } // Dry run with glob pattern let (stdout, stderr) = test_env.jj_cmd_ok( &workspace_root, &["git", "push", "-b=glob:bookmark?", "--dry-run"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Changes to push to origin: Delete bookmark bookmark1 from d13ecdbda2a2 Move sideways bookmark bookmark2 from 8476341eb395 to c4a3c3105d92 Dry-run requested, not pushing. "#); + } // Unmatched bookmark name is error let stderr = test_env.jj_cmd_failure(&workspace_root, &["git", "push", "-b=foo"]); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Error: No such bookmark: foo "###); + } let stderr = test_env.jj_cmd_failure( &workspace_root, &["git", "push", "-b=foo", "-b=glob:?bookmark"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Error: No matching bookmarks for patterns: foo, ?bookmark "###); + } let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--all"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Changes to push to origin: Delete bookmark bookmark1 from d13ecdbda2a2 @@ -592,7 +768,9 @@ fn test_git_push_multiple() { my-bookmark: yqosqzyt c4a3c310 (empty) foo @origin: yqosqzyt c4a3c310 (empty) foo "###); + } let stdout = test_env.jj_cmd_success(&workspace_root, &["log", "-rall()"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @r###" @ yqosqzyt test.user@example.com 2001-02-03 08:05:17 bookmark2 my-bookmark c4a3c310 │ (empty) foo @@ -602,26 +780,36 @@ fn test_git_push_multiple() { ├─╯ (empty) description 1 ◆ zzzzzzzz root() 00000000 "###); + } } -#[test] -fn test_git_push_changes() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_push_changes(subprocess: bool) { let (test_env, workspace_root) = set_up(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.jj_cmd_ok(&workspace_root, &["describe", "-m", "foo"]); std::fs::write(workspace_root.join("file"), "contents").unwrap(); test_env.jj_cmd_ok(&workspace_root, &["new", "-m", "bar"]); std::fs::write(workspace_root.join("file"), "modified").unwrap(); let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--change", "@"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Creating bookmark push-yostqsxwqrlt for revision yostqsxwqrlt Changes to push to origin: Add bookmark push-yostqsxwqrlt to cf1a53a8800a "#); + } // test pushing two changes at once std::fs::write(workspace_root.join("file"), "modified2").unwrap(); let stderr = test_env.jj_cmd_failure(&workspace_root, &["git", "push", "-c=(@|@-)"]); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Error: Revset "(@|@-)" resolved to more than one revision Hint: The revset "(@|@-)" resolved to these revisions: @@ -629,23 +817,32 @@ fn test_git_push_changes() { yqosqzyt a050abf4 foo Hint: Prefix the expression with 'all:' to allow any number of revisions (i.e. 'all:(@|@-)'). "###); + } // test pushing two changes at once, part 2 let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "-c=all:(@|@-)"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Creating bookmark push-yqosqzytrlsw for revision yqosqzytrlsw Changes to push to origin: Move sideways bookmark push-yostqsxwqrlt from cf1a53a8800a to 16c169664e9f Add bookmark push-yqosqzytrlsw to a050abf4ff07 "#); + } // specifying the same change twice doesn't break things std::fs::write(workspace_root.join("file"), "modified3").unwrap(); let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "-c=all:(@|@)"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Changes to push to origin: Move sideways bookmark push-yostqsxwqrlt from 16c169664e9f to ef6313d50ac1 "#); + } // specifying the same bookmark with --change/--bookmark doesn't break things std::fs::write(workspace_root.join("file"), "modified4").unwrap(); @@ -653,11 +850,15 @@ fn test_git_push_changes() { &workspace_root, &["git", "push", "-c=@", "-b=push-yostqsxwqrlt"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Changes to push to origin: Move sideways bookmark push-yostqsxwqrlt from ef6313d50ac1 to c1e65d3a64ce "#); + } // try again with --change that moves the bookmark forward std::fs::write(workspace_root.join("file"), "modified5").unwrap(); @@ -672,28 +873,36 @@ fn test_git_push_changes() { ], ); let stdout = test_env.jj_cmd_success(&workspace_root, &["status"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @r###" Working copy changes: M file Working copy : yostqsxw 38cb417c bar Parent commit: yqosqzyt a050abf4 push-yostqsxwqrlt* push-yqosqzytrlsw | foo "###); + } let (stdout, stderr) = test_env.jj_cmd_ok( &workspace_root, &["git", "push", "-c=@", "-b=push-yostqsxwqrlt"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Changes to push to origin: Move sideways bookmark push-yostqsxwqrlt from c1e65d3a64ce to 38cb417ce3a6 "#); + } let stdout = test_env.jj_cmd_success(&workspace_root, &["status"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @r###" Working copy changes: M file Working copy : yostqsxw 38cb417c push-yostqsxwqrlt | bar Parent commit: yqosqzyt a050abf4 push-yqosqzytrlsw | foo "###); + } // Test changing `git.push-bookmark-prefix`. It causes us to push again. let (stdout, stderr) = test_env.jj_cmd_ok( @@ -705,12 +914,16 @@ fn test_git_push_changes() { "--change=@", ], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Creating bookmark test-yostqsxwqrlt for revision yostqsxwqrlt Changes to push to origin: Add bookmark test-yostqsxwqrlt to 38cb417ce3a6 "#); + } // Test deprecation warning for `git.push-branch-prefix` let (stdout, stderr) = test_env.jj_cmd_ok( @@ -722,18 +935,26 @@ fn test_git_push_changes() { "--change=@", ], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r" Warning: Deprecated config: git.push-branch-prefix is renamed to git.push-bookmark-prefix Creating bookmark branch-yostqsxwqrlt for revision yostqsxwqrlt Changes to push to origin: Add bookmark branch-yostqsxwqrlt to 38cb417ce3a6 "); + } } -#[test] -fn test_git_push_revisions() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_push_revisions(subprocess: bool) { let (test_env, workspace_root) = set_up(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.jj_cmd_ok(&workspace_root, &["describe", "-m", "foo"]); std::fs::write(workspace_root.join("file"), "contents").unwrap(); test_env.jj_cmd_ok(&workspace_root, &["new", "-m", "bar"]); @@ -749,69 +970,95 @@ fn test_git_push_revisions() { &workspace_root, &["git", "push", "--allow-new", "-r=none()"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Warning: No bookmarks point to the specified revisions: none() Nothing changed. "###); + } // Push a revision with no bookmarks let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--allow-new", "-r=@--"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Warning: No bookmarks point to the specified revisions: @-- Nothing changed. "###); + } // Push a revision with a single bookmark let (stdout, stderr) = test_env.jj_cmd_ok( &workspace_root, &["git", "push", "--allow-new", "-r=@-", "--dry-run"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Changes to push to origin: Add bookmark bookmark-1 to 5f432a855e59 Dry-run requested, not pushing. "#); + } // Push multiple revisions of which some have bookmarks let (stdout, stderr) = test_env.jj_cmd_ok( &workspace_root, &["git", "push", "--allow-new", "-r=@--", "-r=@-", "--dry-run"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Warning: No bookmarks point to the specified revisions: @-- Changes to push to origin: Add bookmark bookmark-1 to 5f432a855e59 Dry-run requested, not pushing. "#); + } // Push a revision with a multiple bookmarks let (stdout, stderr) = test_env.jj_cmd_ok( &workspace_root, &["git", "push", "--allow-new", "-r=@", "--dry-run"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Changes to push to origin: Add bookmark bookmark-2a to 84f499037f5c Add bookmark bookmark-2b to 84f499037f5c Dry-run requested, not pushing. "#); + } // Repeating a commit doesn't result in repeated messages about the bookmark let (stdout, stderr) = test_env.jj_cmd_ok( &workspace_root, &["git", "push", "--allow-new", "-r=@-", "-r=@-", "--dry-run"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Changes to push to origin: Add bookmark bookmark-1 to 5f432a855e59 Dry-run requested, not pushing. "#); + } } -#[test] -fn test_git_push_mixed() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_push_mixed(subprocess: bool) { let (test_env, workspace_root) = set_up(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.jj_cmd_ok(&workspace_root, &["describe", "-m", "foo"]); std::fs::write(workspace_root.join("file"), "contents").unwrap(); test_env.jj_cmd_ok(&workspace_root, &["new", "-m", "bar"]); @@ -833,11 +1080,13 @@ fn test_git_push_mixed() { "-r=@", ], ); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r" Creating bookmark push-yqosqzytrlsw for revision yqosqzytrlsw Error: Refusing to create new remote bookmark bookmark-1@origin Hint: Use --allow-new to push new bookmark. Use --remote to specify the remote to push to. "); + } let (stdout, stderr) = test_env.jj_cmd_ok( &workspace_root, @@ -850,7 +1099,10 @@ fn test_git_push_mixed() { "-r=@", ], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Creating bookmark push-yqosqzytrlsw for revision yqosqzytrlsw Changes to push to origin: @@ -859,11 +1111,16 @@ fn test_git_push_mixed() { Add bookmark bookmark-2a to 84f499037f5c Add bookmark bookmark-2b to 84f499037f5c "#); + } } -#[test] -fn test_git_push_existing_long_bookmark() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_push_existing_long_bookmark(subprocess: bool) { let (test_env, workspace_root) = set_up(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.jj_cmd_ok(&workspace_root, &["describe", "-m", "foo"]); std::fs::write(workspace_root.join("file"), "contents").unwrap(); test_env.jj_cmd_ok( @@ -876,16 +1133,24 @@ fn test_git_push_existing_long_bookmark() { ); let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--change=@"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Changes to push to origin: Add bookmark push-19b790168e73f7a73a98deae21e807c0 to a050abf4ff07 "#); + } } -#[test] -fn test_git_push_unsnapshotted_change() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_push_unsnapshotted_change(subprocess: bool) { let (test_env, workspace_root) = set_up(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.jj_cmd_ok(&workspace_root, &["describe", "-m", "foo"]); std::fs::write(workspace_root.join("file"), "contents").unwrap(); test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--change", "@"]); @@ -893,9 +1158,13 @@ fn test_git_push_unsnapshotted_change() { test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--change", "@"]); } -#[test] -fn test_git_push_conflict() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_push_conflict(subprocess: bool) { let (test_env, workspace_root) = set_up(); + if subprocess { + test_env.set_up_git_subprocessing(); + } std::fs::write(workspace_root.join("file"), "first").unwrap(); test_env.jj_cmd_ok(&workspace_root, &["commit", "-m", "first"]); std::fs::write(workspace_root.join("file"), "second").unwrap(); @@ -905,25 +1174,33 @@ fn test_git_push_conflict() { test_env.jj_cmd_ok(&workspace_root, &["bookmark", "create", "my-bookmark"]); test_env.jj_cmd_ok(&workspace_root, &["describe", "-m", "third"]); let stderr = test_env.jj_cmd_failure(&workspace_root, &["git", "push", "--all"]); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Error: Won't push commit e2221a796300 since it has conflicts Hint: Rejected commit: yostqsxw e2221a79 my-bookmark | (conflict) third "###); + } } -#[test] -fn test_git_push_no_description() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_push_no_description(subprocess: bool) { let (test_env, workspace_root) = set_up(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.jj_cmd_ok(&workspace_root, &["bookmark", "create", "my-bookmark"]); test_env.jj_cmd_ok(&workspace_root, &["describe", "-m="]); let stderr = test_env.jj_cmd_failure( &workspace_root, &["git", "push", "--allow-new", "--bookmark", "my-bookmark"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r" Error: Won't push commit 5b36783cd11c since it has no description Hint: Rejected commit: yqosqzyt 5b36783c my-bookmark | (empty) (no description set) "); + } test_env.jj_cmd_ok( &workspace_root, &[ @@ -937,9 +1214,13 @@ fn test_git_push_no_description() { ); } -#[test] -fn test_git_push_no_description_in_immutable() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_push_no_description_in_immutable(subprocess: bool) { let (test_env, workspace_root) = set_up(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.jj_cmd_ok(&workspace_root, &["bookmark", "create", "imm"]); test_env.jj_cmd_ok(&workspace_root, &["describe", "-m="]); test_env.jj_cmd_ok(&workspace_root, &["new", "-m", "foo"]); @@ -956,10 +1237,12 @@ fn test_git_push_no_description_in_immutable() { "--dry-run", ], ); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r" Error: Won't push commit 5b36783cd11c since it has no description Hint: Rejected commit: yqosqzyt 5b36783c imm | (empty) (no description set) "); + } test_env.add_config(r#"revset-aliases."immutable_heads()" = "imm""#); let (stdout, stderr) = test_env.jj_cmd_ok( @@ -972,17 +1255,25 @@ fn test_git_push_no_description_in_immutable() { "--dry-run", ], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Changes to push to origin: Add bookmark my-bookmark to ea7373507ad9 Dry-run requested, not pushing. "#); + } } -#[test] -fn test_git_push_missing_author() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_push_missing_author(subprocess: bool) { let (test_env, workspace_root) = set_up(); + if subprocess { + test_env.set_up_git_subprocessing(); + } let run_without_var = |var: &str, args: &[&str]| { test_env .jj_cmd(&workspace_root, args) @@ -996,25 +1287,33 @@ fn test_git_push_missing_author() { &workspace_root, &["git", "push", "--allow-new", "--bookmark", "missing-name"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r" Error: Won't push commit 944313939bbd since it has no author and/or committer set Hint: Rejected commit: vruxwmqv 94431393 missing-name | (empty) initial "); + } run_without_var("JJ_EMAIL", &["new", "root()", "-m=initial"]); run_without_var("JJ_EMAIL", &["bookmark", "create", "missing-email"]); let stderr = test_env.jj_cmd_failure( &workspace_root, &["git", "push", "--allow-new", "--bookmark=missing-email"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r" Error: Won't push commit 59354714f789 since it has no author and/or committer set Hint: Rejected commit: kpqxywon 59354714 missing-email | (empty) initial "); + } } -#[test] -fn test_git_push_missing_author_in_immutable() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_push_missing_author_in_immutable(subprocess: bool) { let (test_env, workspace_root) = set_up(); + if subprocess { + test_env.set_up_git_subprocessing(); + } let run_without_var = |var: &str, args: &[&str]| { test_env .jj_cmd(&workspace_root, args) @@ -1039,10 +1338,12 @@ fn test_git_push_missing_author_in_immutable() { "--dry-run", ], ); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r" Error: Won't push commit 011f740bf8b5 since it has no author and/or committer set Hint: Rejected commit: yostqsxw 011f740b imm | (empty) no author email "); + } test_env.add_config(r#"revset-aliases."immutable_heads()" = "imm""#); let (stdout, stderr) = test_env.jj_cmd_ok( @@ -1055,17 +1356,25 @@ fn test_git_push_missing_author_in_immutable() { "--dry-run", ], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Changes to push to origin: Add bookmark my-bookmark to 68fdae89de4f Dry-run requested, not pushing. "#); + } } -#[test] -fn test_git_push_missing_committer() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_push_missing_committer(subprocess: bool) { let (test_env, workspace_root) = set_up(); + if subprocess { + test_env.set_up_git_subprocessing(); + } let run_without_var = |var: &str, args: &[&str]| { test_env .jj_cmd(&workspace_root, args) @@ -1079,10 +1388,12 @@ fn test_git_push_missing_committer() { &workspace_root, &["git", "push", "--allow-new", "--bookmark=missing-name"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r" Error: Won't push commit 4fd190283d1a since it has no author and/or committer set Hint: Rejected commit: yqosqzyt 4fd19028 missing-name | (empty) no committer name "); + } test_env.jj_cmd_ok(&workspace_root, &["new", "root()"]); test_env.jj_cmd_ok(&workspace_root, &["bookmark", "create", "missing-email"]); run_without_var("JJ_EMAIL", &["describe", "-m=no committer email"]); @@ -1090,10 +1401,12 @@ fn test_git_push_missing_committer() { &workspace_root, &["git", "push", "--allow-new", "--bookmark=missing-email"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r" Error: Won't push commit eab97428a6ec since it has no author and/or committer set Hint: Rejected commit: kpqxywon eab97428 missing-email | (empty) no committer email "); + } // Test message when there are multiple reasons (missing committer and // description) @@ -1102,15 +1415,21 @@ fn test_git_push_missing_committer() { &workspace_root, &["git", "push", "--allow-new", "--bookmark=missing-email"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r" Error: Won't push commit 1143ed607f54 since it has no description and it has no author and/or committer set Hint: Rejected commit: kpqxywon 1143ed60 missing-email | (empty) (no description set) "); + } } -#[test] -fn test_git_push_missing_committer_in_immutable() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_push_missing_committer_in_immutable(subprocess: bool) { let (test_env, workspace_root) = set_up(); + if subprocess { + test_env.set_up_git_subprocessing(); + } let run_without_var = |var: &str, args: &[&str]| { test_env .jj_cmd(&workspace_root, args) @@ -1136,10 +1455,12 @@ fn test_git_push_missing_committer_in_immutable() { "--dry-run", ], ); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r" Error: Won't push commit 7e61dc727a8f since it has no author and/or committer set Hint: Rejected commit: yostqsxw 7e61dc72 imm | (empty) no committer email "); + } test_env.add_config(r#"revset-aliases."immutable_heads()" = "imm""#); let (stdout, stderr) = test_env.jj_cmd_ok( @@ -1152,26 +1473,39 @@ fn test_git_push_missing_committer_in_immutable() { "--dry-run", ], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Changes to push to origin: Add bookmark my-bookmark to c79f85e90b4a Dry-run requested, not pushing. "#); + } } -#[test] -fn test_git_push_deleted() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_push_deleted(subprocess: bool) { let (test_env, workspace_root) = set_up(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.jj_cmd_ok(&workspace_root, &["bookmark", "delete", "bookmark1"]); let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--deleted"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Changes to push to origin: Delete bookmark bookmark1 from d13ecdbda2a2 "#); + } let stdout = test_env.jj_cmd_success(&workspace_root, &["log", "-rall()"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @r#" @ yqosqzyt test.user@example.com 2001-02-03 08:05:13 5b36783c │ (empty) (no description set) @@ -1181,16 +1515,25 @@ fn test_git_push_deleted() { ├─╯ (empty) description 1 ◆ zzzzzzzz root() 00000000 "#); + } let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--deleted"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Nothing changed. "###); + } } -#[test] -fn test_git_push_conflicting_bookmarks() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_push_conflicting_bookmarks(subprocess: bool) { let (test_env, workspace_root) = set_up(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.add_config("git.auto-local-bookmark = true"); let git_repo = { let mut git_repo_path = workspace_root.clone(); @@ -1208,6 +1551,7 @@ fn test_git_push_conflicting_bookmarks() { test_env.jj_cmd_ok(&workspace_root, &["new", "root()", "-m=description 3"]); test_env.jj_cmd_ok(&workspace_root, &["bookmark", "create", "bookmark2"]); test_env.jj_cmd_ok(&workspace_root, &["git", "fetch"]); + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &workspace_root), @r###" bookmark1: xtvrqkyv d13ecdbd (empty) description 1 @origin: xtvrqkyv d13ecdbd (empty) description 1 @@ -1216,6 +1560,7 @@ fn test_git_push_conflicting_bookmarks() { + rlzusymt 8476341e (empty) description 2 @origin (behind by 1 commits): rlzusymt 8476341e (empty) description 2 "###); + } let bump_bookmark1 = || { test_env.jj_cmd_ok(&workspace_root, &["new", "bookmark1", "-m=bump"]); @@ -1224,50 +1569,68 @@ fn test_git_push_conflicting_bookmarks() { // Conflicting bookmark at @ let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--allow-new"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Warning: Bookmark bookmark2 is conflicted Hint: Run `jj bookmark list` to inspect, and use `jj bookmark set` to fix it up. Nothing changed. "###); + } // --bookmark should be blocked by conflicting bookmark let stderr = test_env.jj_cmd_failure( &workspace_root, &["git", "push", "--allow-new", "--bookmark", "bookmark2"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Error: Bookmark bookmark2 is conflicted Hint: Run `jj bookmark list` to inspect, and use `jj bookmark set` to fix it up. "###); + } // --all shouldn't be blocked by conflicting bookmark bump_bookmark1(); let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--all"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Warning: Bookmark bookmark2 is conflicted Hint: Run `jj bookmark list` to inspect, and use `jj bookmark set` to fix it up. Changes to push to origin: Move forward bookmark bookmark1 from d13ecdbda2a2 to 8df52121b022 "#); + } // --revisions shouldn't be blocked by conflicting bookmark bump_bookmark1(); let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--allow-new", "-rall()"]); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); + } + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Warning: Bookmark bookmark2 is conflicted Hint: Run `jj bookmark list` to inspect, and use `jj bookmark set` to fix it up. Changes to push to origin: Move forward bookmark bookmark1 from 8df52121b022 to 345e1f64a64d "#); + } } -#[test] -fn test_git_push_deleted_untracked() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_push_deleted_untracked(subprocess: bool) { let (test_env, workspace_root) = set_up(); + if subprocess { + test_env.set_up_git_subprocessing(); + } // Absent local bookmark shouldn't be considered "deleted" compared to // non-tracking remote bookmark. @@ -1277,18 +1640,26 @@ fn test_git_push_deleted_untracked() { &["bookmark", "untrack", "bookmark1@origin"], ); let (_stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--deleted"]); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Nothing changed. "###); + } let stderr = test_env.jj_cmd_failure(&workspace_root, &["git", "push", "--bookmark=bookmark1"]); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Error: No such bookmark: bookmark1 "###); + } } -#[test] -fn test_git_push_tracked_vs_all() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_push_tracked_vs_all(subprocess: bool) { let (test_env, workspace_root) = set_up(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.jj_cmd_ok(&workspace_root, &["new", "bookmark1", "-mmoved bookmark1"]); test_env.jj_cmd_ok(&workspace_root, &["bookmark", "set", "bookmark1"]); test_env.jj_cmd_ok(&workspace_root, &["new", "bookmark2", "-mmoved bookmark2"]); @@ -1298,6 +1669,7 @@ fn test_git_push_tracked_vs_all() { &["bookmark", "untrack", "bookmark1@origin"], ); test_env.jj_cmd_ok(&workspace_root, &["bookmark", "create", "bookmark3"]); + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &workspace_root), @r###" bookmark1: vruxwmqv db059e3f (empty) moved bookmark1 bookmark1@origin: xtvrqkyv d13ecdbd (empty) description 1 @@ -1305,34 +1677,41 @@ fn test_git_push_tracked_vs_all() { @origin: rlzusymt 8476341e (empty) description 2 bookmark3: znkkpsqq 1aa4f1f2 (empty) moved bookmark2 "###); + } // At this point, only bookmark2 is still tracked. `jj git push --tracked` would // try to push it and no other bookmarks. let (_stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--tracked", "--dry-run"]); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Changes to push to origin: Delete bookmark bookmark2 from 8476341eb395 Dry-run requested, not pushing. "#); + } // Untrack the last remaining tracked bookmark. test_env.jj_cmd_ok( &workspace_root, &["bookmark", "untrack", "bookmark2@origin"], ); + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &workspace_root), @r###" bookmark1: vruxwmqv db059e3f (empty) moved bookmark1 bookmark1@origin: xtvrqkyv d13ecdbd (empty) description 1 bookmark2@origin: rlzusymt 8476341e (empty) description 2 bookmark3: znkkpsqq 1aa4f1f2 (empty) moved bookmark2 "###); + } // Now, no bookmarks are tracked. --tracked does not push anything let (_stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--tracked"]); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Nothing changed. "###); + } // All bookmarks are still untracked. // - --all tries to push bookmark1, but fails because a bookmark with the same @@ -1351,17 +1730,23 @@ fn test_git_push_tracked_vs_all() { // - We could consider showing some hint on `jj bookmark untrack // bookmark2@origin` instead of showing an error here. let (_stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--all"]); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Warning: Non-tracking remote bookmark bookmark1@origin exists Hint: Run `jj bookmark track bookmark1@origin` to import the remote bookmark. Changes to push to origin: Add bookmark bookmark3 to 1aa4f1f2ef7f "#); + } } -#[test] -fn test_git_push_moved_forward_untracked() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_push_moved_forward_untracked(subprocess: bool) { let (test_env, workspace_root) = set_up(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.jj_cmd_ok(&workspace_root, &["new", "bookmark1", "-mmoved bookmark1"]); test_env.jj_cmd_ok(&workspace_root, &["bookmark", "set", "bookmark1"]); @@ -1370,16 +1755,22 @@ fn test_git_push_moved_forward_untracked() { &["bookmark", "untrack", "bookmark1@origin"], ); let (_stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--allow-new"]); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Warning: Non-tracking remote bookmark bookmark1@origin exists Hint: Run `jj bookmark track bookmark1@origin` to import the remote bookmark. Nothing changed. "###); + } } -#[test] -fn test_git_push_moved_sideways_untracked() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_push_moved_sideways_untracked(subprocess: bool) { let (test_env, workspace_root) = set_up(); + if subprocess { + test_env.set_up_git_subprocessing(); + } test_env.jj_cmd_ok(&workspace_root, &["new", "root()", "-mmoved bookmark1"]); test_env.jj_cmd_ok( @@ -1391,16 +1782,22 @@ fn test_git_push_moved_sideways_untracked() { &["bookmark", "untrack", "bookmark1@origin"], ); let (_stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--allow-new"]); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" Warning: Non-tracking remote bookmark bookmark1@origin exists Hint: Run `jj bookmark track bookmark1@origin` to import the remote bookmark. Nothing changed. "###); + } } -#[test] -fn test_git_push_to_remote_named_git() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_push_to_remote_named_git(subprocess: bool) { let (test_env, workspace_root) = set_up(); + if subprocess { + test_env.set_up_git_subprocessing(); + } let git_repo = { let mut git_repo_path = workspace_root.clone(); git_repo_path.extend([".jj", "repo", "store", "git"]); @@ -1410,12 +1807,14 @@ fn test_git_push_to_remote_named_git() { let stderr = test_env.jj_cmd_failure(&workspace_root, &["git", "push", "--all", "--remote=git"]); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Changes to push to git: Add bookmark bookmark1 to d13ecdbda2a2 Add bookmark bookmark2 to 8476341eb395 Error: Git remote named 'git' is reserved for local Git repository "#); + } } #[test] diff --git a/docs/config.md b/docs/config.md index c139bd25f0..8be03491d9 100644 --- a/docs/config.md +++ b/docs/config.md @@ -1210,6 +1210,28 @@ from the private set. Private commits prevent their descendants from being pushed, since doing so would require pushing the private commit as well. +### Git subprocessing behaviour + +By default, Git remote interactions are handled by [`libgit2`](https://github.com/libgit2/libgit2). +This sometimes causes [SSH problems](https://github.com/jj-vcs/jj/issues/4979) that +cannot be solved by `jj` directly. + +To sidestep this, there is an option to spawn a `git` subprocess to handle those +remote interactions: + +```toml +[git] +subprocess = true +``` + +Additionally, if `git` is not on your OS path, or you want to specify a +particular binary, you can: + +```toml +[git] +executable-path = "/path/to/git" +``` + ## Filesystem monitor In large repositories, it may be beneficial to use a "filesystem monitor" to diff --git a/flake.nix b/flake.nix index 68a36b047f..b2fb22cb8a 100644 --- a/flake.nix +++ b/flake.nix @@ -76,6 +76,9 @@ # for signing tests gnupg openssh + + # for git subprocess test + git ]; env = { @@ -111,6 +114,7 @@ RUSTFLAGS = pkgs.lib.optionalString pkgs.stdenv.isLinux "-C link-arg=-fuse-ld=mold"; NIX_JJ_GIT_HASH = self.rev or ""; CARGO_INCREMENTAL = "0"; + TEST_GIT_EXECUTABLE_PATH = pkgs.lib.getExe pkgs.git; }; postInstall = '' diff --git a/lib/src/config/misc.toml b/lib/src/config/misc.toml index b9801bdb8a..6158ebd33c 100644 --- a/lib/src/config/misc.toml +++ b/lib/src/config/misc.toml @@ -12,6 +12,8 @@ register_snapshot_trigger = false [git] abandon-unreachable-commits = true auto-local-bookmark = false +subprocess = false +executable-path = "git" [operation] hostname = "" diff --git a/lib/src/git.rs b/lib/src/git.rs index 7261991334..3f3bb52851 100644 --- a/lib/src/git.rs +++ b/lib/src/git.rs @@ -22,6 +22,7 @@ use std::default::Default; use std::fmt; use std::io::Read; use std::num::NonZeroU32; +use std::path::Path; use std::path::PathBuf; use std::str; @@ -36,6 +37,8 @@ use crate::backend::CommitId; use crate::backend::TreeValue; use crate::commit::Commit; use crate::git_backend::GitBackend; +use crate::git_subprocess::GitSubprocessContext; +use crate::git_subprocess::GitSubprocessError; use crate::index::Index; use crate::merged_tree::MergedTree; use crate::object_id::ObjectId; @@ -131,6 +134,38 @@ impl RefSpec { } } +/// Helper struct that matches a refspec with its expected location in the +/// remote it's being pushed to +pub(crate) struct RefToPush<'a> { + pub(crate) refspec: &'a RefSpec, + pub(crate) expected_location: Option<&'a CommitId>, +} + +impl<'a> RefToPush<'a> { + fn new(refspec: &'a RefSpec, expected_locations: &'a HashMap<&str, Option<&CommitId>>) -> Self { + let expected_location = *expected_locations.get(refspec.destination.as_str()).expect( + "The refspecs and the expected locations were both constructed from the same source \ + of truth. This means the lookup should always work.", + ); + + RefToPush { + refspec, + expected_location, + } + } + + pub(crate) fn to_git_lease(&self) -> String { + format!( + "{}:{}", + self.refspec.destination, + self.expected_location + .map(|x| x.to_string()) + .as_deref() + .unwrap_or("") + ) + } +} + pub fn parse_git_ref(ref_name: &str) -> Option { if let Some(branch_name) = ref_name.strip_prefix("refs/heads/") { // Git CLI says 'HEAD' is not a valid branch name @@ -1427,9 +1462,11 @@ pub enum GitFetchError { // TODO: I'm sure there are other errors possible, such as transport-level errors. #[error("Unexpected git error when fetching")] InternalGitError(#[from] git2::Error), + #[error(transparent)] + Subprocess(#[from] GitSubprocessError), } -fn fetch_options( +fn git2_fetch_options( callbacks: RemoteCallbacks<'_>, depth: Option, ) -> git2::FetchOptions<'_> { @@ -1455,8 +1492,11 @@ struct GitFetch<'a> { mut_repo: &'a mut MutableRepo, git_repo: &'a git2::Repository, git_settings: &'a GitSettings, + // for git2 only fetch_options: git2::FetchOptions<'a>, fetched: Vec, + // for subprocess only + depth: Option, } impl<'a> GitFetch<'a> { @@ -1465,6 +1505,7 @@ impl<'a> GitFetch<'a> { git_repo: &'a git2::Repository, git_settings: &'a GitSettings, fetch_options: git2::FetchOptions<'a>, + depth: Option, ) -> Self { GitFetch { mut_repo, @@ -1472,29 +1513,15 @@ impl<'a> GitFetch<'a> { git_settings, fetch_options, fetched: vec![], + depth, } } - /// Perform a `git fetch` on the local git repo, updating the - /// remote-tracking branches in the git repo. - /// - /// Keeps track of the {branch_names, remote_name} pair the refs can be - /// subsequently imported into the `jj` repo by calling `import_refs()`. - fn fetch( - &mut self, - branch_names: &[StringPattern], + fn expand_refspecs( remote_name: &str, - ) -> Result, GitFetchError> { - let mut remote = self.git_repo.find_remote(remote_name).map_err(|err| { - if is_remote_not_found_err(&err) { - GitFetchError::NoSuchRemote(remote_name.to_string()) - } else { - GitFetchError::InternalGitError(err) - } - })?; - // At this point, we are only updating Git's remote tracking branches, not the - // local branches. - let refspecs: Vec<_> = branch_names + branch_names: &[StringPattern], + ) -> Result, GitFetchError> { + branch_names .iter() .map(|pattern| { pattern @@ -1510,10 +1537,30 @@ impl<'a> GitFetch<'a> { format!("refs/remotes/{remote_name}/{glob}"), ) }) + .ok_or(GitFetchError::InvalidBranchPattern) }) - .map(|refspec| refspec.map(|r| r.to_git_format())) - .collect::>() - .ok_or(GitFetchError::InvalidBranchPattern)?; + .collect() + } + + fn git2_fetch( + &mut self, + branch_names: &[StringPattern], + remote_name: &str, + ) -> Result, GitFetchError> { + let mut remote = self.git_repo.find_remote(remote_name).map_err(|err| { + if is_remote_not_found_err(&err) { + GitFetchError::NoSuchRemote(remote_name.to_string()) + } else { + GitFetchError::InternalGitError(err) + } + })?; + // At this point, we are only updating Git's remote tracking branches, not the + // local branches. + let refspecs: Vec = Self::expand_refspecs(remote_name, branch_names)? + .iter() + .map(|refspec| refspec.to_git_format()) + .collect(); + if refspecs.is_empty() { // Don't fall back to the base refspecs. return Ok(None); @@ -1531,11 +1578,6 @@ impl<'a> GitFetch<'a> { None, )?; - self.fetched.push(FetchedBranches { - branches: branch_names.to_vec(), - remote: remote_name.to_string(), - }); - // TODO: We could make it optional to get the default branch since we only care // about it on clone. let mut default_branch = None; @@ -1554,6 +1596,85 @@ impl<'a> GitFetch<'a> { Ok(default_branch) } + fn subprocess_fetch( + &mut self, + branch_names: &[StringPattern], + remote_name: &str, + ) -> Result, GitFetchError> { + // check the remote exists + // TODO: we should ideally find a way to do this without git2 + let _remote = self.git_repo.find_remote(remote_name).map_err(|err| { + if is_remote_not_found_err(&err) { + GitFetchError::NoSuchRemote(remote_name.to_string()) + } else { + GitFetchError::InternalGitError(err) + } + })?; + // At this point, we are only updating Git's remote tracking branches, not the + // local branches. + let mut remaining_refspecs: Vec<_> = Self::expand_refspecs(remote_name, branch_names)?; + if remaining_refspecs.is_empty() { + // Don't fall back to the base refspecs. + return Ok(None); + } + + let git_ctx = + GitSubprocessContext::from_git2(self.git_repo, &self.git_settings.executable_path); + + let mut branches_to_prune = Vec::new(); + // git unfortunately errors out if one of the many refspecs is not found + // + // our approach is to filter out failures and retry, + // until either all have failed or an attempt has succeeded + // + // even more unfortunately, git errors out one refspec at a time, + // meaning that the below cycle runs in O(#failed refspecs) + while let Some(failing_refspec) = + git_ctx.spawn_fetch(remote_name, self.depth, &remaining_refspecs)? + { + remaining_refspecs.retain(|r| r.source.as_ref() != Some(&failing_refspec)); + + if let Some(branch_name) = failing_refspec.strip_prefix("refs/heads/") { + branches_to_prune.push(format!("{remote_name}/{branch_name}")); + } + } + + // Even if git fetch has --prune, if a branch is not found it will not be + // pruned on fetch + git_ctx.spawn_branch_prune(&branches_to_prune)?; + + // TODO: We could make it optional to get the default branch since we only care + // about it on clone. + let default_branch = git_ctx.spawn_remote_show(remote_name)?; + tracing::debug!(default_branch = default_branch); + + Ok(default_branch) + } + + /// Perform a `git fetch` on the local git repo, updating the + /// remote-tracking branches in the git repo. + /// + /// Keeps track of the {branch_names, remote_name} pair the refs can be + /// subsequently imported into the `jj` repo by calling `import_refs()`. + fn fetch( + &mut self, + branch_names: &[StringPattern], + remote_name: &str, + ) -> Result, GitFetchError> { + let default_branch = if self.git_settings.subprocess { + self.subprocess_fetch(branch_names, remote_name) + } else { + self.git2_fetch(branch_names, remote_name) + }; + + self.fetched.push(FetchedBranches { + branches: branch_names.to_vec(), + remote: remote_name.to_string(), + }); + + default_branch + } + /// Import the previously fetched remote-tracking branches into the jj repo /// and update jj's local branches. We also import local tags since remote /// tags should have been merged by Git. @@ -1600,6 +1721,10 @@ pub struct GitFetchStats { pub import_stats: GitImportStats, } +// FIXME: this function has too many arguments (duplication between spawning a +// git process and git2 parts of the code). It is expected to be removed soon in +// lieu of the new GitFetch API, to be used directly by the CLI, removing the +// problem #[tracing::instrument(skip(mut_repo, git_repo, callbacks))] pub fn fetch( mut_repo: &mut MutableRepo, @@ -1614,7 +1739,12 @@ pub fn fetch( mut_repo, git_repo, git_settings, - fetch_options(callbacks, depth), + if git_settings.subprocess { + git2::FetchOptions::default() + } else { + git2_fetch_options(callbacks, depth) + }, + depth, ); let default_branch = git_fetch.fetch(branch_names, remote_name)?; let import_stats = git_fetch.import_refs()?; @@ -1642,6 +1772,8 @@ pub enum GitPushError { // and errors caused by the remote rejecting the push. #[error("Unexpected git error when pushing")] InternalGitError(#[from] git2::Error), + #[error(transparent)] + Subprocess(#[from] GitSubprocessError), } #[derive(Clone, Debug)] @@ -1663,6 +1795,7 @@ pub struct GitRefUpdate { pub fn push_branches( mut_repo: &mut MutableRepo, git_repo: &git2::Repository, + git_settings: &GitSettings, remote_name: &str, targets: &GitBranchPushTargets, callbacks: RemoteCallbacks<'_>, @@ -1676,7 +1809,14 @@ pub fn push_branches( new_target: update.new_target.clone(), }) .collect_vec(); - push_updates(mut_repo, git_repo, remote_name, &ref_updates, callbacks)?; + push_updates( + mut_repo, + git_repo, + git_settings, + remote_name, + &ref_updates, + callbacks, + )?; // TODO: add support for partially pushed refs? we could update the view // excluding rejected refs, but the transaction would be aborted anyway @@ -1698,10 +1838,15 @@ pub fn push_branches( pub fn push_updates( repo: &dyn Repo, git_repo: &git2::Repository, + git_settings: &GitSettings, remote_name: &str, updates: &[GitRefUpdate], callbacks: RemoteCallbacks<'_>, ) -> Result<(), GitPushError> { + if remote_name == REMOTE_NAME_FOR_LOCAL_GIT_REPO { + return Err(GitPushError::RemoteReservedForLocalGitRepo); + } + let mut qualified_remote_refs_expected_locations = HashMap::new(); let mut refspecs = vec![]; for update in updates { @@ -1723,18 +1868,29 @@ pub fn push_updates( } // TODO(ilyagr): `push_refs`, or parts of it, should probably be inlined. This // requires adjusting some tests. - let refspecs: Vec = refspecs.iter().map(RefSpec::to_git_format).collect(); - push_refs( - repo, - git_repo, - remote_name, - &qualified_remote_refs_expected_locations, - &refspecs, - callbacks, - ) + + if git_settings.subprocess { + subprocess_push_refs( + git_repo, + &git_settings.executable_path, + remote_name, + &qualified_remote_refs_expected_locations, + &refspecs, + ) + } else { + let refspecs: Vec = refspecs.iter().map(RefSpec::to_git_format).collect(); + git2_push_refs( + repo, + git_repo, + remote_name, + &qualified_remote_refs_expected_locations, + &refspecs, + callbacks, + ) + } } -fn push_refs( +fn git2_push_refs( repo: &dyn Repo, git_repo: &git2::Repository, remote_name: &str, @@ -1742,9 +1898,6 @@ fn push_refs( refspecs: &[String], callbacks: RemoteCallbacks<'_>, ) -> Result<(), GitPushError> { - if remote_name == REMOTE_NAME_FOR_LOCAL_GIT_REPO { - return Err(GitPushError::RemoteReservedForLocalGitRepo); - } let mut remote = git_repo.find_remote(remote_name).map_err(|err| { if is_remote_not_found_err(&err) { GitPushError::NoSuchRemote(remote_name.to_string()) @@ -1812,7 +1965,6 @@ fn push_refs( unexpectedly at {actual_remote_location:?} on the server as opposed \ to the expected {expected_remote_location:?}", ); - failed_push_negotiations.push(dst_refname.to_string()); } } @@ -1850,6 +2002,9 @@ fn push_refs( if remaining_remote_refs.is_empty() { Ok(()) } else { + // TODO: this is probably dead code right now + // The only way this would happen is if a push fails from some + // other reason other than a lease failing (which our tests don't cover) Err(GitPushError::RefUpdateRejected( remaining_remote_refs .iter() @@ -1861,6 +2016,62 @@ fn push_refs( } } +fn subprocess_push_refs( + git_repo: &git2::Repository, + git_executable_path: &Path, + remote_name: &str, + qualified_remote_refs_expected_locations: &HashMap<&str, Option<&CommitId>>, + refspecs: &[RefSpec], +) -> Result<(), GitPushError> { + // check the remote exists + // TODO: we should ideally find a way to do this without git2 + let _remote = git_repo.find_remote(remote_name).map_err(|err| { + if is_remote_not_found_err(&err) { + GitPushError::NoSuchRemote(remote_name.to_string()) + } else { + GitPushError::InternalGitError(err) + } + })?; + let git_ctx = GitSubprocessContext::from_git2(git_repo, git_executable_path); + + let mut remaining_remote_refs: HashSet<_> = qualified_remote_refs_expected_locations + .keys() + .copied() + .collect(); + + let refs_to_push: Vec = refspecs + .iter() + .map(|full_refspec| RefToPush::new(full_refspec, qualified_remote_refs_expected_locations)) + .collect(); + + let (failed_ref_matches, successful_pushes) = git_ctx.spawn_push(remote_name, &refs_to_push)?; + + for remote_ref in successful_pushes { + remaining_remote_refs.remove(remote_ref.as_str()); + } + + if !failed_ref_matches.is_empty() { + let mut refs_in_unexpected_locations = failed_ref_matches; + refs_in_unexpected_locations.sort(); + Err(GitPushError::RefInUnexpectedLocation( + refs_in_unexpected_locations, + )) + } else if remaining_remote_refs.is_empty() { + Ok(()) + } else { + // TODO: this is probably dead code right now + // The only way this would happen is if a push fails from some + // other reason other than a lease failing (which our tests don't cover) + Err(GitPushError::RefUpdateRejected( + remaining_remote_refs + .iter() + .sorted() + .map(|name| name.to_string()) + .collect(), + )) + } +} + #[derive(Debug, Clone, PartialEq, Eq)] enum PushAllowReason { NormalMatch, diff --git a/lib/src/git_subprocess.rs b/lib/src/git_subprocess.rs new file mode 100644 index 0000000000..66f4571a96 --- /dev/null +++ b/lib/src/git_subprocess.rs @@ -0,0 +1,603 @@ +// Copyright 2025 The Jujutsu Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +use std::num::NonZeroU32; +use std::path::Path; +use std::path::PathBuf; +use std::process::Command; +use std::process::Output; +use std::process::Stdio; + +use bstr::ByteSlice; +use thiserror::Error; + +use crate::git::RefSpec; +use crate::git::RefToPush; + +#[derive(Error, Debug)] +pub enum SplitGitDirError { + #[error("The Git dir should be an absolute path")] + RelativePath, + + #[error("The last directory is neither 'git' or '.git' dir")] + MissingGitDir, +} + +#[derive(Error, Debug)] +pub enum GitSubprocessError { + #[error("Could not find repository at '{0}'")] + NoSuchRepository(String), + #[error("Could not execute the git process, found in the OS path '{path}'")] + SpawnInPath { + path: PathBuf, + #[source] + error: std::io::Error, + }, + #[error("Could not execute git process at specified path '{path}'")] + Spawn { + path: PathBuf, + #[source] + error: std::io::Error, + }, + #[error("Failed to wait for the git process")] + Wait(std::io::Error), + #[error("Git process failed: {0}")] + External(String), + #[error("Git dir `{path}` is malformed: could not extract workspace root from it")] + MalformedGitDir { + path: PathBuf, + #[source] + error: SplitGitDirError, + }, +} + +/// Context for creating git subprocesses +pub(crate) struct GitSubprocessContext<'a> { + git_dir: PathBuf, + git_executable_path: &'a Path, +} + +impl<'a> GitSubprocessContext<'a> { + pub(crate) fn new(git_dir: impl Into, git_executable_path: &'a Path) -> Self { + GitSubprocessContext { + git_dir: git_dir.into(), + git_executable_path, + } + } + + pub(crate) fn from_git2(git_repo: &git2::Repository, git_executable_path: &'a Path) -> Self { + Self::new(git_repo.path(), git_executable_path) + } + + /// Create the Git command + fn create_command(&self) -> Command { + let mut git_cmd = Command::new(self.git_executable_path); + // TODO: here we are passing the full path to the git_dir, which can lead to UNC + // bugs in Windows. The ideal way to do this is to pass the workspace + // root to Command::current_dir and then pass a relative path to the git + // dir + git_cmd + .arg("--bare") + .arg("--git-dir") + .arg(&self.git_dir) + .stdin(Stdio::null()) + .stderr(Stdio::piped()); + + git_cmd + } + + /// Spawn the git command + fn spawn_cmd(&self, mut git_cmd: Command) -> Result { + tracing::debug!(cmd = ?git_cmd, "spawning a git subprocess"); + let child_git = git_cmd.spawn().map_err(|error| { + if self.git_executable_path.is_absolute() { + GitSubprocessError::Spawn { + path: self.git_executable_path.to_path_buf(), + error, + } + } else { + GitSubprocessError::SpawnInPath { + path: self.git_executable_path.to_path_buf(), + error, + } + } + })?; + + child_git + .wait_with_output() + .map_err(GitSubprocessError::Wait) + } + + /// Perform a git fetch + /// + /// This returns a fully qualified ref that wasn't fetched successfully + /// Note that git only returns one failed ref at a time + pub(crate) fn spawn_fetch( + &self, + remote_name: &str, + depth: Option, + refspecs: &[RefSpec], + ) -> Result, GitSubprocessError> { + if refspecs.is_empty() { + return Ok(None); + } + let mut command = self.create_command(); + command.stdout(Stdio::piped()); + command + .arg("fetch") + // attempt to prune stale refs + .arg("--prune"); + if let Some(d) = depth { + command.arg(format!("--depth={d}")); + } + command.arg("--").arg(remote_name); + command.args(refspecs.iter().map(|x| x.to_git_format())); + + let output = self.spawn_cmd(command)?; + + parse_git_fetch_output(output) + } + + /// Prune particular branches + pub(crate) fn spawn_branch_prune( + &self, + branches_to_prune: &[String], + ) -> Result<(), GitSubprocessError> { + if branches_to_prune.is_empty() { + return Ok(()); + } + let mut command = self.create_command(); + command.stdout(Stdio::null()); + command.args(["branch", "--remotes", "--delete", "--"]); + command.args(branches_to_prune); + + let output = self.spawn_cmd(command)?; + + // we name the type to make sure that it is not meant to be used + let () = parse_git_branch_prune_output(output)?; + + Ok(()) + } + + /// How we retrieve the remote's default branch: + /// + /// `git remote show ` + /// + /// dumps a lot of information about the remote, with a line such as: + /// ` HEAD branch: ` + pub(crate) fn spawn_remote_show( + &self, + remote_name: &str, + ) -> Result, GitSubprocessError> { + let mut command = self.create_command(); + command.stdout(Stdio::piped()); + command.args(["remote", "show", "--", remote_name]); + let output = self.spawn_cmd(command)?; + + let output = parse_git_remote_show_output(output)?; + + // find the HEAD branch line in the output + parse_git_remote_show_default_branch(&output.stdout) + } + + /// Push references to git + /// + /// All pushes are forced, using --force-with-lease to perform a test&set + /// operation on the remote repository + /// + /// Return tuple with + /// 1. refs that failed to push + /// 2. refs that succeeded to push + pub(crate) fn spawn_push( + &self, + remote_name: &str, + references: &[RefToPush], + ) -> Result<(Vec, Vec), GitSubprocessError> { + let mut command = self.create_command(); + command.stdout(Stdio::piped()); + command.args(["push", "--porcelain"]); + command.args( + references + .iter() + .map(|reference| format!("--force-with-lease={}", reference.to_git_lease())), + ); + command.args(["--", remote_name]); + // with --force-with-lease we cannot have the forced refspec, + // as it ignores the lease + command.args( + references + .iter() + .map(|r| r.refspec.to_git_format_not_forced()), + ); + + let output = self.spawn_cmd(command)?; + + parse_git_push_output(output) + } +} + +/// Generate a GitSubprocessError::ExternalGitError if the stderr output was not +/// recognizable +fn external_git_error(stderr: &[u8]) -> GitSubprocessError { + GitSubprocessError::External(format!( + "External git program failed:\n{}", + stderr.to_str_lossy() + )) +} + +/// Parse no such remote errors output from git +/// +/// Returns the remote that wasn't found +/// +/// To say this, git prints out a lot of things, but the first line is of the +/// form: +/// `fatal: '' does not appear to be a git repository` +/// or +/// `fatal: '': Could not resolve host: invalid-remote +fn parse_no_such_remote(stderr: &[u8]) -> Option { + let first_line = stderr.lines().next()?; + let suffix = first_line + .strip_prefix(b"fatal: '") + .or(first_line.strip_prefix(b"fatal: unable to access '"))?; + + suffix + .strip_suffix(b"' does not appear to be a git repository") + .or(suffix.strip_suffix(b"': Could not resolve host: invalid-remote")) + .map(|remote| remote.to_str_lossy().into_owned()) +} + +/// Parse error from refspec not present on the remote +/// +/// This returns +/// Some(local_ref) that wasn't found by the remote +/// None if this wasn't the error +/// +/// On git fetch even though --prune is specified, if a particular +/// refspec is asked for but not present in the remote, git will error out. +/// +/// Git only reports one of these errors at a time, so we only look at the first +/// line +/// +/// The first line is of the form: +/// `fatal: couldn't find remote ref refs/heads/` +fn parse_no_remote_ref(stderr: &[u8]) -> Option { + let first_line = stderr.lines().next()?; + first_line + .strip_prefix(b"fatal: couldn't find remote ref ") + .map(|refname| refname.to_str_lossy().into_owned()) +} + +/// Parse remote tracking branch not found +/// +/// This returns true if the error was detected +/// +/// if a branch is asked for but is not present, jj will detect it post-hoc +/// so, we want to ignore these particular errors with git +/// +/// The first line is of the form: +/// `error: remote-tracking branch '' not found` +fn parse_no_remote_tracking_branch(stderr: &[u8]) -> Option { + let first_line = stderr.lines().next()?; + + let suffix = first_line.strip_prefix(b"error: remote-tracking branch '")?; + + suffix + .strip_suffix(b"' not found.") + .or(suffix.strip_suffix(b"' not found")) + .map(|branch| branch.to_str_lossy().into_owned()) +} + +// return the fully qualified ref that failed to fetch +// +// note that git fetch only returns one error at a time +fn parse_git_fetch_output(output: Output) -> Result, GitSubprocessError> { + if output.status.success() { + return Ok(None); + } + + // There are some git errors we want to parse out + if let Some(remote) = parse_no_such_remote(&output.stderr) { + return Err(GitSubprocessError::NoSuchRepository(remote)); + } + + if let Some(refspec) = parse_no_remote_ref(&output.stderr) { + return Ok(Some(refspec)); + } + + if parse_no_remote_tracking_branch(&output.stderr).is_some() { + return Ok(None); + } + + Err(external_git_error(&output.stderr)) +} + +fn parse_git_branch_prune_output(output: Output) -> Result<(), GitSubprocessError> { + if output.status.success() { + return Ok(()); + } + + // There are some git errors we want to parse out + if parse_no_remote_tracking_branch(&output.stderr).is_some() { + return Ok(()); + } + + Err(external_git_error(&output.stderr)) +} + +fn parse_git_remote_show_output(output: Output) -> Result { + if output.status.success() { + return Ok(output); + } + + // There are some git errors we want to parse out + if let Some(remote) = parse_no_such_remote(&output.stderr) { + return Err(GitSubprocessError::NoSuchRepository(remote)); + } + + Err(external_git_error(&output.stderr)) +} + +fn parse_git_remote_show_default_branch( + stdout: &[u8], +) -> Result, GitSubprocessError> { + Ok(stdout + .to_str() + .map_err(|e| { + GitSubprocessError::External(format!("git remote output is not utf-8: {e:?}")) + })? + .lines() + .map(|x| x.trim()) + .find(|x| x.starts_with("HEAD branch:")) + .and_then(|x| x.split(" ").last().map(|y| y.trim().to_string())) + .filter(|branch_name| branch_name != "(unknown)")) +} + +// git-push porcelain has the following format (per line) +// `\t:\t\t()` +// +// is one of: +// ' ' for a successfully pushed fast-forward; +// + for a successful forced update +// - for a successfully deleted ref +// * for a successfully pushed new ref +// ! for a ref that was rejected or failed to push; and +// = for a ref that was up to date and did not need pushing. +// +// : is the refspec +// +// is extra info (commit ranges or reason for rejected) +// at times the summary is omitted +// +// is a human-readable explanation +fn parse_ref_pushes(stdout: &[u8]) -> Result<(Vec, Vec), GitSubprocessError> { + if !stdout.starts_with(b"To ") { + return Err(GitSubprocessError::External(format!( + "Git push output unfamiliar:\n{}", + stdout.to_str_lossy() + ))); + } + + let mut pushed_refs = Vec::new(); + let mut rejected_refs = Vec::new(); + for (idx, line) in stdout + .lines() + .skip(1) + .take_while(|line| line != b"Done") + .enumerate() + { + // format: + // \t\t\t + // sometimes the summary is omitted + let create_error = || { + GitSubprocessError::External(format!( + "Line #{idx} of git-push has unknown format: {}", + line.to_str_lossy() + )) + }; + let mut it = line.split_str("\t"); + let flag = it.next().ok_or_else(create_error)?; + let reference = it.next().ok_or_else(create_error)?; + // we capture the remaining elements to ensure the line is well formed + let _summary_or_comment = it.next().ok_or_else(create_error)?; + let _comment_opt = it.next(); + if it.next().is_some() { + return Err(create_error()); + } + + let full_refspec = reference + .to_str() + .map_err(|e| { + format!( + "Line #{} of git-push has non-utf8 refspec {}: {:?}", + idx, + reference.to_str_lossy(), + e + ) + }) + .map_err(GitSubprocessError::External)?; + + let reference = if let Some(reference) = full_refspec.split(":").last() { + Ok(reference.to_string()) + } else { + Err(GitSubprocessError::External(format!( + "Line #{idx} of git-push has full refspec without named ref: {full_refspec}" + ))) + }?; + + match flag { + // ' ' for a successfully pushed fast-forward; + // + for a successful forced update + // - for a successfully deleted ref + // * for a successfully pushed new ref + // = for a ref that was up to date and did not need pushing. + b"+" | b"-" | b"*" | b"=" | b" " => { + pushed_refs.push(reference); + } + // ! for a ref that was rejected or failed to push; and + b"!" => { + rejected_refs.push(reference); + } + unknown => { + return Err(GitSubprocessError::External(format!( + "Line #{} of git-push starts with an unknown flag '{}': '{}'", + idx, + String::from_utf8_lossy(unknown), + String::from_utf8_lossy(line), + ))); + } + } + } + + Ok((rejected_refs, pushed_refs)) +} + +// on Ok, return a tuple with +// 1. list of failed references from test and set +// 2. list of successful references pushed +fn parse_git_push_output(output: Output) -> Result<(Vec, Vec), GitSubprocessError> { + if output.status.success() { + // TODO: figure out how to report `remote: ` logging nicely + // In sum, git apparently supports the remote repo sending a message to the + // local, which we should probably forward nicely + // + // If we support this, we can be a bit more strict, and say that messages other + // than these constitute an error + let ref_pushes = parse_ref_pushes(&output.stdout)?; + return Ok(ref_pushes); + } + + if let Some(remote) = parse_no_such_remote(&output.stderr) { + return Err(GitSubprocessError::NoSuchRepository(remote)); + } + + if output + .stderr + .starts_with_str("error: failed to push some refs to ") + { + parse_ref_pushes(&output.stdout) + } else { + Err(external_git_error(&output.stderr)) + } +} + +#[cfg(test)] +mod test { + use super::*; + + const SAMPLE_NO_SUCH_REPOSITORY_ERROR: &[u8] = + br###"fatal: unable to access 'origin': Could not resolve host: invalid-remote +fatal: Could not read from remote repository. + +Please make sure you have the correct access rights +and the repository exists. "###; + const SAMPLE_NO_SUCH_REMOTE_ERROR: &[u8] = + br###"fatal: 'origin' does not appear to be a git repository +fatal: Could not read from remote repository. + +Please make sure you have the correct access rights +and the repository exists. "###; + const SAMPLE_NO_REMOTE_REF_ERROR: &[u8] = b"fatal: couldn't find remote ref refs/heads/noexist"; + const SAMPLE_NO_REMOTE_TRACKING_BRANCH_ERROR: &[u8] = + b"error: remote-tracking branch 'bookmark' not found"; + const SAMPLE_PUSH_REFS_ERROR: &[u8] = b"To origin +*\tdeadbeef:refs/heads/bookmark1\tdeadbeef\t[new branch] ++\tdeadbeef:refs/heads/bookmark2\tdeadbeef\t[new branch] +-\tdeadbeef:refs/heads/bookmark3\tdeadbeef\t[new branch] + \tdeadbeef:refs/heads/bookmark4\tdeadbeef\t[new branch] +=\tdeadbeef:refs/heads/bookmark5\tdeadbeef\t[new branch] +!\tdeadbeef:refs/heads/bookmark6\tdeadbeef\t[new branch] +Done"; + const SAMPLE_OK_STDERR: &[u8] = b""; + + #[test] + fn test_parse_no_such_remote() { + assert_eq!( + parse_no_such_remote(SAMPLE_NO_SUCH_REPOSITORY_ERROR), + Some("origin".to_string()) + ); + assert_eq!( + parse_no_such_remote(SAMPLE_NO_SUCH_REMOTE_ERROR), + Some("origin".to_string()) + ); + assert_eq!(parse_no_such_remote(SAMPLE_NO_REMOTE_REF_ERROR), None); + assert_eq!( + parse_no_such_remote(SAMPLE_NO_REMOTE_TRACKING_BRANCH_ERROR), + None + ); + assert_eq!(parse_no_such_remote(SAMPLE_PUSH_REFS_ERROR), None); + assert_eq!(parse_no_such_remote(SAMPLE_OK_STDERR), None); + } + + #[test] + fn test_parse_no_remote_ref() { + assert_eq!(parse_no_remote_ref(SAMPLE_NO_SUCH_REPOSITORY_ERROR), None); + assert_eq!(parse_no_remote_ref(SAMPLE_NO_SUCH_REMOTE_ERROR), None); + assert_eq!( + parse_no_remote_ref(SAMPLE_NO_REMOTE_REF_ERROR), + Some("refs/heads/noexist".to_string()) + ); + assert_eq!( + parse_no_remote_ref(SAMPLE_NO_REMOTE_TRACKING_BRANCH_ERROR), + None + ); + assert_eq!(parse_no_remote_ref(SAMPLE_PUSH_REFS_ERROR), None); + assert_eq!(parse_no_remote_ref(SAMPLE_OK_STDERR), None); + } + + #[test] + fn test_parse_no_remote_tracking_branch() { + assert_eq!( + parse_no_remote_tracking_branch(SAMPLE_NO_SUCH_REPOSITORY_ERROR), + None + ); + assert_eq!( + parse_no_remote_tracking_branch(SAMPLE_NO_SUCH_REMOTE_ERROR), + None + ); + assert_eq!( + parse_no_remote_tracking_branch(SAMPLE_NO_REMOTE_REF_ERROR), + None + ); + assert_eq!( + parse_no_remote_tracking_branch(SAMPLE_NO_REMOTE_TRACKING_BRANCH_ERROR), + Some("bookmark".to_string()) + ); + assert_eq!( + parse_no_remote_tracking_branch(SAMPLE_PUSH_REFS_ERROR), + None + ); + assert_eq!(parse_no_remote_tracking_branch(SAMPLE_OK_STDERR), None); + } + + #[test] + fn test_parse_ref_pushes() { + assert!(parse_ref_pushes(SAMPLE_NO_SUCH_REPOSITORY_ERROR).is_err()); + assert!(parse_ref_pushes(SAMPLE_NO_SUCH_REMOTE_ERROR).is_err()); + assert!(parse_ref_pushes(SAMPLE_NO_REMOTE_REF_ERROR).is_err()); + assert!(parse_ref_pushes(SAMPLE_NO_REMOTE_TRACKING_BRANCH_ERROR).is_err()); + let (failed, success) = parse_ref_pushes(SAMPLE_PUSH_REFS_ERROR).unwrap(); + assert_eq!(failed, vec!["refs/heads/bookmark6".to_string()]); + assert_eq!( + success, + vec![ + "refs/heads/bookmark1".to_string(), + "refs/heads/bookmark2".to_string(), + "refs/heads/bookmark3".to_string(), + "refs/heads/bookmark4".to_string(), + "refs/heads/bookmark5".to_string(), + ] + ); + assert!(parse_ref_pushes(SAMPLE_OK_STDERR).is_err()); + } +} diff --git a/lib/src/lib.rs b/lib/src/lib.rs index c9f64868db..95c801ca2f 100644 --- a/lib/src/lib.rs +++ b/lib/src/lib.rs @@ -65,6 +65,8 @@ pub mod git { } #[cfg(feature = "git")] pub mod git_backend; +#[cfg(feature = "git")] +mod git_subprocess; pub mod gitignore; pub mod gpg_signing; pub mod graph; diff --git a/lib/src/settings.rs b/lib/src/settings.rs index f10d24ef16..f361baf7c4 100644 --- a/lib/src/settings.rs +++ b/lib/src/settings.rs @@ -14,6 +14,7 @@ #![allow(missing_docs)] +use std::path::PathBuf; use std::str::FromStr; use std::sync::Arc; use std::sync::Mutex; @@ -58,6 +59,8 @@ struct UserSettingsData { pub struct GitSettings { pub auto_local_bookmark: bool, pub abandon_unreachable_commits: bool, + pub subprocess: bool, + pub executable_path: PathBuf, } impl GitSettings { @@ -65,6 +68,8 @@ impl GitSettings { Ok(GitSettings { auto_local_bookmark: settings.get_bool("git.auto-local-bookmark")?, abandon_unreachable_commits: settings.get_bool("git.abandon-unreachable-commits")?, + subprocess: settings.get_bool("git.subprocess")?, + executable_path: settings.get("git.executable-path")?, }) } } @@ -74,6 +79,8 @@ impl Default for GitSettings { GitSettings { auto_local_bookmark: false, abandon_unreachable_commits: true, + subprocess: false, + executable_path: PathBuf::from("git"), } } } diff --git a/lib/tests/test_git.rs b/lib/tests/test_git.rs index 719e11da23..b5c46ce2a3 100644 --- a/lib/tests/test_git.rs +++ b/lib/tests/test_git.rs @@ -114,6 +114,19 @@ fn get_git_repo(repo: &Arc) -> git2::Repository { get_git_backend(repo).open_git_repo().unwrap() } +fn get_git_settings(subprocess: bool) -> GitSettings { + let executable_path = std::env::var("TEST_GIT_EXECUTABLE_PATH") + .as_ref() + .map(Path::new) + .unwrap_or(Path::new("git")) + .to_owned(); + GitSettings { + subprocess, + executable_path, + ..Default::default() + } +} + #[test] fn test_import_refs() { let git_settings = GitSettings { @@ -2528,10 +2541,11 @@ fn test_init() { assert!(!repo.view().heads().contains(&jj_id(&initial_git_commit))); } -#[test] -fn test_fetch_empty_repo() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_fetch_empty_repo(subprocess: bool) { let test_data = GitRepoData::create(); - let git_settings = GitSettings::default(); + let git_settings = get_git_settings(subprocess); let mut tx = test_data.repo.start_transaction(); let stats = git::fetch( @@ -2551,14 +2565,15 @@ fn test_fetch_empty_repo() { assert_eq!(tx.repo_mut().view().bookmarks().count(), 0); } -#[test] -fn test_fetch_initial_commit_head_is_not_set() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_fetch_initial_commit_head_is_not_set(subprocess: bool) { let test_data = GitRepoData::create(); + let initial_git_commit = empty_git_commit(&test_data.origin_repo, "refs/heads/main", &[]); let git_settings = GitSettings { auto_local_bookmark: true, - ..Default::default() + ..get_git_settings(subprocess) }; - let initial_git_commit = empty_git_commit(&test_data.origin_repo, "refs/heads/main", &[]); let mut tx = test_data.repo.start_transaction(); let stats = git::fetch( @@ -2602,13 +2617,10 @@ fn test_fetch_initial_commit_head_is_not_set() { ); } -#[test] -fn test_fetch_initial_commit_head_is_set() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_fetch_initial_commit_head_is_set(subprocess: bool) { let test_data = GitRepoData::create(); - let git_settings = GitSettings { - auto_local_bookmark: true, - ..Default::default() - }; let initial_git_commit = empty_git_commit(&test_data.origin_repo, "refs/heads/main", &[]); test_data.origin_repo.set_head("refs/heads/main").unwrap(); let new_git_commit = empty_git_commit( @@ -2620,6 +2632,10 @@ fn test_fetch_initial_commit_head_is_set() { .origin_repo .reference("refs/tags/v1.0", new_git_commit.id(), false, "") .unwrap(); + let git_settings = GitSettings { + auto_local_bookmark: true, + ..get_git_settings(subprocess) + }; let mut tx = test_data.repo.start_transaction(); let stats = git::fetch( @@ -2637,14 +2653,15 @@ fn test_fetch_initial_commit_head_is_set() { assert!(stats.import_stats.abandoned_commits.is_empty()); } -#[test] -fn test_fetch_success() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_fetch_success(subprocess: bool) { let mut test_data = GitRepoData::create(); + let initial_git_commit = empty_git_commit(&test_data.origin_repo, "refs/heads/main", &[]); let git_settings = GitSettings { auto_local_bookmark: true, - ..Default::default() + ..get_git_settings(subprocess) }; - let initial_git_commit = empty_git_commit(&test_data.origin_repo, "refs/heads/main", &[]); let mut tx = test_data.repo.start_transaction(); git::fetch( @@ -2719,14 +2736,15 @@ fn test_fetch_success() { ); } -#[test] -fn test_fetch_prune_deleted_ref() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_fetch_prune_deleted_ref(subprocess: bool) { let test_data = GitRepoData::create(); + let commit = empty_git_commit(&test_data.origin_repo, "refs/heads/main", &[]); let git_settings = GitSettings { auto_local_bookmark: true, - ..Default::default() + ..get_git_settings(subprocess) }; - let commit = empty_git_commit(&test_data.origin_repo, "refs/heads/main", &[]); let mut tx = test_data.repo.start_transaction(); git::fetch( @@ -2771,14 +2789,15 @@ fn test_fetch_prune_deleted_ref() { .is_absent()); } -#[test] -fn test_fetch_no_default_branch() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_fetch_no_default_branch(subprocess: bool) { let test_data = GitRepoData::create(); + let initial_git_commit = empty_git_commit(&test_data.origin_repo, "refs/heads/main", &[]); let git_settings = GitSettings { auto_local_bookmark: true, - ..Default::default() + ..get_git_settings(subprocess) }; - let initial_git_commit = empty_git_commit(&test_data.origin_repo, "refs/heads/main", &[]); let mut tx = test_data.repo.start_transaction(); git::fetch( @@ -2819,11 +2838,12 @@ fn test_fetch_no_default_branch() { assert_eq!(stats.default_branch, None); } -#[test] -fn test_fetch_empty_refspecs() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_fetch_empty_refspecs(subprocess: bool) { let test_data = GitRepoData::create(); - let git_settings = GitSettings::default(); empty_git_commit(&test_data.origin_repo, "refs/heads/main", &[]); + let git_settings = get_git_settings(subprocess); // Base refspecs shouldn't be respected let mut tx = test_data.repo.start_transaction(); @@ -2849,11 +2869,12 @@ fn test_fetch_empty_refspecs() { .is_absent()); } -#[test] -fn test_fetch_no_such_remote() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_fetch_no_such_remote(subprocess: bool) { let test_data = GitRepoData::create(); - let git_settings = GitSettings::default(); let mut tx = test_data.repo.start_transaction(); + let git_settings = get_git_settings(subprocess); let result = git::fetch( tx.repo_mut(), &test_data.git_repo, @@ -3045,13 +3066,15 @@ fn set_up_push_repos(settings: &UserSettings, temp_dir: &TempDir) -> PushTestSet } } -#[test] -fn test_push_bookmarks_success() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_push_bookmarks_success(subprocess: bool) { let settings = testutils::user_settings(); let temp_dir = testutils::new_temp_dir(); let mut setup = set_up_push_repos(&settings, &temp_dir); let clone_repo = get_git_repo(&setup.jj_repo); let mut tx = setup.jj_repo.start_transaction(); + let git_settings = get_git_settings(subprocess); let targets = GitBranchPushTargets { branch_updates: vec![( @@ -3065,6 +3088,7 @@ fn test_push_bookmarks_success() { let result = git::push_branches( tx.repo_mut(), &clone_repo, + &git_settings, "origin", &targets, git::RemoteCallbacks::default(), @@ -3110,13 +3134,15 @@ fn test_push_bookmarks_success() { assert!(!tx.repo_mut().has_changes()); } -#[test] -fn test_push_bookmarks_deletion() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_push_bookmarks_deletion(subprocess: bool) { let settings = testutils::user_settings(); let temp_dir = testutils::new_temp_dir(); let mut setup = set_up_push_repos(&settings, &temp_dir); let clone_repo = get_git_repo(&setup.jj_repo); let mut tx = setup.jj_repo.start_transaction(); + let git_settings = get_git_settings(subprocess); let source_repo = git2::Repository::open(&setup.source_repo_dir).unwrap(); // Test the setup @@ -3134,6 +3160,7 @@ fn test_push_bookmarks_deletion() { let result = git::push_branches( tx.repo_mut(), &get_git_repo(&setup.jj_repo), + &git_settings, "origin", &targets, git::RemoteCallbacks::default(), @@ -3162,13 +3189,15 @@ fn test_push_bookmarks_deletion() { assert!(!tx.repo_mut().has_changes()); } -#[test] -fn test_push_bookmarks_mixed_deletion_and_addition() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_push_bookmarks_mixed_deletion_and_addition(subprocess: bool) { let settings = testutils::user_settings(); let temp_dir = testutils::new_temp_dir(); let mut setup = set_up_push_repos(&settings, &temp_dir); let clone_repo = get_git_repo(&setup.jj_repo); let mut tx = setup.jj_repo.start_transaction(); + let git_settings = get_git_settings(subprocess); let targets = GitBranchPushTargets { branch_updates: vec![ @@ -3191,6 +3220,7 @@ fn test_push_bookmarks_mixed_deletion_and_addition() { let result = git::push_branches( tx.repo_mut(), &clone_repo, + &git_settings, "origin", &targets, git::RemoteCallbacks::default(), @@ -3231,12 +3261,14 @@ fn test_push_bookmarks_mixed_deletion_and_addition() { assert!(!tx.repo_mut().has_changes()); } -#[test] -fn test_push_bookmarks_not_fast_forward() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_push_bookmarks_not_fast_forward(subprocess: bool) { let settings = testutils::user_settings(); let temp_dir = testutils::new_temp_dir(); let setup = set_up_push_repos(&settings, &temp_dir); let mut tx = setup.jj_repo.start_transaction(); + let git_settings = get_git_settings(subprocess); let targets = GitBranchPushTargets { branch_updates: vec![( @@ -3250,6 +3282,7 @@ fn test_push_bookmarks_not_fast_forward() { let result = git::push_branches( tx.repo_mut(), &get_git_repo(&setup.jj_repo), + &git_settings, "origin", &targets, git::RemoteCallbacks::default(), @@ -3269,11 +3302,13 @@ fn test_push_bookmarks_not_fast_forward() { // may want to add tests for when a bookmark unexpectedly moved backwards or // unexpectedly does not exist for bookmark deletion. -#[test] -fn test_push_updates_unexpectedly_moved_sideways_on_remote() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_push_updates_unexpectedly_moved_sideways_on_remote(subprocess: bool) { let settings = testutils::user_settings(); let temp_dir = testutils::new_temp_dir(); let setup = set_up_push_repos(&settings, &temp_dir); + let git_settings = get_git_settings(subprocess); // The main bookmark is actually at `main_commit` on the remote. If we expect // it to be at `sideways_commit`, it unexpectedly moved sideways from our @@ -3295,6 +3330,7 @@ fn test_push_updates_unexpectedly_moved_sideways_on_remote() { git::push_updates( setup.jj_repo.as_ref(), &get_git_repo(&setup.jj_repo), + &git_settings, "origin", &targets, git::RemoteCallbacks::default(), @@ -3334,11 +3370,13 @@ fn test_push_updates_unexpectedly_moved_sideways_on_remote() { ); } -#[test] -fn test_push_updates_unexpectedly_moved_forward_on_remote() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_push_updates_unexpectedly_moved_forward_on_remote(subprocess: bool) { let settings = testutils::user_settings(); let temp_dir = testutils::new_temp_dir(); let setup = set_up_push_repos(&settings, &temp_dir); + let git_settings = get_git_settings(subprocess); // The main bookmark is actually at `main_commit` on the remote. If we // expected it to be at `parent_of_commit`, it unexpectedly moved forward @@ -3362,6 +3400,7 @@ fn test_push_updates_unexpectedly_moved_forward_on_remote() { git::push_updates( setup.jj_repo.as_ref(), &get_git_repo(&setup.jj_repo), + &git_settings, "origin", &targets, git::RemoteCallbacks::default(), @@ -3388,19 +3427,29 @@ fn test_push_updates_unexpectedly_moved_forward_on_remote() { Err(GitPushError::RefInUnexpectedLocation(_)) ); - // Moving the bookmark *forwards* is OK, as an exception matching our bookmark - // conflict resolution rules - assert_matches!( - attempt_push_expecting_parent(Some(setup.child_of_main_commit.id().clone())), - Ok(()) - ); + if subprocess { + // git is strict about honouring the expected location on --force-with-lease + assert_matches!( + attempt_push_expecting_parent(Some(setup.child_of_main_commit.id().clone())), + Err(GitPushError::RefInUnexpectedLocation(_)) + ); + } else { + // Moving the bookmark *forwards* is OK, as an exception matching our bookmark + // conflict resolution rules + assert_matches!( + attempt_push_expecting_parent(Some(setup.child_of_main_commit.id().clone())), + Ok(()) + ); + } } -#[test] -fn test_push_updates_unexpectedly_exists_on_remote() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_push_updates_unexpectedly_exists_on_remote(subprocess: bool) { let settings = testutils::user_settings(); let temp_dir = testutils::new_temp_dir(); let setup = set_up_push_repos(&settings, &temp_dir); + let git_settings = get_git_settings(subprocess); // The main bookmark is actually at `main_commit` on the remote. In this test, // we expect it to not exist on the remote at all. @@ -3420,6 +3469,7 @@ fn test_push_updates_unexpectedly_exists_on_remote() { git::push_updates( setup.jj_repo.as_ref(), &get_git_repo(&setup.jj_repo), + &git_settings, "origin", &targets, git::RemoteCallbacks::default(), @@ -3431,22 +3481,34 @@ fn test_push_updates_unexpectedly_exists_on_remote() { Err(GitPushError::RefInUnexpectedLocation(_)) ); - // We *can* move the bookmark forward even if we didn't expect it to exist - assert_matches!( - attempt_push_expecting_absence(Some(setup.child_of_main_commit.id().clone())), - Ok(()) - ); + if subprocess { + // Git is strict with enforcing the expected location + assert_matches!( + attempt_push_expecting_absence(Some(setup.child_of_main_commit.id().clone())), + Err(GitPushError::RefInUnexpectedLocation(_)) + ); + } else { + // In git2: We *can* move the bookmark forward even if we didn't expect it to + // exist + assert_matches!( + attempt_push_expecting_absence(Some(setup.child_of_main_commit.id().clone())), + Ok(()) + ); + } } -#[test] -fn test_push_updates_success() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_push_updates_success(subprocess: bool) { let settings = testutils::user_settings(); let temp_dir = testutils::new_temp_dir(); let setup = set_up_push_repos(&settings, &temp_dir); + let git_settings = get_git_settings(subprocess); let clone_repo = get_git_repo(&setup.jj_repo); let result = git::push_updates( setup.jj_repo.as_ref(), &clone_repo, + &git_settings, "origin", &[GitRefUpdate { qualified_name: "refs/heads/main".to_string(), @@ -3476,14 +3538,17 @@ fn test_push_updates_success() { assert_eq!(new_target, Some(new_oid)); } -#[test] -fn test_push_updates_no_such_remote() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_push_updates_no_such_remote(subprocess: bool) { let settings = testutils::user_settings(); let temp_dir = testutils::new_temp_dir(); let setup = set_up_push_repos(&settings, &temp_dir); + let git_settings = get_git_settings(subprocess); let result = git::push_updates( setup.jj_repo.as_ref(), &get_git_repo(&setup.jj_repo), + &git_settings, "invalid-remote", &[GitRefUpdate { qualified_name: "refs/heads/main".to_string(), @@ -3495,14 +3560,17 @@ fn test_push_updates_no_such_remote() { assert!(matches!(result, Err(GitPushError::NoSuchRemote(_)))); } -#[test] -fn test_push_updates_invalid_remote() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_push_updates_invalid_remote(subprocess: bool) { let settings = testutils::user_settings(); let temp_dir = testutils::new_temp_dir(); let setup = set_up_push_repos(&settings, &temp_dir); + let git_settings = get_git_settings(subprocess); let result = git::push_updates( setup.jj_repo.as_ref(), &get_git_repo(&setup.jj_repo), + &git_settings, "http://invalid-remote", &[GitRefUpdate { qualified_name: "refs/heads/main".to_string(),