git: prohibit creation of remote named "git"

#1690

CHANGELOG.md

@@ -53,6 +53,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * `jj git push` will now push all branches in the range `remote_branches()..@`
   instead of only branches pointing to `@` or `@-`.
 
+* It's no longer allowed to create a Git remote named "git". Use `jj git remote
+  rename` to rename the existing remote.
+  [#1690](https://github.com/martinvonz/jj/issues/1690)
+
 ### New features
 
 * Default template for `jj log` now does not show irrelevant information

cli/tests/test_git_remotes.rs

@@ -74,6 +74,13 @@ fn test_git_remote_add() {
     insta::assert_snapshot!(stderr, @r###"
     Error: Git remote named 'foo' already exists
     "###);
+    let stderr = test_env.jj_cmd_failure(
+        &repo_path,
+        &["git", "remote", "add", "git", "http://example.com/repo/git"],
+    );
+    insta::assert_snapshot!(stderr, @r###"
+    Error: Git remote named 'git' is reserved for local Git repository
+    "###);
     let stdout = test_env.jj_cmd_success(&repo_path, &["git", "remote", "list"]);
     insta::assert_snapshot!(stdout, @r###"
     foo http://example.com/repo/foo
@@ -102,6 +109,10 @@ fn test_git_remote_rename() {
     insta::assert_snapshot!(stderr, @r###"
     Error: Git remote named 'baz' already exists
     "###);
+    let stderr = test_env.jj_cmd_failure(&repo_path, &["git", "remote", "rename", "foo", "git"]);
+    insta::assert_snapshot!(stderr, @r###"
+    Error: Git remote named 'git' is reserved for local Git repository
+    "###);
     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"]);
@@ -110,3 +121,24 @@ fn test_git_remote_rename() {
     baz http://example.com/repo/baz
     "###);
 }
+
+#[test]
+fn test_git_remote_named_git() {
+    let test_env = TestEnvironment::default();
+
+    // Existing remote named 'git' shouldn't block the repo initialization.
+    let repo_path = test_env.env_root().join("repo");
+    let git_repo = git2::Repository::init(&repo_path).unwrap();
+    git_repo
+        .remote("git", "http://example.com/repo/repo")
+        .unwrap();
+    test_env.jj_cmd_success(&repo_path, &["init", "--git-repo=."]);
+
+    // The remote can be renamed.
+    let stdout = test_env.jj_cmd_success(&repo_path, &["git", "remote", "rename", "git", "bar"]);
+    insta::assert_snapshot!(stdout, @"");
+    let stdout = test_env.jj_cmd_success(&repo_path, &["git", "remote", "list"]);
+    insta::assert_snapshot!(stdout, @r###"
+    bar http://example.com/repo/repo
+    "###);
+}

lib/src/git.rs

@@ -561,6 +561,11 @@ pub enum GitRemoteManagementError {
     NoSuchRemote(String),
     #[error("Git remote named '{0}' already exists")]
     RemoteAlreadyExists(String),
+    #[error(
+        "Git remote named '{name}' is reserved for local Git repository",
+        name = REMOTE_NAME_FOR_LOCAL_GIT_REPO
+    )]
+    RemoteReservedForLocalGitRepo,
     #[error(transparent)]
     InternalGitError(git2::Error),
 }
@@ -587,6 +592,9 @@ pub fn add_remote(
     remote_name: &str,
     url: &str,
 ) -> Result<(), GitRemoteManagementError> {
+    if remote_name == REMOTE_NAME_FOR_LOCAL_GIT_REPO {
+        return Err(GitRemoteManagementError::RemoteReservedForLocalGitRepo);
+    }
     git_repo.remote(remote_name, url).map_err(|err| {
         if is_remote_exists_err(&err) {
             GitRemoteManagementError::RemoteAlreadyExists(remote_name.to_owned())
@@ -638,6 +646,9 @@ pub fn rename_remote(
     old_remote_name: &str,
     new_remote_name: &str,
 ) -> Result<(), GitRemoteManagementError> {
+    if new_remote_name == REMOTE_NAME_FOR_LOCAL_GIT_REPO {
+        return Err(GitRemoteManagementError::RemoteReservedForLocalGitRepo);
+    }
     git_repo
         .remote_rename(old_remote_name, new_remote_name)
         .map_err(|err| {