From 86109b7641756678484a1fd2826bf47f7d83ead9 Mon Sep 17 00:00:00 2001 From: bsdinis Date: Wed, 25 Dec 2024 20:19:32 +0000 Subject: [PATCH] cli: shell out to git 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 shelling out to `git` 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] shell = true ``` Or via a CLI argument, e.g.: `jj git clone --shell ` 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 with shell-out enabled: ``` $ cargo build ``` Clone a private repo: ``` $ jj git clone --shell ``` Create new commit and push ``` $ echo "TEST" > this_is_a_test_file.txt $ jj describe -m 'test commit' $ jj git push --shell -b ``` --- cli/src/commands/git/clone.rs | 23 +- cli/src/commands/git/fetch.rs | 4 +- cli/src/commands/git/push.rs | 12 +- cli/src/config-schema.json | 5 + cli/src/git_util.rs | 66 ++-- cli/tests/cli-reference@.md.snap | 1 + cli/tests/test_git_clone.rs | 237 ++++++++++++-- cli/tests/test_git_fetch.rs | 344 +++++++++++++++++--- cli/tests/test_git_push.rs | 438 ++++++++++++++++++++++---- lib/src/config/misc.toml | 1 + lib/src/git.rs | 464 ++++++++++++++++++++++----- lib/src/git_shell.rs | 517 +++++++++++++++++++++++++++++++ lib/src/lib.rs | 1 + lib/tests/test_git.rs | 111 ++++--- 14 files changed, 1943 insertions(+), 281 deletions(-) create mode 100644 lib/src/git_shell.rs diff --git a/cli/src/commands/git/clone.rs b/cli/src/commands/git/clone.rs index 743fd5e844..b357a301da 100644 --- a/cli/src/commands/git/clone.rs +++ b/cli/src/commands/git/clone.rs @@ -34,6 +34,7 @@ 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_shell; use crate::git_util::get_git_repo; use crate::git_util::map_git_error; use crate::git_util::print_git_import_stats; @@ -129,6 +130,7 @@ 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 shell = get_config_git_shell(command)?; let clone_result = do_git_clone( ui, command, @@ -137,6 +139,7 @@ pub fn cmd_git_clone( remote_name, &source, &canonical_wc_path, + shell, ); if clone_result.is_err() { let clean_up_dirs = || -> io::Result<()> { @@ -196,6 +199,7 @@ fn do_git_clone( remote_name: &str, source: &str, wc_path: &Path, + shell: bool, ) -> Result<(WorkspaceCommandHelper, GitFetchStats), CommandError> { let (workspace, repo) = if colocate { Workspace::init_colocated_git(command.settings(), wc_path)? @@ -214,7 +218,7 @@ fn do_git_clone( let mut fetch_tx = workspace_command.start_transaction(); let git_settings = command.settings().git_settings()?; - let stats = with_remote_git_callbacks(ui, None, |cb| { + let stats = with_remote_git_callbacks(ui, None, shell, |cb| { git::fetch( fetch_tx.repo_mut(), &git_repo, @@ -223,14 +227,27 @@ fn do_git_clone( cb, &git_settings, depth, + shell, ) }) .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::GitImportError(err) => CommandError::from(err), GitFetchError::InternalGitError(err) => map_git_error(err), + GitFetchError::GitForkError(err) => CommandError::with_message( + crate::command_error::CommandErrorKind::Internal, + "external git process failed", + err, + ), + GitFetchError::ExternalGitError(err) => { + CommandError::new(crate::command_error::CommandErrorKind::Internal, err) + } + GitFetchError::PathConversionError(path) => CommandError::new( + crate::command_error::CommandErrorKind::Internal, + format!("failed to convert path {} to string", path.display()), + ), 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 bf1226d436..35f0c334f9 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_shell; 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 shell_flag = get_config_git_shell(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,7 @@ 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, &remotes, &args.branch, shell_flag)?; 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 92e014f302..08f11eb2e5 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_shell; 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 shell_flag = get_config_git_shell(command)?; let git_repo = get_git_repo(workspace_command.repo().store())?; let remote = if let Some(name) = &args.remote { @@ -313,9 +315,13 @@ 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), + shell_flag, + |cb| git::push_branches(tx.repo_mut(), &git_repo, &remote, &targets, cb, shell_flag), + ) .map_err(|err| match err { GitPushError::InternalGitError(err) => map_git_error(err), GitPushError::RefInUnexpectedLocation(refs) => user_error_with_hint( diff --git a/cli/src/config-schema.json b/cli/src/config-schema.json index 8a02e71165..9b95ef8055 100644 --- a/cli/src/config-schema.json +++ b/cli/src/config-schema.json @@ -362,6 +362,11 @@ "type": "string", "description": "The remote to which commits are pushed", "default": "origin" + }, + "shell": { + "type": "boolean", + "description": "Whether jj shells out to git for network operations (push/fetch/clone)", + "default": false } } }, diff --git a/cli/src/git_util.rs b/cli/src/git_util.rs index 00d97d6288..4fb9edd74c 100644 --- a/cli/src/git_util.rs +++ b/cli/src/git_util.rs @@ -24,6 +24,7 @@ use std::process::Stdio; use std::time::Instant; use itertools::Itertools; +use jj_lib::config::ConfigGetError; use jj_lib::git; use jj_lib::git::FailedRefExport; use jj_lib::git::FailedRefExportReason; @@ -40,6 +41,7 @@ use jj_lib::str_util::StringPattern; use jj_lib::workspace::Workspace; use unicode_width::UnicodeWidthStr; +use crate::cli_util::CommandHelper; use crate::cli_util::WorkspaceCommandTransaction; use crate::command_error::user_error; use crate::command_error::user_error_with_hint; @@ -257,29 +259,35 @@ type SidebandProgressCallback<'a> = &'a mut dyn FnMut(&[u8]); pub fn with_remote_git_callbacks( ui: &Ui, sideband_progress_callback: Option>, + shell: 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 shell { + 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( @@ -462,11 +470,12 @@ pub fn git_fetch( git_repo: &git2::Repository, remotes: &[String], branch: &[StringPattern], + shell: bool, ) -> 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, shell, |cb| { git::fetch( tx.repo_mut(), git_repo, @@ -475,6 +484,7 @@ pub fn git_fetch( cb, &git_settings, None, + shell, ) }) .map_err(|err| match err { @@ -535,3 +545,17 @@ fn warn_if_branches_not_found( Ok(()) } + +pub(crate) fn get_config_git_shell(command: &CommandHelper) -> Result { + command + .config_env() + .resolve_config(command.raw_config())? + .get::("git.shell") + .or_else(|e| { + if matches!(e, ConfigGetError::NotFound { .. }) { + Ok(false) + } else { + Err(e) + } + }) +} diff --git a/cli/tests/cli-reference@.md.snap b/cli/tests/cli-reference@.md.snap index 49fe4c2156..9ed477a40e 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/test_git_clone.rs b/cli/tests/test_git_clone.rs index d0ebdedca6..470f701e77 100644 --- a/cli/tests/test_git_clone.rs +++ b/cli/tests/test_git_clone.rs @@ -16,6 +16,8 @@ use std::path; use std::path::Path; use std::path::PathBuf; +use test_case::test_case; + use crate::common::get_stderr_string; use crate::common::get_stdout_string; use crate::common::TestEnvironment; @@ -47,27 +49,34 @@ 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; "shell out to git for remote calls")] +fn test_git_clone(shell: bool) { let test_env = TestEnvironment::default(); test_env.add_config("git.auto-local-bookmark = true"); + if shell { + test_env.add_config("git.shell = true") + } 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::allow_duplicates! { insta::assert_snapshot!(stdout, @""); 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::assert_snapshot!(stderr, @r###" Fetching into new repo in "$TEST_ENV/clone" @@ -77,15 +86,18 @@ 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::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(); @@ -95,11 +107,13 @@ 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::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 @@ -110,46 +124,57 @@ 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::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::assert_snapshot!(stderr, @r###" Fetching into new repo in "$TEST_ENV/nested/path/to/repo" @@ -159,30 +184,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; "shell out to git for remote calls")] +fn test_git_clone_bad_source(shell: bool) { let test_env = TestEnvironment::default(); + if shell { + test_env.add_config("git.shell = true") + } 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; "shell out to git for remote calls")] +fn test_git_clone_colocate(shell: bool) { let test_env = TestEnvironment::default(); test_env.add_config("git.auto-local-bookmark = true"); + if shell { + test_env.add_config("git.shell = true") + } let git_repo_path = test_env.env_root().join("source"); let git_repo = git2::Repository::init(git_repo_path).unwrap(); @@ -191,20 +229,24 @@ fn test_git_clone_colocate() { test_env.env_root(), &["git", "clone", "source", "empty", "--colocate"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); 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); @@ -213,6 +255,7 @@ fn test_git_clone_colocate() { test_env.env_root(), &["git", "clone", "source", "clone", "--colocate"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r###" Fetching into new repo in "$TEST_ENV/clone" @@ -222,6 +265,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()); @@ -250,6 +294,7 @@ 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/ @@ -263,14 +308,17 @@ fn test_git_clone_colocate() { @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::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(); @@ -283,11 +331,13 @@ 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::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 @@ -301,11 +351,13 @@ 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::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()); @@ -315,9 +367,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()); @@ -326,9 +380,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(); @@ -336,9 +392,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(); @@ -346,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 "###); + } // Clone into a nested path let (stdout, stderr) = test_env.jj_cmd_ok( @@ -361,6 +421,7 @@ fn test_git_clone_colocate() { "--colocate", ], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r###" Fetching into new repo in "$TEST_ENV/nested/path/to/repo" @@ -370,11 +431,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; "shell out to git for remote calls")] +fn test_git_clone_remote_default_bookmark(shell: bool) { let test_env = TestEnvironment::default(); + if shell { + test_env.add_config("git.shell = true") + } let git_repo_path = test_env.env_root().join("source"); let git_repo = git2::Repository::init(git_repo_path).unwrap(); set_up_non_empty_git_repo(&git_repo); @@ -392,6 +458,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 @@ -408,20 +475,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 @@ -437,11 +508,13 @@ fn test_git_clone_remote_default_bookmark() { 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 @@ -457,20 +530,27 @@ fn test_git_clone_remote_default_bookmark() { 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; "shell out to git for remote calls")] +fn test_git_clone_ignore_working_copy(shell: bool) { let test_env = TestEnvironment::default(); + if shell { + test_env.add_config("git.shell = true") + } let git_repo_path = test_env.env_root().join("source"); let git_repo = git2::Repository::init(git_repo_path).unwrap(); set_up_non_empty_git_repo(&git_repo); @@ -480,33 +560,43 @@ 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::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; "shell out to git for remote calls")] +fn test_git_clone_at_operation(shell: bool) { let test_env = TestEnvironment::default(); + if shell { + test_env.add_config("git.shell = true") + } let git_repo_path = test_env.env_root().join("source"); let git_repo = git2::Repository::init(git_repo_path).unwrap(); set_up_non_empty_git_repo(&git_repo); @@ -515,15 +605,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; "shell out to git for remote calls")] +fn test_git_clone_with_remote_name(shell: bool) { let test_env = TestEnvironment::default(); test_env.add_config("git.auto-local-bookmark = true"); + if shell { + test_env.add_config("git.shell = true") + } let git_repo_path = test_env.env_root().join("source"); let git_repo = git2::Repository::init(git_repo_path).unwrap(); set_up_non_empty_git_repo(&git_repo); @@ -533,6 +629,7 @@ 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::assert_snapshot!(stderr, @r#" Fetching into new repo in "$TEST_ENV/clone" @@ -542,11 +639,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; "shell out to git for remote calls")] +fn test_git_clone_trunk_deleted(shell: bool) { let test_env = TestEnvironment::default(); + if shell { + test_env.add_config("git.shell = true") + } let git_repo_path = test_env.env_root().join("source"); let git_repo = git2::Repository::init(git_repo_path).unwrap(); set_up_non_empty_git_repo(&git_repo); @@ -554,6 +656,7 @@ 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::assert_snapshot!(stderr, @r#" Fetching into new repo in "$TEST_ENV/clone" @@ -563,16 +666,20 @@ 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::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) @@ -584,10 +691,11 @@ fn test_git_clone_trunk_deleted() { 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] -fn test_git_clone_with_depth() { +fn test_git_clone_with_depth_git2() { let test_env = TestEnvironment::default(); test_env.add_config("git.auto-local-bookmark = true"); let git_repo_path = test_env.env_root().join("source"); @@ -600,15 +708,64 @@ fn test_git_clone_with_depth() { 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) "#); + } } +// this test has slightly different behaviour from git2, because it doesn't +// support shallow clones on a local transport #[test] -fn test_git_clone_invalid_immutable_heads() { +fn test_git_clone_with_depth_shell() { + let test_env = TestEnvironment::default(); + test_env.add_config("git.auto-local-bookmark = true"); + test_env.add_config("git.shell = true"); + let git_repo_path = test_env.env_root().join("source"); + let clone_path = test_env.env_root().join("clone"); + let git_repo = git2::Repository::init(git_repo_path).unwrap(); + set_up_non_empty_git_repo(&git_repo); + + // local transport *does* work in normal git + // we check everything works + 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::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::assert_snapshot!(stderr, @""); + } +} + +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "shell out to git for remote calls")] +fn test_git_clone_invalid_immutable_heads(shell: bool) { let test_env = TestEnvironment::default(); + if shell { + test_env.add_config("git.shell = true") + } let git_repo_path = test_env.env_root().join("source"); let git_repo = git2::Repository::init(git_repo_path).unwrap(); set_up_non_empty_git_repo(&git_repo); @@ -620,6 +777,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 @@ -627,11 +785,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; "shell out to git for remote calls")] +fn test_git_clone_malformed(shell: bool) { let test_env = TestEnvironment::default(); + if shell { + test_env.add_config("git.shell = true") + } 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"); @@ -642,6 +805,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 @@ -649,31 +813,40 @@ 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) "#); + } } 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..36f851b7a6 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,43 +74,60 @@ 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; "shell out to git for remote calls")] +fn test_git_fetch_with_default_config(shell: bool) { let test_env = TestEnvironment::default(); + if shell { + test_env.add_config("git.shell = 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@origin: oputwtnw ffecd2d6 message "###); + } } -#[test] -fn test_git_fetch_default_remote() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "shell out to git for remote calls")] +fn test_git_fetch_default_remote(shell: bool) { let test_env = TestEnvironment::default(); + if shell { + test_env.add_config("git.shell = true") + } 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; "shell out to git for remote calls")] +fn test_git_fetch_single_remote(shell: bool) { let test_env = TestEnvironment::default(); + if shell { + test_env.add_config("git.shell = true") + } 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 @@ -117,11 +136,16 @@ fn test_git_fetch_single_remote() { 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; "shell out to git for remote calls")] +fn test_git_fetch_single_remote_all_remotes_flag(shell: bool) { let test_env = TestEnvironment::default(); + if shell { + test_env.add_config("git.shell = true") + } 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 +155,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; "shell out to git for remote calls")] +fn test_git_fetch_single_remote_from_arg(shell: bool) { let test_env = TestEnvironment::default(); + if shell { + test_env.add_config("git.shell = true") + } 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; "shell out to git for remote calls")] +fn test_git_fetch_single_remote_from_config(shell: bool) { let test_env = TestEnvironment::default(); + if shell { + test_env.add_config("git.shell = true") + } 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 +198,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; "shell out to git for remote calls")] +fn test_git_fetch_multiple_remotes(shell: bool) { let test_env = TestEnvironment::default(); + if shell { + test_env.add_config("git.shell = true") + } 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 +223,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; "shell out to git for remote calls")] +fn test_git_fetch_all_remotes(shell: bool) { let test_env = TestEnvironment::default(); + if shell { + test_env.add_config("git.shell = true") + } 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 +247,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; "shell out to git for remote calls")] +fn test_git_fetch_multiple_remotes_from_config(shell: bool) { let test_env = TestEnvironment::default(); + if shell { + test_env.add_config("git.shell = true") + } 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 +272,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; "shell out to git for remote calls")] +fn test_git_fetch_nonexistent_remote(shell: bool) { let test_env = TestEnvironment::default(); + if shell { + test_env.add_config("git.shell = 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"); @@ -237,34 +297,46 @@ 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' "###); // 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; "shell out to git for remote calls")] +fn test_git_fetch_nonexistent_remote_from_config(shell: bool) { let test_env = TestEnvironment::default(); + if shell { + test_env.add_config("git.shell = 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.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; "shell out to git for remote calls")] +fn test_git_fetch_from_remote_named_git(shell: bool) { let test_env = TestEnvironment::default(); + if shell { + test_env.add_config("git.shell = true") + } 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,14 +348,17 @@ 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::assert_snapshot!(stderr, @""); @@ -294,10 +369,12 @@ 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 @@ -306,20 +383,27 @@ fn test_git_fetch_from_remote_named_git() { 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; "shell out to git for remote calls")] +fn test_git_fetch_prune_before_updating_tips(shell: bool) { let test_env = TestEnvironment::default(); + if shell { + test_env.add_config("git.shell = true") + } 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 +414,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; "shell out to git for remote calls")] +fn test_git_fetch_conflicting_bookmarks(shell: bool) { let test_env = TestEnvironment::default(); + if shell { + test_env.add_config("git.shell = true") + } 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 +437,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; "shell out to git for remote calls")] +fn test_git_fetch_conflicting_bookmarks_colocated(shell: bool) { let test_env = TestEnvironment::default(); + if shell { + test_env.add_config("git.shell = true") + } 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 +491,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 +499,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 +531,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; "shell out to git for remote calls")] +fn test_git_fetch_all(shell: bool) { let test_env = TestEnvironment::default(); + if shell { + test_env.add_config("git.shell = true") + } 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 +546,18 @@ 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::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 @@ -493,10 +604,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 +621,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 +629,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* @@ -574,11 +689,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; "shell out to git for remote calls")] +fn test_git_fetch_some_of_many_bookmarks(shell: bool) { let test_env = TestEnvironment::default(); + if shell { + test_env.add_config("git.shell = true") + } 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 +707,18 @@ 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::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,14 +729,18 @@ 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 @@ -624,9 +751,11 @@ fn test_git_fetch_some_of_many_bookmarks() { @ 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::assert_snapshot!(stderr, @r###" bookmark: b@origin [new] tracked @@ -643,11 +772,13 @@ fn test_git_fetch_some_of_many_bookmarks() { 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::assert_snapshot!(stderr, @r###" bookmark: a1@origin [new] tracked @@ -664,9 +795,11 @@ 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::assert_snapshot!(stderr, @r###" Nothing changed. @@ -682,10 +815,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 +832,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 +840,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,10 +852,12 @@ 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::assert_snapshot!(stderr, @r###" bookmark: a1@origin [updated] tracked @@ -752,12 +891,14 @@ 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::assert_snapshot!(stderr, @r###" bookmark: a2@origin [updated] tracked @@ -788,11 +929,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; "shell out to git for remote calls")] +fn test_git_fetch_bookmarks_some_missing(shell: bool) { let test_env = TestEnvironment::default(); + if shell { + test_env.add_config("git.shell = true") + } 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 +949,13 @@ 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::assert_snapshot!(get_bookmark_output(&test_env, &repo_path), @""); + } // multiple missing bookmarks, implicit remotes (@origin) let (_stdout, stderr) = test_env.jj_cmd_ok( @@ -816,15 +964,18 @@ 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::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 "###); @@ -832,6 +983,7 @@ fn test_git_fetch_bookmarks_some_missing() { origin: oputwtnw ffecd2d6 message @origin: oputwtnw ffecd2d6 message "###); + } // multiple existing bookmark, explicit remotes, each bookmark is only in one // remote. @@ -842,6 +994,7 @@ 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 @@ -854,6 +1007,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,6 +1016,7 @@ 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. @@ -874,13 +1029,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; "shell out to git for remote calls")] +fn test_git_fetch_undo(shell: bool) { let test_env = TestEnvironment::default(); + if shell { + test_env.add_config("git.shell = true") + } 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 +1048,18 @@ 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::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,12 +1070,14 @@ 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::assert_snapshot!(stderr, @r###" bookmark: a1@origin [new] tracked @@ -927,7 +1092,9 @@ 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::assert_snapshot!(stderr, @r#" Undid operation: eb2029853b02 (2001-02-03 08:05:18) fetch from git remote(s) origin @@ -937,9 +1104,11 @@ 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::assert_snapshot!(stderr, @r###" bookmark: b@origin [new] tracked @@ -951,13 +1120,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; "shell out to git for remote calls")] +fn test_fetch_undo_what(shell: bool) { let test_env = TestEnvironment::default(); + if shell { + test_env.add_config("git.shell = true") + } 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 +1139,18 @@ 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::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 @@ -988,10 +1165,12 @@ fn test_fetch_undo_what() { // Initial state we will try to return to after `op restore`. There are no // bookmarks. 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::assert_snapshot!(stderr, @r###" bookmark: b@origin [new] tracked @@ -1007,6 +1186,7 @@ fn test_fetch_undo_what() { 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,6 +1194,7 @@ fn test_fetch_undo_what() { &repo_path, &["op", "restore", "--what", "repo", &base_operation_id], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r#" Restored to operation: eac759b9ab75 (2001-02-03 08:05:07) add workspace 'default' @@ -1022,15 +1203,18 @@ fn test_fetch_undo_what() { 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,6 +1227,7 @@ fn test_fetch_undo_what() { &base_operation_id, ], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r#" Restored to operation: eac759b9ab75 (2001-02-03 08:05:07) add workspace 'default' @@ -1050,40 +1235,52 @@ fn test_fetch_undo_what() { 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; "shell out to git for remote calls")] +fn test_git_fetch_remove_fetch(shell: bool) { let test_env = TestEnvironment::default(); + if shell { + test_env.add_config("git.shell = true") + } 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::assert_snapshot!(stderr, @r###" bookmark: origin@origin [new] tracked @@ -1094,52 +1291,69 @@ fn test_git_fetch_remove_fetch() { + 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; "shell out to git for remote calls")] +fn test_git_fetch_rename_fetch(shell: bool) { let test_env = TestEnvironment::default(); + if shell { + test_env.add_config("git.shell = true") + } 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::assert_snapshot!(stderr, @r###" Nothing changed. "###); + } } -#[test] -fn test_git_fetch_removed_bookmark() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "shell out to git for remote calls")] +fn test_git_fetch_removed_bookmark(shell: bool) { let test_env = TestEnvironment::default(); + if shell { + test_env.add_config("git.shell = true") + } 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 +1361,18 @@ 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::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,9 +1383,11 @@ 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::assert_snapshot!(stderr, @r###" bookmark: a1@origin [new] tracked @@ -1187,6 +1406,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,6 +1414,7 @@ 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::assert_snapshot!(stderr, @r###" Nothing changed. @@ -1209,10 +1430,12 @@ 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::assert_snapshot!(stderr, @r###" bookmark: a2@origin [deleted] untracked @@ -1227,11 +1450,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; "shell out to git for remote calls")] +fn test_git_fetch_removed_parent_bookmark(shell: bool) { let test_env = TestEnvironment::default(); + if shell { + test_env.add_config("git.shell = true") + } 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 +1467,18 @@ 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::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,9 +1489,11 @@ 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::assert_snapshot!(stderr, @r###" bookmark: a1@origin [new] tracked @@ -1279,6 +1512,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,6 +1526,7 @@ fn test_git_fetch_removed_parent_bookmark() { "git", "fetch", "--branch", "master", "--branch", "trunk1", "--branch", "a1", ], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r###" bookmark: a1@origin [deleted] untracked @@ -1308,11 +1543,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; "shell out to git for remote calls")] +fn test_git_fetch_remote_only_bookmark(shell: bool) { let test_env = TestEnvironment::default(); + if shell { + test_env.add_config("git.shell = true") + } test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); let repo_path = test_env.env_root().join("repo"); @@ -1347,10 +1587,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,6 +1608,7 @@ 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 @@ -1377,4 +1620,5 @@ fn test_git_fetch_remote_only_bookmark() { @origin: mzyxwzks 9f01a0e0 message feature2@origin: mzyxwzks 9f01a0e0 message "###); + } } diff --git a/cli/tests/test_git_push.rs b/cli/tests/test_git_push.rs index 3a868329d9..7cd82fa9cb 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,39 @@ 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; "shell out to git for remote calls")] +fn test_git_push_nothing(shell: bool) { let (test_env, workspace_root) = set_up(); + if shell { + test_env.add_config("git.shell = true") + } // 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::assert_snapshot!(stderr, @r###" Nothing changed. "###); + } } -#[test] -fn test_git_push_current_bookmark() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "shell out to git for remote calls")] +fn test_git_push_current_bookmark(shell: bool) { let (test_env, workspace_root) = set_up(); + if shell { + test_env.add_config("git.shell = true") + } 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 +94,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,11 +102,13 @@ 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::assert_snapshot!(stderr, @r#" Changes to push to origin: @@ -99,7 +116,9 @@ fn test_git_push_current_bookmark() { 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::assert_snapshot!(stderr, @r#" Changes to push to origin: @@ -114,6 +133,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 +149,31 @@ 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::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::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; "shell out to git for remote calls")] +fn test_git_push_parent_bookmark(shell: bool) { let (test_env, workspace_root) = set_up(); + if shell { + test_env.add_config("git.shell = true") + } 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 +183,61 @@ 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::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; "shell out to git for remote calls")] +fn test_git_push_no_matching_bookmark(shell: bool) { let (test_env, workspace_root) = set_up(); + if shell { + test_env.add_config("git.shell = true") + } 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::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; "shell out to git for remote calls")] +fn test_git_push_matching_bookmark_unchanged(shell: bool) { let (test_env, workspace_root) = set_up(); + if shell { + test_env.add_config("git.shell = true") + } 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::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; "shell out to git for remote calls")] +fn test_git_push_other_remote_has_bookmark(shell: bool) { let (test_env, workspace_root) = set_up(); + if shell { + test_env.add_config("git.shell = true") + } 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 +261,22 @@ 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::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::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 +289,22 @@ fn test_git_push_other_remote_has_bookmark() { &workspace_root, &["git", "push", "--allow-new", "--remote=other"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); 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; "shell out to git for remote calls")] +fn test_git_push_forward_unexpectedly_moved(shell: bool) { let (test_env, workspace_root) = set_up(); + if shell { + test_env.add_config("git.shell = true") + } // Move bookmark1 forward on the remote let origin_path = test_env.env_root().join("origin"); @@ -264,29 +320,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; "shell out to git for remote calls")] +fn test_git_push_sideways_unexpectedly_moved(shell: bool) { let (test_env, workspace_root) = set_up(); + if shell { + test_env.add_config("git.shell = true") + } // 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 +360,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; "shell out to git for remote calls")] +fn test_git_push_deletion_unexpectedly_moved(shell: bool) { let (test_env, workspace_root) = set_up(); + if shell { + test_env.add_config("git.shell = true") + } // 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; "shell out to git for remote calls")] +fn test_git_push_unexpectedly_deleted(shell: bool) { let (test_env, workspace_root) = set_up(); + if shell { + test_env.add_config("git.shell = true") + } // 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 +456,54 @@ 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::allow_duplicates! { 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; "shell out to git for remote calls")] +fn test_git_push_creation_unexpectedly_already_exists(shell: bool) { let (test_env, workspace_root) = set_up(); + if shell { + test_env.add_config("git.shell = true") + } // Forget bookmark1 locally test_env.jj_cmd_ok(&workspace_root, &["bookmark", "forget", "bookmark1"]); @@ -416,24 +512,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; "shell out to git for remote calls")] +fn test_git_push_locally_created_and_rewritten(shell: bool) { let (test_env, workspace_root) = set_up(); + if shell { + test_env.add_config("git.shell = true") + } // Ensure that remote bookmarks aren't tracked automatically test_env.add_config("git.auto-local-bookmark = false"); @@ -441,20 +545,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 +572,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; "shell out to git for remote calls")] +fn test_git_push_multiple(shell: bool) { let (test_env, workspace_root) = set_up(); + if shell { + test_env.add_config("git.shell = true") + } test_env.jj_cmd_ok(&workspace_root, &["bookmark", "delete", "bookmark1"]); test_env.jj_cmd_ok( &workspace_root, @@ -481,6 +597,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,9 +605,11 @@ 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::assert_snapshot!(stderr, @r#" Changes to push to origin: @@ -499,6 +618,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,6 +631,7 @@ fn test_git_push_multiple() { "--dry-run", ], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r#" Changes to push to origin: @@ -518,6 +639,7 @@ fn test_git_push_multiple() { 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,6 +654,7 @@ fn test_git_push_multiple() { "--dry-run", ], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r#" Changes to push to origin: @@ -539,11 +662,13 @@ fn test_git_push_multiple() { 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::assert_snapshot!(stderr, @r#" Changes to push to origin: @@ -551,21 +676,27 @@ fn test_git_push_multiple() { 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::assert_snapshot!(stderr, @r#" Changes to push to origin: @@ -579,7 +710,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 +722,34 @@ 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; "shell out to git for remote calls")] +fn test_git_push_changes(shell: bool) { let (test_env, workspace_root) = set_up(); + if shell { + test_env.add_config("git.shell = true") + } 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::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,8 +757,10 @@ 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::assert_snapshot!(stderr, @r#" Creating bookmark push-yqosqzytrlsw for revision yqosqzytrlsw @@ -625,14 +768,17 @@ fn test_git_push_changes() { 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::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 +786,13 @@ fn test_git_push_changes() { &workspace_root, &["git", "push", "-c=@", "-b=push-yostqsxwqrlt"], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); 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 +807,34 @@ 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::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 +846,14 @@ fn test_git_push_changes() { "--change=@", ], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); 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,6 +865,7 @@ fn test_git_push_changes() { "--change=@", ], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r#" Warning: Config git.push-branch-prefix is deprecated. Please switch to git.push-bookmark-prefix @@ -716,11 +873,16 @@ fn test_git_push_changes() { 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; "shell out to git for remote calls")] +fn test_git_push_revisions(shell: bool) { let (test_env, workspace_root) = set_up(); + if shell { + test_env.add_config("git.shell = true") + } 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,34 +898,41 @@ 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::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::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::assert_snapshot!(stderr, @r#" Warning: No bookmarks point to the specified revisions: @-- @@ -771,11 +940,13 @@ fn test_git_push_revisions() { 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::assert_snapshot!(stderr, @r#" Changes to push to origin: @@ -783,22 +954,29 @@ fn test_git_push_revisions() { 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::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; "shell out to git for remote calls")] +fn test_git_push_mixed(shell: bool) { let (test_env, workspace_root) = set_up(); + if shell { + test_env.add_config("git.shell = true") + } 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 +998,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,6 +1017,7 @@ fn test_git_push_mixed() { "-r=@", ], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r#" Creating bookmark push-yqosqzytrlsw for revision yqosqzytrlsw @@ -846,11 +1027,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; "shell out to git for remote calls")] +fn test_git_push_existing_long_bookmark(shell: bool) { let (test_env, workspace_root) = set_up(); + if shell { + test_env.add_config("git.shell = true") + } 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 +1049,22 @@ 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::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; "shell out to git for remote calls")] +fn test_git_push_unsnapshotted_change(shell: bool) { let (test_env, workspace_root) = set_up(); + if shell { + test_env.add_config("git.shell = true") + } 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 +1072,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; "shell out to git for remote calls")] +fn test_git_push_conflict(shell: bool) { let (test_env, workspace_root) = set_up(); + if shell { + test_env.add_config("git.shell = true") + } 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 +1088,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; "shell out to git for remote calls")] +fn test_git_push_no_description(shell: bool) { let (test_env, workspace_root) = set_up(); + if shell { + test_env.add_config("git.shell = true") + } 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 +1128,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; "shell out to git for remote calls")] +fn test_git_push_no_description_in_immutable(shell: bool) { let (test_env, workspace_root) = set_up(); + if shell { + test_env.add_config("git.shell = true") + } 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 +1151,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 +1169,23 @@ fn test_git_push_no_description_in_immutable() { "--dry-run", ], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); 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; "shell out to git for remote calls")] +fn test_git_push_missing_author(shell: bool) { let (test_env, workspace_root) = set_up(); + if shell { + test_env.add_config("git.shell = true") + } let run_without_var = |var: &str, args: &[&str]| { test_env .jj_cmd(&workspace_root, args) @@ -983,25 +1199,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; "shell out to git for remote calls")] +fn test_git_push_missing_author_in_immutable(shell: bool) { let (test_env, workspace_root) = set_up(); + if shell { + test_env.add_config("git.shell = true") + } let run_without_var = |var: &str, args: &[&str]| { test_env .jj_cmd(&workspace_root, args) @@ -1026,10 +1250,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 +1268,23 @@ fn test_git_push_missing_author_in_immutable() { "--dry-run", ], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); 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; "shell out to git for remote calls")] +fn test_git_push_missing_committer(shell: bool) { let (test_env, workspace_root) = set_up(); + if shell { + test_env.add_config("git.shell = true") + } let run_without_var = |var: &str, args: &[&str]| { test_env .jj_cmd(&workspace_root, args) @@ -1066,10 +1298,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 +1311,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 +1325,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; "shell out to git for remote calls")] +fn test_git_push_missing_committer_in_immutable(shell: bool) { let (test_env, workspace_root) = set_up(); + if shell { + test_env.add_config("git.shell = true") + } let run_without_var = |var: &str, args: &[&str]| { test_env .jj_cmd(&workspace_root, args) @@ -1123,10 +1365,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 +1383,35 @@ fn test_git_push_missing_committer_in_immutable() { "--dry-run", ], ); + insta::allow_duplicates! { insta::assert_snapshot!(stdout, @""); 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; "shell out to git for remote calls")] +fn test_git_push_deleted(shell: bool) { let (test_env, workspace_root) = set_up(); + if shell { + test_env.add_config("git.shell = true") + } 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::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 +1421,23 @@ 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::assert_snapshot!(stderr, @r###" Nothing changed. "###); + } } -#[test] -fn test_git_push_conflicting_bookmarks() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "shell out to git for remote calls")] +fn test_git_push_conflicting_bookmarks(shell: bool) { let (test_env, workspace_root) = set_up(); + if shell { + test_env.add_config("git.shell = true") + } test_env.add_config("git.auto-local-bookmark = true"); let git_repo = { let mut git_repo_path = workspace_root.clone(); @@ -1195,6 +1455,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 +1464,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,26 +1473,31 @@ 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::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::assert_snapshot!(stderr, @r#" Warning: Bookmark bookmark2 is conflicted @@ -1238,11 +1505,13 @@ fn test_git_push_conflicting_bookmarks() { 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::assert_snapshot!(stderr, @r#" Warning: Bookmark bookmark2 is conflicted @@ -1250,11 +1519,16 @@ fn test_git_push_conflicting_bookmarks() { 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; "shell out to git for remote calls")] +fn test_git_push_deleted_untracked(shell: bool) { let (test_env, workspace_root) = set_up(); + if shell { + test_env.add_config("git.shell = true") + } // Absent local bookmark shouldn't be considered "deleted" compared to // non-tracking remote bookmark. @@ -1264,18 +1538,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; "shell out to git for remote calls")] +fn test_git_push_tracked_vs_all(shell: bool) { let (test_env, workspace_root) = set_up(); + if shell { + test_env.add_config("git.shell = true") + } 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 +1567,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 +1575,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 +1628,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; "shell out to git for remote calls")] +fn test_git_push_moved_forward_untracked(shell: bool) { let (test_env, workspace_root) = set_up(); + if shell { + test_env.add_config("git.shell = true") + } test_env.jj_cmd_ok(&workspace_root, &["new", "bookmark1", "-mmoved bookmark1"]); test_env.jj_cmd_ok(&workspace_root, &["bookmark", "set", "bookmark1"]); @@ -1357,16 +1653,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; "shell out to git for remote calls")] +fn test_git_push_moved_sideways_untracked(shell: bool) { let (test_env, workspace_root) = set_up(); + if shell { + test_env.add_config("git.shell = true") + } test_env.jj_cmd_ok(&workspace_root, &["new", "root()", "-mmoved bookmark1"]); test_env.jj_cmd_ok( @@ -1378,16 +1680,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; "shell out to git for remote calls")] +fn test_git_push_to_remote_named_git(shell: bool) { let (test_env, workspace_root) = set_up(); + if shell { + test_env.add_config("git.shell = true") + } let git_repo = { let mut git_repo_path = workspace_root.clone(); git_repo_path.extend([".jj", "repo", "store", "git"]); @@ -1397,12 +1705,14 @@ fn test_git_push_to_remote_named_git() { let stderr = test_env.jj_cmd_failure(&workspace_root, &["git", "push", "--all", "--remote=git"]); + insta::allow_duplicates! { insta::assert_snapshot!(stderr, @r#" Changes to push to git: Add bookmark bookmark1 to d13ecdbda2a2 Add bookmark bookmark2 to 8476341eb395 Error: Git remote named 'git' is reserved for local Git repository "#); + } } fn get_bookmark_output(test_env: &TestEnvironment, repo_path: &Path) -> String { diff --git a/lib/src/config/misc.toml b/lib/src/config/misc.toml index d432be25f9..a19b5a374d 100644 --- a/lib/src/config/misc.toml +++ b/lib/src/config/misc.toml @@ -12,6 +12,7 @@ register_snapshot_trigger = false [git] abandon-unreachable-commits = true # auto-local-bookmark = false +# shell = false [operation] # hostname = diff --git a/lib/src/git.rs b/lib/src/git.rs index e52e9baa91..c9f00b5757 100644 --- a/lib/src/git.rs +++ b/lib/src/git.rs @@ -19,9 +19,11 @@ use std::collections::BTreeMap; use std::collections::HashMap; use std::collections::HashSet; use std::default::Default; +use std::ffi::OsStr; use std::fmt; use std::io::Read; use std::num::NonZeroU32; +use std::path::Path; use std::path::PathBuf; use std::str; @@ -34,6 +36,7 @@ use crate::backend::BackendError; use crate::backend::CommitId; use crate::commit::Commit; use crate::git_backend::GitBackend; +use crate::git_shell; use crate::index::Index; use crate::object_id::ObjectId; use crate::op_store::RefTarget; @@ -110,6 +113,34 @@ fn to_remote_branch<'a>(parsed_ref: &'a RefName, remote_name: &str) -> Option<&' } } +/// 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 { + if git_dir.file_name() == Some(OsStr::new(".git")) { + git_dir + .parent() + .map(|x| x.to_path_buf()) + .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().map(|x| x.to_path_buf()).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() + )) + } +} + /// Returns true if the `parsed_ref` won't be imported because its remote name /// is reserved. /// @@ -1078,6 +1109,21 @@ fn is_remote_not_found_err(err: &git2::Error) -> bool { ) } +fn repository_not_found_err(err: &git2::Error) -> Option<&str> { + let mut s = None; + if matches!( + (err.class(), err.code()), + (git2::ErrorClass::Repository, git2::ErrorCode::GenericError) + ) && err.message().starts_with("could not find repository at ") + { + let mut sp = err.message().split('\''); + sp.next(); + s = sp.next(); + } + + s +} + fn is_remote_exists_err(err: &git2::Error) -> bool { matches!( (err.class(), err.code()), @@ -1227,9 +1273,32 @@ 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("Failed to fork git process")] + GitForkError(#[from] std::io::Error), + #[error("git process failed: {0}")] + ExternalGitError(String), + #[error("failed to convert path")] + PathConversionError(std::path::PathBuf), } -fn fetch_options( +fn shell_fetch_options( + callbacks: RemoteCallbacks<'_>, + depth: Option, +) -> git2::FetchOptions<'_> { + let mut proxy_options = git2::ProxyOptions::new(); + proxy_options.auto(); + + let mut fetch_options = git2::FetchOptions::new(); + fetch_options.proxy_options(proxy_options); + fetch_options.remote_callbacks(callbacks.into_git()); + if let Some(depth) = depth { + fetch_options.depth(depth.get().try_into().unwrap_or(i32::MAX)); + } + + fetch_options +} + +fn git2_fetch_options( callbacks: RemoteCallbacks<'_>, depth: Option, ) -> git2::FetchOptions<'_> { @@ -1257,6 +1326,8 @@ struct GitFetch<'a> { git_settings: &'a GitSettings, fetch_options: git2::FetchOptions<'a>, fetched: Vec, + depth: Option, + shell: bool, } impl<'a> GitFetch<'a> { @@ -1265,6 +1336,8 @@ impl<'a> GitFetch<'a> { git_repo: &'a git2::Repository, git_settings: &'a GitSettings, fetch_options: git2::FetchOptions<'a>, + depth: Option, + shell: bool, ) -> Self { GitFetch { mut_repo, @@ -1272,29 +1345,16 @@ impl<'a> GitFetch<'a> { git_settings, fetch_options, fetched: vec![], + depth, + shell, } } - /// 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 @@ -1307,14 +1367,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"); @@ -1348,6 +1433,79 @@ impl<'a> GitFetch<'a> { Ok(default_branch) } + fn shell_fetch( + &mut self, + branch_names: &[StringPattern], + remote_name: &str, + ) -> 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_dir = self.git_repo.path().to_str().ok_or_else(|| { + GitFetchError::PathConversionError(self.git_repo.path().to_path_buf()) + })?; + + // TODO: there is surely a better way to get this? it seems like it's stored in + // the workspace, but that seems inaccessible here + let work_tree = work_tree_from_git_dir(self.git_repo.path()) + .map(|p| { + p.to_str() + .map(|x| x.to_string()) + .ok_or(GitFetchError::PathConversionError(p)) + }) + .map_err(GitFetchError::ExternalGitError)??; + + let mut prunes = Vec::new(); + for refspec in refspecs { + git_shell::spawn_git_fetch( + git_dir, + &work_tree, + remote_name, + self.depth, + &refspec, + &mut prunes, + )?; + } + for branch in prunes { + git_shell::spawn_git_branch_prune(git_dir, &work_tree, 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_shell::spawn_git_remote_show(git_dir, &work_tree, 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 self.shell { + self.shell_fetch(branch_names, remote_name) + } 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. @@ -1403,12 +1561,20 @@ pub fn fetch( callbacks: RemoteCallbacks<'_>, git_settings: &GitSettings, depth: Option, + shell: bool, ) -> Result { + // git fetch remote_name branch_names let mut git_fetch = GitFetch::new( mut_repo, git_repo, git_settings, - fetch_options(callbacks, depth), + if shell { + shell_fetch_options(callbacks, depth) + } else { + git2_fetch_options(callbacks, depth) + }, + depth, + shell, ); let default_branch = git_fetch.fetch(branch_names, remote_name)?; let import_stats = git_fetch.import_refs()?; @@ -1436,6 +1602,40 @@ pub enum GitPushError { // and errors caused by the remote rejecting the push. #[error("Unexpected git error when pushing")] InternalGitError(#[from] git2::Error), + #[error("Failed to fork git process")] + GitForkError(#[from] IoError), + #[error("git process failed: {0}")] + ExternalGitError(String), + #[error("failed to convert path")] + PathConversionError(std::path::PathBuf), +} +impl From for GitPushError { + fn from(value: std::io::Error) -> Self { + GitPushError::GitForkError(IoError(value)) + } +} + +/// New-type for comparable io errors +pub struct IoError(std::io::Error); +impl PartialEq for IoError { + fn eq(&self, other: &Self) -> bool { + self.0.kind().eq(&other.0.kind()) + } +} +impl std::error::Error for IoError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + Some(&self.0) + } +} +impl std::fmt::Display for IoError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.0.fmt(f) + } +} +impl std::fmt::Debug for IoError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.0.fmt(f) + } } #[derive(Clone, Debug)] @@ -1460,6 +1660,7 @@ pub fn push_branches( remote_name: &str, targets: &GitBranchPushTargets, callbacks: RemoteCallbacks<'_>, + shell: bool, ) -> Result<(), GitPushError> { let ref_updates = targets .branch_updates @@ -1470,7 +1671,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, + remote_name, + &ref_updates, + callbacks, + shell, + )?; // TODO: add support for partially pushed refs? we could update the view // excluding rejected refs, but the transaction would be aborted anyway @@ -1495,6 +1703,7 @@ pub fn push_updates( remote_name: &str, updates: &[GitRefUpdate], callbacks: RemoteCallbacks<'_>, + shell: bool, ) -> Result<(), GitPushError> { let mut qualified_remote_refs_expected_locations = HashMap::new(); let mut refspecs = vec![]; @@ -1517,17 +1726,79 @@ 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 shell { + shell_push_refs( + repo, + git_repo, + remote_name, + &qualified_remote_refs_expected_locations, + &refspecs, + callbacks, + ) + } else { + git2_push_refs( + repo, + git_repo, + remote_name, + &qualified_remote_refs_expected_locations, + &refspecs, + callbacks, + ) + } +} + +fn check_allow_push( + repo: &dyn Repo, + dst_refname: &str, + actual_remote_location: Option<&CommitId>, + expected_remote_location: Option<&CommitId>, + local_location: Option<&CommitId>, + failed_ref_matches: &mut Vec, +) -> bool { + match allow_push( + repo.index(), + actual_remote_location, + expected_remote_location, + local_location, + ) { + Ok(PushAllowReason::NormalMatch) => {} + Ok(PushAllowReason::UnexpectedNoop) => { + tracing::info!( + "The push of {dst_refname} is unexpectedly a no-op, the remote branch is already \ + at {actual_remote_location:?}. We expected it to be at \ + {expected_remote_location:?}. We don't consider this an error.", + ); + } + Ok(PushAllowReason::ExceptionalFastforward) => { + // TODO(ilyagr): We could consider printing a user-facing message at + // this point. + tracing::info!( + "We allow the push of {dst_refname} to {local_location:?}, even though it is \ + unexpectedly at {actual_remote_location:?} on the server rather than the \ + expected {expected_remote_location:?}. The desired location is a descendant of \ + the actual location, and the actual location is a descendant of the expected \ + location.", + ); + } + Err(()) => { + // While we show debug info in the message with `--debug`, + // there's probably no need to show the detailed commit + // locations to the user normally. They should do a `jj git + // fetch`, and the resulting branch conflicts should contain + // all the information they need. + tracing::info!( + "Cannot push {dst_refname} to {local_location:?}; it is at unexpectedly at \ + {actual_remote_location:?} on the server as opposed to the expected \ + {expected_remote_location:?}", + ); + failed_ref_matches.push(dst_refname.to_string()); + return false; + } + } + true } -fn push_refs( +fn git2_push_refs( repo: &dyn Repo, git_repo: &git2::Repository, remote_name: &str, @@ -1568,47 +1839,14 @@ fn push_refs( |oid: git2::Oid| (!oid.is_zero()).then(|| CommitId::from_bytes(oid.as_bytes())); let actual_remote_location = oid_to_maybe_commitid(update.src()); let local_location = oid_to_maybe_commitid(update.dst()); - - match allow_push( - repo.index(), + check_allow_push( + repo, + dst_refname, actual_remote_location.as_ref(), expected_remote_location, local_location.as_ref(), - ) { - Ok(PushAllowReason::NormalMatch) => {} - Ok(PushAllowReason::UnexpectedNoop) => { - tracing::info!( - "The push of {dst_refname} is unexpectedly a no-op, the remote branch \ - is already at {actual_remote_location:?}. We expected it to be at \ - {expected_remote_location:?}. We don't consider this an error.", - ); - } - Ok(PushAllowReason::ExceptionalFastforward) => { - // TODO(ilyagr): We could consider printing a user-facing message at - // this point. - tracing::info!( - "We allow the push of {dst_refname} to {local_location:?}, even \ - though it is unexpectedly at {actual_remote_location:?} on the \ - server rather than the expected {expected_remote_location:?}. The \ - desired location is a descendant of the actual location, and the \ - actual location is a descendant of the expected location.", - ); - } - Err(()) => { - // While we show debug info in the message with `--debug`, - // there's probably no need to show the detailed commit - // locations to the user normally. They should do a `jj git - // fetch`, and the resulting branch conflicts should contain - // all the information they need. - tracing::info!( - "Cannot push {dst_refname} to {local_location:?}; it is at \ - unexpectedly at {actual_remote_location:?} on the server as opposed \ - to the expected {expected_remote_location:?}", - ); - - failed_push_negotiations.push(dst_refname.to_string()); - } - } + &mut failed_push_negotiations, + ); } if failed_push_negotiations.is_empty() { Ok(()) @@ -1654,6 +1892,90 @@ fn push_refs( } } +fn shell_push_refs( + repo: &dyn Repo, + git_repo: &git2::Repository, + remote_name: &str, + qualified_remote_refs_expected_locations: &HashMap<&str, Option<&CommitId>>, + refspecs: &[String], + _callbacks: RemoteCallbacks<'_>, +) -> Result<(), GitPushError> { + if remote_name == REMOTE_NAME_FOR_LOCAL_GIT_REPO { + return Err(GitPushError::RemoteReservedForLocalGitRepo); + } + let git_dir = git_repo + .path() + .to_str() + .ok_or_else(|| GitPushError::PathConversionError(git_repo.path().to_path_buf()))?; + let work_tree = work_tree_from_git_dir(git_repo.path()) + .map(|p| { + p.to_str() + .map(|x| x.to_string()) + .ok_or(GitPushError::PathConversionError(p)) + }) + .map_err(GitPushError::ExternalGitError)??; + + let mut remaining_remote_refs: HashSet<_> = qualified_remote_refs_expected_locations + .keys() + .copied() + .collect(); + let mut failed_ref_matches = vec![]; + for remote_refspec in refspecs { + let remote_ref_opt = remote_refspec.split(":").last(); + let dest_local_location = remote_refspec + .split(":") + .next() + .map(|x| { + if x.starts_with("+") { + let mut chars = x.chars(); + chars.next(); + chars.as_str() + } else { + x + } + }) + .and_then(|x| if x.trim().is_empty() { None } else { Some(x) }) + .map(|x| CommitId::try_from_hex(x).expect("non hex")); + if let Some(remote_ref) = remote_ref_opt { + if let Some(expected_remote_location) = + qualified_remote_refs_expected_locations.get(remote_ref) + { + let actual_remote_location = + git_shell::spawn_git_ls_remote(git_dir, &work_tree, remote_name, remote_ref)?; + if !check_allow_push( + repo, + remote_ref, + actual_remote_location.as_ref(), + *expected_remote_location, + dest_local_location.as_ref(), + &mut failed_ref_matches, + ) { + continue; + } + } + } + git_shell::spawn_git_push(git_dir, &work_tree, remote_name, remote_refspec)?; + if let Some(remote_ref) = remote_ref_opt { + 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_shell.rs b/lib/src/git_shell.rs new file mode 100644 index 0000000000..e3353be23b --- /dev/null +++ b/lib/src/git_shell.rs @@ -0,0 +1,517 @@ +use std::num::NonZeroU32; +use std::process::Command; +use std::process::Output; +use std::process::Stdio; + +use crate::backend::CommitId; +use crate::git::GitFetchError; +use crate::git::GitPushError; + +fn convert_git_fetch_output_to_fetch_result( + output: Output, + prunes: &mut Vec, +) -> Result { + // None means signal termination: + // 128 is the base for the signal exit codes (128 + signo) + let code = output.status.code(); + if code == Some(0) { + return Ok(output); + } + + let lossy_err = String::from_utf8_lossy(&output.stderr).to_string(); + let stderr = String::from_utf8(output.stderr.clone()).map_err(|e| { + GitFetchError::ExternalGitError(format!( + "external git program failed with non-utf8 output: {e:?}\n--- stderr ---\n{lossy_err}", + )) + })?; + + // There are some git errors we want to parse out + + // GitFetchError::NoSuchRemote + // + // 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` + let git_err = { + let mut git_err = None; + if let Some(first_line) = stderr.lines().next() { + if first_line.starts_with("fatal: '") + && first_line.ends_with("' does not appear to be a git repository") + { + let mut split = first_line.split('\''); + split.next(); // ignore prefix + let branch_name = split.next(); + if let Some(bname) = branch_name { + git_err = Some(Err(GitFetchError::NoSuchRemote(bname.to_string()))); + } + } + } + + git_err + }; + if let Some(e) = git_err { + return e; + } + + // even though --prune is specified, if a particular refspec is asked for but + // not present in the remote, git will error out. + // we ignore it explicitly + // + // The first line is of the form: + // `fatal: couldn't find remote ref refs/heads/` + if let Some(first_line) = stderr.lines().next() { + if first_line.starts_with("fatal: couldn't find remote ref refs/heads/") { + let mut sp = first_line.split("refs/heads/"); + sp.next(); + prunes.push(sp.next().unwrap().to_string()); + return Ok(output); + } + } + + Err(GitFetchError::ExternalGitError(format!( + "external git program failed:\n{stderr}", + ))) +} + +pub(crate) fn spawn_git_fetch( + git_dir: &str, + work_tree: &str, + remote_name: &str, + depth: Option, + refspec: &str, + prunes: &mut Vec, +) -> Result<(), GitFetchError> { + let depth_arg = depth.map(|x| format!("--depth={x}")); + tracing::debug!( + "shelling out to `git --git-dir={} --work-tree={} fetch --prune{} {} {}", + git_dir, + work_tree, + depth_arg + .as_ref() + .map(|x| format!(" {x}")) + .unwrap_or("".to_string()), + remote_name, + refspec + ); + + let remote_git = { + let mut cmd = Command::new("git"); + + cmd.arg(format!("--git-dir={git_dir}")) + .arg(format!("--work-tree={work_tree}")) + .arg("fetch") + .arg("--prune"); + + if let Some(depth) = depth_arg { + cmd.arg(depth); + } + + cmd.arg(remote_name) + .arg(refspec) + .stdin(Stdio::null()) + .stderr(Stdio::piped()) + .spawn() + }?; + + let output = remote_git.wait_with_output()?; + let _ = convert_git_fetch_output_to_fetch_result(output, prunes)?; + + Ok(()) +} + +fn convert_git_remote_show_output_to_fetch_result( + output: Output, + prunes: &mut Vec, +) -> Result { + // None means signal termination: + // 128 is the base for the signal exit codes (128 + signo) + let code = output.status.code(); + if code == Some(0) { + return Ok(output); + } + + let lossy_err = String::from_utf8_lossy(&output.stderr).to_string(); + let stderr = String::from_utf8(output.stderr.clone()).map_err(|e| { + GitFetchError::ExternalGitError(format!( + "external git program failed with non-utf8 output: {e:?}\n--- stderr ---\n{lossy_err}", + )) + })?; + + // There are some git errors we want to parse out + + // GitFetchError::NoSuchRemote + // + // 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` + let git_err = { + let mut git_err = None; + if let Some(first_line) = stderr.lines().next() { + if first_line.starts_with("fatal: '") + && first_line.ends_with("' does not appear to be a git repository") + { + let mut split = first_line.split('\''); + split.next(); // ignore prefix + let branch_name = split.next(); + if let Some(bname) = branch_name { + git_err = Some(Err(GitFetchError::NoSuchRemote(bname.to_string()))); + } + } + } + + git_err + }; + if let Some(e) = git_err { + return e; + } + + // if a particular refspec is asked for but not present in the remote, git will + // error out. we ignore it explicitly + // + // The first line is of the form: + // `fatal: couldn't find remote ref refs/heads/` + if let Some(first_line) = stderr.lines().next() { + if first_line.starts_with("fatal: couldn't find remote ref refs/heads/") { + let mut sp = first_line.split("refs/heads/"); + sp.next(); + prunes.push(sp.next().unwrap().to_string()); + return Ok(output); + } + } + + Err(GitFetchError::ExternalGitError(format!( + "external git program failed:\n{stderr}", + ))) +} + +// 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_git_remote_show( + git_dir: &str, + work_tree: &str, + remote_name: &str, +) -> Result, GitFetchError> { + tracing::debug!( + "shelling out to `git --git-dir={git_dir} --work-tree={work_tree} remote show \ + {remote_name}`" + ); + + let remote_git = Command::new("git") + .arg(format!("--git-dir={git_dir}")) + .arg(format!("--work-tree={work_tree}")) + .arg("remote") + .arg("show") + .arg(remote_name) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn()?; + + let output = remote_git.wait_with_output()?; + let output = convert_git_remote_show_output_to_fetch_result(output, &mut vec![])?; + + // find the HEAD branch line in the output + let branch_name = String::from_utf8(output.stdout) + .map_err(|e| { + GitFetchError::ExternalGitError(format!("git remote output is not utf-8: {e:?}")) + })? + .lines() + .map(|x| x.trim()) + .find(|x| x.starts_with("HEAD branch:")) + .and_then(|x| x.split(" ").last().map(|y| y.trim().to_string())); + + // 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) +} + +fn convert_git_branch_prune_output_to_fetch_result( + output: Output, +) -> Result { + // None means signal termination: + // 128 is the base for the signal exit codes (128 + signo) + let code = output.status.code(); + if code == Some(0) { + return Ok(output); + } + + let lossy_err = String::from_utf8_lossy(&output.stderr).to_string(); + let stderr = String::from_utf8(output.stderr.clone()).map_err(|e| { + GitFetchError::ExternalGitError(format!( + "external git program failed with non-utf8 output: {e:?}\n--- stderr ---\n{lossy_err}", + )) + })?; + + // There are some git errors we want to parse out + + // 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` + if let Some(first_line) = stderr.lines().next() { + if first_line.starts_with("error: remote-tracking branch '") + && first_line.ends_with("' not found") + { + return Ok(output); + } + } + + Err(GitFetchError::ExternalGitError(format!( + "external git program failed:\n{stderr}", + ))) +} + +pub(crate) fn spawn_git_branch_prune( + git_dir: &str, + work_tree: &str, + remote_name: &str, + branch_name: &str, +) -> Result<(), GitFetchError> { + tracing::debug!( + "shelling out to `git --git-dir={git_dir} --work-tree={work_tree} branch --remotes \ + --delete {remote_name}/{branch_name}`" + ); + let branch_git = Command::new("git") + .arg(format!("--git-dir={git_dir}")) + .arg(format!("--work-tree={work_tree}")) + .arg("branch") + .arg("--remotes") + .arg("--delete") + .arg(format!("{remote_name}/{branch_name}")) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::piped()) + .spawn()?; + + let output = branch_git.wait_with_output()?; + convert_git_branch_prune_output_to_fetch_result(output)?; + + Ok(()) +} + +fn convert_git_push_output_to_push_result(output: Output) -> Result { + // None means signal termination: + // 128 is the base for the signal exit codes (128 + signo) + let code = output.status.code(); + if code == Some(0) { + return Ok(output); + } + + let lossy_err = String::from_utf8_lossy(&output.stderr).to_string(); + let stderr = String::from_utf8(output.stderr.clone()).map_err(|e| { + GitPushError::ExternalGitError(format!( + "external git program failed with non-utf8 output: {e:?}\n--- stderr ---\n{lossy_err}", + )) + })?; + + // GitPushError::NoSuchRemote + // + // 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` + let git_err = { + let mut git_err = None; + if let Some(first_line) = stderr.lines().next() { + if first_line.starts_with("fatal: '") + && first_line.ends_with("' does not appear to be a git repository") + { + let mut split = first_line.split('\''); + split.next(); // ignore prefix + let branch_name = split.next(); + if let Some(bname) = branch_name { + git_err = Some(Err(GitPushError::NoSuchRemote(bname.to_string()))); + } + } + } + + git_err + }; + if let Some(e) = git_err { + return e; + } + + // GitPushError::NoSuchRemote + // + // If the remote repo is specified with the URL directly it gives a slightly + // different error `fatal: '' does not appear to be a git + // repository` + let git_err = { + let mut git_err = None; + if let Some(first_line) = stderr.lines().next() { + if first_line.starts_with("fatal: unable to access '") + && first_line.ends_with("': Could not resolve host: invalid-remote") + { + let mut split = first_line.split('\''); + split.next(); // ignore prefix + let branch_name = split.next(); + if let Some(bname) = branch_name { + git_err = Some(Err(GitPushError::NoSuchRemote(bname.to_string()))); + } + } + } + + git_err + }; + if let Some(e) = git_err { + return e; + } + + Err(GitPushError::ExternalGitError(format!( + "external git program failed:\n{stderr}" + ))) +} + +pub(crate) fn spawn_git_push( + git_dir: &str, + work_tree: &str, + remote_name: &str, + refspec: &str, +) -> Result<(), GitPushError> { + tracing::debug!( + "shelling out to `git --git-dir={} --work-tree={} push {} {}", + git_dir, + work_tree, + remote_name, + refspec + ); + + let remote_git = Command::new("git") + .arg(format!("--git-dir={git_dir}")) + .arg(format!("--work-tree={work_tree}")) + .arg("push") + .arg(remote_name) + .arg(refspec) + .stdin(Stdio::null()) + .stderr(Stdio::piped()) + .spawn()?; + + let output = remote_git.wait_with_output()?; + let _ = convert_git_push_output_to_push_result(output)?; + + Ok(()) +} + +fn convert_git_ls_remote_to_push_result(output: Output) -> Result { + // None means signal termination: + // 128 is the base for the signal exit codes (128 + signo) + let code = output.status.code(); + if code == Some(0) { + return Ok(output); + } + + let lossy_err = String::from_utf8_lossy(&output.stderr).to_string(); + let stderr = String::from_utf8(output.stderr.clone()).map_err(|e| { + GitPushError::ExternalGitError(format!( + "external git program failed with non-utf8 output: {e:?}\n--- stderr ---\n{lossy_err}", + )) + })?; + + // GitPushError::NoSuchRemote + // + // 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` + let git_err = { + let mut git_err = None; + if let Some(first_line) = stderr.lines().next() { + if first_line.starts_with("fatal: '") + && first_line.ends_with("' does not appear to be a git repository") + { + let mut split = first_line.split('\''); + split.next(); // ignore prefix + let branch_name = split.next(); + if let Some(bname) = branch_name { + git_err = Some(Err(GitPushError::NoSuchRemote(bname.to_string()))); + } + } + } + + git_err + }; + if let Some(e) = git_err { + return e; + } + + // GitPushError::NoSuchRemote + // + // If the remote repo is specified with the URL directly it gives a slightly + // different error `fatal: '' does not appear to be a git + // repository` + let git_err = { + let mut git_err = None; + if let Some(first_line) = stderr.lines().next() { + if first_line.starts_with("fatal: unable to access '") + && first_line.ends_with("': Could not resolve host: invalid-remote") + { + let mut split = first_line.split('\''); + split.next(); // ignore prefix + let branch_name = split.next(); + if let Some(bname) = branch_name { + git_err = Some(Err(GitPushError::NoSuchRemote(bname.to_string()))); + } + } + } + + git_err + }; + if let Some(e) = git_err { + return e; + } + + Err(GitPushError::ExternalGitError(format!( + "external git program failed:\n{stderr}" + ))) +} + +pub(crate) fn spawn_git_ls_remote( + git_dir: &str, + work_tree: &str, + remote_name: &str, + remote_ref: &str, +) -> Result, GitPushError> { + tracing::debug!( + "shelling out to `git --git-dir={} --work-tree={} ls-remote {} {}", + git_dir, + work_tree, + remote_name, + remote_ref + ); + + let remote_git = Command::new("git") + .arg(format!("--git-dir={git_dir}")) + .arg(format!("--work-tree={work_tree}")) + .arg("ls-remote") + .arg(remote_name) + .arg(remote_ref) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn()?; + + let output = remote_git.wait_with_output()?; + let output = convert_git_ls_remote_to_push_result(output)?; + + let hex_str = String::from_utf8(output.stdout).map_err(|e| { + GitPushError::ExternalGitError(format!("git rev-parse gave non-utf8 output: {e:?}")) + })?; + + if let Some(commit_str) = hex_str.split_whitespace().next() { + let commit_id = CommitId::try_from_hex(commit_str).map_err(|e| { + GitPushError::ExternalGitError(format!("git ls-remote gave non hex output: {e:?}")) + })?; + + return Ok(Some(commit_id)); + } + + Ok(None) +} diff --git a/lib/src/lib.rs b/lib/src/lib.rs index 677b60829d..3a20c71c8b 100644 --- a/lib/src/lib.rs +++ b/lib/src/lib.rs @@ -53,6 +53,7 @@ pub mod fsmonitor; pub mod git; #[cfg(feature = "git")] pub mod git_backend; +mod git_shell; 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 558bc2982c..f6d5af0301 100644 --- a/lib/tests/test_git.rs +++ b/lib/tests/test_git.rs @@ -2232,8 +2232,9 @@ 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; "shell out to git for remote calls")] +fn test_fetch_empty_repo(shell: bool) { let test_data = GitRepoData::create(); let git_settings = GitSettings::default(); @@ -2246,6 +2247,7 @@ fn test_fetch_empty_repo() { git::RemoteCallbacks::default(), &git_settings, None, + shell, ) .unwrap(); // No default bookmark and no refs @@ -2255,8 +2257,9 @@ 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; "shell out to git for remote calls")] +fn test_fetch_initial_commit_head_is_not_set(shell: bool) { let test_data = GitRepoData::create(); let git_settings = GitSettings { auto_local_bookmark: true, @@ -2273,6 +2276,7 @@ fn test_fetch_initial_commit_head_is_not_set() { git::RemoteCallbacks::default(), &git_settings, None, + shell, ) .unwrap(); // No default bookmark because the origin repo's HEAD wasn't set @@ -2306,8 +2310,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; "shell out to git for remote calls")] +fn test_fetch_initial_commit_head_is_set(shell: bool) { let test_data = GitRepoData::create(); let git_settings = GitSettings { auto_local_bookmark: true, @@ -2334,6 +2339,7 @@ fn test_fetch_initial_commit_head_is_set() { git::RemoteCallbacks::default(), &git_settings, None, + shell, ) .unwrap(); @@ -2341,8 +2347,9 @@ 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; "shell out to git for remote calls")] +fn test_fetch_success(shell: bool) { let mut test_data = GitRepoData::create(); let git_settings = GitSettings { auto_local_bookmark: true, @@ -2359,6 +2366,7 @@ fn test_fetch_success() { git::RemoteCallbacks::default(), &git_settings, None, + shell, ) .unwrap(); test_data.repo = tx.commit("test").unwrap(); @@ -2383,6 +2391,7 @@ fn test_fetch_success() { git::RemoteCallbacks::default(), &git_settings, None, + shell, ) .unwrap(); // The default bookmark is "main" @@ -2423,8 +2432,9 @@ fn test_fetch_success() { ); } -#[test] -fn test_fetch_prune_deleted_ref() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "shell out to git for remote calls")] +fn test_fetch_prune_deleted_ref(shell: bool) { let test_data = GitRepoData::create(); let git_settings = GitSettings { auto_local_bookmark: true, @@ -2441,6 +2451,7 @@ fn test_fetch_prune_deleted_ref() { git::RemoteCallbacks::default(), &git_settings, None, + shell, ) .unwrap(); // Test the setup @@ -2465,6 +2476,7 @@ fn test_fetch_prune_deleted_ref() { git::RemoteCallbacks::default(), &git_settings, None, + shell, ) .unwrap(); assert_eq!(stats.import_stats.abandoned_commits, vec![jj_id(&commit)]); @@ -2475,8 +2487,9 @@ 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; "shell out to git for remote calls")] +fn test_fetch_no_default_branch(shell: bool) { let test_data = GitRepoData::create(); let git_settings = GitSettings { auto_local_bookmark: true, @@ -2493,6 +2506,7 @@ fn test_fetch_no_default_branch() { git::RemoteCallbacks::default(), &git_settings, None, + shell, ) .unwrap(); @@ -2517,14 +2531,16 @@ fn test_fetch_no_default_branch() { git::RemoteCallbacks::default(), &git_settings, None, + shell, ) .unwrap(); // There is no default bookmark assert_eq!(stats.default_branch, None); } -#[test] -fn test_fetch_empty_refspecs() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "shell out to git for remote calls")] +fn test_fetch_empty_refspecs(shell: bool) { let test_data = GitRepoData::create(); let git_settings = GitSettings::default(); empty_git_commit(&test_data.origin_repo, "refs/heads/main", &[]); @@ -2539,6 +2555,7 @@ fn test_fetch_empty_refspecs() { git::RemoteCallbacks::default(), &git_settings, None, + shell, ) .unwrap(); assert!(tx @@ -2553,8 +2570,9 @@ 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; "shell out to git for remote calls")] +fn test_fetch_no_such_remote(shell: bool) { let test_data = GitRepoData::create(); let git_settings = GitSettings::default(); let mut tx = test_data.repo.start_transaction(); @@ -2566,6 +2584,7 @@ fn test_fetch_no_such_remote() { git::RemoteCallbacks::default(), &git_settings, None, + shell, ); assert!(matches!(result, Err(GitFetchError::NoSuchRemote(_)))); } @@ -2663,8 +2682,9 @@ 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; "shell out to git for remote calls")] +fn test_push_bookmarks_success(shell: bool) { let settings = testutils::user_settings(); let temp_dir = testutils::new_temp_dir(); let mut setup = set_up_push_repos(&settings, &temp_dir); @@ -2686,6 +2706,7 @@ fn test_push_bookmarks_success() { "origin", &targets, git::RemoteCallbacks::default(), + shell, ); assert_eq!(result, Ok(())); @@ -2728,8 +2749,9 @@ 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; "shell out to git for remote calls")] +fn test_push_bookmarks_deletion(shell: bool) { let settings = testutils::user_settings(); let temp_dir = testutils::new_temp_dir(); let mut setup = set_up_push_repos(&settings, &temp_dir); @@ -2755,6 +2777,7 @@ fn test_push_bookmarks_deletion() { "origin", &targets, git::RemoteCallbacks::default(), + shell, ); assert_eq!(result, Ok(())); @@ -2780,8 +2803,9 @@ 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; "shell out to git for remote calls")] +fn test_push_bookmarks_mixed_deletion_and_addition(shell: bool) { let settings = testutils::user_settings(); let temp_dir = testutils::new_temp_dir(); let mut setup = set_up_push_repos(&settings, &temp_dir); @@ -2812,6 +2836,7 @@ fn test_push_bookmarks_mixed_deletion_and_addition() { "origin", &targets, git::RemoteCallbacks::default(), + shell, ); assert_eq!(result, Ok(())); @@ -2849,8 +2874,9 @@ 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; "shell out to git for remote calls")] +fn test_push_bookmarks_not_fast_forward(shell: bool) { let settings = testutils::user_settings(); let temp_dir = testutils::new_temp_dir(); let setup = set_up_push_repos(&settings, &temp_dir); @@ -2871,6 +2897,7 @@ fn test_push_bookmarks_not_fast_forward() { "origin", &targets, git::RemoteCallbacks::default(), + shell, ); assert_eq!(result, Ok(())); @@ -2887,8 +2914,9 @@ 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; "shell out to git for remote calls")] +fn test_push_updates_unexpectedly_moved_sideways_on_remote(shell: bool) { let settings = testutils::user_settings(); let temp_dir = testutils::new_temp_dir(); let setup = set_up_push_repos(&settings, &temp_dir); @@ -2916,6 +2944,7 @@ fn test_push_updates_unexpectedly_moved_sideways_on_remote() { "origin", &targets, git::RemoteCallbacks::default(), + shell, ) }; @@ -2952,8 +2981,9 @@ 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; "shell out to git for remote calls")] +fn test_push_updates_unexpectedly_moved_forward_on_remote(shell: bool) { let settings = testutils::user_settings(); let temp_dir = testutils::new_temp_dir(); let setup = set_up_push_repos(&settings, &temp_dir); @@ -2983,6 +3013,7 @@ fn test_push_updates_unexpectedly_moved_forward_on_remote() { "origin", &targets, git::RemoteCallbacks::default(), + shell, ) }; @@ -3014,8 +3045,9 @@ fn test_push_updates_unexpectedly_moved_forward_on_remote() { ); } -#[test] -fn test_push_updates_unexpectedly_exists_on_remote() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "shell out to git for remote calls")] +fn test_push_updates_unexpectedly_exists_on_remote(shell: bool) { let settings = testutils::user_settings(); let temp_dir = testutils::new_temp_dir(); let setup = set_up_push_repos(&settings, &temp_dir); @@ -3041,6 +3073,7 @@ fn test_push_updates_unexpectedly_exists_on_remote() { "origin", &targets, git::RemoteCallbacks::default(), + shell, ) }; @@ -3056,8 +3089,9 @@ fn test_push_updates_unexpectedly_exists_on_remote() { ); } -#[test] -fn test_push_updates_success() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "shell out to git for remote calls")] +fn test_push_updates_success(shell: bool) { let settings = testutils::user_settings(); let temp_dir = testutils::new_temp_dir(); let setup = set_up_push_repos(&settings, &temp_dir); @@ -3072,6 +3106,7 @@ fn test_push_updates_success() { new_target: Some(setup.child_of_main_commit.id().clone()), }], git::RemoteCallbacks::default(), + shell, ); assert_eq!(result, Ok(())); @@ -3094,8 +3129,9 @@ 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; "shell out to git for remote calls")] +fn test_push_updates_no_such_remote(shell: bool) { let settings = testutils::user_settings(); let temp_dir = testutils::new_temp_dir(); let setup = set_up_push_repos(&settings, &temp_dir); @@ -3109,12 +3145,14 @@ fn test_push_updates_no_such_remote() { new_target: Some(setup.child_of_main_commit.id().clone()), }], git::RemoteCallbacks::default(), + shell, ); assert!(matches!(result, Err(GitPushError::NoSuchRemote(_)))); } -#[test] -fn test_push_updates_invalid_remote() { +#[test_case(false; "use git2 for remote calls")] +#[test_case(true; "shell out to git for remote calls")] +fn test_push_updates_invalid_remote(shell: bool) { let settings = testutils::user_settings(); let temp_dir = testutils::new_temp_dir(); let setup = set_up_push_repos(&settings, &temp_dir); @@ -3128,6 +3166,7 @@ fn test_push_updates_invalid_remote() { new_target: Some(setup.child_of_main_commit.id().clone()), }], git::RemoteCallbacks::default(), + shell, ); assert!(matches!(result, Err(GitPushError::NoSuchRemote(_)))); }