Skip to content

Commit

Permalink
cli: git push: describe private commits that would fail push
Browse files Browse the repository at this point in the history
  • Loading branch information
@shanesveller
shanesveller committed Dec 4, 2024
1 parent 644e5f0 commit 5553baf
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 31 deletions.
21 changes: 18 additions & 3 deletions cli/src/commands/git/push.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
Expand Down
24 changes: 16 additions & 8 deletions cli/tests/test_git_private_commits.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()""#);
Expand All @@ -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(
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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*')'
");
}
50 changes: 30 additions & 20 deletions cli/tests/test_git_push.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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,
&[
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -1062,19 +1068,21 @@ 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"]);
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 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)
Expand All @@ -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]
Expand Down Expand Up @@ -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(
Expand Down

0 comments on commit 5553baf

Please sign in to comment.