[PATCH v1 1/2] checkout: factor out functions to new lib file

[PATCH v1 1/2] checkout: factor out functions to new lib file

[Thomas Gummerer]

Factor the functions out, so they can be re-used from other places.  In
particular these functions will be re-used in builtin/worktree.c to make
git worktree add dwim more.

While there add a description to the function.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---

I'm not sure where these functions should go ideally, they don't seem
to fit in any existing library file, so I created a new one for now.
Any suggestions for a better home are welcome!

 Makefile           |  1 +
 builtin/checkout.c | 41 +----------------------------------------
 checkout.c         | 43 +++++++++++++++++++++++++++++++++++++++++++
 checkout.h         | 14 ++++++++++++++
 4 files changed, 59 insertions(+), 40 deletions(-)
 create mode 100644 checkout.c
 create mode 100644 checkout.h

diff --git a/Makefile b/Makefile
index cd75985991..8d603c7443 100644
--- a/Makefile
+++ b/Makefile
@@ -757,6 +757,7 @@ LIB_OBJS += branch.o
 LIB_OBJS += bulk-checkin.o
 LIB_OBJS += bundle.o
 LIB_OBJS += cache-tree.o
+LIB_OBJS += checkout.o
 LIB_OBJS += color.o
 LIB_OBJS += column.o
 LIB_OBJS += combine-diff.o
diff --git a/builtin/checkout.c b/builtin/checkout.c
index fc4f8fd2ea..9e1cfd10b3 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1,5 +1,6 @@
 #include "builtin.h"
 #include "config.h"
+#include "checkout.h"
 #include "lockfile.h"
 #include "parse-options.h"
 #include "refs.h"
@@ -874,46 +875,6 @@ static int git_checkout_config(const char *var, const char *value, void *cb)
 	return git_xmerge_config(var, value, NULL);
 }
 
