From a115ea8f9d594ffafbc9492075a8c7b63500f97c Mon Sep 17 00:00:00 2001 From: Shane Sveller Date: Fri, 29 Nov 2024 15:16:38 -0600 Subject: [PATCH] cli: git push: describe private commits that would fail push --- cli/src/commands/git/push.rs | 21 +++++++++-- cli/tests/test_git_private_commits.rs | 24 ++++++++----- cli/tests/test_git_push.rs | 50 ++++++++++++++++----------- 3 files changed, 64 insertions(+), 31 deletions(-) diff --git a/cli/src/commands/git/push.rs b/cli/src/commands/git/push.rs index d3608bf86f..866dba2650 100644 --- a/cli/src/commands/git/push.rs +++ b/cli/src/commands/git/push.rs @@ -402,15 +402,30 @@ fn validate_commits_ready_to_push( if commit.has_conflict()? { reasons.push("it has conflicts"); } - if !args.allow_private && is_private(commit.id())? { + let is_private = is_private(commit.id())?; + if !args.allow_private && is_private { reasons.push("it is private"); } if !reasons.is_empty() { - return Err(user_error(format!( + let mut error = user_error(format!( "Won't push commit {} since {}", short_commit_hash(commit.id()), reasons.join(" and ") - ))); + )); + error.add_formatted_hint_with(|formatter| { + write!(formatter, "Rejected commit: ")?; + workspace_helper.write_commit_summary(formatter, &commit)?; + Ok(()) + }); + if !args.allow_private && is_private { + error.add_hint(format!( + "Configured git.private-commits: '{}'", + settings + .get_string("git.private-commits") + .expect("should have private-commits setting") + )); + } + return Err(error); } } Ok(()) diff --git a/cli/tests/test_git_private_commits.rs b/cli/tests/test_git_private_commits.rs index fadd5ca05b..e806a1bfe7 100644 --- a/cli/tests/test_git_private_commits.rs +++ b/cli/tests/test_git_private_commits.rs @@ -88,9 +88,11 @@ fn test_git_private_commits_block_pushing() { // Will not push when a pushed commit is contained in git.private-commits test_env.add_config(r#"git.private-commits = "description(glob:'private*')""#); let stderr = test_env.jj_cmd_failure(&workspace_root, &["git", "push", "--all"]); - insta::assert_snapshot!(stderr, @r###" + insta::assert_snapshot!(stderr, @r" Error: Won't push commit aa3058ff8663 since it is private - "###); + Hint: Rejected commit: yqosqzyt aa3058ff main* | (empty) private 1 + Hint: Configured git.private-commits: 'description(glob:'private*')' + "); // May push when the commit is removed from git.private-commits test_env.add_config(r#"git.private-commits = "none()""#); @@ -114,9 +116,11 @@ fn test_git_private_commits_can_be_overridden() { // Will not push when a pushed commit is contained in git.private-commits test_env.add_config(r#"git.private-commits = "description(glob:'private*')""#); let stderr = test_env.jj_cmd_failure(&workspace_root, &["git", "push", "--all"]); - insta::assert_snapshot!(stderr, @r###" + insta::assert_snapshot!(stderr, @r" Error: Won't push commit aa3058ff8663 since it is private - "###); + Hint: Rejected commit: yqosqzyt aa3058ff main* | (empty) private 1 + Hint: Configured git.private-commits: 'description(glob:'private*')' + "); // May push when the commit is removed from git.private-commits let (_, stderr) = test_env.jj_cmd_ok( @@ -166,9 +170,11 @@ fn test_git_private_commits_not_directly_in_line_block_pushing() { &workspace_root, &["git", "push", "--allow-new", "-b=bookmark1"], ); - insta::assert_snapshot!(stderr, @r###" + insta::assert_snapshot!(stderr, @r" Error: Won't push commit f1253a9b1ea9 since it is private - "###); + Hint: Rejected commit: yqosqzyt f1253a9b (empty) private 1 + Hint: Configured git.private-commits: 'description(glob:'private*')' + "); } #[test] @@ -270,7 +276,9 @@ fn test_git_private_commits_are_evaluated_separately_for_each_remote() { &workspace_root, &["git", "push", "--remote=other", "-b=main"], ); - insta::assert_snapshot!(stderr, @r###" + insta::assert_snapshot!(stderr, @r" Error: Won't push commit 36b7ecd11ad9 since it is private - "###); + Hint: Rejected commit: znkkpsqq 36b7ecd1 (empty) private 1 + Hint: Configured git.private-commits: 'description(glob:'private*')' + "); } diff --git a/cli/tests/test_git_push.rs b/cli/tests/test_git_push.rs index 467bdf4a82..ac5c7cf022 100644 --- a/cli/tests/test_git_push.rs +++ b/cli/tests/test_git_push.rs @@ -894,9 +894,10 @@ 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::assert_snapshot!(stderr, @r###" + insta::assert_snapshot!(stderr, @r" Error: Won't push commit 73c265a92cfd since it has conflicts - "###); + Hint: Rejected commit: yostqsxw 73c265a9 my-bookmark | (conflict) third + "); } #[test] @@ -908,9 +909,10 @@ fn test_git_push_no_description() { &workspace_root, &["git", "push", "--allow-new", "--bookmark", "my-bookmark"], ); - insta::assert_snapshot!(stderr, @r###" + 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, &[ @@ -943,9 +945,10 @@ fn test_git_push_no_description_in_immutable() { "--dry-run", ], ); - insta::assert_snapshot!(stderr, @r###" + 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( @@ -982,18 +985,20 @@ fn test_git_push_missing_author() { &workspace_root, &["git", "push", "--allow-new", "--bookmark", "missing-name"], ); - insta::assert_snapshot!(stderr, @r###" + 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::assert_snapshot!(stderr, @r###" + 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] @@ -1023,9 +1028,10 @@ fn test_git_push_missing_author_in_immutable() { "--dry-run", ], ); - insta::assert_snapshot!(stderr, @r###" + 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( @@ -1062,9 +1068,10 @@ fn test_git_push_missing_committer() { &workspace_root, &["git", "push", "--allow-new", "--bookmark=missing-name"], ); - insta::assert_snapshot!(stderr, @r###" + 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"]); @@ -1072,9 +1079,10 @@ fn test_git_push_missing_committer() { &workspace_root, &["git", "push", "--allow-new", "--bookmark=missing-email"], ); - insta::assert_snapshot!(stderr, @r###" + 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) @@ -1083,9 +1091,10 @@ fn test_git_push_missing_committer() { &workspace_root, &["git", "push", "--allow-new", "--bookmark=missing-email"], ); - insta::assert_snapshot!(stderr, @r###" + 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] @@ -1116,9 +1125,10 @@ fn test_git_push_missing_committer_in_immutable() { "--dry-run", ], ); - insta::assert_snapshot!(stderr, @r###" + 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(