Re: [PATCH v3 2/3] worktree: make add <path> <branch> dwim
From: Eric Sunshine <hidden>
Date: 2017-11-19 08:32:05
On Sat, Nov 18, 2017 at 5:47 PM, Thomas Gummerer [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Currently 'git worktree add <path> <branch>', errors out when 'branch' is not a local branch. It has no additional dwim'ing features that one might expect. Make it behave more like 'git checkout <branch>' when the branch doesn't exist locally, but a remote tracking branch uniquely matches the desired branch name, i.e. create a new branch from the remote tracking branch and set the upstream to the remote tracking branch. As 'git worktree add' currently just dies in this situation, there are no backwards compatibility worries when introducing this feature. Signed-off-by: Thomas Gummerer <redacted> ---diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt@@ -52,6 +52,13 @@ is linked to the current repository, sharing everything except working +If <branch> is not found, and neither `-b` nor `-B` nor `--detach` are +used, but there does exist a tracking branch in exactly one remote +(call it <remote>) with a matching name, treat as equivalent to +------------ +$ git worktree add -b <branch> <path> <remote>/<branch> +------------
The example from which this was copied in git-checkout.txt shows the --track option being used, which makes it clear that the new local branch will track the remote-tracking branch. git-worktree, of course, does not have a --track option, but would it make sense to mention explicitly in the prose that the newly-created local branch tracks the remote one? (Or are readers sufficiently savvy to intuit it?)
quoted hunk ↗ jump to hunk
diff --git a/builtin/worktree.c b/builtin/worktree.c index 7b9307aa58..05fc902bcc 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c@@ -1,4 +1,5 @@ #include "cache.h" +#include "checkout.h" #include "config.h" #include "builtin.h" #include "dir.h"@@ -386,6 +387,20 @@ static int add(int ac, const char **av, const char *prefix) opts.new_branch = xstrndup(s, n); } + if (ac == 2 && !opts.new_branch && !opts.detach) { + struct object_id oid; + struct commit *commit; + const char *remote; + + commit = lookup_commit_reference_by_name(branch); + if (!commit) + remote = unique_tracking_name(branch, &oid); + if (!commit && remote) { + opts.new_branch = branch; + branch = remote; + } + }
You can simplify the above conditionals to:
commit = ...
if (!commit) {
remote = ...
if (remote) {
...
}
}
So, although you're not passing --track explicitly to the "git branch"
invocation just below, you get --track for free since it's the default
behavior when creating a new local branch from a remote one, correct?
(Just checking my understanding.)
quoted hunk ↗ jump to hunk
if (opts.new_branch) { struct child_process cp = CHILD_PROCESS_INIT; cp.git_cmd = 1;diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh@@ -6,6 +6,16 @@ test_description='test git worktree add' +# Is branch "refs/heads/$1" set to pull from "$2/$3"? +test_branch_upstream () { + printf "%s\n" "$2" "refs/heads/$3" >expect.upstream && + { + git config "branch.$1.remote" && + git config "branch.$1.merge" + } >actual.upstream && + test_cmp expect.upstream actual.upstream +}
Not a big deal, but it wouldn't hurt to move this function down to the point where it is first needed...
quoted hunk ↗ jump to hunk
test_expect_success 'setup' ' test_commit init '@@ -314,4 +324,40 @@ test_expect_success 'rename a branch under bisect not allowed' ' +test_expect_success '"add" <path> <non-existent-branch> fails' ' + test_must_fail git worktree add foo non-existent +' + +test_expect_success '"add" <path> <branch> dwims' ' + test_when_finished rm -rf repo_upstream && + test_when_finished rm -rf repo_dwim && + test_when_finished rm -rf foo &&
Also not a big deal, but could all be combined into a single invocation:
test_when_finished rm -rf repo_upstream repo_dwim foo &&
+ git init repo_upstream && + ( + cd repo_upstream && + test_commit upstream_master && + git checkout -b foo && + test_commit a_foo + ) && + git init repo_dwim && + ( + cd repo_dwim && + test_commit dwim_master && + git remote add repo_upstream ../repo_upstream && + git config remote.repo_upstream.fetch \ + "refs/heads/*:refs/remotes/repo_upstream/*" && + git fetch --all && + test_must_fail git worktree add -b foo ../foo foo && + test_must_fail git worktree add --detach ../foo foo && + git worktree add ../foo foo + ) && + ( + cd foo && + test_branch_upstream foo repo_upstream foo && + git rev-parse repo_upstream/foo >expect && + git rev-parse foo >actual && + test_cmp expect actual + ) +'