git: add git.private-commits setting for preventing commits from bein…

…g pushed

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

CHANGELOG.md

@@ -67,6 +67,8 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 * New `diff_contains()` revset function can be used to search diffs.
 
+* 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.

cli/src/commands/git/push.rs

@@ -25,7 +25,7 @@ use jj_lib::refs::{
     classify_branch_push_action, BranchPushAction, BranchPushUpdate, LocalAndRemoteRef,
 };
 use jj_lib::repo::Repo;
-use jj_lib::revset::RevsetExpression;
+use jj_lib::revset::{self, RevsetExpression};
 use jj_lib::settings::{ConfigResultExt as _, UserSettings};
 use jj_lib::str_util::StringPattern;
 use jj_lib::view::View;
@@ -279,36 +279,59 @@ pub fn cmd_git_push(
             &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
+        let workspace_helper = tx.base_workspace_helper();
+
+        let git_settings = command.settings().git_settings();
+        let is_private = if let Some(private_commits) = git_settings.private_commits {
+            let private_commits_revset = revset::parse(
+                private_commits.as_str(),
+                &tx.base_workspace_helper().revset_parse_context(),
+            )?;
+            Some(
+                workspace_helper
+                    .attach_revset_evaluator(private_commits_revset)?
+                    .evaluate()?
+                    .containing_fn(),
+            )
+        } else {
+            None
+        };
+
+        for commit in workspace_helper
+            .attach_revset_evaluator(commits_to_push)?
+            .evaluate_to_commits()?
         {
-            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 ")
-            )));
+            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.as_ref().map_or(false, |f| f(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 ")
+                )));
+            }
         }
     }
 

cli/tests/runner.rs

@@ -51,6 +51,7 @@ mod test_next_prev_commands;
 mod test_obslog_command;
 mod test_operations;
 mod test_parallelize_command;
+mod test_private_commits;
 mod test_rebase_command;
 mod test_repo_change_report;
 mod test_resolve_command;

cli/tests/test_private_commits.rs

@@ -0,0 +1,227 @@
+// Copyright 2023 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"]);
+
+    // Will not push when a pushed commit is contained in git.private-commits
+    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
+    "###);
+}

docs/config.md

@@ -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:*')"
+```
+
+Any commit descending from a private commit is also prevented from being pushed,
+since doing so would require pushing the private commit as well.
+
+If a commit is in `git.private-commits` but is already on the remote, then it is
+ignored and does not prevent pushing its descendants. Commits that are immutable
+are also ignored.
+
 ## Filesystem monitor
 
 In large repositories, it may be beneficial to use a "filesystem monitor" to

lib/src/settings.rs

@@ -42,6 +42,7 @@ pub struct RepoSettings {
 pub struct GitSettings {
     pub auto_local_branch: bool,
     pub abandon_unreachable_commits: bool,
+    pub private_commits: Option<String>,
 }
 
 impl GitSettings {
@@ -51,6 +52,7 @@ impl GitSettings {
             abandon_unreachable_commits: config
                 .get_bool("git.abandon-unreachable-commits")
                 .unwrap_or(true),
+            private_commits: config.get_string("git.private-commits").ok(),
         }
     }
 }
@@ -60,6 +62,7 @@ impl Default for GitSettings {
         GitSettings {
             auto_local_branch: false,
             abandon_unreachable_commits: true,
+            private_commits: None,
         }
     }
 }