-struct tracking_name_data {
-	/* const */ char *src_ref;
-	char *dst_ref;
-	struct object_id *dst_oid;
-	int unique;
-};
-
-static int check_tracking_name(struct remote *remote, void *cb_data)
-{
-	struct tracking_name_data *cb = cb_data;
-	struct refspec query;
-	memset(&query, 0, sizeof(struct refspec));
-	query.src = cb->src_ref;
-	if (remote_find_tracking(remote, &query) ||
-	    get_oid(query.dst, cb->dst_oid)) {
-		free(query.dst);
-		return 0;
-	}
-	if (cb->dst_ref) {
-		free(query.dst);
-		cb->unique = 0;
-		return 0;
-	}
-	cb->dst_ref = query.dst;
-	return 0;
-}
-
-static const char *unique_tracking_name(const char *name, struct object_id *oid)
-{
-	struct tracking_name_data cb_data = { NULL, NULL, NULL, 1 };
-	cb_data.src_ref = xstrfmt("refs/heads/%s", name);
-	cb_data.dst_oid = oid;
-	for_each_remote(check_tracking_name, &cb_data);
-	free(cb_data.src_ref);
-	if (cb_data.unique)
-		return cb_data.dst_ref;
-	free(cb_data.dst_ref);
-	return NULL;
-}
-
 static int parse_branchname_arg(int argc, const char **argv,
 				int dwim_new_local_branch_ok,
 				struct branch_info *new,
diff --git a/checkout.c b/checkout.c
new file mode 100644
index 0000000000..a9cf378453
--- /dev/null
+++ b/checkout.c
@@ -0,0 +1,43 @@
+#include "cache.h"
+
+#include "remote.h"
+
+struct tracking_name_data {
+	/* const */ char *src_ref;
+	char *dst_ref;
+	struct object_id *dst_oid;
+	int unique;
+};
+
+static int check_tracking_name(struct remote *remote, void *cb_data)
+{
+	struct tracking_name_data *cb = cb_data;
+	struct refspec query;
+	memset(&query, 0, sizeof(struct refspec));
+	query.src = cb->src_ref;
+	if (remote_find_tracking(remote, &query) ||
+	    get_oid(query.dst, cb->dst_oid)) {
+		free(query.dst);
+		return 0;
+	}
+	if (cb->dst_ref) {
+		free(query.dst);
+		cb->unique = 0;
+		return 0;
+	}
+	cb->dst_ref = query.dst;
+	return 0;
+}
+
+const char *unique_tracking_name(const char *name, struct object_id *oid)
+{
+	struct tracking_name_data cb_data = { NULL, NULL, NULL, 1 };
+	cb_data.src_ref = xstrfmt("refs/heads/%s", name);
+	cb_data.dst_oid = oid;
+	for_each_remote(check_tracking_name, &cb_data);
+	free(cb_data.src_ref);
+	if (cb_data.unique)
+		return cb_data.dst_ref;
+	free(cb_data.dst_ref);
+	return NULL;
+}
diff --git a/checkout.h b/checkout.h
new file mode 100644
index 0000000000..9272fe1449
--- /dev/null
+++ b/checkout.h
@@ -0,0 +1,14 @@
+#ifndef CHECKOUT_H
+#define CHECKOUT_H
+
+#include "git-compat-util.h"
+#include "cache.h"
+
+/*
+ * Check if the branch name uniquely matches a branch name on a remote
+ * tracking branch.  Return the name of the remote if such a branch
+ * exists, NULL otherwise.
+ */
+extern const char *unique_tracking_name(const char *name, struct object_id *oid);
+
+#endif /* CHECKOUT_H */
-- 
2.15.0.403.gc27cc4dac6

[PATCH v1 2/2] worktree: make add dwim

[Thomas Gummerer]

Currently git worktree add creates a new branch, that matches the HEAD
of whichever worktree we were on when calling "git worktree add".

Add a --guess flag to git worktree add, that makes it behave like the
dwim machinery in 'git checkout <new-branch>', i.e. check if the new
branch name uniquely matches the branch name of a remote tracking
branch, and if so check out that branch, and set the upstream to the
remote tracking branch.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---

I'm a bit torn about hiding his behind an additional flag in git
worktree add or not.  I would like to have the feature without the
additional flag, but it might break some peoples expectations, so
dunno.

 builtin/worktree.c      | 14 +++++++++-
 t/t2025-worktree-add.sh | 70 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 83 insertions(+), 1 deletion(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 7b9307aa58..5740d1f8a3 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"
@@ -341,6 +342,7 @@ static int add(int ac, const char **av, const char *prefix)
 	const char *new_branch_force = NULL;
 	char *path;
 	const char *branch;
+	int dwim_new_branch = 0;
 	struct option options[] = {
 		OPT__FORCE(&opts.force, N_("checkout <branch> even if already checked out in other worktree")),
 		OPT_STRING('b', NULL, &opts.new_branch, N_("branch"),
@@ -350,6 +352,7 @@ static int add(int ac, const char **av, const char *prefix)
 		OPT_BOOL(0, "detach", &opts.detach, N_("detach HEAD at named commit")),
 		OPT_BOOL(0, "checkout", &opts.checkout, N_("populate the new working tree")),
 		OPT_BOOL(0, "lock", &opts.keep_locked, N_("keep the new working tree locked")),
+		OPT_BOOL(0, "guess", &dwim_new_branch, N_("checkout upstream branch if there's a unique match")),
 		OPT_END()
 	};
 
@@ -363,6 +366,7 @@ static int add(int ac, const char **av, const char *prefix)
 
 	path = prefix_filename(prefix, av[0]);
 	branch = ac < 2 ? "HEAD" : av[1];
+	dwim_new_branch = ac < 2 ? dwim_new_branch : 0;
 
 	if (!strcmp(branch, "-"))
 		branch = "@{-1}";
@@ -387,13 +391,21 @@ static int add(int ac, const char **av, const char *prefix)
 	}
 
 	if (opts.new_branch) {
+		struct object_id oid;
+		const char *remote;
 		struct child_process cp = CHILD_PROCESS_INIT;
+
+		remote = unique_tracking_name(opts.new_branch, &oid);
+
 		cp.git_cmd = 1;
 		argv_array_push(&cp.args, "branch");
 		if (opts.force_new_branch)
 			argv_array_push(&cp.args, "--force");
 		argv_array_push(&cp.args, opts.new_branch);
-		argv_array_push(&cp.args, branch);
+		if (dwim_new_branch && remote)
+			argv_array_push(&cp.args, remote);
+		else
+			argv_array_push(&cp.args, branch);
 		if (run_command(&cp))
 			return -1;
 		branch = opts.new_branch;
diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index b5c47ac602..b37c279787 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -6,6 +6,16 @@ test_description='test git worktree add'
 
 . "$TEST_DIRECTORY"/lib-rebase.sh
 
+# 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
+}
+
 test_expect_success 'setup' '
 	test_commit init
 '
@@ -314,4 +324,64 @@ test_expect_success 'rename a branch under bisect not allowed' '
 	test_must_fail git branch -M under-bisect bisect-with-new-name
 '
 
+test_expect_success 'git worktree add does not dwim' '
+	test_when_finished rm -rf repo_a &&
+	test_when_finished rm -rf repo_b &&
+	test_when_finished rm -rf foo &&
+	git init repo_a &&
+	(
+		cd repo_a &&
+		test_commit a_master &&
+		git checkout -b foo &&
+		test_commit a_foo
+	) &&
+	git init repo_b &&
+	(
+		cd repo_b &&
+		test_commit b_master &&
+		git remote add repo_a ../repo_a &&
+		git config remote.repo_a.fetch \
+			"+refs/heads/*:refs/remotes/other_a/*" &&
+		git fetch --all &&
+		git worktree add ../foo
+	) &&
+	(
+		cd foo &&
+		! test_branch_upstream foo repo_a foo &&
+		git rev-parse other_a/foo >expect &&
+		git rev-parse foo >actual &&
+		! test_cmp expect actual
+	)
+'
+
+test_expect_success 'git worktree add --guess dwims' '
+	test_when_finished rm -rf repo_a &&
+	test_when_finished rm -rf repo_b &&
+	test_when_finished rm -rf foo &&
+	git init repo_a &&
+	(
+		cd repo_a &&
+		test_commit a_master &&
+		git checkout -b foo &&
+		test_commit a_foo
+	) &&
+	git init repo_b &&
+	(
+		cd repo_b &&
+		test_commit b_master &&
+		git remote add repo_a ../repo_a &&
+		git config remote.repo_a.fetch \
+			"+refs/heads/*:refs/remotes/other_a/*" &&
+		git fetch --all &&
+		git worktree add --guess ../foo
+	) &&
+	(
+		cd foo &&
+		test_branch_upstream foo repo_a foo &&
+		git rev-parse other_a/foo >expect &&
+		git rev-parse foo >actual &&
+		test_cmp expect actual
+	)
+'
+
 test_done
-- 
2.15.0.403.gc27cc4dac6

Re: [PATCH v1 2/2] worktree: make add dwim

[Junio C Hamano]

Thomas Gummerer <t.gummerer@gmail.com> writes:

> I'm a bit torn about hiding his behind an additional flag in git
> worktree add or not.  I would like to have the feature without the
> additional flag, but it might break some peoples expectations, so
> dunno.

Yeah, I agree with the sentiment.  But what expectation would we be
breaking, I wonder (see below)?

At the conceptual level, it is even more unfortunate that "git
worktree --help" says this for "git worktree add <path> [<branch>]":

    If `<branch>` is omitted and neither `-b` nor `-B` nor
    `--detach` used, then, as a convenience, a new branch based at
    HEAD is created automatically, as if `-b $(basename <path>)` was
    specified.

which means that it already does a DWIM, namely "since you didn't
say what branch to create to track what other branch, we'll fork one
for you from the HEAD, and give a name to it".  That may be a useful
DWIM for some existing users sometimes, and you may even find it
useful some of the time but not always.  Different people mean
different things in different situations, and there is no single
definition for DWIMming that would fit all of them.

Which in turn means that the new variable name DWIM_NEW_BRANCH and
the new option name GUESS are way underspecified.  You'd need to
name them after the "kind" of dwim; otherwise, not only the future
additions for new (third) kind of dwim would become cumbersome, but
readers of the code would be confused if they are about the existing
dwim (fork a new branch from HEAD and give it a name guessed by the
pathname) or your new dwim.

This also needs a documentation update.  Unlike the existing DWIM,
it is totally unclear when you are dwimming in absence of which
option.

I am guessing that, when you do not have a branch "topic" but your
upstream does, saying

    $ git worktree add ../a-new-worktree topic

would create "refs/heads/topic" to build on top of
"refs/remotes/origin/topic" just like "git checkout topic" would.

IOW, when fully spelled out:

    $ git branch -t -b topic origin/topic
    $ git worktree add ../a-new-worktree topic

is what your DWIM does?  Am I following you correctly?

If so, as long as the new DWIM kicks in ONLY when "topic" does not
exist, I suspect that there is no backward compatibility worries.
The command line

    $ git worktree add ../a-new-worktree topic

would just have failed because three was no 'topic' branch yet, no?

Re: [PATCH v1 2/2] worktree: make add dwim

[Thomas Gummerer]

On 11/13, Junio C Hamano wrote:
> Thomas Gummerer <t.gummerer@gmail.com> writes:
> 
> > I'm a bit torn about hiding his behind an additional flag in git
> > worktree add or not.  I would like to have the feature without the
> > additional flag, but it might break some peoples expectations, so
> > dunno.
> 
> Yeah, I agree with the sentiment.  But what expectation would we be
> breaking, I wonder (see below)?
> 
> At the conceptual level, it is even more unfortunate that "git
> worktree --help" says this for "git worktree add <path> [<branch>]":
> 
>     If `<branch>` is omitted and neither `-b` nor `-B` nor
>     `--detach` used, then, as a convenience, a new branch based at
>     HEAD is created automatically, as if `-b $(basename <path>)` was
>     specified.
> 
> which means that it already does a DWIM, namely "since you didn't
> say what branch to create to track what other branch, we'll fork one
> for you from the HEAD, and give a name to it".  That may be a useful
> DWIM for some existing users sometimes, and you may even find it
> useful some of the time but not always.  Different people mean
> different things in different situations, and there is no single
> definition for DWIMming that would fit all of them.
> 
> Which in turn means that the new variable name DWIM_NEW_BRANCH and
> the new option name GUESS are way underspecified.  You'd need to
> name them after the "kind" of dwim; otherwise, not only the future
> additions for new (third) kind of dwim would become cumbersome, but
> readers of the code would be confused if they are about the existing
> dwim (fork a new branch from HEAD and give it a name guessed by the
> pathname) or your new dwim.

Yeah I definitely agree with that.  I was mainly thinking about my
personal use case with --guess, and didn't fully think through that it
might mean different things to other people.

> This also needs a documentation update.  Unlike the existing DWIM,
> it is totally unclear when you are dwimming in absence of which
> option.

Sorry about that, I totally forgot about the documentation for this.
Will add in the next round.

> I am guessing that, when you do not have a branch "topic" but your
> upstream does, saying
> 
>     $ git worktree add ../a-new-worktree topic
> 
> would create "refs/heads/topic" to build on top of
> "refs/remotes/origin/topic" just like "git checkout topic" would.
> 
> IOW, when fully spelled out:
> 
>     $ git branch -t -b topic origin/topic
>     $ git worktree add ../a-new-worktree topic
> 
> is what your DWIM does?  Am I following you correctly?

It's not quite what my DWIM does/what I originally intended my DWIM to
do.  What it does is when

    $ git worktree add ../topic

and omitting the branch argument, it would behave the way you spelled
out, i.e.

    $ git branch -t -b topic origin/topic
    $ git worktree add ../topic topic

instead of just checking it out from HEAD.  I must confess I missed
the branch argument, so that still behaves the old way with my code,
while what you have above would make a lot more sense.

> If so, as long as the new DWIM kicks in ONLY when "topic" does not
> exist, I suspect that there is no backward compatibility worries.
> The command line
> 
>     $ git worktree add ../a-new-worktree topic
> 
> would just have failed because three was no 'topic' branch yet, no?

Indeed, with this there would not be any backwards compatility
worries.

Ideally I'd still like to make

    $ git worktree add ../topic

work as described above, to avoid having to type 'topic' twice (the
directory name matches the branch name for the vast majority of my
worktrees) but that should then come behind a flag/config option?
Following your reasoning above it should probably be called something
other than --guess.

Maybe --guess-remote and worktree.guessRemote would do?  I'm quite bad
at naming though, so other suggestions are very welcome.

Re: [PATCH v1 2/2] worktree: make add dwim

[Eric Sunshine]

On Tue, Nov 14, 2017 at 3:45 AM, Thomas Gummerer <t.gummerer@gmail.com> wrote:
> On 11/13, Junio C Hamano wrote:
>> If so, as long as the new DWIM kicks in ONLY when "topic" does not
>> exist, I suspect that there is no backward compatibility worries.
>> The command line
>>
>>     $ git worktree add ../a-new-worktree topic
>>
>> would just have failed because three was no 'topic' branch yet, no?
>
> Indeed, with this there would not be any backwards compatility
> worries.
>
> Ideally I'd still like to make
>
>     $ git worktree add ../topic
>
> work as described above, to avoid having to type 'topic' twice (the
> directory name matches the branch name for the vast majority of my
> worktrees) but that should then come behind a flag/config option?
> Following your reasoning above it should probably be called something
> other than --guess.
>
> Maybe --guess-remote and worktree.guessRemote would do?  I'm quite bad
> at naming though, so other suggestions are very welcome.

For my own edification...

git worktree add ../dir branch

* Checks out branch into ../dir if branch exists.

* Errors out if branch does not exist. However, if we consider the
history of the implementation of worktrees[*1*], then this really
ought to try the "origin/branch -> branch" DWIM before erroring-out.
Implementing this DWIM could be considered a regression fix according
to [*1*] and, as Junio points out, should not pose backward
compatibility worries.

git worktree add ../topic

* Correctly errors out, refusing to create a new branch named "topic",
if "topic" is already a branch.

* Creates a new branch named "topic" if no such local branch exists.

The desired new DWIMing would change the second bullet point to:

* If no local branch named "topic" exists, DWIM it from "origin/topic"
if possible, else create a new local branch named "topic".

But that's a behavior change since, with the existing implementation,
a newly created local "topic" has no relation to, and does not track,
any origin/topic branch. The proposed --guess option is to avoid users
being hit by this backward incompatibility, however, one could also
view the proposed DWIMing as some sort of bug fix since, at least
some, users would expect the DWIMing since it already takes place
elsewhere.

So, at least two options exist:

* Just make the new DWIMing the default behavior, accepting that it
might bite a few people. Fallout can be mitigated somewhat by
prominently announcing that the DWIMing took place, in which case the
user can take corrective action (say, by choosing a different worktree
name); nothing is lost and no damage done since this is happening only
at worktree creation time.

* Add a new option to enable DWIMing; perhaps name it -t/--track,
which is familiar terminology and still gives you a relatively short
and sweet "git worktree add -t ../topic".

Given the mentioned mitigation factor and that some/many users likely
would expect it to DWIM "origin/topic -> topic" anyhow, I'd lean in
favor of the first option (but perhaps I'm too daring with other
people's workflows).

FOOTNOTES

[*1*]: When Duy first implemented worktree support, he incorporated it
directly into the git-checkout command ("git checkout --to worktree
..."), which means that he got all the git-checkout features for free,
including the "origin/branch -> branch" DWIM. When worktree support
was later moved to git-worktree, it lost most of the features
inherited implicitly from git-checkout, such as -b, -B, --detach, so
those were added back to git-worktree explicitly. However, at that
early stage, git-worktree was still piggy-backing atop git-checkout,
thus likely was still getting the "origin/branch -> branch" DWIM for
free. A final iteration converted git-worktree away from heavyweight
git-checkout to lightweight git-reset, at which point he DWIMing was
lost. If you take this history into account, then loss of
"origin/branch -> branch" DWIMing is a regression, so restoring it
could be considered a bug fix.

Re: [PATCH v1 2/2] worktree: make add dwim

[Eric Sunshine]

On Tue, Nov 14, 2017 at 3:14 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> For my own edification...
> [...]
> git worktree add ../topic
>
> * Correctly errors out, refusing to create a new branch named "topic",
> if "topic" is already a branch.

By the way, there's an additional DWIM that could be done here instead
of erroring out. Specifically, for "git worktree add ../topic":

* If branch "topic" exists, check it out (rather than refusing to
create a new branch named "topic").

* If origin/topic exists, DWIM local "topic" branch into existence.

* Otherwise, create new local branch "topic".

> * Creates a new branch named "topic" if no such local branch exists.
>
> The desired new DWIMing would change the second bullet point to:
>
> * If no local branch named "topic" exists, DWIM it from "origin/topic"
> if possible, else create a new local branch named "topic".

Re: [PATCH v1 2/2] worktree: make add dwim

[Thomas Gummerer]

On 11/14, Eric Sunshine wrote:
> On Tue, Nov 14, 2017 at 3:14 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> > For my own edification...
> > [...]
> > git worktree add ../topic
> >
> > * Correctly errors out, refusing to create a new branch named "topic",
> > if "topic" is already a branch.
> 
> By the way, there's an additional DWIM that could be done here instead
> of erroring out. Specifically, for "git worktree add ../topic":
> 
> * If branch "topic" exists, check it out (rather than refusing to
> create a new branch named "topic").

I think this would be a good improvement either way as I suspect this
is what users would hope for, and as it currently just dies there are
less backwards compatibility worries.

> * If origin/topic exists, DWIM local "topic" branch into existence.
> 
> * Otherwise, create new local branch "topic".
> 
> > * Creates a new branch named "topic" if no such local branch exists.
> >
> > The desired new DWIMing would change the second bullet point to:
> >
> > * If no local branch named "topic" exists, DWIM it from "origin/topic"
> > if possible, else create a new local branch named "topic".

Re: [PATCH v1 2/2] worktree: make add dwim

[Thomas Gummerer]

On 11/15, Thomas Gummerer wrote:
> On 11/14, Eric Sunshine wrote:
> > On Tue, Nov 14, 2017 at 3:14 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> > > For my own edification...
> > > [...]
> > > git worktree add ../topic
> > >
> > > * Correctly errors out, refusing to create a new branch named "topic",
> > > if "topic" is already a branch.
> > 
> > By the way, there's an additional DWIM that could be done here instead
> > of erroring out. Specifically, for "git worktree add ../topic":
> > 
> > * If branch "topic" exists, check it out (rather than refusing to
> > create a new branch named "topic").
> 
> I think this would be a good improvement either way as I suspect this
> is what users would hope for, and as it currently just dies there are
> less backwards compatibility worries.

While I still think this would be an improvement, after thinking about
it a bit more I think this is somewhat orthogonal to what I'm trying
to achieve with this patch series.  Therefore I didn't implement this
yet, but I'm still thinking of implementing this in a separate topic.

> > * If origin/topic exists, DWIM local "topic" branch into existence.
> > 
> > * Otherwise, create new local branch "topic".
> > 
> > > * Creates a new branch named "topic" if no such local branch exists.
> > >
> > > The desired new DWIMing would change the second bullet point to:
> > >
> > > * If no local branch named "topic" exists, DWIM it from "origin/topic"
> > > if possible, else create a new local branch named "topic".

Re: [PATCH v1 2/2] worktree: make add dwim

[Thomas Gummerer]

On 11/14, Eric Sunshine wrote:
> On Tue, Nov 14, 2017 at 3:45 AM, Thomas Gummerer <t.gummerer@gmail.com> wrote:
> > On 11/13, Junio C Hamano wrote:
> >> If so, as long as the new DWIM kicks in ONLY when "topic" does not
> >> exist, I suspect that there is no backward compatibility worries.
> >> The command line
> >>
> >>     $ git worktree add ../a-new-worktree topic
> >>
> >> would just have failed because three was no 'topic' branch yet, no?
> >
> > Indeed, with this there would not be any backwards compatility
> > worries.
> >
> > Ideally I'd still like to make
> >
> >     $ git worktree add ../topic
> >
> > work as described above, to avoid having to type 'topic' twice (the
> > directory name matches the branch name for the vast majority of my
> > worktrees) but that should then come behind a flag/config option?
> > Following your reasoning above it should probably be called something
> > other than --guess.
> >
> > Maybe --guess-remote and worktree.guessRemote would do?  I'm quite bad
> > at naming though, so other suggestions are very welcome.
> 
> For my own edification...
> 
> git worktree add ../dir branch
> 
> * Checks out branch into ../dir if branch exists.
> 
> * Errors out if branch does not exist. However, if we consider the
> history of the implementation of worktrees[*1*], then this really
> ought to try the "origin/branch -> branch" DWIM before erroring-out.
> Implementing this DWIM could be considered a regression fix according
> to [*1*] and, as Junio points out, should not pose backward
> compatibility worries.

Agreed, I think it is not very controversial that this would be an
improvement.

> git worktree add ../topic
> 
> * Correctly errors out, refusing to create a new branch named "topic",
> if "topic" is already a branch.
> 
> * Creates a new branch named "topic" if no such local branch exists.
> 
> The desired new DWIMing would change the second bullet point to:
> 
> * If no local branch named "topic" exists, DWIM it from "origin/topic"
> if possible, else create a new local branch named "topic".
> 
> But that's a behavior change since, with the existing implementation,
> a newly created local "topic" has no relation to, and does not track,
> any origin/topic branch. The proposed --guess option is to avoid users
> being hit by this backward incompatibility, however, one could also
> view the proposed DWIMing as some sort of bug fix since, at least
> some, users would expect the DWIMing since it already takes place
> elsewhere.

I'm not sure we can call it a bug fix anymore, since as Junio pointed
out the current behaviour of creating a new branch at HEAD is
documented in the man page.

However git-worktree is also still marked as experimental in the man
page, so we could allow ourselves to be a little bit more lax when it
comes to backwards compatibility, especially because it is easy to
take corrective action after the new DWIMing happened.

> So, at least two options exist:
> 
> * Just make the new DWIMing the default behavior, accepting that it
> might bite a few people. Fallout can be mitigated somewhat by
> prominently announcing that the DWIMing took place, in which case the
> user can take corrective action (say, by choosing a different worktree
> name); nothing is lost and no damage done since this is happening only
> at worktree creation time.
> 
> * Add a new option to enable DWIMing; perhaps name it -t/--track,
> which is familiar terminology and still gives you a relatively short
> and sweet "git worktree add -t ../topic".
> 
> Given the mentioned mitigation factor and that some/many users likely
> would expect it to DWIM "origin/topic -> topic" anyhow, I'd lean in
> favor of the first option (but perhaps I'm too daring with other
> people's workflows).

Yeah, I'm leaning towards the first option as well, but I'm clearly
biased as that's how I'd like it to behave, and others might want the
other behaviour.  Unfortunately I don't know many worktree users, so I
can't tell what the general consensus on this would be.

I guess the second option would be the safer one, and we can still
switch that to be the default at some point if we wish to do so
later.

tl;dr I have no idea which of the options would be better :)

> FOOTNOTES
> 
> [*1*]: When Duy first implemented worktree support, he incorporated it
> directly into the git-checkout command ("git checkout --to worktree
> ..."), which means that he got all the git-checkout features for free,
> including the "origin/branch -> branch" DWIM. When worktree support
> was later moved to git-worktree, it lost most of the features
> inherited implicitly from git-checkout, such as -b, -B, --detach, so
> those were added back to git-worktree explicitly. However, at that
> early stage, git-worktree was still piggy-backing atop git-checkout,
> thus likely was still getting the "origin/branch -> branch" DWIM for
> free. A final iteration converted git-worktree away from heavyweight
> git-checkout to lightweight git-reset, at which point he DWIMing was
> lost. If you take this history into account, then loss of
> "origin/branch -> branch" DWIMing is a regression, so restoring it
> could be considered a bug fix.

Re: [PATCH v1 2/2] worktree: make add dwim

[Eric Sunshine]

On Wed, Nov 15, 2017 at 3:50 AM, Thomas Gummerer <t.gummerer@gmail.com> wrote:
> On 11/14, Eric Sunshine wrote:
>> git worktree add ../topic
>> [...]
>> The desired new DWIMing would change the second bullet point to:
>>
>> * If no local branch named "topic" exists, DWIM it from "origin/topic"
>> if possible, else create a new local branch named "topic".
>>
>> But that's a behavior change since, with the existing implementation,
>> a newly created local "topic" has no relation to, and does not track,
>> any origin/topic branch. The proposed --guess option is to avoid users
>> being hit by this backward incompatibility, however, one could also
>> view the proposed DWIMing as some sort of bug fix since, at least
>> some, users would expect the DWIMing since it already takes place
>> elsewhere.
>
> I'm not sure we can call it a bug fix anymore, since as Junio pointed
> out the current behaviour of creating a new branch at HEAD is
> documented in the man page.
>
> However git-worktree is also still marked as experimental in the man
> page, so we could allow ourselves to be a little bit more lax when it
> comes to backwards compatibility, especially because it is easy to
> take corrective action after the new DWIMing happened.

Yep, my leaning toward making this new DWIMing default (without a
--guess or --track option) also is based in part on git-worktree still
being marked "experimental".

>> So, at least two options exist:
>>
>> * Just make the new DWIMing the default behavior, accepting that it
>> might bite a few people. Fallout can be mitigated somewhat by
>> prominently announcing that the DWIMing took place, in which case the
>> user can take corrective action (say, by choosing a different worktree
>> name); nothing is lost and no damage done since this is happening only
>> at worktree creation time.
>>
>> * Add a new option to enable DWIMing; perhaps name it -t/--track,
>> which is familiar terminology and still gives you a relatively short
>> and sweet "git worktree add -t ../topic".
>>
>> Given the mentioned mitigation factor and that some/many users likely
>> would expect it to DWIM "origin/topic -> topic" anyhow, I'd lean in
>> favor of the first option (but perhaps I'm too daring with other
>> people's workflows).
>
> Yeah, I'm leaning towards the first option as well, but I'm clearly
> biased as that's how I'd like it to behave, and others might want the
> other behaviour.  Unfortunately I don't know many worktree users, so I
> can't tell what the general consensus on this would be.

Aside from the mentioned mitigation factor, which somewhat eases my
worries about backward incompatibility, one genuine concern is
breaking existing scripts. At the very least, if the new DWIM becomes
default, there might need to be some escape hatch for scripts to opt
out of it.

> I guess the second option would be the safer one, and we can still
> switch that to be the default at some point if we wish to do so
> later.

The longer you wait, the less likely you'll have the chance since
git-worktree will (presumably) gain more users and be used in more
scripts as time passes. So, if the new DWIMing is to become the
default, better to do so earlier rather than later.

> tl;dr I have no idea which of the options would be better :)

I'm probably too cavalier and shortsighted (at least on this topic) to
make a well-informed decision about it. Junio probably has a better
feeling about whether such a change of behavior makes sense at this
late date, and, of course, it's his decision whether to accept such a
change into his tree.

Re: [PATCH v1 1/2] checkout: factor out functions to new lib file

[Junio C Hamano]

Thomas Gummerer <t.gummerer@gmail.com> writes:

> Factor the functions out, so they can be re-used from other places.  In
> particular these functions will be re-used in builtin/worktree.c to make
> git worktree add dwim more.
>
> While there add a description to the function.
>
> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
> ---
>
> I'm not sure where these functions should go ideally, they don't seem
> to fit in any existing library file, so I created a new one for now.
> Any suggestions for a better home are welcome!
>
>  Makefile           |  1 +
>  builtin/checkout.c | 41 +----------------------------------------
>  checkout.c         | 43 +++++++++++++++++++++++++++++++++++++++++++
>  checkout.h         | 14 ++++++++++++++
>  4 files changed, 59 insertions(+), 40 deletions(-)
>  create mode 100644 checkout.c
>  create mode 100644 checkout.h

I am not sure either.  I've always considered that entry.c is the
libified thing codepath that want to do a "checkout" should call,
but the functions that you are moving is at a "higher layer" that
does not concern the core-git data structures (i.e. the 'tracking'
is a mere "user" of the ref API, unlike things in entry.c such as
checkout_entry() that is a more "maintainer" of the index integrity),
so perhaps it makes sense to have new file for it.

> diff --git a/checkout.c b/checkout.c
> new file mode 100644
> index 0000000000..a9cf378453
> --- /dev/null
> +++ b/checkout.c
> @@ -0,0 +1,43 @@
> +#include "cache.h"
> +

A useless blank line.

> +#include "remote.h"
> +

This one is useful ;-)

> +struct tracking_name_data {
> ...
> diff --git a/checkout.h b/checkout.h
> new file mode 100644
> index 0000000000..9272fe1449
> --- /dev/null
> +++ b/checkout.h
> @@ -0,0 +1,14 @@
> +#ifndef CHECKOUT_H
> +#define CHECKOUT_H
> +
> +#include "git-compat-util.h"
> +#include "cache.h"

Try "git grep -e git-compat-util Documentation/CodingGuidelines" and
just keep inclusion of "cache.h".

Re: [PATCH v1 1/2] checkout: factor out functions to new lib file

[Thomas Gummerer]

On 11/13, Junio C Hamano wrote:
> Thomas Gummerer <t.gummerer@gmail.com> writes:
> 
> > Factor the functions out, so they can be re-used from other places.  In
> > particular these functions will be re-used in builtin/worktree.c to make
> > git worktree add dwim more.
> >
> > While there add a description to the function.
> >
> > Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
> > ---
> >
> > I'm not sure where these functions should go ideally, they don't seem
> > to fit in any existing library file, so I created a new one for now.
> > Any suggestions for a better home are welcome!
> >
> >  Makefile           |  1 +
> >  builtin/checkout.c | 41 +----------------------------------------
> >  checkout.c         | 43 +++++++++++++++++++++++++++++++++++++++++++
> >  checkout.h         | 14 ++++++++++++++
> >  4 files changed, 59 insertions(+), 40 deletions(-)
> >  create mode 100644 checkout.c
> >  create mode 100644 checkout.h
> 
> I am not sure either.  I've always considered that entry.c is the
> libified thing codepath that want to do a "checkout" should call,
> but the functions that you are moving is at a "higher layer" that
> does not concern the core-git data structures (i.e. the 'tracking'
> is a mere "user" of the ref API, unlike things in entry.c such as
> checkout_entry() that is a more "maintainer" of the index integrity),
> so perhaps it makes sense to have new file for it.

Makes sense, let me keep it as a new file then.

> > diff --git a/checkout.c b/checkout.c
> > new file mode 100644
> > index 0000000000..a9cf378453
> > --- /dev/null
> > +++ b/checkout.c
> > @@ -0,0 +1,43 @@
> > +#include "cache.h"
> > +
> 
> A useless blank line.
> 
> > +#include "remote.h"
> > +
> 
> This one is useful ;-)
> 
> > +struct tracking_name_data {
> > ...
> > diff --git a/checkout.h b/checkout.h
> > new file mode 100644
> > index 0000000000..9272fe1449
> > --- /dev/null
> > +++ b/checkout.h
> > @@ -0,0 +1,14 @@
> > +#ifndef CHECKOUT_H
> > +#define CHECKOUT_H
> > +
> > +#include "git-compat-util.h"
> > +#include "cache.h"
> 
> Try "git grep -e git-compat-util Documentation/CodingGuidelines" and
> just keep inclusion of "cache.h".

Ah I forgot about that.  Will fix, thanks!

[PATCH v2 1/3] checkout: factor out functions to new lib file

[Thomas Gummerer]

Factor the functions out, so they can be re-used from other places.  In
particular these functions will be re-used in builtin/worktree.c to make
git worktree add dwim more.

While there add some docs to the function.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
The previous round of this series is at
https://public-inbox.org/git/20171112134305.3949-1-t.gummerer@gmail.com/.
Thanks Junio and Eric for the comments on the previous round!

 Makefile           |  1 +
 builtin/checkout.c | 41 +----------------------------------------
 checkout.c         | 42 ++++++++++++++++++++++++++++++++++++++++++
 checkout.h         | 13 +++++++++++++
 4 files changed, 57 insertions(+), 40 deletions(-)
 create mode 100644 checkout.c
 create mode 100644 checkout.h

diff --git a/Makefile b/Makefile
index cd75985991..8d603c7443 100644
--- a/Makefile
+++ b/Makefile
@@ -757,6 +757,7 @@ LIB_OBJS += branch.o
 LIB_OBJS += bulk-checkin.o
 LIB_OBJS += bundle.o
 LIB_OBJS += cache-tree.o
+LIB_OBJS += checkout.o
 LIB_OBJS += color.o
 LIB_OBJS += column.o
 LIB_OBJS += combine-diff.o
diff --git a/builtin/checkout.c b/builtin/checkout.c
index fc4f8fd2ea..9e1cfd10b3 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1,5 +1,6 @@
 #include "builtin.h"
 #include "config.h"
+#include "checkout.h"
 #include "lockfile.h"
 #include "parse-options.h"
 #include "refs.h"
@@ -874,46 +875,6 @@ static int git_checkout_config(const char *var, const char *value, void *cb)
 	return git_xmerge_config(var, value, NULL);
 }
 
-struct tracking_name_data {
-	/* const */ char *src_ref;
-	char *dst_ref;
-	struct object_id *dst_oid;
-	int unique;
-};
-
-static int check_tracking_name(struct remote *remote, void *cb_data)
-{
-	struct tracking_name_data *cb = cb_data;
-	struct refspec query;
-	memset(&query, 0, sizeof(struct refspec));
-	query.src = cb->src_ref;
-	if (remote_find_tracking(remote, &query) ||
-	    get_oid(query.dst, cb->dst_oid)) {
-		free(query.dst);
-		return 0;
-	}
-	if (cb->dst_ref) {
-		free(query.dst);
-		cb->unique = 0;
-		return 0;
-	}
-	cb->dst_ref = query.dst;
-	return 0;
-}
-
-static const char *unique_tracking_name(const char *name, struct object_id *oid)
-{
-	struct tracking_name_data cb_data = { NULL, NULL, NULL, 1 };
-	cb_data.src_ref = xstrfmt("refs/heads/%s", name);
-	cb_data.dst_oid = oid;
-	for_each_remote(check_tracking_name, &cb_data);
-	free(cb_data.src_ref);
-	if (cb_data.unique)
-		return cb_data.dst_ref;
-	free(cb_data.dst_ref);
-	return NULL;
-}
-
 static int parse_branchname_arg(int argc, const char **argv,
 				int dwim_new_local_branch_ok,
 				struct branch_info *new,
diff --git a/checkout.c b/checkout.c
new file mode 100644
index 0000000000..b0c744d37a
--- /dev/null
+++ b/checkout.c
@@ -0,0 +1,42 @@
+#include "cache.h"
+#include "remote.h"
+
+struct tracking_name_data {
+	/* const */ char *src_ref;
+	char *dst_ref;
+	struct object_id *dst_oid;
+	int unique;
+};
+
+static int check_tracking_name(struct remote *remote, void *cb_data)
+{
+	struct tracking_name_data *cb = cb_data;
+	struct refspec query;
+	memset(&query, 0, sizeof(struct refspec));
+	query.src = cb->src_ref;
+	if (remote_find_tracking(remote, &query) ||
+	    get_oid(query.dst, cb->dst_oid)) {
+		free(query.dst);
+		return 0;
+	}
+	if (cb->dst_ref) {
+		free(query.dst);
+		cb->unique = 0;
+		return 0;
+	}
+	cb->dst_ref = query.dst;
+	return 0;
+}
+
+const char *unique_tracking_name(const char *name, struct object_id *oid)
+{
+	struct tracking_name_data cb_data = { NULL, NULL, NULL, 1 };
+	cb_data.src_ref = xstrfmt("refs/heads/%s", name);
+	cb_data.dst_oid = oid;
+	for_each_remote(check_tracking_name, &cb_data);
+	free(cb_data.src_ref);
+	if (cb_data.unique)
+		return cb_data.dst_ref;
+	free(cb_data.dst_ref);
+	return NULL;
+}
diff --git a/checkout.h b/checkout.h
new file mode 100644
index 0000000000..9980711179
--- /dev/null
+++ b/checkout.h
@@ -0,0 +1,13 @@
+#ifndef CHECKOUT_H
+#define CHECKOUT_H
+
+#include "cache.h"
+
+/*
+ * Check if the branch name uniquely matches a branch name on a remote
+ * tracking branch.  Return the name of the remote if such a branch
+ * exists, NULL otherwise.
+ */
+extern const char *unique_tracking_name(const char *name, struct object_id *oid);
+
+#endif /* CHECKOUT_H */
-- 
2.15.0.345.gf926f18f3d

[PATCH v2 2/3] worktree: make add <path> <branch> dwim

[Thomas Gummerer]

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 <t.gummerer@gmail.com>
---
 Documentation/git-worktree.txt |  7 +++++++
 builtin/worktree.c             | 15 ++++++++++++++
 t/t2025-worktree-add.sh        | 44 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 66 insertions(+)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index b472acc356..3c7c8c3cee 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -52,6 +52,13 @@ is linked to the current repository, sharing everything except working
 directory specific files such as HEAD, index, etc. `-` may also be
 specified as `<branch>`; it is synonymous with `@{-1}`.
 +
+If <branch> is not found 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>
+------------
++
 If `<branch>` is omitted and neither `-b` nor `-B` nor `--detach` used,
 then, as a convenience, a new branch based at HEAD is created automatically,
 as if `-b $(basename <path>)` was specified.
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 7b9307aa58..92b583ae39 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) {
+		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;
+		}
+	}
+
 	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
index b5c47ac602..e5959800c0 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -6,6 +6,16 @@ test_description='test git worktree add'
 
 . "$TEST_DIRECTORY"/lib-rebase.sh
 
+# 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
+}
+
 test_expect_success 'setup' '
 	test_commit init
 '
@@ -314,4 +324,38 @@ test_expect_success 'rename a branch under bisect not allowed' '
 	test_must_fail git branch -M under-bisect bisect-with-new-name
 '
 
+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 &&
+	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 &&
+		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
+	)
+'
+
 test_done
-- 
2.15.0.345.gf926f18f3d

Re: [PATCH v2 2/3] worktree: make add <path> <branch> dwim

[Thomas Gummerer]

On 11/18, Thomas Gummerer wrote:
> 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 <t.gummerer@gmail.com>
> ---
>  Documentation/git-worktree.txt |  7 +++++++
>  builtin/worktree.c             | 15 ++++++++++++++
>  t/t2025-worktree-add.sh        | 44 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 66 insertions(+)
> 
> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
> index b472acc356..3c7c8c3cee 100644
> --- a/Documentation/git-worktree.txt
> +++ b/Documentation/git-worktree.txt
> @@ -52,6 +52,13 @@ is linked to the current repository, sharing everything except working
>  directory specific files such as HEAD, index, etc. `-` may also be
>  specified as `<branch>`; it is synonymous with `@{-1}`.
>  +
> +If <branch> is not found 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>
> +------------
> ++
>  If `<branch>` is omitted and neither `-b` nor `-B` nor `--detach` used,
>  then, as a convenience, a new branch based at HEAD is created automatically,
>  as if `-b $(basename <path>)` was specified.
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> index 7b9307aa58..92b583ae39 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) {

Err I just realized this doesn't quite make sense.  Similar to the
dwim for 'git worktree add <path>', this condition should include
'!opts.new_branch && !opts.detach'.  In these cases it's still better
to error out, as the user explicitly asked for a new branch with a
different name/asked not to be put onto a branch.  I'll send a v3 with
this fixed in a bit.

> +		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;
> +		}
> +	}
> +
>  	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
> index b5c47ac602..e5959800c0 100755
> --- a/t/t2025-worktree-add.sh
> +++ b/t/t2025-worktree-add.sh
> @@ -6,6 +6,16 @@ test_description='test git worktree add'
>  
>  . "$TEST_DIRECTORY"/lib-rebase.sh
>  
> +# 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
> +}
> +
>  test_expect_success 'setup' '
>  	test_commit init
>  '
> @@ -314,4 +324,38 @@ test_expect_success 'rename a branch under bisect not allowed' '
>  	test_must_fail git branch -M under-bisect bisect-with-new-name
>  '
>  
> +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 &&
> +	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 &&
> +		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
> +	)
> +'
> +
>  test_done
> -- 
> 2.15.0.345.gf926f18f3d
> 

