diff --git a/cli/src/commands/git/push.rs b/cli/src/commands/git/push.rs index 632a3f4e38a..a84a842c974 100644 --- a/cli/src/commands/git/push.rs +++ b/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::>(); + 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 {}", diff --git a/cli/src/config/revsets.toml b/cli/src/config/revsets.toml index f91b3930933..b1b71721802 100644 --- a/cli/src/config/revsets.toml +++ b/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()' diff --git a/cli/src/revset_util.rs b/cli/src/revset_util.rs index 96d3adeb80b..41e671b59cf 100644 --- a/cli/src/revset_util.rs +++ b/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, 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<'_>, diff --git a/cli/tests/runner.rs b/cli/tests/runner.rs index 0a05d943f37..41c614dc0a0 100644 --- a/cli/tests/runner.rs +++ b/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; diff --git a/cli/tests/test_private_commits.rs b/cli/tests/test_private_commits.rs new file mode 100644 index 00000000000..dcefb1a1238 --- /dev/null +++ b/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 + "###); +} diff --git a/docs/revsets.md b/docs/revsets.md index ada858a24d1..ba476de6019 100644 --- a/docs/revsets.md +++ b/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