From 8bc4574ee582d6d7ccc4269edbb95c1be92e5f9b Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Tue, 12 Jul 2022 21:14:02 -0700 Subject: [PATCH] cli: push only branches pointing to `@` by default Since we now allow pushing open commits, we can implement support for pushing the "current" branch by defining a "current" branch as any branch pointing to `@`. That definition of a current/active seems to have been the consensus in discussion #411. Closes #246. --- CHANGELOG.md | 3 ++ docs/git-comparison.md | 2 +- src/commands.rs | 46 +++++++++++++++++++++++++++-- tests/test_git_push.rs | 67 ++++++++++++++++++++++++++++++++++++++---- 4 files changed, 109 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fe4c6d260..98875ab69 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * `jj git push` no longer aborts if you attempt to push an open commit (but it now aborts if a commit does not have a description). +* `jj git push` now pushes only branches pointing to the `@` by default. Use + `--all` to push all branches. + ### New features * `jj rebase` now accepts a `--branch/-b ` argument, which can be used diff --git a/docs/git-comparison.md b/docs/git-comparison.md index f04483e0b..c4f804a05 100644 --- a/docs/git-comparison.md +++ b/docs/git-comparison.md @@ -108,7 +108,7 @@ parent. Update a remote repo with all branches from the local repo - jj git push [--remote <remote>] (there is no + jj git push --all [--remote <remote>] (there is no support for pushing from non-Git repos yet) git push --all [<remote>] diff --git a/src/commands.rs b/src/commands.rs index 5a3c9be9e..ec01dfa8f 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -1858,9 +1858,11 @@ struct GitCloneArgs { /// Push to a Git remote /// -/// By default, all branches are pushed. Use `--branch` if you want to push only -/// one branch. +/// By default, pushes any branches pointing to `@`. Use `--branch` to push a +/// specific branch. Use `--all` to push all branches. Use `--change` to +/// generate a branch name based on a specific commit's change ID. #[derive(clap::Args, Clone, Debug)] +#[clap(group(ArgGroup::new("what").args(&["branch", "all", "change"])))] struct GitPushArgs { /// The remote to push to (only named remotes are supported) #[clap(long, default_value = "origin")] @@ -1868,6 +1870,9 @@ struct GitPushArgs { /// Push only this branch #[clap(long)] branch: Option, + /// Push all branches + #[clap(long)] + all: bool, /// Push this commit by creating a branch based on its change ID #[clap(long)] change: Option, @@ -5123,7 +5128,7 @@ fn cmd_git_push( branch_name, &args.remote, branch_name )?; } - } else { + } else if args.all { // TODO: Is it useful to warn about conflicted branches? for (branch_name, branch_target) in workspace_command.repo().view().branches() { let push_action = classify_branch_push_action(branch_target, &args.remote); @@ -5138,6 +5143,41 @@ fn cmd_git_push( } tx = workspace_command .start_transaction(&format!("push all branches to git remote {}", &args.remote)); + } else { + match workspace_command + .repo() + .view() + .get_checkout(&workspace_command.workspace_id()) + { + None => { + return Err(UserError( + "Nothing checked out in this workspace".to_string(), + )); + } + Some(checkout) => { + let desired_target = Some(RefTarget::Normal(checkout.clone())); + for (branch_name, branch_target) in workspace_command.repo().view().branches() { + if branch_target.local_target == desired_target { + let push_action = classify_branch_push_action(branch_target, &args.remote); + match push_action { + BranchPushAction::AlreadyMatches => {} + BranchPushAction::LocalConflicted => {} + BranchPushAction::RemoteConflicted => {} + BranchPushAction::Update(update) => { + branch_updates.push((branch_name.clone(), update)); + } + } + } + } + } + } + if branch_updates.is_empty() { + return Err(UserError("No current branch.".to_string())); + } + tx = workspace_command.start_transaction(&format!( + "push current branch(es) to git remote {}", + &args.remote + )); } if branch_updates.is_empty() { diff --git a/tests/test_git_push.rs b/tests/test_git_push.rs index c5aae65ea..fd59a546d 100644 --- a/tests/test_git_push.rs +++ b/tests/test_git_push.rs @@ -64,14 +64,71 @@ fn set_up() -> (TestEnvironment, PathBuf) { fn test_git_push_nothing() { let (test_env, workspace_root) = set_up(); // No branches to push yet - let stdout = test_env.jj_cmd_success(&workspace_root, &["git", "push"]); + let stdout = test_env.jj_cmd_success(&workspace_root, &["git", "push", "--all"]); insta::assert_snapshot!(stdout, @r###" Nothing changed. "###); } #[test] -fn test_git_push_success() { +fn test_git_push_current_branch() { + let (test_env, workspace_root) = set_up(); + // Update some branches. `branch1` is not a current branch, but `branch2` and + // `my-branch` are. + test_env.jj_cmd_success( + &workspace_root, + &["describe", "branch1", "-m", "modified branch1 commit"], + ); + test_env.jj_cmd_success( + &workspace_root, + &["branch", "set", "--allow-backwards", "branch2"], + ); + test_env.jj_cmd_success(&workspace_root, &["branch", "create", "my-branch"]); + test_env.jj_cmd_success(&workspace_root, &["describe", "-m", "foo"]); + // Check the setup + let stdout = test_env.jj_cmd_success(&workspace_root, &["branch", "list"]); + insta::assert_snapshot!(stdout, @r###" + branch1: 5d0d85ed3da7 modified branch1 commit + @origin (ahead by 1 commits, behind by 1 commits): 545acdb23f70 description + branch2: 7840c9885676 foo + @origin (ahead by 1 commits, behind by 1 commits): 545acdb23f70 description + my-branch: 7840c9885676 foo + "###); + // First dry-run. `branch1` should not get pushed. + let stdout = test_env.jj_cmd_success(&workspace_root, &["git", "push", "--dry-run"]); + insta::assert_snapshot!(stdout, @r###" + Branch changes to push to origin: + Move branch branch2 from 545acdb23f70 to 7840c9885676 + Add branch my-branch to 7840c9885676 + Dry-run requested, not pushing. + "###); + let stdout = test_env.jj_cmd_success(&workspace_root, &["git", "push"]); + insta::assert_snapshot!(stdout, @r###" + Branch changes to push to origin: + Move branch branch2 from 545acdb23f70 to 7840c9885676 + Add branch my-branch to 7840c9885676 + "###); + let stdout = test_env.jj_cmd_success(&workspace_root, &["branch", "list"]); + insta::assert_snapshot!(stdout, @r###" + branch1: 5d0d85ed3da7 modified branch1 commit + @origin (ahead by 1 commits, behind by 1 commits): 545acdb23f70 description + branch2: 7840c9885676 foo + my-branch: 7840c9885676 foo + "###); +} + +#[test] +fn test_git_push_no_current_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_all() { let (test_env, workspace_root) = set_up(); test_env.jj_cmd_success(&workspace_root, &["branch", "delete", "branch1"]); test_env.jj_cmd_success( @@ -90,7 +147,7 @@ fn test_git_push_success() { my-branch: 7840c9885676 foo "###); // First dry-run - let stdout = test_env.jj_cmd_success(&workspace_root, &["git", "push", "--dry-run"]); + let stdout = test_env.jj_cmd_success(&workspace_root, &["git", "push", "--all", "--dry-run"]); insta::assert_snapshot!(stdout, @r###" Branch changes to push to origin: Delete branch branch1 from 545acdb23f70 @@ -98,7 +155,7 @@ fn test_git_push_success() { Add branch my-branch to 7840c9885676 Dry-run requested, not pushing. "###); - let stdout = test_env.jj_cmd_success(&workspace_root, &["git", "push"]); + let stdout = test_env.jj_cmd_success(&workspace_root, &["git", "push", "--all"]); insta::assert_snapshot!(stdout, @r###" Branch changes to push to origin: Delete branch branch1 from 545acdb23f70 @@ -133,7 +190,7 @@ fn test_git_push_conflict() { test_env.jj_cmd_success(&workspace_root, &["rebase", "-r", "@", "-d", "@--"]); test_env.jj_cmd_success(&workspace_root, &["branch", "set", "my-branch"]); test_env.jj_cmd_success(&workspace_root, &["close", "-m", "third"]); - let stderr = test_env.jj_cmd_failure(&workspace_root, &["git", "push"]); + let stderr = test_env.jj_cmd_failure(&workspace_root, &["git", "push", "--all"]); insta::assert_snapshot!(stderr, @r###" Error: Won't push commit 50ccff1aeab0 since it has conflicts "###);