[PATCH v2 3/3] worktree: make add <path> dwim

[Thomas Gummerer]

Currently 'git worktree add <path>' creates a new branch named after the
basename of the <path>, that matches the HEAD of whichever worktree we
were on when calling "git worktree add <path>".

Make 'git worktree add <path> behave more like the dwim machinery in
'git checkout <new-branch>', i.e. check if the new branch name uniquely
matches the branch name of a remote tracking branch, and if so check out
that branch and set the upstream to the remote tracking branch.

This is a change of behaviour compared to the current behaviour, where
we create a new branch matching HEAD.  However as 'git worktree' is
still an experimental feature, and it's easy to notice/correct the
behaviour in case it's not what the user desired it's probably okay to
break existing behaviour here.

In order to also satisfy users who want the current behaviour of
creating a new branch from HEAD, add a '--no-track' flag, which disables
the new behaviour, and keeps the old behaviour of creating a new branch
from the head of the current worktree.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
I went back and forth between hiding this behing a flag, and making it
default and having a flag for getting the old behaviour back.

In the end I went for making the new behaviour the default, because
the 'worktree' feature is still experimental, and it's easy to correct
the behaviour if it's not what was desired.  I also think this is the
more intuitve behaviour, but clearly I'm biased because *I* want to it
to behave this way.

