From 7269d596892d1d89a2c45c9912067235e66ed28e Mon Sep 17 00:00:00 2001 From: bsdinis Date: Wed, 25 Dec 2024 20:19:32 +0000 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. Users can either enable shelling out to git in a config file: ```toml [git] subprocess = true ``` 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 --shell ``` 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 --shell -b ``` --- .github/workflows/build.yml | 15 +- CHANGELOG.md | 5 + cli/src/commands/git/clone.rs | 52 ++- cli/src/commands/git/fetch.rs | 11 +- cli/src/commands/git/push.rs | 31 +- cli/src/config-schema.json | 9 + cli/src/git_util.rs | 84 +++-- cli/tests/cli-reference@.md.snap | 1 + cli/tests/common/mod.rs | 10 + cli/tests/test_git_clone.rs | 331 ++++++++++++++--- cli/tests/test_git_fetch.rs | 497 +++++++++++++++++++++++--- cli/tests/test_git_push.rs | 590 +++++++++++++++++++++++++++---- flake.nix | 7 + lib/src/config/misc.toml | 2 + lib/src/git.rs | 252 +++++++++++-- lib/src/git_subprocess.rs | 442 +++++++++++++++++++++++ lib/src/lib.rs | 2 + lib/tests/test_git.rs | 185 +++++++--- 18 files changed, 2243 insertions(+), 283 deletions(-) create mode 100644 lib/src/git_subprocess.rs diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 40882a5bd0..4de89374ae 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -86,8 +86,19 @@ jobs: tool: nextest - name: Build run: cargo build --workspace --all-targets --verbose ${{ matrix.cargo_flags }} - - name: Test - run: cargo nextest run --workspace --all-targets --verbose ${{ matrix.cargo_flags }} + - name: Test [non-windows] + if: ${{ matrix.os != 'windows-latest' }} + run: | + export TEST_GIT_PATH="$(which git)"; + 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: | + $env:TEST_GIT_PATH='C:\Program Files\Git\bin\git.exe' + cargo nextest run --workspace --all-targets --verbose ${{ matrix.cargo_flags }} env: RUST_BACKTRACE: 1 CARGO_TERM_COLOR: always diff --git a/CHANGELOG.md b/CHANGELOG.md index 37021a6b00..732ed33efc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,8 @@ 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. + * `jj evolog` now accepts `--reversed`. * `jj restore` now supports `-i`/`--interactive` selection. @@ -61,6 +63,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixed bugs +* Fixes ssh bugs when interacting with git remotes due to `libgit2`'s limitations. + [#4979](https://github.com/jj-vcs/jj/issues/4979) + * Fixed diff selection by external tools with `jj split`/`commit -i FILESETS`. [#5252](https://github.com/jj-vcs/jj/issues/5252) diff --git a/cli/src/commands/git/clone.rs b/cli/src/commands/git/clone.rs index 944a76d7ab..126f336287 100644 --- a/cli/src/commands/git/clone.rs +++ b/cli/src/commands/git/clone.rs @@ -30,10 +30,12 @@ use super::write_repository_level_trunk_alias; use crate::cli_util::CommandHelper; use crate::cli_util::WorkspaceCommandHelper; use crate::command_error::cli_error; +use crate::command_error::internal_error; use crate::command_error::user_error; use crate::command_error::user_error_with_message; use crate::command_error::CommandError; use crate::commands::git::maybe_add_gitignore; +use crate::git_util::get_config_git_path; use crate::git_util::get_git_repo; use crate::git_util::map_git_error; use crate::git_util::print_git_import_stats; @@ -97,6 +99,13 @@ fn is_empty_dir(path: &Path) -> bool { } } +struct CloneArgs<'a> { + depth: Option, + remote_name: &'a str, + source: &'a str, + wc_path: &'a Path, +} + pub fn cmd_git_clone( ui: &mut Ui, command: &CommandHelper, @@ -129,14 +138,18 @@ pub fn cmd_git_clone( // `/some/path/.` let canonical_wc_path = dunce::canonicalize(&wc_path) .map_err(|err| user_error_with_message(format!("Failed to create {wc_path_str}"), err))?; + let git_executable_path = get_config_git_path(command)?; let clone_result = do_git_clone( ui, command, + git_executable_path.as_deref(), args.colocate, - args.depth, - remote_name, - &source, - &canonical_wc_path, + CloneArgs { + depth: args.depth, + remote_name, + source: &source, + wc_path: &canonical_wc_path, + }, ); if clone_result.is_err() { let clean_up_dirs = || -> io::Result<()> { @@ -191,12 +204,17 @@ pub fn cmd_git_clone( fn do_git_clone( ui: &mut Ui, command: &CommandHelper, + git_executable_path: Option<&Path>, colocate: bool, - depth: Option, - remote_name: &str, - source: &str, - wc_path: &Path, + clone_args: CloneArgs<'_>, ) -> Result<(WorkspaceCommandHelper, GitFetchStats), CommandError> { + let CloneArgs { + depth, + remote_name, + source, + wc_path, + } = clone_args; + let settings = command.settings_for_new_workspace(wc_path)?; let (workspace, repo) = if colocate { Workspace::init_colocated_git(&settings, wc_path)? @@ -215,10 +233,11 @@ fn do_git_clone( 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_executable_path.is_some(), |cb| { git::fetch( fetch_tx.repo_mut(), &git_repo, + git_executable_path, remote_name, &[StringPattern::everything()], cb, @@ -227,11 +246,22 @@ fn do_git_clone( ) }) .map_err(|err| match err { - GitFetchError::NoSuchRemote(_) => { - panic!("shouldn't happen as we just created the git remote") + GitFetchError::NoSuchRemote(repo_name) => { + user_error(format!("Could not find repository at '{repo_name}'")) + } + GitFetchError::GitCommandNotFound(path) => { + if path == Path::new("git") { + user_error("Could not find a `git` executable in the OS path") + } else { + user_error(format!( + "Could not execute provided git path: `{}`", + path.display() + )) + } } GitFetchError::GitImportError(err) => CommandError::from(err), GitFetchError::InternalGitError(err) => map_git_error(err), + GitFetchError::Subprocess(err) => internal_error(err), GitFetchError::InvalidBranchPattern => { unreachable!("we didn't provide any globs") } diff --git a/cli/src/commands/git/fetch.rs b/cli/src/commands/git/fetch.rs index 3ed3d360d5..e724e3046b 100644 --- a/cli/src/commands/git/fetch.rs +++ b/cli/src/commands/git/fetch.rs @@ -23,6 +23,7 @@ use crate::cli_util::CommandHelper; use crate::command_error::CommandError; use crate::commands::git::get_single_remote; use crate::complete; +use crate::git_util::get_config_git_path; use crate::git_util::get_git_repo; use crate::git_util::git_fetch; use crate::ui::Ui; @@ -69,6 +70,7 @@ pub fn cmd_git_fetch( args: &GitFetchArgs, ) -> Result<(), CommandError> { let mut workspace_command = command.workspace_helper(ui)?; + let git_executable_path = get_config_git_path(command)?; let git_repo = get_git_repo(workspace_command.repo().store())?; let remotes = if args.all_remotes { get_all_remotes(&git_repo)? @@ -78,7 +80,14 @@ pub fn cmd_git_fetch( args.remotes.clone() }; let mut tx = workspace_command.start_transaction(); - git_fetch(ui, &mut tx, &git_repo, &remotes, &args.branch)?; + git_fetch( + ui, + &mut tx, + &git_repo, + git_executable_path.as_deref(), + &remotes, + &args.branch, + )?; tx.finish( ui, format!("fetch from git remote(s) {}", remotes.iter().join(",")), diff --git a/cli/src/commands/git/push.rs b/cli/src/commands/git/push.rs index 4c748c59dc..a5fd442895 100644 --- a/cli/src/commands/git/push.rs +++ b/cli/src/commands/git/push.rs @@ -49,6 +49,7 @@ use crate::command_error::CommandError; use crate::commands::git::get_single_remote; use crate::complete; use crate::formatter::Formatter; +use crate::git_util::get_config_git_path; use crate::git_util::get_git_repo; use crate::git_util::map_git_error; use crate::git_util::with_remote_git_callbacks; @@ -166,6 +167,7 @@ pub fn cmd_git_push( args: &GitPushArgs, ) -> Result<(), CommandError> { let mut workspace_command = command.workspace_helper(ui)?; + let git_executable_path = get_config_git_path(command)?; let git_repo = get_git_repo(workspace_command.repo().store())?; let remote = if let Some(name) = &args.remote { @@ -313,9 +315,22 @@ 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) - }) + + with_remote_git_callbacks( + ui, + Some(&mut sideband_progress_callback), + git_executable_path.is_some(), + |cb| { + git::push_branches( + tx.repo_mut(), + &git_repo, + git_executable_path.as_deref(), + &remote, + &targets, + cb, + ) + }, + ) .map_err(|err| match err { GitPushError::InternalGitError(err) => map_git_error(err), GitPushError::RefInUnexpectedLocation(refs) => user_error_with_hint( @@ -327,6 +342,16 @@ pub fn cmd_git_push( "Try fetching from the remote, then make the bookmark point to where you want it to \ be, and push again.", ), + GitPushError::GitCommandNotFound(path) => { + if path == std::path::Path::new("git") { + user_error("Could not find a `git` executable in the OS path") + } else { + user_error(format!( + "Could not execute provided git path: `{}`", + path.display() + )) + } + } _ => user_error(err), })?; writer.flush(ui)?; diff --git a/cli/src/config-schema.json b/cli/src/config-schema.json index c4a705626d..f1f2af8998 100644 --- a/cli/src/config-schema.json +++ b/cli/src/config-schema.json @@ -362,6 +362,15 @@ "type": "string", "description": "The remote to which commits are pushed", "default": "origin" + }, + "subprocess": { + "type": "boolean", + "description": "Whether jj spawns a git subprocess for network operations (push/fetch/clone)", + "default": false + }, + "path": { + "type": "string", + "description": "Path to the git executable" } } }, diff --git a/cli/src/git_util.rs b/cli/src/git_util.rs index 2935bf7f33..7d6638b94e 100644 --- a/cli/src/git_util.rs +++ b/cli/src/git_util.rs @@ -28,6 +28,7 @@ use std::time::Instant; use crossterm::terminal::Clear; use crossterm::terminal::ClearType; use itertools::Itertools; +use jj_lib::config::ConfigGetError; use jj_lib::fmt_util::binary_prefix; use jj_lib::git; use jj_lib::git::FailedRefExport; @@ -46,6 +47,7 @@ use jj_lib::workspace::Workspace; use unicode_width::UnicodeWidthStr; use crate::cleanup_guard::CleanupGuard; +use crate::cli_util::CommandHelper; use crate::cli_util::WorkspaceCommandTransaction; use crate::command_error::user_error; use crate::command_error::user_error_with_hint; @@ -263,29 +265,40 @@ type SidebandProgressCallback<'a> = &'a mut dyn FnMut(&[u8]); pub fn with_remote_git_callbacks( ui: &Ui, sideband_progress_callback: Option>, + subprocess: bool, 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 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( @@ -610,16 +623,18 @@ pub fn git_fetch( ui: &mut Ui, tx: &mut WorkspaceCommandTransaction, git_repo: &git2::Repository, + git_executable_path: Option<&Path>, remotes: &[String], branch: &[StringPattern], ) -> Result<(), CommandError> { 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_executable_path.is_some(), |cb| { git::fetch( tx.repo_mut(), git_repo, + git_executable_path, remote, branch, cb, @@ -643,6 +658,16 @@ pub fn git_fetch( } GitFetchError::GitImportError(err) => err.into(), GitFetchError::InternalGitError(err) => map_git_error(err), + GitFetchError::GitCommandNotFound(path) => { + if path == std::path::Path::new("git") { + user_error("Could not find a `git` executable in the OS path") + } else { + user_error(format!( + "Could not execute provided git path: `{}`", + path.display() + )) + } + } _ => user_error(err), })?; print_git_import_stats(ui, tx.repo(), &stats.import_stats, true)?; @@ -686,6 +711,23 @@ fn warn_if_branches_not_found( Ok(()) } +pub(crate) fn get_config_git_subprocess(command: &CommandHelper) -> Result { + command.settings().get_bool("git.subprocess") +} + +pub(crate) fn get_config_git_path( + command: &CommandHelper, +) -> Result, ConfigGetError> { + if get_config_git_subprocess(command)? { + command + .settings() + .get::("git.executable_path") + .map(Some) + } else { + Ok(None) + } +} + #[cfg(test)] mod tests { use insta::assert_snapshot; diff --git a/cli/tests/cli-reference@.md.snap b/cli/tests/cli-reference@.md.snap index e507055fd1..0f20cd259d 100644 --- a/cli/tests/cli-reference@.md.snap +++ b/cli/tests/cli-reference@.md.snap @@ -1,6 +1,7 @@ --- source: cli/tests/test_generate_md_cli_help.rs description: "AUTO-GENERATED FILE, DO NOT EDIT. This cli reference is generated by a test as an `insta` snapshot. MkDocs includes this snapshot from docs/cli-reference.md." +snapshot_kind: text --- diff --git a/cli/tests/common/mod.rs b/cli/tests/common/mod.rs index e1a883934c..2b8d92e49e 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,15 @@ 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_path) = std::env::var("TEST_GIT_PATH") { + self.add_config(format!("git.executable_path = '{}'", git_path.trim())); + } + } + 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 8b3d47166c..0e6db9ca14 100644 --- a/cli/tests/test_git_clone.rs +++ b/cli/tests/test_git_clone.rs @@ -17,6 +17,7 @@ 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; @@ -50,28 +51,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 +90,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 +113,15 @@ 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###" + } + insta::allow_duplicates! { + 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' + "#); + } assert!(!test_env.env_root().join("failed").exists()); // Failed clone shouldn't remove the existing destination directory @@ -113,47 +132,62 @@ 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###" + } + insta::allow_duplicates! { + 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' + "#); + } 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 +196,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 +241,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 +269,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 +281,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 +310,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 +351,15 @@ 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###" + } + insta::allow_duplicates! { + 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' + "#); + } assert!(!test_env.env_root().join("failed").exists()); // Failed clone shouldn't remove the existing destination directory @@ -304,11 +373,15 @@ 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###" + } + insta::allow_duplicates! { + 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' + "#); + } 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 +391,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 +404,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 +416,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 +428,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 +445,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 +457,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 +484,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 +494,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 +503,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 +530,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 +554,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 +592,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 +639,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 +663,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 +675,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_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); @@ -557,7 +692,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 @@ -566,16 +704,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) @@ -583,10 +727,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] @@ -671,29 +818,76 @@ fn test_git_clone_conditional_config() { "); } -#[test] -fn test_git_clone_with_depth() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "spawn a git subprocess for remote calls")] +fn test_git_clone_with_depth(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 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); - // 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( - test_env.env_root(), - &["git", "clone", "--depth", "1", "source", "clone"], - ); - insta::assert_snapshot!(stderr, @r#" + // git does support shallow clones on the local transport, so it will work + // (we cannot replicate git2's erroneous behaviour wrt git) + if subprocess { + let (stdout, stderr) = test_env.jj_cmd_ok( + test_env.env_root(), + &["git", "clone", "--depth", "1", "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 + 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::allow_duplicates! { + 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::allow_duplicates! { + insta::assert_snapshot!(stderr, @""); + } + } else { + // 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( + test_env.env_root(), + &["git", "clone", "--depth", "1", "source", "clone"], + ); + insta::allow_duplicates! { + insta::assert_snapshot!(stderr, @r#" Fetching into new repo in "$TEST_ENV/clone" Error: shallow fetch is not supported by the local transport; class=Net (12) "#); + } + } } -#[test] -fn test_git_clone_invalid_immutable_heads() { +#[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); @@ -705,6 +899,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 @@ -712,11 +907,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"); @@ -727,6 +927,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 @@ -734,31 +935,77 @@ 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"]); + insta::assert_snapshot!(stderr, @r#" + Fetching into new repo in "$TEST_ENV/clone" + Error: Could not find a `git` executable in the OS path + "#); +} + +#[test] +fn test_git_clone_no_git_executable_with_path() { + let mut test_env = TestEnvironment::default(); + test_env.add_config("git.subprocess = true"); + test_env.add_config("git.executable_path = '/this/is/an/invalid/path'"); + 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!(stderr, @r#" + Fetching into new repo in "$TEST_ENV/clone" + Error: Could not execute provided git path: `/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 b655f834f2..0ab3017800 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,34 +299,48 @@ 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"); @@ -276,15 +352,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 +375,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 +422,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 +445,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 +499,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 +507,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 +539,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 +554,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 +578,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 +613,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 +626,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 +643,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 +651,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 +663,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 +675,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 +688,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 +705,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 +721,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 +739,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 +763,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 +807,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 +841,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 +863,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 +880,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 +888,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 +900,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 +930,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 +945,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 +976,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 +989,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"); @@ -803,11 +1009,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( @@ -816,22 +1026,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. @@ -842,10 +1060,13 @@ fn test_git_fetch_bookmarks_some_missing() { "rem2", ], ); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r###" bookmark: rem1@rem1 [new] tracked bookmark: rem2@rem2 [new] tracked "###); + } + insta::allow_duplicates! { insta::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @r###" origin: oputwtnw ffecd2d6 message @origin: oputwtnw ffecd2d6 message @@ -854,6 +1075,7 @@ fn test_git_fetch_bookmarks_some_missing() { rem2: yszkquru 2497a8a0 message @rem2: yszkquru 2497a8a0 message "###); + } // multiple bookmarks, one exists, one doesn't let (_stdout, stderr) = test_env.jj_cmd_ok( @@ -862,10 +1084,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 @@ -874,13 +1099,18 @@ fn test_git_fetch_bookmarks_some_missing() { rem2: yszkquru 2497a8a0 message @rem2: yszkquru 2497a8a0 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(); @@ -888,15 +1118,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 @@ -907,13 +1142,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 @@ -927,8 +1166,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 "#); @@ -937,10 +1180,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 "###); @@ -951,13 +1198,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(); @@ -965,15 +1217,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 @@ -984,18 +1241,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 @@ -1003,10 +1268,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 @@ -1014,23 +1282,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( @@ -1043,103 +1319,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(); @@ -1147,15 +1463,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 @@ -1166,16 +1487,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 @@ -1187,6 +1514,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"]); @@ -1194,10 +1522,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 @@ -1209,11 +1542,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. @@ -1227,11 +1564,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(); @@ -1239,15 +1581,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 @@ -1258,16 +1605,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 @@ -1279,6 +1632,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:*"]); @@ -1292,13 +1646,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 @@ -1308,11 +1667,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"); @@ -1347,10 +1711,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( @@ -1366,15 +1732,46 @@ 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 "###); + } +} + +#[test] +fn test_git_fetch_no_git_executable() { + let mut test_env = TestEnvironment::default(); + test_env.add_config("git.subprocess = 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.add_env_var("PATH", ""); + let stderr = test_env.jj_cmd_failure(&repo_path, &["git", "fetch"]); + insta::assert_snapshot!(stderr, @"Error: Could not find a `git` executable in the OS path"); +} + +#[test] +fn test_git_fetch_no_git_executable_with_path() { + let mut test_env = TestEnvironment::default(); + test_env.add_config("git.subprocess = true"); + test_env.add_config("git.executable_path = '/this/is/an/invalid/path'"); + 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.add_env_var("PATH", ""); + let stderr = test_env.jj_cmd_failure(&repo_path, &["git", "fetch"]); + insta::assert_snapshot!(stderr, @"Error: Could not execute provided git path: `/this/is/an/invalid/path`"); } diff --git a/cli/tests/test_git_push.rs b/cli/tests/test_git_push.rs index a7ab1b9478..7b47a31d3d 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,20 +578,25 @@ 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. "); + } let (_stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--allow-new"]); + 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 @@ -463,16 +605,23 @@ fn test_git_push_locally_created_and_rewritten() { my: vruxwmqv eaf7a52c (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 eaf7a52c8906 "); + } } -#[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, @@ -481,6 +630,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 @@ -488,10 +638,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 @@ -499,6 +653,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, @@ -511,13 +666,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, @@ -532,41 +691,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 @@ -579,7 +753,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 @@ -589,26 +765,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: @@ -616,23 +802,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(); @@ -640,11 +835,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(); @@ -659,28 +858,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( @@ -692,12 +899,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( @@ -709,18 +920,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"]); @@ -736,69 +955,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"]); @@ -820,11 +1065,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, @@ -837,7 +1084,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: @@ -846,11 +1096,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( @@ -863,16 +1118,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", "@"]); @@ -880,9 +1143,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(); @@ -892,25 +1159,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, &[ @@ -924,9 +1199,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"]); @@ -943,10 +1222,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( @@ -959,17 +1240,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) @@ -983,25 +1272,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) @@ -1026,10 +1323,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( @@ -1042,17 +1341,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) @@ -1066,10 +1373,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"]); @@ -1077,10 +1386,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) @@ -1089,15 +1400,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) @@ -1123,10 +1440,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( @@ -1139,26 +1458,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) @@ -1168,16 +1500,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(); @@ -1195,6 +1536,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 @@ -1203,6 +1545,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"]); @@ -1211,50 +1554,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. @@ -1264,18 +1625,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"]); @@ -1285,6 +1654,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 @@ -1292,34 +1662,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 @@ -1338,17 +1715,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"]); @@ -1357,16 +1740,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( @@ -1378,16 +1767,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"]); @@ -1397,12 +1792,63 @@ 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] +fn test_git_push_no_git_executable() { + let (mut test_env, workspace_root) = set_up(); + test_env.add_config("git.subprocess = true"); + // Show the setup. `insta` has trouble if this is done inside `set_up()` + test_env.add_config(r#"revset-aliases."immutable_heads()" = "none()""#); + test_env.jj_cmd_ok(&workspace_root, &["edit", "bookmark1"]); + test_env.jj_cmd_ok( + &workspace_root, + &["describe", "-m", "modified bookmark1 commit"], + ); + test_env.jj_cmd_ok(&workspace_root, &["new", "-m", "non-empty description"]); + std::fs::write(workspace_root.join("file"), "file").unwrap(); + + test_env.add_env_var("PATH", ""); + let stderr = test_env.jj_cmd_failure(&workspace_root, &["git", "push"]); + + insta::assert_snapshot!(stderr, @r" + Changes to push to origin: + Move sideways bookmark bookmark1 from d13ecdbda2a2 to e612d524a5c6 + Error: Could not find a `git` executable in the OS path + "); +} + +#[test] +fn test_git_push_no_git_executable_with_path() { + let (mut test_env, workspace_root) = set_up(); + test_env.add_config("git.subprocess = true"); + test_env.add_config("git.executable_path = '/this/is/an/invalid/path'"); + // Show the setup. `insta` has trouble if this is done inside `set_up()` + test_env.add_config(r#"revset-aliases."immutable_heads()" = "none()""#); + test_env.jj_cmd_ok(&workspace_root, &["edit", "bookmark1"]); + test_env.jj_cmd_ok( + &workspace_root, + &["describe", "-m", "modified bookmark1 commit"], + ); + test_env.jj_cmd_ok(&workspace_root, &["new", "-m", "non-empty description"]); + std::fs::write(workspace_root.join("file"), "file").unwrap(); + + test_env.add_env_var("PATH", ""); + let stderr = test_env.jj_cmd_failure(&workspace_root, &["git", "push"]); + + insta::assert_snapshot!(stderr, @r" + Changes to push to origin: + Move sideways bookmark bookmark1 from d13ecdbda2a2 to e612d524a5c6 + Error: Could not execute provided git path: `/this/is/an/invalid/path` + "); } fn get_bookmark_output(test_env: &TestEnvironment, repo_path: &Path) -> String { diff --git a/flake.nix b/flake.nix index a751e062cb..38d4aa9c7b 100644 --- a/flake.nix +++ b/flake.nix @@ -117,6 +117,10 @@ NIX_JJ_GIT_HASH = self.rev or ""; CARGO_INCREMENTAL = "0"; + # give git path to tests + # (PATH variable is unset in the testing environment) + env.TEST_GIT_PATH = pkgs.lib.getExe pkgs.git; + preCheck = '' export RUST_BACKTRACE=1 ''; @@ -197,6 +201,9 @@ gnupg openssh + # To run git subprocess checks + git + # For building the documentation website uv ] ++ darwinDeps ++ linuxNativeDeps; diff --git a/lib/src/config/misc.toml b/lib/src/config/misc.toml index b9801bdb8a..6fd4aa41a8 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 aafee76b94..299dd1f2f5 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; @@ -35,6 +36,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; @@ -1215,6 +1218,19 @@ fn is_remote_not_found_err(err: &git2::Error) -> bool { ) } +fn repository_not_found_err(err: &git2::Error) -> Option<&str> { + if err.class() == git2::ErrorClass::Repository + && err.code() == git2::ErrorCode::GenericError + && err.message().starts_with("could not find repository at ") + { + let mut sp = err.message().split('\''); + sp.next(); + sp.next() + } else { + None + } +} + fn is_remote_exists_err(err: &git2::Error) -> bool { matches!( (err.class(), err.code()), @@ -1372,9 +1388,13 @@ 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("Could not find `git` executable: provided path {0:?}")] + GitCommandNotFound(PathBuf), + #[error("Git subprocess failed")] + Subprocess(GitSubprocessError), } -fn fetch_options( +fn git2_fetch_options( callbacks: RemoteCallbacks<'_>, depth: Option, ) -> git2::FetchOptions<'_> { @@ -1402,6 +1422,8 @@ struct GitFetch<'a> { git_settings: &'a GitSettings, fetch_options: git2::FetchOptions<'a>, fetched: Vec, + depth: Option, + git_executable_path: Option<&'a Path>, } impl<'a> GitFetch<'a> { @@ -1410,6 +1432,8 @@ impl<'a> GitFetch<'a> { git_repo: &'a git2::Repository, git_settings: &'a GitSettings, fetch_options: git2::FetchOptions<'a>, + depth: Option, + git_executable_path: Option<&'a Path>, ) -> Self { GitFetch { mut_repo, @@ -1417,29 +1441,16 @@ impl<'a> GitFetch<'a> { git_settings, fetch_options, fetched: vec![], + depth, + git_executable_path, } } - /// 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 @@ -1452,14 +1463,39 @@ impl<'a> GitFetch<'a> { .map(|glob| format!("+refs/heads/{glob}:refs/remotes/{remote_name}/{glob}")) }) .collect::>() - .ok_or(GitFetchError::InvalidBranchPattern)?; + .ok_or(GitFetchError::InvalidBranchPattern) + } + + 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)?; if refspecs.is_empty() { // Don't fall back to the base refspecs. return Ok(None); } tracing::debug!("remote.download"); - remote.download(&refspecs, Some(&mut self.fetch_options))?; + remote + .download(&refspecs, Some(&mut self.fetch_options)) + .map_err(|err| { + if let Some(s) = repository_not_found_err(&err) { + GitFetchError::NoSuchRemote(s.to_string()) + } else { + GitFetchError::InternalGitError(err) + } + })?; tracing::debug!("remote.prune"); remote.prune(None)?; tracing::debug!("remote.update_tips"); @@ -1493,6 +1529,66 @@ impl<'a> GitFetch<'a> { Ok(default_branch) } + fn subprocess_fetch( + &mut self, + branch_names: &[StringPattern], + remote_name: &str, + git_path: &Path, + ) -> Result, GitFetchError> { + // 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)?; + if refspecs.is_empty() { + // Don't fall back to the base refspecs. + return Ok(None); + } + + let git_ctx = GitSubprocessContext::from_git2(self.git_repo, git_path)?; + + let mut prunes = Vec::new(); + for refspec in refspecs { + git_ctx.spawn_fetch(remote_name, self.depth, &refspec, &mut prunes)?; + } + + // Even though --prune is specified on git fetch, if a refspec is asked + // for but not present in the remote git will not remove the local reference. + // Instead, git will error out, saying it couldn't find the remote reference + // + // On git fetch we collect these errors and then prune them manually + for branch in prunes { + git_ctx.spawn_branch_prune(remote_name, &branch)?; + } + + 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 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> { + if let Some(git_path) = &self.git_executable_path { + self.subprocess_fetch(branch_names, remote_name, git_path) + } else { + self.git2_fetch(branch_names, remote_name) + } + } + /// 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. @@ -1539,21 +1635,34 @@ 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 +#[allow(clippy::too_many_arguments)] #[tracing::instrument(skip(mut_repo, git_repo, callbacks))] pub fn fetch( mut_repo: &mut MutableRepo, git_repo: &git2::Repository, + git_executable_path: Option<&Path>, remote_name: &str, branch_names: &[StringPattern], callbacks: RemoteCallbacks<'_>, git_settings: &GitSettings, depth: Option, ) -> Result { + // git fetch remote_name branch_names let mut git_fetch = GitFetch::new( mut_repo, git_repo, git_settings, - fetch_options(callbacks, depth), + if git_executable_path.is_some() { + git2::FetchOptions::default() + } else { + git2_fetch_options(callbacks, depth) + }, + depth, + git_executable_path, ); let default_branch = git_fetch.fetch(branch_names, remote_name)?; let import_stats = git_fetch.import_refs()?; @@ -1581,6 +1690,10 @@ pub enum GitPushError { // and errors caused by the remote rejecting the push. #[error("Unexpected git error when pushing")] InternalGitError(#[from] git2::Error), + #[error("Could not find `git` executable: provided path {0:?}")] + GitCommandNotFound(PathBuf), + #[error("Git subprocess failed")] + Subprocess(GitSubprocessError), } #[derive(Clone, Debug)] @@ -1602,6 +1715,7 @@ pub struct GitRefUpdate { pub fn push_branches( mut_repo: &mut MutableRepo, git_repo: &git2::Repository, + git_executable_path: Option<&Path>, remote_name: &str, targets: &GitBranchPushTargets, callbacks: RemoteCallbacks<'_>, @@ -1615,7 +1729,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_executable_path, + 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 @@ -1637,6 +1758,7 @@ pub fn push_branches( pub fn push_updates( repo: &dyn Repo, git_repo: &git2::Repository, + git_executable_path: Option<&Path>, remote_name: &str, updates: &[GitRefUpdate], callbacks: RemoteCallbacks<'_>, @@ -1662,17 +1784,27 @@ pub fn push_updates( } // TODO(ilyagr): `push_refs`, or parts of it, should probably be inlined. This // requires adjusting some tests. - push_refs( - repo, - git_repo, - remote_name, - &qualified_remote_refs_expected_locations, - &refspecs, - callbacks, - ) + if let Some(git_path) = git_executable_path { + subprocess_push_refs( + git_repo, + git_path, + remote_name, + &qualified_remote_refs_expected_locations, + &refspecs, + ) + } else { + 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, @@ -1750,7 +1882,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()); } } @@ -1799,6 +1930,59 @@ fn push_refs( } } +fn subprocess_push_refs( + git_repo: &git2::Repository, + git_path: &Path, + remote_name: &str, + qualified_remote_refs_expected_locations: &HashMap<&str, Option<&CommitId>>, + refspecs: &[String], +) -> Result<(), GitPushError> { + if remote_name == REMOTE_NAME_FOR_LOCAL_GIT_REPO { + return Err(GitPushError::RemoteReservedForLocalGitRepo); + } + + let git_ctx = GitSubprocessContext::from_git2(git_repo, git_path)?; + + let mut remaining_remote_refs: HashSet<_> = qualified_remote_refs_expected_locations + .keys() + .copied() + .collect(); + let mut failed_ref_matches = vec![]; + for full_refspec in refspecs { + let remote_ref = full_refspec.split(":").last(); + let expected_remote_location = remote_ref.and_then(|remote_ref| { + qualified_remote_refs_expected_locations + .get(remote_ref) + .and_then(|x| x.map(|y| y.to_string())) + }); + git_ctx.spawn_push( + remote_name, + remote_ref, + full_refspec, + expected_remote_location.as_deref(), + &mut failed_ref_matches, + )?; + if let Some(remote_ref) = remote_ref { + remaining_remote_refs.remove(remote_ref); + } + } + + if !failed_ref_matches.is_empty() { + failed_ref_matches.sort(); + Err(GitPushError::RefInUnexpectedLocation(failed_ref_matches)) + } else if remaining_remote_refs.is_empty() { + Ok(()) + } else { + 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..3de9ef1365 --- /dev/null +++ b/lib/src/git_subprocess.rs @@ -0,0 +1,442 @@ +use std::ffi::OsStr; +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::GitFetchError; +use crate::git::GitPushError; + +#[derive(Error, Debug)] +pub enum GitSubprocessError { + #[error("Failed to execute the git process")] + Spawn(std::io::Error), + #[error("Failed to wait for the git process")] + Wait(std::io::Error), + #[error("Failed to canonicalize path: {0}")] + PathCanonicalization(std::path::PathBuf, std::io::Error), + #[error("Git process failed: {0}")] + External(String), + #[error("Could not obtain path of the work tree: {0}")] + WorkTreePath(String), +} + +#[derive(Error, Debug)] +pub enum GitError { + #[error("No git remote named '{0}'")] + NoSuchRemote(String), + #[error("Could not find `git` executable: provided path {0:?}")] + GitCommandNotFound(PathBuf), + #[error("Git subprocess failed")] + Subprocess(#[from] GitSubprocessError), +} + +impl From for GitFetchError { + fn from(value: GitError) -> Self { + match value { + GitError::NoSuchRemote(remote) => GitFetchError::NoSuchRemote(remote), + GitError::GitCommandNotFound(git_path) => GitFetchError::GitCommandNotFound(git_path), + GitError::Subprocess(error) => GitFetchError::Subprocess(error), + } + } +} + +impl From for GitPushError { + fn from(value: GitError) -> Self { + match value { + GitError::NoSuchRemote(remote) => GitPushError::NoSuchRemote(remote), + GitError::GitCommandNotFound(git_path) => GitPushError::GitCommandNotFound(git_path), + GitError::Subprocess(error) => GitPushError::Subprocess(error), + } + } +} + +/// Context for creating git subprocesses +pub(crate) struct GitSubprocessContext<'a> { + git_dir: PathBuf, + work_tree: PathBuf, + git_path: &'a Path, +} + +fn canonicalize(path: impl AsRef) -> Result { + dunce::canonicalize(&path) + .map_err(|e| GitSubprocessError::PathCanonicalization(path.as_ref().to_path_buf(), e)) +} + +impl<'a> GitSubprocessContext<'a> { + pub(crate) fn new( + git_dir: impl Into, + work_tree: impl Into, + git_path: &'a Path, + ) -> Self { + GitSubprocessContext { + git_dir: git_dir.into(), + work_tree: work_tree.into(), + git_path, + } + } + + pub(crate) fn from_git2( + git_repo: &git2::Repository, + git_path: &'a Path, + ) -> Result { + let git_dir = canonicalize(git_repo.path())?; + let work_tree = work_tree_from_git_dir(git_repo.path()) + .map_err(GitSubprocessError::WorkTreePath) + .map(canonicalize)??; + + Ok(Self::new(git_dir, work_tree, git_path)) + } + + /// Spawn a git process + fn spawn(&self, args: I, stdout: Stdio) -> Result + where + I: IntoIterator, + S: AsRef, + { + let mut git_cmd = Command::new(self.git_path); + // we cd into the work tree to avoid + git_cmd + .args([ + OsStr::new("--git-dir"), + self.git_dir.as_ref(), + OsStr::new("--work-tree"), + self.work_tree.as_ref(), + ]) + .args(args) + .stdin(Stdio::null()) + .stdout(stdout) + .stderr(Stdio::piped()); + tracing::debug!(cmd = ?git_cmd, "spawning a git subprocess"); + let child_git = git_cmd.spawn().map_err(|e| { + if e.kind() == std::io::ErrorKind::NotFound { + GitError::GitCommandNotFound(self.git_path.to_path_buf()) + } else { + GitSubprocessError::Spawn(e).into() + } + })?; + + let output = child_git + .wait_with_output() + .map_err(GitSubprocessError::Wait)?; + Ok(output) + } + + /// Perform a git fetch + pub(crate) fn spawn_fetch( + &self, + remote_name: &str, + depth: Option, + refspec: &str, + prunes: &mut Vec, + ) -> Result<(), GitError> { + let args = { + let mut a = vec!["fetch".to_string(), "--prune".to_string()]; + if let Some(d) = depth { + a.push(format!("--depth={d}")); + } + a.push("--".to_string()); + a.push(remote_name.to_string()); + a.push(refspec.to_string()); + a + }; + + let output = self.spawn(&args, Stdio::inherit())?; + // we name the type to make sure that it is not meant to be used + let _: () = parse_git_fetch_output(output, prunes)?; + + 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, GitError> { + let output = self.spawn(["remote", "show", "--", remote_name], Stdio::piped())?; + + let output = parse_git_remote_show_output(output)?; + + // find the HEAD branch line in the output + let branch_name = String::from_utf8(output.stdout) + .map_err(|e| { + GitSubprocessError::External(format!("git remote output is not utf-8: {e:?}")) + }) + .map_err(GitError::Subprocess)? + .lines() + .map(|x| x.trim()) + .find(|x| x.starts_with("HEAD branch:")) + .and_then(|x| x.split(" ").last().map(|y| y.trim().to_string())); + + // git will output (unknown) if there is no default branch. we want it to be a + // none value + if let Some(x) = branch_name.as_deref() { + if x == "(unknown)" { + return Ok(None); + } + } + Ok(branch_name) + } + + /// Prune particular branches + /// + /// Even if git fetch has --prune, if a branch is not found it will not be + /// pruned on fetch + pub(crate) fn spawn_branch_prune( + &self, + remote_name: &str, + branch_name: &str, + ) -> Result<(), GitError> { + let refname = format!("{remote_name}/{branch_name}"); + let output = self.spawn( + ["branch", "--remotes", "--delete", "--", &refname], + Stdio::null(), + )?; + + // we name the type to make sure that it is not meant to be used + let _: () = parse_git_branch_prune_output(output)?; + + Ok(()) + } + + /// Push references to git + /// + /// All pushes are forced, using --force-with-lease to perform a test&set + /// operation on the remote repository + pub(crate) fn spawn_push( + &self, + remote_name: &str, + remote_ref: Option<&str>, + full_refspec: &str, + expected_remote_location: Option<&str>, + failed_ref_matches: &mut Vec, + ) -> Result<(), GitError> { + // we use --force-with-lease, so we are already force pushing + // (which is the behaviour a leading + signifies) + // + // if we leave the leading +, the test and set operation is ignored and + // it becomes a regular forced push + let full_refspec = full_refspec.strip_prefix("+").unwrap_or(full_refspec); + let args = { + let mut a = vec!["push".to_string()]; + if let Some(refname) = remote_ref { + a.push(format!( + "--force-with-lease={refname}:{}", + expected_remote_location.unwrap_or("") + )); + } + + a.extend_from_slice(&[ + "--".to_string(), + remote_name.to_string(), + full_refspec.to_string(), + ]); + a + }; + + let output = self.spawn(&args, Stdio::inherit())?; + + // parse git push output returns true if the test and set operation failed + // because a reference moved on the remote + let test_and_set_failed = parse_git_push_output(output)?; + + if test_and_set_failed { + if let Some(dst_reference) = remote_ref { + failed_ref_matches.push(dst_reference.to_string()); + } + } + + Ok(()) + } +} + +/// Get the work tree dir from the git dir +/// +/// There are two possible options: +/// - on a bare git repo, the dir has a parent named .jj that sits on the +/// workspace root +/// - on a colocated .git dir, it is already on the workspace root +fn work_tree_from_git_dir(git_dir: &Path) -> Result<&Path, String> { + if git_dir.file_name() == Some(OsStr::new(".git")) { + git_dir + .parent() + .ok_or(format!("git repo had no parent: {}", git_dir.display())) + } else if git_dir.file_name() == Some(OsStr::new("git")) { + let mut it = git_dir + .ancestors() + .skip_while(|dir| !dir.ends_with(Path::new(".jj"))); + it.next().ok_or(format!( + "could not find .jj dir in git dir path: {}", + git_dir.display() + )) + } else { + Err(format!( + "git dir is not named `git` nor `.git`: {}", + git_dir.display() + )) + } +} + +/// Generate a GitError::ExternalGitError if the stderr output was not +/// recognizable +fn external_git_error(stderr: &[u8]) -> GitError { + GitError::Subprocess(GitSubprocessError::External(format!( + "External git program failed:\n{}", + stderr.to_str_lossy() + ))) +} + +/// Parse no such remote errors output from git +/// +/// 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]) -> Result<(), String> { + if let Some(first_line) = stderr.lines().next() { + if (first_line.starts_with_str("fatal: '") + && first_line.ends_with_str("' does not appear to be a git repository")) + || (first_line.starts_with_str("fatal: unable to access '") + && first_line.ends_with_str("': Could not resolve host: invalid-remote")) + { + let mut split = first_line.split_str("\'"); + split.next(); // ignore prefix + let branch_name = split.next(); + if let Some(bname) = branch_name { + return Err(bname.to_str_lossy().to_string()); + } + } + } + + Ok(()) +} + +/// Parse error from refspec not present on the remote +/// +/// This returns the branch to prune if it is found, 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. +/// +/// The first line is of the form: +/// `fatal: couldn't find remote ref refs/heads/` +fn parse_no_remote_ref(stderr: &[u8]) -> Option { + if let Some(first_line) = stderr.lines().next() { + if first_line.starts_with_str("fatal: couldn't find remote ref refs/heads/") { + let mut sp = first_line.split_str("refs/heads/"); + sp.next(); + return Some(sp.next().unwrap().to_str_lossy().to_string()); + } + } + + None +} + +/// 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]) -> bool { + if let Some(first_line) = stderr.lines().next() { + return first_line.starts_with_str("error: remote-tracking branch") + && (first_line.ends_with_str("not found") || first_line.ends_with_str("not found.")); + } + + false +} + +/// Parse errors from --force-with-lease +/// +/// Return true if there was an error +/// +/// When a ref has moved on the remote, git responds with +/// ` +/// To +/// ! [rejected] -> (stale info) +/// error: failed to push some refs to +/// ` +/// +/// Because we only push one branch at a time, +/// we just look at the last line to report +/// whether this was the problem or not. +/// It is up to the caller to know which ref it was trying to push +fn parse_force_with_lease_error(stderr: &[u8]) -> bool { + stderr + .lines() + .last() + .map(|x| x.starts_with_str("error: failed to push some refs to ")) + .unwrap_or(false) +} + +fn parse_git_fetch_output(output: Output, prunes: &mut Vec) -> Result<(), GitError> { + if output.status.success() { + return Ok(()); + } + + // There are some git errors we want to parse out + parse_no_such_remote(&output.stderr).map_err(GitError::NoSuchRemote)?; + + if let Some(branch_name) = parse_no_remote_ref(&output.stderr) { + prunes.push(branch_name); + return Ok(()); + } + + if parse_no_remote_tracking_branch(&output.stderr) { + 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 + parse_no_such_remote(&output.stderr).map_err(GitError::NoSuchRemote)?; + + Err(external_git_error(&output.stderr)) +} + +fn parse_git_branch_prune_output(output: Output) -> Result<(), GitError> { + if output.status.success() { + return Ok(()); + } + + // There are some git errors we want to parse out + if parse_no_remote_tracking_branch(&output.stderr) { + return Ok(()); + } + + Err(external_git_error(&output.stderr)) +} + +// true indicates force_with_lease failed due to mismatch +// with expected and actual reference locations on the remote +fn parse_git_push_output(output: Output) -> Result { + if output.status.success() { + return Ok(false); + } + + parse_no_such_remote(&output.stderr).map_err(GitError::NoSuchRemote)?; + if parse_force_with_lease_error(&output.stderr) { + return Ok(true); + } + + Err(external_git_error(&output.stderr)) +} 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/tests/test_git.rs b/lib/tests/test_git.rs index 8e8452d884..62a967feb0 100644 --- a/lib/tests/test_git.rs +++ b/lib/tests/test_git.rs @@ -114,6 +114,23 @@ fn get_git_repo(repo: &Arc) -> git2::Repository { get_git_backend(repo).open_git_repo().unwrap() } +fn get_git_executable_path(subprocess: bool) -> Option { + subprocess + .then(|| { + std::env::var("TEST_GIT_PATH") + .map(|x| x.into()) + .or_else(|e| { + if matches!(e, std::env::VarError::NotPresent) { + Ok("git".into()) + } else { + Err(e) + } + }) + }) + .transpose() + .unwrap() +} + #[test] fn test_import_refs() { let git_settings = GitSettings { @@ -2524,15 +2541,18 @@ 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_executable_path = get_git_executable_path(subprocess); let mut tx = test_data.repo.start_transaction(); let stats = git::fetch( tx.repo_mut(), &test_data.git_repo, + git_executable_path.as_deref(), "origin", &[StringPattern::everything()], git::RemoteCallbacks::default(), @@ -2547,19 +2567,22 @@ 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 git_settings = GitSettings { auto_local_bookmark: true, ..Default::default() }; let initial_git_commit = empty_git_commit(&test_data.origin_repo, "refs/heads/main", &[]); + let git_executable_path = get_git_executable_path(subprocess); let mut tx = test_data.repo.start_transaction(); let stats = git::fetch( tx.repo_mut(), &test_data.git_repo, + git_executable_path.as_deref(), "origin", &[StringPattern::everything()], git::RemoteCallbacks::default(), @@ -2598,8 +2621,9 @@ 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, @@ -2616,11 +2640,13 @@ fn test_fetch_initial_commit_head_is_set() { .origin_repo .reference("refs/tags/v1.0", new_git_commit.id(), false, "") .unwrap(); + let git_executable_path = get_git_executable_path(subprocess); let mut tx = test_data.repo.start_transaction(); let stats = git::fetch( tx.repo_mut(), &test_data.git_repo, + git_executable_path.as_deref(), "origin", &[StringPattern::everything()], git::RemoteCallbacks::default(), @@ -2633,19 +2659,22 @@ 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 git_settings = GitSettings { auto_local_bookmark: true, ..Default::default() }; let initial_git_commit = empty_git_commit(&test_data.origin_repo, "refs/heads/main", &[]); + let git_executable_path = get_git_executable_path(subprocess); let mut tx = test_data.repo.start_transaction(); git::fetch( tx.repo_mut(), &test_data.git_repo, + git_executable_path.as_deref(), "origin", &[StringPattern::everything()], git::RemoteCallbacks::default(), @@ -2670,6 +2699,7 @@ fn test_fetch_success() { let stats = git::fetch( tx.repo_mut(), &test_data.git_repo, + git_executable_path.as_deref(), "origin", &[StringPattern::everything()], git::RemoteCallbacks::default(), @@ -2715,19 +2745,22 @@ 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 git_settings = GitSettings { auto_local_bookmark: true, ..Default::default() }; let commit = empty_git_commit(&test_data.origin_repo, "refs/heads/main", &[]); + let git_executable_path = get_git_executable_path(subprocess); let mut tx = test_data.repo.start_transaction(); git::fetch( tx.repo_mut(), &test_data.git_repo, + git_executable_path.as_deref(), "origin", &[StringPattern::everything()], git::RemoteCallbacks::default(), @@ -2752,6 +2785,7 @@ fn test_fetch_prune_deleted_ref() { let stats = git::fetch( tx.repo_mut(), &test_data.git_repo, + git_executable_path.as_deref(), "origin", &[StringPattern::everything()], git::RemoteCallbacks::default(), @@ -2767,19 +2801,22 @@ 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 git_settings = GitSettings { auto_local_bookmark: true, ..Default::default() }; let initial_git_commit = empty_git_commit(&test_data.origin_repo, "refs/heads/main", &[]); + let git_executable_path = get_git_executable_path(subprocess); let mut tx = test_data.repo.start_transaction(); git::fetch( tx.repo_mut(), &test_data.git_repo, + git_executable_path.as_deref(), "origin", &[StringPattern::everything()], git::RemoteCallbacks::default(), @@ -2804,6 +2841,7 @@ fn test_fetch_no_default_branch() { let stats = git::fetch( tx.repo_mut(), &test_data.git_repo, + git_executable_path.as_deref(), "origin", &[StringPattern::everything()], git::RemoteCallbacks::default(), @@ -2815,17 +2853,20 @@ 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_executable_path = get_git_executable_path(subprocess); // Base refspecs shouldn't be respected let mut tx = test_data.repo.start_transaction(); git::fetch( tx.repo_mut(), &test_data.git_repo, + git_executable_path.as_deref(), "origin", &[], git::RemoteCallbacks::default(), @@ -2845,14 +2886,17 @@ 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_executable_path = get_git_executable_path(subprocess); let result = git::fetch( tx.repo_mut(), &test_data.git_repo, + git_executable_path.as_deref(), "invalid-remote", &[StringPattern::everything()], git::RemoteCallbacks::default(), @@ -2955,13 +2999,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_executable_path = get_git_executable_path(subprocess); let targets = GitBranchPushTargets { branch_updates: vec![( @@ -2975,6 +3021,7 @@ fn test_push_bookmarks_success() { let result = git::push_branches( tx.repo_mut(), &clone_repo, + git_executable_path.as_deref(), "origin", &targets, git::RemoteCallbacks::default(), @@ -3020,13 +3067,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_executable_path = get_git_executable_path(subprocess); let source_repo = git2::Repository::open(&setup.source_repo_dir).unwrap(); // Test the setup @@ -3044,6 +3093,7 @@ fn test_push_bookmarks_deletion() { let result = git::push_branches( tx.repo_mut(), &get_git_repo(&setup.jj_repo), + git_executable_path.as_deref(), "origin", &targets, git::RemoteCallbacks::default(), @@ -3072,13 +3122,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_executable_path = get_git_executable_path(subprocess); let targets = GitBranchPushTargets { branch_updates: vec![ @@ -3101,6 +3153,7 @@ fn test_push_bookmarks_mixed_deletion_and_addition() { let result = git::push_branches( tx.repo_mut(), &clone_repo, + git_executable_path.as_deref(), "origin", &targets, git::RemoteCallbacks::default(), @@ -3141,12 +3194,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_executable_path = get_git_executable_path(subprocess); let targets = GitBranchPushTargets { branch_updates: vec![( @@ -3160,6 +3215,7 @@ fn test_push_bookmarks_not_fast_forward() { let result = git::push_branches( tx.repo_mut(), &get_git_repo(&setup.jj_repo), + git_executable_path.as_deref(), "origin", &targets, git::RemoteCallbacks::default(), @@ -3179,11 +3235,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_executable_path = get_git_executable_path(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 @@ -3205,6 +3263,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_executable_path.as_deref(), "origin", &targets, git::RemoteCallbacks::default(), @@ -3244,11 +3303,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_executable_path = get_git_executable_path(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 @@ -3272,6 +3333,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_executable_path.as_deref(), "origin", &targets, git::RemoteCallbacks::default(), @@ -3298,19 +3360,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_executable_path = get_git_executable_path(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. @@ -3330,6 +3402,7 @@ fn test_push_updates_unexpectedly_exists_on_remote() { git::push_updates( setup.jj_repo.as_ref(), &get_git_repo(&setup.jj_repo), + git_executable_path.as_deref(), "origin", &targets, git::RemoteCallbacks::default(), @@ -3341,22 +3414,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_executable_path = get_git_executable_path(subprocess); let clone_repo = get_git_repo(&setup.jj_repo); let result = git::push_updates( setup.jj_repo.as_ref(), &clone_repo, + git_executable_path.as_deref(), "origin", &[GitRefUpdate { qualified_name: "refs/heads/main".to_string(), @@ -3386,14 +3471,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_executable_path = get_git_executable_path(subprocess); let result = git::push_updates( setup.jj_repo.as_ref(), &get_git_repo(&setup.jj_repo), + git_executable_path.as_deref(), "invalid-remote", &[GitRefUpdate { qualified_name: "refs/heads/main".to_string(), @@ -3405,14 +3493,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_executable_path = get_git_executable_path(subprocess); let result = git::push_updates( setup.jj_repo.as_ref(), &get_git_repo(&setup.jj_repo), + git_executable_path.as_deref(), "http://invalid-remote", &[GitRefUpdate { qualified_name: "refs/heads/main".to_string(),