ok/jj
1
0
Fork 0
forked from mirrors/jj
Code Pull requests Activity

git: add git.private-commits setting for preventing commits from being pushed

Browse source 
The user can define the setting `git.private-commits` as they desire. For
example:

    git.private-commits = 'description(glob:"wip:*")'

If any commits are in this revset, then the push is aborted.

If a commit would be private but already exists on the remote, then it does
not block pushes, nor do its descendents block pushes unless they are also
contained in `git.private-commits`.

Closes #3376
This commit is contained in:
Stephen Jennings 2024-07-05 14:47:46 -07:00
parent da221eb888
commit 03b6d380f5
5 changed files with 324 additions and 50 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
CHANGELOG.md
View file

@ -73,6 +73,8 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
* New command `jj operation show` that can show the changes made in a single
operation.
* New config setting `git.private-commits` to prevent commits from being pushed.
### Fixed bugs
* `jj diff --git` no longer shows the contents of binary files.

127
cli/src/commands/git/push.rs
View file

@ -18,6 +18,7 @@ use std::{fmt, io};
use clap::ArgGroup;
use itertools::Itertools;
use jj_lib::backend::CommitId;
use jj_lib::git::{self, GitBranchPushTargets, GitPushError};
use jj_lib::object_id::ObjectId;
use jj_lib::op_store::RefTarget;
@ -261,56 +262,7 @@ pub fn cmd_git_push(
);
}
// Check if there are conflicts in any commits we're about to push that haven't
// already been pushed.
let new_heads = branch_updates
.iter()
.filter_map(|(_, update)| update.new_target.clone())
.collect_vec();
let old_heads = repo
.view()
.remote_branches(&remote)
.flat_map(|(_, old_head)| old_head.target.added_ids())
.cloned()
.collect_vec();
// (old_heads | immutable_heads() | root())..new_heads
let commits_to_push = RevsetExpression::commits(old_heads)
.union(&revset_util::parse_immutable_heads_expression(
&tx.base_workspace_helper().revset_parse_context(),
)?)
.range(&RevsetExpression::commits(new_heads));
for commit in tx
.base_workspace_helper()
.attach_revset_evaluator(commits_to_push)?
.evaluate_to_commits()?
{
let commit = commit?;
let mut reasons = vec![];
if commit.description().is_empty() && !args.allow_empty_description {
reasons.push("it has no description");
}
if commit.author().name.is_empty()
|| commit.author().name == UserSettings::USER_NAME_PLACEHOLDER
|| commit.author().email.is_empty()
|| commit.author().email == UserSettings::USER_EMAIL_PLACEHOLDER
|| commit.committer().name.is_empty()
|| commit.committer().name == UserSettings::USER_NAME_PLACEHOLDER
|| commit.committer().email.is_empty()
|| commit.committer().email == UserSettings::USER_EMAIL_PLACEHOLDER
{
reasons.push("it has no author and/or committer set");
}
if commit.has_conflict()? {
reasons.push("it has conflicts");
}
if !reasons.is_empty() {
return Err(user_error(format!(
"Won't push commit {} since {}",
short_commit_hash(commit.id()),
reasons.join(" and ")
)));
}
}
validate_commits_ready_to_push(&branch_updates, &remote, &tx, command, args)?;
writeln!(ui.status(), "Branch changes to push to {}:", &remote)?;
for (branch_name, update) in &branch_updates {
@ -387,6 +339,81 @@ pub fn cmd_git_push(
Ok(())
}
/// Validates that the commits that will be pushed are ready (have authorship
/// information, are not conflicted, etc.)
fn validate_commits_ready_to_push(
branch_updates: &[(String, BranchPushUpdate)],
remote: &str,
tx: &WorkspaceCommandTransaction,
command: &CommandHelper,
args: &GitPushArgs,
) -> Result<(), CommandError> {
let workspace_helper = tx.base_workspace_helper();
let repo = workspace_helper.repo();
let new_heads = branch_updates
.iter()
.filter_map(|(_, update)| update.new_target.clone())
.collect_vec();
let old_heads = repo
.view()
.remote_branches(remote)
.flat_map(|(_, old_head)| old_head.target.added_ids())
.cloned()
.collect_vec();
let commits_to_push = RevsetExpression::commits(old_heads)
.union(&revset_util::parse_immutable_heads_expression(
&tx.base_workspace_helper().revset_parse_context(),
)?)
.range(&RevsetExpression::commits(new_heads));
let config = command.settings().config();
let is_private = if let Ok(revset) = config.get_string("git.private-commits") {
workspace_helper
.parse_revset(&RevisionArg::from(revset))?
.evaluate()?
.containing_fn()
} else {
Box::new(|_: &CommitId| false)
};
for commit in workspace_helper
.attach_revset_evaluator(commits_to_push)?
.evaluate_to_commits()?
{
let commit = commit?;
let mut reasons = vec![];
if commit.description().is_empty() && !args.allow_empty_description {
reasons.push("it has no description");
}
if commit.author().name.is_empty()
|| commit.author().name == UserSettings::USER_NAME_PLACEHOLDER
|| commit.author().email.is_empty()
|| commit.author().email == UserSettings::USER_EMAIL_PLACEHOLDER
|| commit.committer().name.is_empty()
|| commit.committer().name == UserSettings::USER_NAME_PLACEHOLDER
|| commit.committer().email.is_empty()
|| commit.committer().email == UserSettings::USER_EMAIL_PLACEHOLDER
{
reasons.push("it has no author and/or committer set");
}
if commit.has_conflict()? {
reasons.push("it has conflicts");
}
if is_private(commit.id()) {
reasons.push("it is private");
}
if !reasons.is_empty() {
return Err(user_error(format!(
"Won't push commit {} since {}",
short_commit_hash(commit.id()),
reasons.join(" and ")
)));
}
}
Ok(())
}
fn get_default_push_remote(
ui: &Ui,
settings: &UserSettings,

1
cli/tests/runner.rs
View file

@ -36,6 +36,7 @@ mod test_git_colocated;
mod test_git_fetch;
mod test_git_import_export;
mod test_git_init;
mod test_git_private_commits;
mod test_git_push;
mod test_git_remotes;
mod test_git_submodule;

226
cli/tests/test_git_private_commits.rs Normal file
View file

@ -0,0 +1,226 @@
// Copyright 2024 The Jujutsu Authors
//
// 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 std::path::{Path, PathBuf};
use crate::common::TestEnvironment;
fn set_up() -> (TestEnvironment, PathBuf) {
let test_env = TestEnvironment::default();
test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "origin"]);
let origin_path = test_env.env_root().join("origin");
let origin_git_repo_path = origin_path
.join(".jj")
.join("repo")
.join("store")
.join("git");
test_env.jj_cmd_ok(&origin_path, &["describe", "-m=public 1"]);
test_env.jj_cmd_ok(&origin_path, &["new", "-m=public 2"]);
test_env.jj_cmd_ok(&origin_path, &["branch", "create", "main"]);
test_env.jj_cmd_ok(&origin_path, &["git", "export"]);
test_env.jj_cmd_ok(
test_env.env_root(),
&[
"git",
"clone",
"--config-toml=git.auto-local-branch=true",
origin_git_repo_path.to_str().unwrap(),
"local",
],
);
let workspace_root = test_env.env_root().join("local");
(test_env, workspace_root)
}
fn set_up_remote_at_main(test_env: &TestEnvironment, workspace_root: &Path, remote_name: &str) {
test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", remote_name]);
let other_path = test_env.env_root().join(remote_name);
let other_git_repo_path = other_path
.join(".jj")
.join("repo")
.join("store")
.join("git");
test_env.jj_cmd_ok(
workspace_root,
&[
"git",
"remote",
"add",
remote_name,
other_git_repo_path.to_str().unwrap(),
],
);
test_env.jj_cmd_ok(
workspace_root,
&["git", "push", "--remote", remote_name, "-b=main"],
);
}
#[test]
fn test_git_private_commits_block_pushing() {
let (test_env, workspace_root) = set_up();
test_env.jj_cmd_ok(&workspace_root, &["new", "main", "-m=private 1"]);
test_env.jj_cmd_ok(&workspace_root, &["branch", "set", "main"]);
// 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###"
Error: Won't push commit aa3058ff8663 since it is private
"###);
// May push when the commit is removed from git.private-commits
test_env.add_config(r#"git.private-commits = "none()""#);
let (_, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--all"]);
insta::assert_snapshot!(stderr, @r###"
Branch changes to push to origin:
Move forward branch main from 7eb97bf230ad to aa3058ff8663
Warning: The working-copy commit in workspace 'default' became immutable, so a new commit has been created on top of it.
Working copy now at: znkkpsqq 2e1adf47 (empty) (no description set)
Parent commit : yqosqzyt aa3058ff main | (empty) private 1
"###);
}
#[test]
fn test_git_private_commits_are_not_checked_if_immutable() {
let (test_env, workspace_root) = set_up();
test_env.jj_cmd_ok(&workspace_root, &["new", "main", "-m=private 1"]);
test_env.jj_cmd_ok(&workspace_root, &["branch", "set", "main"]);
test_env.add_config(r#"git.private-commits = "description(glob:'private*')""#);
test_env.add_config(r#"revset-aliases."immutable_heads()" = "all()""#);
let (_, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--all"]);
insta::assert_snapshot!(stderr, @r###"
Branch changes to push to origin:
Move forward branch main from 7eb97bf230ad to aa3058ff8663
Warning: The working-copy commit in workspace 'default' became immutable, so a new commit has been created on top of it.
Working copy now at: yostqsxw dce4a15c (empty) (no description set)
Parent commit : yqosqzyt aa3058ff main | (empty) private 1
"###);
}
#[test]
fn test_git_private_commits_not_directly_in_line_block_pushing() {
let (test_env, workspace_root) = set_up();
// New private commit descended from root()
test_env.jj_cmd_ok(&workspace_root, &["new", "root()", "-m=private 1"]);
test_env.jj_cmd_ok(&workspace_root, &["new", "main", "@", "-m=public 3"]);
test_env.jj_cmd_ok(&workspace_root, &["branch", "create", "branch1"]);
test_env.add_config(r#"git.private-commits = "description(glob:'private*')""#);
let stderr = test_env.jj_cmd_failure(&workspace_root, &["git", "push", "-b=branch1"]);
insta::assert_snapshot!(stderr, @r###"
Error: Won't push commit f1253a9b1ea9 since it is private
"###);
}
#[test]
fn test_git_private_commits_descending_from_commits_pushed_do_not_block_pushing() {
let (test_env, workspace_root) = set_up();
test_env.jj_cmd_ok(&workspace_root, &["new", "main", "-m=public 3"]);
test_env.jj_cmd_ok(&workspace_root, &["branch", "move", "main"]);
test_env.jj_cmd_ok(&workspace_root, &["new", "-m=private 1"]);
test_env.add_config(r#"git.private-commits = "description(glob:'private*')""#);
let (_, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "-b=main"]);
insta::assert_snapshot!(stderr, @r###"
Branch changes to push to origin:
Move forward branch main from 7eb97bf230ad to 05ef53bc99ec
"###);
}
#[test]
fn test_git_private_commits_already_on_the_remote_do_not_block_push() {
let (test_env, workspace_root) = set_up();
// Start a branch before a "private" commit lands in main
test_env.jj_cmd_ok(&workspace_root, &["branch", "create", "branch1", "-r=main"]);
// Push a commit that would become a private_root if it weren't already on
// the remote
test_env.jj_cmd_ok(&workspace_root, &["new", "main", "-m=private 1"]);
test_env.jj_cmd_ok(&workspace_root, &["new", "-m=public 3"]);
test_env.jj_cmd_ok(&workspace_root, &["branch", "set", "main"]);
let (_, stderr) =
test_env.jj_cmd_ok(&workspace_root, &["git", "push", "-b=main", "-b=branch1"]);
insta::assert_snapshot!(stderr, @r###"
Branch changes to push to origin:
Move forward branch main from 7eb97bf230ad to fbb352762352
Add branch branch1 to 7eb97bf230ad
Warning: The working-copy commit in workspace 'default' became immutable, so a new commit has been created on top of it.
Working copy now at: kpqxywon a7b08364 (empty) (no description set)
Parent commit : yostqsxw fbb35276 main | (empty) public 3
"###);
test_env.add_config(r#"git.private-commits = "description(glob:'private*')""#);
// Since "private 1" is already on the remote, pushing it should be allowed
test_env.jj_cmd_ok(&workspace_root, &["branch", "set", "branch1", "-r=main"]);
let (_, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--all"]);
insta::assert_snapshot!(stderr, @r###"
Branch changes to push to origin:
Move forward branch branch1 from 7eb97bf230ad to fbb352762352
"###);
// Ensure that the already-pushed commit doesn't block a new branch from
// being pushed
test_env.jj_cmd_ok(
&workspace_root,
&["new", "description('private 1')", "-m=public 4"],
);
test_env.jj_cmd_ok(&workspace_root, &["branch", "create", "branch2"]);
let (_, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "-b=branch2"]);
insta::assert_snapshot!(stderr, @r###"
Branch changes to push to origin:
Add branch branch2 to ee5b808b0b95
"###);
}
#[test]
fn test_git_private_commits_are_evaluated_separately_for_each_remote() {
let (test_env, workspace_root) = set_up();
set_up_remote_at_main(&test_env, &workspace_root, "other");
test_env.add_config(r#"revset-aliases."immutable_heads()" = "none()""#);
// Push a commit that would become a private_root if it weren't already on
// the remote
test_env.jj_cmd_ok(&workspace_root, &["new", "main", "-m=private 1"]);
test_env.jj_cmd_ok(&workspace_root, &["new", "-m=public 3"]);
test_env.jj_cmd_ok(&workspace_root, &["branch", "set", "main"]);
let (_, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "-b=main"]);
insta::assert_snapshot!(stderr, @r###"
Branch changes to push to origin:
Move forward branch main from 7eb97bf230ad to d8632ce893ab
"###);
test_env.add_config(r#"git.private-commits = "description(glob:'private*')""#);
// But pushing to a repo that doesn't have the private commit yet is still
// blocked
let stderr = test_env.jj_cmd_failure(
&workspace_root,
&["git", "push", "--remote=other", "-b=main"],
);
insta::assert_snapshot!(stderr, @r###"
Error: Won't push commit 36b7ecd11ad9 since it is private
"###);
}

18
docs/config.md
View file

@ -809,6 +809,24 @@ example:
git.push-branch-prefix = "martinvonz/push-"
### Set of private commits
You can configure the set of private commits by setting `git.private-commits` to
a revset. The value is a revset of commits that Jujutsu will refuse to push. If
unset, all commits are eligible to be pushed.
```toml
# Prevent pushing work in progress or anything explicitly labeled "private"
git.private-commits = "description(glob:'wip:*') | description(glob:'private:*')"
```
If a commit is in `git.private-commits` but is already on the remote, then it is
not considered a private commit. Commits that are immutable are also excluded
from the private set.
Private commits prevent their descendants from being pushed, since doing so
would require pushing the private commit as well.
## Filesystem monitor
In large repositories, it may be beneficial to use a "filesystem monitor" to