I'm happy to keep the old behaviour the default and hide the new
behaviour behind a flag if we feel this is too much of a change in
behaviour at this point.

 Documentation/git-worktree.txt | 15 ++++++++---
 builtin/worktree.c             |  9 +++++++
 t/t2025-worktree-add.sh        | 60 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 81 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 3c7c8c3cee..11cac104d9 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -60,9 +60,18 @@ $ git worktree add -b <branch> <path> <remote>/<branch>
 ------------
 +
 If `<branch>` is omitted and neither `-b` nor `-B` nor `--detach` used,
-then, as a convenience, a new branch based at HEAD is created automatically,
-as if `-b $(basename <path>)` was specified.
-
+then, as a convenience, if there exists a tracking branch in exactly
+one remote (call it <remote>) matching the basename of the path
+(call it <branch>), treat it as equivalent to
+------------
+$ git worktree add -b <branch> <path> <remote>/<branch>
+------------
+If no tracking branch exists in exactly one remote, <branch> is
+created based on HEAD, as if `-b $(basename <path>)` was specified.
++
+To disable the behaviour of trying to match the basename of <path> to
+a remote, and always create a new branch from HEAD, the `--no-track`
+flag can be passed to `git worktree add`.
 list::
 
 List details of each worktree.  The main worktree is listed first, followed by
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 92b583ae39..82088415b8 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -342,6 +342,7 @@ static int add(int ac, const char **av, const char *prefix)
 	const char *new_branch_force = NULL;
 	char *path;
 	const char *branch;
