diff --git a/CHANGELOG.md b/CHANGELOG.md index 23445692a..d7bff91f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -71,6 +71,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 some additional insight into what is happening behind the scenes. Note: This is not comprehensively supported by all operations yet. +* (#493) When exporting branches to Git, we used to fail if some branches could + not be exported (e.g. because Git doesn't allow a branch called `main` and + another branch called `main/sub`). We now print a warning about these branches + instead. + ### Fixed bugs * (#463) A bug in the export of branches to Git caused spurious conflicted diff --git a/src/cli_util.rs b/src/cli_util.rs index fd439c331..94b59f0a1 100644 --- a/src/cli_util.rs +++ b/src/cli_util.rs @@ -839,7 +839,8 @@ impl WorkspaceCommandHelper { if self.working_copy_shared_with_git { self.export_head_to_git(mut_repo)?; let git_repo = self.repo.store().git_repo().unwrap(); - git::export_refs(mut_repo, &git_repo)?; + let failed_branches = git::export_refs(mut_repo, &git_repo)?; + print_failed_git_export(ui, &failed_branches)?; } let maybe_old_commit = tx .base_repo() @@ -883,6 +884,22 @@ pub fn print_checkout_stats(ui: &mut Ui, stats: CheckoutStats) -> Result<(), std Ok(()) } +pub fn print_failed_git_export( + ui: &mut Ui, + failed_branches: &[String], +) -> Result<(), std::io::Error> { + if !failed_branches.is_empty() { + ui.write_warn("Failed to export some branches:\n")?; + let mut formatter = ui.stderr_formatter(); + for branch_name in failed_branches { + formatter.write_str(" ")?; + formatter.with_label("branch", |formatter| formatter.write_str(branch_name))?; + formatter.write_str("\n")?; + } + } + Ok(()) +} + /// Expands "~/" to "$HOME/" as Git seems to do for e.g. core.excludesFile. fn expand_git_path(path_str: String) -> PathBuf { if let Some(remainder) = path_str.strip_prefix("~/") { diff --git a/src/commands.rs b/src/commands.rs index e1e29d59d..8a3ec1d4b 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -53,9 +53,9 @@ use maplit::{hashmap, hashset}; use pest::Parser; use crate::cli_util::{ - print_checkout_stats, resolve_base_revs, short_commit_description, short_commit_hash, - user_error, user_error_with_hint, write_commit_summary, Args, CommandError, CommandHelper, - WorkspaceCommandHelper, + print_checkout_stats, print_failed_git_export, resolve_base_revs, short_commit_description, + short_commit_hash, user_error, user_error_with_hint, write_commit_summary, Args, CommandError, + CommandHelper, WorkspaceCommandHelper, }; use crate::formatter::{Formatter, PlainTextFormatter}; use crate::graphlog::{AsciiGraphDrawer, Edge}; @@ -4542,8 +4542,9 @@ fn cmd_git_export( let repo = workspace_command.repo(); let git_repo = get_git_repo(repo.store())?; let mut tx = workspace_command.start_transaction("export git refs"); - git::export_refs(tx.mut_repo(), &git_repo)?; + let failed_branches = git::export_refs(tx.mut_repo(), &git_repo)?; workspace_command.finish_transaction(ui, tx)?; + print_failed_git_export(ui, &failed_branches)?; Ok(()) } diff --git a/tests/test_git_colocated.rs b/tests/test_git_colocated.rs index 247d86ab1..5436d8dcb 100644 --- a/tests/test_git_colocated.rs +++ b/tests/test_git_colocated.rs @@ -16,7 +16,7 @@ use std::path::Path; use git2::Oid; -use crate::common::TestEnvironment; +use crate::common::{get_stderr_string, TestEnvironment}; pub mod common; @@ -178,6 +178,42 @@ fn test_git_colocated_branches() { "###); } +#[test] +fn test_git_colocated_conflicting_git_refs() { + let test_env = TestEnvironment::default(); + let workspace_root = test_env.env_root().join("repo"); + git2::Repository::init(&workspace_root).unwrap(); + test_env.jj_cmd_success(&workspace_root, &["init", "--git-repo", "."]); + let assert = test_env + .jj_cmd(&workspace_root, &["branch", "create", ""]) + .assert() + .success() + .stdout(""); + insta::assert_snapshot!(get_stderr_string(&assert), @r###" + Failed to export some branches: + + "###); + let assert = test_env + .jj_cmd(&workspace_root, &["branch", "create", "main"]) + .assert() + .success() + .stdout(""); + insta::assert_snapshot!(get_stderr_string(&assert), @r###" + Failed to export some branches: + + "###); + let assert = test_env + .jj_cmd(&workspace_root, &["branch", "create", "main/sub"]) + .assert() + .success() + .stdout(""); + insta::assert_snapshot!(get_stderr_string(&assert), @r###" + Failed to export some branches: + + main/sub + "###); +} + fn get_log_output(test_env: &TestEnvironment, workspace_root: &Path) -> String { test_env.jj_cmd_success(workspace_root, &["log", "-T", "commit_id \" \" branches"]) } diff --git a/tests/test_git_export.rs b/tests/test_git_export.rs new file mode 100644 index 000000000..486c0e370 --- /dev/null +++ b/tests/test_git_export.rs @@ -0,0 +1,39 @@ +// Copyright 2022 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use crate::common::{get_stderr_string, TestEnvironment}; + +pub mod common; + +#[test] +fn test_git_export_conflicting_git_refs() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]); + let repo_path = test_env.env_root().join("repo"); + + // TODO: Make it an error to try to create a branch with an empty name + test_env.jj_cmd_success(&repo_path, &["branch", "create", ""]); + test_env.jj_cmd_success(&repo_path, &["branch", "create", "main"]); + test_env.jj_cmd_success(&repo_path, &["branch", "create", "main/sub"]); + let assert = test_env + .jj_cmd(&repo_path, &["git", "export"]) + .assert() + .success() + .stdout(""); + insta::assert_snapshot!(get_stderr_string(&assert), @r###" + Failed to export some branches: + + main/sub + "###); +}