revset: add private_roots revset for preventing commits from being pu…

…shed

The user can define the revset alias `private_roots()` as they desire. For
example:

    revset-aliases.'private_roots()' = '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 `private_roots()`.

Closes #3376

cli/src/commands/git/push.rs

@@ -279,6 +279,22 @@ pub fn cmd_git_push(
             &tx.base_workspace_helper().revset_parse_context(),
         )?)
         .range(&RevsetExpression::commits(new_heads));
+
+    // Only the roots themselves need to be checked if they're being pushed. If
+    // a root is already on the remote, then it is "grandfathered in" and
+    // shouldn't block any descendents from being pushed (unless the descendents
+    // are in private_roots()).
+    let private_commits_revset = revset_util::parse_private_roots_expression(
+        &tx.base_workspace_helper().revset_parse_context(),
+    )?;
+
+    let private_commits = tx
+        .base_workspace_helper()
+        .attach_revset_evaluator(private_commits_revset)?
+        .evaluate_to_commits()?
+        .filter_map(|ch| ch.map(|c| c.id().to_owned()).ok())
+        .collect::<HashSet<_>>();
+
     for commit in tx
         .base_workspace_helper()
         .attach_revset_evaluator(commits_to_push)?
@@ -303,6 +319,9 @@ pub fn cmd_git_push(
         if commit.has_conflict()? {
             reasons.push("it has conflicts");
         }
+        if private_commits.contains(commit.id()) {
+            reasons.push("it is private");
+        }
         if !reasons.is_empty() {
             return Err(user_error(format!(
                 "Won't push commit {} since {}",

cli/src/config/revsets.toml

@@ -21,3 +21,5 @@ latest(
 'immutable_heads()' = 'trunk() | tags() | untracked_remote_branches()'
 'immutable()' = '::(immutable_heads() | root())'
 'mutable()' = '~immutable()'
+
+'private_roots()' = 'none()'

cli/src/revset_util.rs

@@ -37,6 +37,7 @@ use crate::templater::TemplateRenderer;
 use crate::ui::Ui;
 
 const BUILTIN_IMMUTABLE_HEADS: &str = "immutable_heads";
+const BUILTIN_PRIVATE_ROOTS: &str = "private_roots";
 
 #[derive(Debug, Error)]
 pub enum UserRevsetEvaluationError {
@@ -182,6 +183,17 @@ pub fn parse_immutable_expression(
     Ok(parse_immutable_heads_expression(context)?.ancestors())
 }
 
+/// Parses user-configured expression defining the roots of the private set.
+pub fn parse_private_roots_expression(
+    context: &RevsetParseContext,
+) -> Result<Rc<RevsetExpression>, RevsetParseError> {
+    let (_, _, private_roots_string) = context
+        .aliases_map()
+        .get_function(BUILTIN_PRIVATE_ROOTS, 0)
+        .unwrap();
+    revset::parse(private_roots_string, context)
+}
+
 pub(super) fn evaluate_revset_to_single_commit<'a>(
     revision_str: &str,
     expression: &RevsetExpressionEvaluator<'_>,

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,211 @@
+// 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::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: &PathBuf, 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 the commit is contained in private_roots()::
+    test_env.add_config(r#"revset-aliases."private_roots()" = "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
+    "###);
+
+    // Will not push when the commit is removed from private_roots()::
+    test_env.add_config(r#"revset-aliases."private_roots()" = "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 the commit is contained in private_roots()::
+    test_env.add_config(r#"revset-aliases."private_roots()" = "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#"revset-aliases."private_roots()" = "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_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#"revset-aliases."private_roots()" = "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#"revset-aliases."private_roots()" = "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/revsets.md

@@ -412,6 +412,11 @@ for a comprehensive list.
   *not* change whether a commit is immutable. To do that, edit
   `immutable_heads()`.
 
+* `private_roots()`: The set of commits that `jj` treats as private and will
+  refuse to push. For example, setting this to `description(glob:"wip:*")` will
+  prevent accidentally pushing any commit that has a description starting with
+  "wip:". Resolves to `none()` by default.
+
 
 ## The `all:` modifier