diff --git a/CHANGELOG.md b/CHANGELOG.md index acbd3409f..f946917f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 function parameter. For example, `author(foo@)` is now an error, and a revset alias `'revset-aliases.foo@' = '@'` will be ignored. +* `jj git push` will now push all branches in the range `remote_branches()..@` + instead of only branches pointing to `@` or `@-`. + ### New features * Default template for `jj log` now does not show irrelevant information diff --git a/cli/src/commands/git.rs b/cli/src/commands/git.rs index f877fc384..25a05ca9c 100644 --- a/cli/src/commands/git.rs +++ b/cli/src/commands/git.rs @@ -16,7 +16,7 @@ use jj_lib::op_store::{BranchTarget, RefTarget}; use jj_lib::refs::{classify_branch_push_action, BranchPushAction, BranchPushUpdate}; use jj_lib::repo::Repo; use jj_lib::repo_path::RepoPath; -use jj_lib::revset::{self, RevsetIteratorExt as _}; +use jj_lib::revset::{self, RevsetExpression, RevsetIteratorExt as _, StringPattern}; use jj_lib::settings::{ConfigResultExt as _, UserSettings}; use jj_lib::store::Store; use jj_lib::view::View; @@ -122,10 +122,10 @@ pub struct GitCloneArgs { /// Push to a Git remote /// -/// By default, pushes any branches pointing to `@`, or `@-` if no branches -/// point to `@`. Use `--branch` to push specific branches. Use `--all` to push -/// all branches. Use `--change` to generate branch names based on the change -/// IDs of specific commits. +/// By default, pushes any branches pointing to +/// `remote_branches(remote=)..@`. Use `--branch` to push specific +/// branches. Use `--all` to push all branches. Use `--change` to generate +/// branch names based on the change IDs of specific commits. #[derive(clap::Args, Clone, Debug)] #[command(group(ArgGroup::new("specific").args(&["branch", "change", "revisions"]).multiple(true)))] #[command(group(ArgGroup::new("what").args(&["all", "deleted"]).conflicts_with("specific")))] @@ -703,7 +703,7 @@ fn cmd_git_push( } } tx_description = format!("push all deleted branches to git remote {remote}"); - } else if !args.branch.is_empty() || !args.change.is_empty() || !args.revisions.is_empty() { + } else { let mut seen_branches = hashset! {}; for branch_name in &args.branch { if !seen_branches.insert(branch_name.clone()) { @@ -771,13 +771,30 @@ fn cmd_git_push( } } - // TODO: Narrow search space to local target commits. - // TODO: Remove redundant CommitId -> Commit -> CommitId round trip. - let revision_commit_ids: HashSet<_> = + let use_default_revset = + args.branch.is_empty() && args.change.is_empty() && args.revisions.is_empty(); + let revision_commit_ids: HashSet<_> = if use_default_revset { + let Some(wc_commit_id) = wc_commit_id else { + return Err(user_error("Nothing checked out in this workspace")); + }; + let current_branches_expression = RevsetExpression::remote_branches( + StringPattern::everything(), + StringPattern::Exact(remote.to_owned()), + ) + .range(&RevsetExpression::commit(wc_commit_id)) + .intersection(&RevsetExpression::branches(StringPattern::everything())); + let current_branches_revset = tx + .base_workspace_helper() + .evaluate_revset(current_branches_expression)?; + current_branches_revset.iter().collect() + } else { + // TODO: Narrow search space to local target commits. + // TODO: Remove redundant CommitId -> Commit -> CommitId round trip. resolve_multiple_nonempty_revsets(&args.revisions, tx.base_workspace_helper(), ui)? .iter() .map(|commit| commit.id().clone()) - .collect(); + .collect() + }; let branches_targeted = find_branches_targeting(repo.view(), |id| revision_commit_ids.contains(id)); for &(branch_name, branch_target) in &branches_targeted { @@ -790,7 +807,7 @@ fn cmd_git_push( Err(message) => writeln!(ui.warning(), "{message}")?, } } - if !args.revisions.is_empty() && branches_targeted.is_empty() { + if (!args.revisions.is_empty() || use_default_revset) && branches_targeted.is_empty() { writeln!( ui.warning(), "No branches point to the specified revisions." @@ -807,39 +824,7 @@ fn cmd_git_push( ), &remote ); - } else { - match wc_commit_id { - None => { - return Err(user_error("Nothing checked out in this workspace")); - } - Some(wc_commit) => { - // Search for branches targeting @ - let mut branches = find_branches_targeting(repo.view(), |id| id == &wc_commit); - if branches.is_empty() { - // Try @- instead if @ is discardable - let commit = repo.store().get_commit(&wc_commit)?; - if commit.is_discardable() { - if let [parent_commit_id] = commit.parent_ids() { - branches = - find_branches_targeting(repo.view(), |id| id == parent_commit_id); - } - } - } - if branches.is_empty() { - return Err(user_error("No current branch.")); - } - for (branch_name, branch_target) in branches { - match classify_branch_update(branch_name, branch_target, &remote) { - Ok(Some(update)) => branch_updates.push((branch_name.clone(), update)), - Ok(None) => {} - Err(message) => return Err(user_error(message)), - } - } - } - } - tx_description = format!("push current branch(es) to git remote {}", &remote); } - if branch_updates.is_empty() { writeln!(ui, "Nothing changed.")?; return Ok(()); diff --git a/cli/tests/test_git_push.rs b/cli/tests/test_git_push.rs index b5ef3ddb6..8e52383de 100644 --- a/cli/tests/test_git_push.rs +++ b/cli/tests/test_git_push.rs @@ -110,7 +110,8 @@ fn test_git_push_parent_branch() { &workspace_root, &["describe", "-m", "modified branch1 commit"], ); - test_env.jj_cmd_success(&workspace_root, &["new"]); + test_env.jj_cmd_success(&workspace_root, &["new", "-m", "non-empty description"]); + std::fs::write(workspace_root.join("file"), "file").unwrap(); let stdout = test_env.jj_cmd_success(&workspace_root, &["git", "push"]); insta::assert_snapshot!(stdout, @r###" Branch changes to push to origin: @@ -119,54 +120,79 @@ fn test_git_push_parent_branch() { } #[test] -fn test_git_no_push_parent_branch_non_empty_commit() { - let (test_env, workspace_root) = set_up(); - test_env.jj_cmd_success(&workspace_root, &["edit", "branch1"]); - test_env.jj_cmd_success( - &workspace_root, - &["describe", "-m", "modified branch1 commit"], - ); - test_env.jj_cmd_success(&workspace_root, &["new"]); - std::fs::write(workspace_root.join("file"), "file").unwrap(); - let stderr = test_env.jj_cmd_failure(&workspace_root, &["git", "push"]); - insta::assert_snapshot!(stderr, @r###" - Error: No current branch. - "###); -} - -#[test] -fn test_git_no_push_parent_branch_description() { - let (test_env, workspace_root) = set_up(); - test_env.jj_cmd_success(&workspace_root, &["edit", "branch1"]); - test_env.jj_cmd_success( - &workspace_root, - &["describe", "-m", "modified branch1 commit"], - ); - test_env.jj_cmd_success(&workspace_root, &["new", "-m", "non-empty description"]); - let stderr = test_env.jj_cmd_failure(&workspace_root, &["git", "push"]); - insta::assert_snapshot!(stderr, @r###" - Error: No current branch. - "###); -} - -#[test] -fn test_git_push_no_current_branch() { +fn test_git_push_no_matching_branch() { let (test_env, workspace_root) = set_up(); test_env.jj_cmd_success(&workspace_root, &["new"]); - let stderr = test_env.jj_cmd_failure(&workspace_root, &["git", "push"]); - insta::assert_snapshot!(stderr, @r###" - Error: No current branch. - "###); -} - -#[test] -fn test_git_push_current_branch_unchanged() { - let (test_env, workspace_root) = set_up(); - test_env.jj_cmd_success(&workspace_root, &["co", "branch1"]); - let stdout = test_env.jj_cmd_success(&workspace_root, &["git", "push"]); + let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push"]); insta::assert_snapshot!(stdout, @r###" Nothing changed. "###); + insta::assert_snapshot!(stderr, @r###" + No branches point to the specified revisions. + "###); +} + +#[test] +fn test_git_push_matching_branch_unchanged() { + let (test_env, workspace_root) = set_up(); + test_env.jj_cmd_success(&workspace_root, &["co", "branch1"]); + let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push"]); + insta::assert_snapshot!(stdout, @r###" + Nothing changed. + "###); + insta::assert_snapshot!(stderr, @r###" + No branches point to the specified revisions. + "###); +} + +/// Test that `jj git push` without arguments pushes a branch to the specified +/// remote even if it's already up to date on another remote +/// (`remote_branches(remote=)..@` vs. `remote_branches()..@`). +#[test] +fn test_git_push_other_remote_has_branch() { + let (test_env, workspace_root) = set_up(); + // Create another remote (but actually the same) + let other_remote_path = test_env + .env_root() + .join("origin") + .join(".jj") + .join("repo") + .join("store") + .join("git"); + test_env.jj_cmd_success( + &workspace_root, + &[ + "git", + "remote", + "add", + "other", + other_remote_path.to_str().unwrap(), + ], + ); + // Modify branch1 and push it to `origin` + test_env.jj_cmd_success(&workspace_root, &["edit", "branch1"]); + test_env.jj_cmd_success(&workspace_root, &["describe", "-m=modified"]); + let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push"]); + insta::assert_snapshot!(stdout, @r###" + Branch changes to push to origin: + Force branch branch1 from 45a3aa29e907 to 50421a29358a + "###); + insta::assert_snapshot!(stderr, @""); + // Since it's already pushed to origin, nothing will happen if push again + let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push"]); + insta::assert_snapshot!(stdout, @r###" + Nothing changed. + "###); + insta::assert_snapshot!(stderr, @r###" + No branches point to the specified revisions. + "###); + // But it will still get pushed to another remote + let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--remote=other"]); + insta::assert_snapshot!(stdout, @r###" + Branch changes to push to other: + Add branch branch1 to 50421a29358a + "###); + insta::assert_snapshot!(stderr, @""); } #[test] @@ -582,9 +608,12 @@ fn test_git_push_conflicting_branches() { }; // Conflicting branch at @ - let stderr = test_env.jj_cmd_failure(&workspace_root, &["git", "push"]); + let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push"]); + insta::assert_snapshot!(stdout, @r###" + Nothing changed. + "###); insta::assert_snapshot!(stderr, @r###" - Error: Branch branch2 is conflicted + Branch branch2 is conflicted "###); // --branch should be blocked by conflicting branch