git: handle remote not found error by remove/rename_remote()

cli/src/cli_util.rs

@@ -32,7 +32,7 @@ use indexmap::IndexSet;
 use itertools::Itertools;
 use jj_lib::backend::{BackendError, ChangeId, CommitId, ObjectId, TreeId};
 use jj_lib::commit::Commit;
-use jj_lib::git::{GitConfigParseError, GitExportError, GitImportError};
+use jj_lib::git::{GitConfigParseError, GitExportError, GitImportError, GitRemoteManagementError};
 use jj_lib::git_backend::GitBackend;
 use jj_lib::gitignore::GitIgnoreFile;
 use jj_lib::hex_util::to_reverse_hex;
@@ -286,6 +286,12 @@ impl From<GitExportError> for CommandError {
     }
 }
 
+impl From<GitRemoteManagementError> for CommandError {
+    fn from(err: GitRemoteManagementError) -> Self {
+        user_error(format!("{err}"))
+    }
+}
+
 impl From<RevsetEvaluationError> for CommandError {
     fn from(err: RevsetEvaluationError) -> Self {
         user_error(format!("{err}"))

cli/src/commands/git.rs

@@ -268,13 +268,9 @@ fn cmd_git_remote_remove(
     let mut workspace_command = command.workspace_helper(ui)?;
     let repo = workspace_command.repo();
     let git_repo = get_git_repo(repo.store())?;
-    if git_repo.find_remote(&args.remote).is_err() {
-        return Err(user_error("Remote doesn't exist"));
-    }
     let mut tx =
         workspace_command.start_transaction(&format!("remove git remote {}", &args.remote));
-    git::remove_remote(tx.mut_repo(), &git_repo, &args.remote)
+    git::remove_remote(tx.mut_repo(), &git_repo, &args.remote)?;
-        .map_err(|err| user_error(err.to_string()))?;
     if tx.mut_repo().has_changes() {
         tx.finish(ui)
     } else {
@@ -290,13 +286,9 @@ fn cmd_git_remote_rename(
     let mut workspace_command = command.workspace_helper(ui)?;
     let repo = workspace_command.repo();
     let git_repo = get_git_repo(repo.store())?;
-    if git_repo.find_remote(&args.old).is_err() {
-        return Err(user_error("Remote doesn't exist"));
-    }
     let mut tx = workspace_command
         .start_transaction(&format!("rename git remote {} to {}", &args.old, &args.new));
-    git::rename_remote(tx.mut_repo(), &git_repo, &args.old, &args.new)
+    git::rename_remote(tx.mut_repo(), &git_repo, &args.old, &args.new)?;
-        .map_err(|err| user_error(err.to_string()))?;
     if tx.mut_repo().has_changes() {
         tx.finish(ui)
     } else {

cli/tests/test_git_remotes.rs

@@ -46,8 +46,9 @@ fn test_git_remotes() {
     insta::assert_snapshot!(stdout, @"bar http://example.com/repo/bar
 ");
     let stderr = test_env.jj_cmd_failure(&repo_path, &["git", "remote", "remove", "nonexistent"]);
-    insta::assert_snapshot!(stderr, @"Error: Remote doesn't exist
+    insta::assert_snapshot!(stderr, @r###"
-");
+    Error: No git remote named 'nonexistent'
+    "###);
 }
 
 #[test]
@@ -61,7 +62,9 @@ fn test_git_remote_rename() {
         &["git", "remote", "add", "foo", "http://example.com/repo/foo"],
     );
     let stderr = test_env.jj_cmd_failure(&repo_path, &["git", "remote", "rename", "bar", "foo"]);
-    insta::assert_snapshot!(stderr, @"Error: Remote doesn't exist\n");
+    insta::assert_snapshot!(stderr, @r###"
+    Error: No git remote named 'bar'
+    "###);
     let stdout = test_env.jj_cmd_success(&repo_path, &["git", "remote", "rename", "foo", "bar"]);
     insta::assert_snapshot!(stdout, @"");
     let stdout = test_env.jj_cmd_success(&repo_path, &["git", "remote", "list"]);

lib/src/git.rs

@@ -548,6 +548,14 @@ pub fn export_some_refs(
     Ok(failed_branches)
 }
 
+#[derive(Debug, Error)]
+pub enum GitRemoteManagementError {
+    #[error("No git remote named '{0}'")]
+    NoSuchRemote(String),
+    #[error(transparent)]
+    InternalGitError(git2::Error),
+}
+
 fn is_remote_not_found_err(err: &git2::Error) -> bool {
     matches!(
         (err.class(), err.code()),
@@ -562,8 +570,14 @@ pub fn remove_remote(
     mut_repo: &mut MutableRepo,
     git_repo: &git2::Repository,
     remote_name: &str,
-) -> Result<(), git2::Error> {
+) -> Result<(), GitRemoteManagementError> {
-    git_repo.remote_delete(remote_name)?;
+    git_repo.remote_delete(remote_name).map_err(|err| {
+        if is_remote_not_found_err(&err) {
+            GitRemoteManagementError::NoSuchRemote(remote_name.to_owned())
+        } else {
+            GitRemoteManagementError::InternalGitError(err)
+        }
+    })?;
     let mut branches_to_delete = vec![];
     for (branch, target) in mut_repo.view().branches() {
         if target.remote_targets.contains_key(remote_name) {
@@ -592,8 +606,16 @@ pub fn rename_remote(
     git_repo: &git2::Repository,
     old_remote_name: &str,
     new_remote_name: &str,
-) -> Result<(), git2::Error> {
+) -> Result<(), GitRemoteManagementError> {
-    git_repo.remote_rename(old_remote_name, new_remote_name)?;
+    git_repo
+        .remote_rename(old_remote_name, new_remote_name)
+        .map_err(|err| {
+            if is_remote_not_found_err(&err) {
+                GitRemoteManagementError::NoSuchRemote(old_remote_name.to_owned())
+            } else {
+                GitRemoteManagementError::InternalGitError(err)
+            }
+        })?;
     mut_repo.rename_remote(old_remote_name, new_remote_name);
     let prefix = format!("refs/remotes/{old_remote_name}/");
     let git_refs = mut_repo