+	int track_dwim = 1;
 	struct option options[] = {
 		OPT__FORCE(&opts.force, N_("checkout <branch> even if already checked out in other worktree")),
 		OPT_STRING('b', NULL, &opts.new_branch, N_("branch"),
@@ -351,6 +352,7 @@ static int add(int ac, const char **av, const char *prefix)
 		OPT_BOOL(0, "detach", &opts.detach, N_("detach HEAD at named commit")),
 		OPT_BOOL(0, "checkout", &opts.checkout, N_("populate the new working tree")),
 		OPT_BOOL(0, "lock", &opts.keep_locked, N_("keep the new working tree locked")),
+		OPT_BOOL(0, "track", &track_dwim, N_("checkout upstream branch if there's a unique match")),
 		OPT_END()
 	};
 
@@ -385,6 +387,13 @@ static int add(int ac, const char **av, const char *prefix)
 		int n;
 		const char *s = worktree_basename(path, &n);
 		opts.new_branch = xstrndup(s, n);
+		if (track_dwim) {
+			struct object_id oid;
+			const char *remote =
+				unique_tracking_name(opts.new_branch, &oid);
+			if (remote)
+				branch = remote;
+		}
 	}
 
 	if (ac == 2) {
diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index e5959800c0..a566d867fe 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -358,4 +358,64 @@ test_expect_success '"add" <path> <branch> dwims' '
 	)
 '
 
