ok/jj
1
0
Fork 0
forked from mirrors/jj

cli: when export to git fails, report failed branches

This adds a warning whenever export to the backing Git repo fails,
whether it's by an explicit `jj git export` or an automatic export. It
might be too spammy to print the message after every failed command in
the colocated case, but let's try it and see.
This commit is contained in:
Martin von Zweigbergk 2022-11-24 20:17:39 -08:00 committed by Martin von Zweigbergk
parent a90ef20976
commit c7fb8709b4
5 changed files with 104 additions and 6 deletions

View file

@ -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

View file

@ -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("~/") {

View file

@ -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(())
}

View file

@ -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"])
}

39
tests/test_git_export.rs Normal file
View file

@ -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
"###);
}