+test_expect_success 'git worktree add --no-track does not set up tracking' '
+	test_when_finished rm -rf repo_a &&
+	test_when_finished rm -rf repo_b &&
+	test_when_finished rm -rf foo &&
+	git init repo_a &&
+	(
+		cd repo_a &&
+		test_commit a_master &&
+		git checkout -b foo &&
+		test_commit a_foo
+	) &&
+	git init repo_b &&
+	(
+		cd repo_b &&
+		test_commit b_master &&
+		git remote add repo_a ../repo_a &&
+		git config remote.repo_a.fetch \
+			"+refs/heads/*:refs/remotes/other_a/*" &&
+		git fetch --all &&
+		git worktree add --no-track ../foo
+	) &&
+	(
+		cd foo &&
+		! test_branch_upstream foo repo_a foo &&
+		git rev-parse other_a/foo >expect &&
+		git rev-parse foo >actual &&
+		! test_cmp expect actual
+	)
+'
+
+test_expect_success 'git worktree add sets up tracking' '
+	test_when_finished rm -rf repo_a &&
+	test_when_finished rm -rf repo_b &&
+	test_when_finished rm -rf foo &&
+	git init repo_a &&
+	(
+		cd repo_a &&
+		test_commit a_master &&
+		git checkout -b foo &&
+		test_commit a_foo
+	) &&
+	git init repo_b &&
+	(
+		cd repo_b &&
+		test_commit b_master &&
+		git remote add repo_a ../repo_a &&
+		git config remote.repo_a.fetch \
+			"+refs/heads/*:refs/remotes/other_a/*" &&
+		git fetch --all &&
+		git worktree add ../foo
+	) &&
+	(
+		cd foo &&
+		test_branch_upstream foo repo_a foo &&
+		git rev-parse other_a/foo >expect &&
+		git rev-parse foo >actual &&
+		test_cmp expect actual
+	)
+'
+
 test_done
-- 
2.15.0.345.gf926f18f3d

[PATCH v3 0/3] make git worktree add dwim more

[Thomas Gummerer]

Sorry about the noise with v2 and v3 so quickly one after another.  I
only too late realized that the new dwim for 'add <path> <branch>'
doesn't make much sense if -b or --detach are given, and it's better
to keep on erroring out in these cases.

The previous rounds were at https://public-inbox.org/git/20171112134305.3949-1-t.gummerer@gmail.com/
and https://public-inbox.org/git/20171118181103.28354-1-t.gummerer@gmail.com/.

In case anybody already started reading v2, an interdiff between the
two versions is below:

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 11cac104d9..eedead2c4c 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -52,9 +52,9 @@ is linked to the current repository, sharing everything except working
 directory specific files such as HEAD, index, etc. `-` may also be
 specified as `<branch>`; it is synonymous with `@{-1}`.
 +
-If <branch> is not found but there does exist a tracking branch in
-exactly one remote (call it <remote>) with a matching name, treat as
-equivalent to
+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>
 ------------
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 82088415b8..b2a6dd020c 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -396,7 +396,7 @@ static int add(int ac, const char **av, const char *prefix)
 		}
 	}
 
-	if (ac == 2) {
+	if (ac == 2 && !opts.new_branch && !opts.detach) {
 		struct object_id oid;
 		struct commit *commit;
 		const char *remote;
diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index a566d867fe..87e233f812 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -347,6 +347,8 @@ test_expect_success '"add" <path> <branch> dwims' '
 		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
 	) &&
 	(

Thomas Gummerer (3):
  checkout: factor out functions to new lib file
  worktree: make add <path> <branch> dwim
  worktree: make add <path> dwim

 Documentation/git-worktree.txt |  22 +++++++--
 Makefile                       |   1 +
 builtin/checkout.c             |  41 +---------------
 builtin/worktree.c             |  24 ++++++++++
 checkout.c                     |  42 ++++++++++++++++
 checkout.h                     |  13 +++++
 t/t2025-worktree-add.sh        | 106 +++++++++++++++++++++++++++++++++++++++++
 7 files changed, 206 insertions(+), 43 deletions(-)
 create mode 100644 checkout.c
 create mode 100644 checkout.h

-- 
2.15.0.345.gf926f18f3

[PATCH v3 1/3] checkout: factor out functions to new lib file

[Thomas Gummerer]

Factor the functions out, so they can be re-used from other places.  In
particular these functions will be re-used in builtin/worktree.c to make
git worktree add dwim more.

While there add some docs to the function.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 Makefile           |  1 +
 builtin/checkout.c | 41 +----------------------------------------
 checkout.c         | 42 ++++++++++++++++++++++++++++++++++++++++++
 checkout.h         | 13 +++++++++++++
 4 files changed, 57 insertions(+), 40 deletions(-)
 create mode 100644 checkout.c
 create mode 100644 checkout.h

diff --git a/Makefile b/Makefile
index cd75985991..8d603c7443 100644
--- a/Makefile
+++ b/Makefile
@@ -757,6 +757,7 @@ LIB_OBJS += branch.o
 LIB_OBJS += bulk-checkin.o
 LIB_OBJS += bundle.o
 LIB_OBJS += cache-tree.o
+LIB_OBJS += checkout.o
 LIB_OBJS += color.o
 LIB_OBJS += column.o
 LIB_OBJS += combine-diff.o
diff --git a/builtin/checkout.c b/builtin/checkout.c
index fc4f8fd2ea..9e1cfd10b3 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1,5 +1,6 @@
 #include "builtin.h"
 #include "config.h"
+#include "checkout.h"
 #include "lockfile.h"
 #include "parse-options.h"
 #include "refs.h"
@@ -874,46 +875,6 @@ static int git_checkout_config(const char *var, const char *value, void *cb)
 	return git_xmerge_config(var, value, NULL);
 }
 
-struct tracking_name_data {
-	/* const */ char *src_ref;
-	char *dst_ref;
-	struct object_id *dst_oid;
-	int unique;
-};
-
-static int check_tracking_name(struct remote *remote, void *cb_data)
-{
-	struct tracking_name_data *cb = cb_data;
-	struct refspec query;
-	memset(&query, 0, sizeof(struct refspec));
-	query.src = cb->src_ref;
-	if (remote_find_tracking(remote, &query) ||
-	    get_oid(query.dst, cb->dst_oid)) {
-		free(query.dst);
-		return 0;
-	}
-	if (cb->dst_ref) {
-		free(query.dst);
-		cb->unique = 0;
-		return 0;
-	}
-	cb->dst_ref = query.dst;
-	return 0;
-}
-
-static const char *unique_tracking_name(const char *name, struct object_id *oid)
-{
-	struct tracking_name_data cb_data = { NULL, NULL, NULL, 1 };
-	cb_data.src_ref = xstrfmt("refs/heads/%s", name);
-	cb_data.dst_oid = oid;
-	for_each_remote(check_tracking_name, &cb_data);
-	free(cb_data.src_ref);
-	if (cb_data.unique)
-		return cb_data.dst_ref;
-	free(cb_data.dst_ref);
-	return NULL;
-}
-
 static int parse_branchname_arg(int argc, const char **argv,
 				int dwim_new_local_branch_ok,
 				struct branch_info *new,
diff --git a/checkout.c b/checkout.c
new file mode 100644
index 0000000000..b0c744d37a
--- /dev/null
+++ b/checkout.c
@@ -0,0 +1,42 @@
+#include "cache.h"
+#include "remote.h"
+
+struct tracking_name_data {
+	/* const */ char *src_ref;
+	char *dst_ref;
+	struct object_id *dst_oid;
+	int unique;
+};
+
+static int check_tracking_name(struct remote *remote, void *cb_data)
+{
+	struct tracking_name_data *cb = cb_data;
+	struct refspec query;
+	memset(&query, 0, sizeof(struct refspec));
+	query.src = cb->src_ref;
+	if (remote_find_tracking(remote, &query) ||
+	    get_oid(query.dst, cb->dst_oid)) {
+		free(query.dst);
+		return 0;
+	}
+	if (cb->dst_ref) {
+		free(query.dst);
+		cb->unique = 0;
+		return 0;
+	}
+	cb->dst_ref = query.dst;
+	return 0;
+}
+
+const char *unique_tracking_name(const char *name, struct object_id *oid)
+{
+	struct tracking_name_data cb_data = { NULL, NULL, NULL, 1 };
+	cb_data.src_ref = xstrfmt("refs/heads/%s", name);
+	cb_data.dst_oid = oid;
+	for_each_remote(check_tracking_name, &cb_data);
+	free(cb_data.src_ref);
+	if (cb_data.unique)
+		return cb_data.dst_ref;
+	free(cb_data.dst_ref);
+	return NULL;
+}
diff --git a/checkout.h b/checkout.h
new file mode 100644
index 0000000000..9980711179
--- /dev/null
+++ b/checkout.h
@@ -0,0 +1,13 @@
+#ifndef CHECKOUT_H
+#define CHECKOUT_H
+
+#include "cache.h"
+
+/*
+ * Check if the branch name uniquely matches a branch name on a remote
+ * tracking branch.  Return the name of the remote if such a branch
+ * exists, NULL otherwise.
+ */
+extern const char *unique_tracking_name(const char *name, struct object_id *oid);
+
+#endif /* CHECKOUT_H */
-- 
2.15.0.345.gf926f18f3

[PATCH v3 2/3] worktree: make add <path> <branch> dwim

[Thomas Gummerer]

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 <t.gummerer@gmail.com>
---
 Documentation/git-worktree.txt |  7 +++++++
 builtin/worktree.c             | 15 ++++++++++++++
 t/t2025-worktree-add.sh        | 46 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 68 insertions(+)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index b472acc356..abf48fecb8 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -52,6 +52,13 @@ is linked to the current repository, sharing everything except working
 directory specific files such as HEAD, index, etc. `-` may also be
 specified as `<branch>`; it is synonymous with `@{-1}`.
 +
+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>
+------------
++
 If `<branch>` is omitted and neither `-b` nor `-B` nor `--detach` used,
 then, as a convenience, a new branch based at HEAD is created automatically,
 as if `-b $(basename <path>)` was specified.
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;
+		}
+	}
+
 	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
index b5c47ac602..e3fc60dd1c 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -6,6 +6,16 @@ test_description='test git worktree add'
 
 . "$TEST_DIRECTORY"/lib-rebase.sh
 
+# 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
+}
+
 test_expect_success 'setup' '
 	test_commit init
 '
@@ -314,4 +324,40 @@ test_expect_success 'rename a branch under bisect not allowed' '
 	test_must_fail git branch -M under-bisect bisect-with-new-name
 '
 
+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 &&
+	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
+	)
+'
+
 test_done
-- 
2.15.0.345.gf926f18f3

Re: [PATCH v3 2/3] worktree: make add <path> <branch> dwim

[Eric Sunshine]

On Sat, Nov 18, 2017 at 5:47 PM, Thomas Gummerer <t.gummerer@gmail.com> wrote:
> 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 <t.gummerer@gmail.com>
> ---
> 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?)

> 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.)

>         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...

>  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
> +       )
> +'

Re: [PATCH v3 2/3] worktree: make add <path> <branch> dwim

[Thomas Gummerer]

On 11/19, Eric Sunshine wrote:
> On Sat, Nov 18, 2017 at 5:47 PM, Thomas Gummerer <t.gummerer@gmail.com> wrote:
> > 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 <t.gummerer@gmail.com>
> > ---
> > 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?)

It is how 'git worktree add -b <branch> <path> <remote>/<branch>'
currently works, which is also not documented anywhere.  However I'm
not sure it's super intuitive, from just reading the command, so I'll
add an explicit mention about it.

> > 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) {
>             ...
>         }
>     }

Will change, thanks!

> 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.)

Yes that's correct.  The default behaviour of git branch does exactly
what we want here, so we're relying on it, instead of gouing through
the trouble of explicitly specifying --track.

We have a test checking the expected behaviour of setting up the
upstream, so in the unlikely event that the behaviour in 'git branch'
changes, we'd still guard against it there.  Therefore I don't think
there's a need to be extra defensive here.

> >         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...

Will do.

> >  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 &&

Thanks, will change!

Thanks a lot for the review!

> > +       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
> > +       )
> > +'

[PATCH v3 3/3] worktree: make add <path> dwim

[Thomas Gummerer]

Currently 'git worktree add <path>' creates a new branch named after the
basename of the <path>, that matches the HEAD of whichever worktree we
were on when calling "git worktree add <path>".

Make 'git worktree add <path> behave more like the dwim machinery in
'git checkout <new-branch>', i.e. check if the new branch name uniquely
matches the branch name of a remote tracking branch, and if so check out
that branch and set the upstream to the remote tracking branch.

This is a change of behaviour compared to the current behaviour, where
we create a new branch matching HEAD.  However as 'git worktree' is
still an experimental feature, and it's easy to notice/correct the
behaviour in case it's not what the user desired it's probably okay to
break existing behaviour here.

In order to also satisfy users who want the current behaviour of
creating a new branch from HEAD, add a '--no-track' flag, which disables
the new behaviour, and keeps the old behaviour of creating a new branch
from the head of the current worktree.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 Documentation/git-worktree.txt | 15 ++++++++---
 builtin/worktree.c             |  9 +++++++
 t/t2025-worktree-add.sh        | 60 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 81 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index abf48fecb8..eedead2c4c 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -60,9 +60,18 @@ $ git worktree add -b <branch> <path> <remote>/<branch>
 ------------
 +
 If `<branch>` is omitted and neither `-b` nor `-B` nor `--detach` used,
-then, as a convenience, a new branch based at HEAD is created automatically,
-as if `-b $(basename <path>)` was specified.
-
+then, as a convenience, if there exists a tracking branch in exactly
+one remote (call it <remote>) matching the basename of the path
+(call it <branch>), treat it as equivalent to
+------------
+$ git worktree add -b <branch> <path> <remote>/<branch>
+------------
+If no tracking branch exists in exactly one remote, <branch> is
+created based on HEAD, as if `-b $(basename <path>)` was specified.
++
+To disable the behaviour of trying to match the basename of <path> to
+a remote, and always create a new branch from HEAD, the `--no-track`
+flag can be passed to `git worktree add`.
 list::
 
 List details of each worktree.  The main worktree is listed first, followed by
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 05fc902bcc..b2a6dd020c 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -342,6 +342,7 @@ static int add(int ac, const char **av, const char *prefix)
 	const char *new_branch_force = NULL;
 	char *path;
 	const char *branch;
+	int track_dwim = 1;
 	struct option options[] = {
 		OPT__FORCE(&opts.force, N_("checkout <branch> even if already checked out in other worktree")),
 		OPT_STRING('b', NULL, &opts.new_branch, N_("branch"),
@@ -351,6 +352,7 @@ static int add(int ac, const char **av, const char *prefix)
 		OPT_BOOL(0, "detach", &opts.detach, N_("detach HEAD at named commit")),
 		OPT_BOOL(0, "checkout", &opts.checkout, N_("populate the new working tree")),
 		OPT_BOOL(0, "lock", &opts.keep_locked, N_("keep the new working tree locked")),
+		OPT_BOOL(0, "track", &track_dwim, N_("checkout upstream branch if there's a unique match")),
 		OPT_END()
 	};
 
@@ -385,6 +387,13 @@ static int add(int ac, const char **av, const char *prefix)
 		int n;
 		const char *s = worktree_basename(path, &n);
 		opts.new_branch = xstrndup(s, n);
+		if (track_dwim) {
+			struct object_id oid;
+			const char *remote =
+				unique_tracking_name(opts.new_branch, &oid);
+			if (remote)
+				branch = remote;
+		}
 	}
 
 	if (ac == 2 && !opts.new_branch && !opts.detach) {
diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index e3fc60dd1c..87e233f812 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -360,4 +360,64 @@ test_expect_success '"add" <path> <branch> dwims' '
 	)
 '
 
+test_expect_success 'git worktree add --no-track does not set up tracking' '
+	test_when_finished rm -rf repo_a &&
+	test_when_finished rm -rf repo_b &&
+	test_when_finished rm -rf foo &&
+	git init repo_a &&
+	(
+		cd repo_a &&
+		test_commit a_master &&
+		git checkout -b foo &&
+		test_commit a_foo
+	) &&
+	git init repo_b &&
+	(
+		cd repo_b &&
+		test_commit b_master &&
+		git remote add repo_a ../repo_a &&
+		git config remote.repo_a.fetch \
+			"+refs/heads/*:refs/remotes/other_a/*" &&
+		git fetch --all &&
+		git worktree add --no-track ../foo
+	) &&
+	(
+		cd foo &&
+		! test_branch_upstream foo repo_a foo &&
+		git rev-parse other_a/foo >expect &&
+		git rev-parse foo >actual &&
+		! test_cmp expect actual
+	)
+'
+
+test_expect_success 'git worktree add sets up tracking' '
+	test_when_finished rm -rf repo_a &&
+	test_when_finished rm -rf repo_b &&
+	test_when_finished rm -rf foo &&
+	git init repo_a &&
+	(
+		cd repo_a &&
+		test_commit a_master &&
+		git checkout -b foo &&
+		test_commit a_foo
+	) &&
+	git init repo_b &&
+	(
+		cd repo_b &&
+		test_commit b_master &&
+		git remote add repo_a ../repo_a &&
+		git config remote.repo_a.fetch \
+			"+refs/heads/*:refs/remotes/other_a/*" &&
+		git fetch --all &&
+		git worktree add ../foo
+	) &&
+	(
+		cd foo &&
+		test_branch_upstream foo repo_a foo &&
+		git rev-parse other_a/foo >expect &&
+		git rev-parse foo >actual &&
+		test_cmp expect actual
+	)
+'
+
 test_done
-- 
2.15.0.345.gf926f18f3

Re: [PATCH v3 3/3] worktree: make add <path> dwim

[Eric Sunshine]

On Sat, Nov 18, 2017 at 5:47 PM, Thomas Gummerer <t.gummerer@gmail.com> wrote:
> Currently 'git worktree add <path>' creates a new branch named after the
> basename of the <path>, that matches the HEAD of whichever worktree we
> were on when calling "git worktree add <path>".
>
> Make 'git worktree add <path> behave more like the dwim machinery in
> 'git checkout <new-branch>', i.e. check if the new branch name uniquely
> matches the branch name of a remote tracking branch, and if so check out
> that branch and set the upstream to the remote tracking branch.
>
> This is a change of behaviour compared to the current behaviour, where
> we create a new branch matching HEAD.  However as 'git worktree' is
> still an experimental feature, and it's easy to notice/correct the
> behaviour in case it's not what the user desired it's probably okay to
> break existing behaviour here.
>
> In order to also satisfy users who want the current behaviour of
> creating a new branch from HEAD, add a '--no-track' flag, which disables
> the new behaviour, and keeps the old behaviour of creating a new branch
> from the head of the current worktree.
>
> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
> ---
> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
> @@ -60,9 +60,18 @@ $ git worktree add -b <branch> <path> <remote>/<branch>
>  ------------
>  If `<branch>` is omitted and neither `-b` nor `-B` nor `--detach` used,
> -then, as a convenience, a new branch based at HEAD is created automatically,
> -as if `-b $(basename <path>)` was specified.
> -
> +then, as a convenience, if there exists a tracking branch in exactly
> +one remote (call it <remote>) matching the basename of the path
> +(call it <branch>), treat it as equivalent to

Inconsistent typesetting. In the context, typesetting is `<foo>`,
whereas in the new content, you've used <foo> for these two.

> +------------
> +$ git worktree add -b <branch> <path> <remote>/<branch>
> +------------
> +If no tracking branch exists in exactly one remote, <branch> is

Typesetting: `<foo>`

> +created based on HEAD, as if `-b $(basename <path>)` was specified.
> ++
> +To disable the behaviour of trying to match the basename of <path> to
> +a remote, and always create a new branch from HEAD, the `--no-track`

Does --[no-]track deserve to be documented in the OPTIONS section like
the other options are?

> +flag can be passed to `git worktree add`.
>  list::

You lost a blank line before "list::".

>  List details of each worktree.  The main worktree is listed first, followed by
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> @@ -351,6 +352,7 @@ static int add(int ac, const char **av, const char *prefix)
> +               OPT_BOOL(0, "track", &track_dwim, N_("checkout upstream branch if there's a unique match")),

git-checkout and git-branch help describe this as "setting upstream"
and "setting up tracking", respectively. Using "checkout" in this help
message could be confusing, especially since git-worktree has a
--no-checkout option; this seems to imply that --track would override
it.

>                 OPT_END()
>         };
>
> diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
> @@ -360,4 +360,64 @@ test_expect_success '"add" <path> <branch> dwims' '
> +test_expect_success 'git worktree add --no-track does not set up tracking' '
> +       test_when_finished rm -rf repo_a &&
> +       test_when_finished rm -rf repo_b &&
> +       test_when_finished rm -rf foo &&
> +       git init repo_a &&
> +       (
> +               cd repo_a &&
> +               test_commit a_master &&
> +               git checkout -b foo &&
> +               test_commit a_foo
> +       ) &&
> +       git init repo_b &&
> +       (
> +               cd repo_b &&
> +               test_commit b_master &&
> +               git remote add repo_a ../repo_a &&
> +               git config remote.repo_a.fetch \
> +                       "+refs/heads/*:refs/remotes/other_a/*" &&
> +               git fetch --all &&
> +               git worktree add --no-track ../foo
> +       ) &&
> +       (
> +               cd foo &&
> +               ! test_branch_upstream foo repo_a foo &&
> +               git rev-parse other_a/foo >expect &&
> +               git rev-parse foo >actual &&
> +               ! test_cmp expect actual
> +       )
> +'

Most of the boilerplate in the three new tests (added in this and the
previous patch) is identical (and very verbose). Perhaps the bulk of
it can be factored out into a function?

> +
> +test_expect_success 'git worktree add sets up tracking' '
> +       test_when_finished rm -rf repo_a &&
> +       test_when_finished rm -rf repo_b &&
> +       test_when_finished rm -rf foo &&
> +       git init repo_a &&
> +       (
> +               cd repo_a &&
> +               test_commit a_master &&
> +               git checkout -b foo &&
> +               test_commit a_foo
> +       ) &&
> +       git init repo_b &&
> +       (
> +               cd repo_b &&
> +               test_commit b_master &&
> +               git remote add repo_a ../repo_a &&
> +               git config remote.repo_a.fetch \
> +                       "+refs/heads/*:refs/remotes/other_a/*" &&
> +               git fetch --all &&
> +               git worktree add ../foo
> +       ) &&
> +       (
> +               cd foo &&
> +               test_branch_upstream foo repo_a foo &&
> +               git rev-parse other_a/foo >expect &&
> +               git rev-parse foo >actual &&
> +               test_cmp expect actual
> +       )
> +'
> +
>  test_done

Re: [PATCH v3 3/3] worktree: make add <path> dwim

[Eric Sunshine]

On Sun, Nov 19, 2017 at 2:04 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Sat, Nov 18, 2017 at 5:47 PM, Thomas Gummerer <t.gummerer@gmail.com> wrote:
>> +To disable the behaviour of trying to match the basename of <path> to
>> +a remote, and always create a new branch from HEAD, the `--no-track`
>
> Does --[no-]track deserve to be documented in the OPTIONS section like
> the other options are?

One other question: Since this is re-using the well-known option name
--no-track, should it also get applied to the "git worktree add -b foo
dir origin/foo" case, as well, which you pointed out (in the patch 2/3
thread) already DWIMs tracking automatically? (I can easily see
someone reporting it as a bug if "git worktree add --no-track -b foo
dir origin/foo" does not suppress tracking.)

Re: [PATCH v3 3/3] worktree: make add <path> dwim

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Sun, Nov 19, 2017 at 2:04 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>> On Sat, Nov 18, 2017 at 5:47 PM, Thomas Gummerer <t.gummerer@gmail.com> wrote:
>>> +To disable the behaviour of trying to match the basename of <path> to
>>> +a remote, and always create a new branch from HEAD, the `--no-track`
>>
>> Does --[no-]track deserve to be documented in the OPTIONS section like
>> the other options are?
>
> One other question: Since this is re-using the well-known option name
> --no-track, should it also get applied to the "git worktree add -b foo
> dir origin/foo" case, as well, which you pointed out (in the patch 2/3
> thread) already DWIMs tracking automatically? (I can easily see
> someone reporting it as a bug if "git worktree add --no-track -b foo
> dir origin/foo" does not suppress tracking.)

Sensible suggestion.  Thanks.

Re: [PATCH v3 3/3] worktree: make add <path> dwim

[Thomas Gummerer]

On 11/19, Eric Sunshine wrote:
> On Sun, Nov 19, 2017 at 2:04 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> > On Sat, Nov 18, 2017 at 5:47 PM, Thomas Gummerer <t.gummerer@gmail.com> wrote:
> >> +To disable the behaviour of trying to match the basename of <path> to
> >> +a remote, and always create a new branch from HEAD, the `--no-track`
> >
> > Does --[no-]track deserve to be documented in the OPTIONS section like
> > the other options are?
> 
> One other question: Since this is re-using the well-known option name
> --no-track, should it also get applied to the "git worktree add -b foo
> dir origin/foo" case, as well, which you pointed out (in the patch 2/3
> thread) already DWIMs tracking automatically? (I can easily see
> someone reporting it as a bug if "git worktree add --no-track -b foo
> dir origin/foo" does not suppress tracking.)

I didn't consider that, I think you are right, and the flag should
apply in that case as well.  I think at that point we may as well pass
this flag through to the 'git branch' call, and let users set up
tracking if they want to, the same way it works in 'git branch'.  Thanks!

Re: [PATCH v3 3/3] worktree: make add <path> dwim

[Junio C Hamano]

Thomas Gummerer <t.gummerer@gmail.com> writes:

> I didn't consider that, I think you are right, and the flag should
> apply in that case as well.  I think at that point we may as well pass
> this flag through to the 'git branch' call, and let users set up
> tracking if they want to, the same way it works in 'git branch'.

OK, so tracking is set up by default in the current implementation
of "git worktree" (even without your proposed update), but we will
stop doing so, and instead take an explicit "--track" option (or
"--no-track" to countermand an earlier "--track" on the command line
and/or a default configured with branch.autosetupmerge) just like
"git branch" does?

I think that it is very sensible thing to make sure that "branch",
"checkout -b" and "worktree", i.e. the three ways to create a branch
to work on (the latter two being short-hands), behave consistently.

Thanks.

Re: [PATCH v3 3/3] worktree: make add <path> dwim

[Thomas Gummerer]

On 11/22, Junio C Hamano wrote:
> Thomas Gummerer <t.gummerer@gmail.com> writes:
> 
> > I didn't consider that, I think you are right, and the flag should
> > apply in that case as well.  I think at that point we may as well pass
> > this flag through to the 'git branch' call, and let users set up
> > tracking if they want to, the same way it works in 'git branch'.
> 
> OK, so tracking is set up by default in the current implementation
> of "git worktree" (even without your proposed update), but we will
> stop doing so, and instead take an explicit "--track" option (or
> "--no-track" to countermand an earlier "--track" on the command line
> and/or a default configured with branch.autosetupmerge) just like
> "git branch" does?

I was a bit brief in the above.  The full story is that tracking is
set up by default if the '<branch>' given is a remote tracking branch,
and isn't set up otherwise, the same way as 'git branch' behaves.

What I'm planning to do is introduce a --[no-]track flag to override
this behaviour.  As 'git worktree' really just calls 'git branch'
internally, the branch.autoSetupMerge configuration is also
respected.

> I think that it is very sensible thing to make sure that "branch",
> "checkout -b" and "worktree", i.e. the three ways to create a branch
> to work on (the latter two being short-hands), behave consistently.

So in summary "branch" and "worktree" already behave consistently, the
plan is just to introduce the same "--[no-]track" flag as branch to
allow users to override the behaviour the same way as they are allowed
in "branch".

> Thanks.