[PATCH] add option -n (--no-checkout) to git-worktree add

By adding option -n, we can make some customizations before checkout, like sparse checkout, etc.

Signed-off-by: Ray Zhang <redacted>
---
 builtin/worktree.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 38b5609..14ca3d9 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c

@@ -21,6 +21,7 @@ static const char * const worktree_usage[] = {
 struct add_opts {
 	int force;
 	int detach;
+	int no_checkout;
 	const char *new_branch;
 	int force_new_branch;
 };

@@ -284,11 +285,13 @@ static int add_worktree(const char *path, const char *refname,
 	if (ret)
 		goto done;
 
-	cp.argv = NULL;
-	argv_array_clear(&cp.args);
-	argv_array_pushl(&cp.args, "reset", "--hard", NULL);
-	cp.env = child_env.argv;
-	ret = run_command(&cp);
+	if (!opts->no_checkout) {
+		cp.argv = NULL;
+		argv_array_clear(&cp.args);
+		argv_array_pushl(&cp.args, "reset", "--hard", NULL);
+		cp.env = child_env.argv;
+		ret = run_command(&cp);
+	}
 	if (!ret) {
 		is_junk = 0;
 		free(junk_work_tree);

@@ -320,6 +323,7 @@ static int add(int ac, const char **av, const char *prefix)
 		OPT_STRING('B', NULL, &new_branch_force, N_("branch"),
 			   N_("create or reset a branch")),
 		OPT_BOOL(0, "detach", &opts.detach, N_("detach HEAD at named commit")),
+		OPT_BOOL('n', "no-checkout", &opts.no_checkout, N_("don't create a checkout")),
 		OPT_END()
 	};
 

--

https://github.com/git/git/pull/217

On Wed, Mar 23, 2016 at 11:08 AM, Ray Zhang [off-list ref] wrote:

add option -n (--no-checkout) to git-worktree add

Alternate:

    worktree: add: introduce --no-checkout option

By adding option -n, we can make some customizations before checkout, like sparse checkout, etc.

This parallels git-clone's --no-checkout. Okay.

Typically, one would not squat on a short option (-n) when first
introducing a feature and would only add the short equivalent after
the option proved popular, however, in this case, as git-clone
supports -n, I suppose finger muscle-memory is a consideration.

By the way, please wrap the commit message at 70-72 characters or so.

Signed-off-by: Ray Zhang <redacted>
---
 builtin/worktree.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

This change needs corresponding documentation
(Documentation/git-worktree.txt) and test (t/t2025-worktree-add.sh)
updates.

Thanks.

By adding this option which defaults to true, we can use the
corresponding --no-checkout to make some customizations before
the checkout, like sparse checkout, etc.

Signed-off-by: Ray Zhang <redacted>
---
 Documentation/git-worktree.txt |  6 +++++-
 builtin/worktree.c             | 15 ++++++++++-----
 t/t2025-worktree-add.sh        |  5 +++++
 3 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 62c76c1..e96fe0f 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt

@@ -9,7 +9,7 @@ git-worktree - Manage multiple working trees
 SYNOPSIS
 --------
 [verse]
-'git worktree add' [-f] [--detach] [-b <new-branch>] <path> [<branch>]
+'git worktree add' [-f] [--detach] [--checkout] [-b <new-branch>] <path> [<branch>]
 'git worktree prune' [-n] [-v] [--expire <expire>]
 'git worktree list' [--porcelain]
 

@@ -87,6 +87,10 @@ OPTIONS
 	With `add`, detach HEAD in the new working tree. See "DETACHED HEAD"
 	in linkgit:git-checkout[1].
 
+--checkout::
+	Default option with `add`, populate the new working tree. Use
+	`--no-checkout` to skip the checkout.
+
 -n::
 --dry-run::
 	With `prune`, do not remove anything; just report what it would

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 38b5609..e677cd7 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c

@@ -21,6 +21,7 @@ static const char * const worktree_usage[] = {
 struct add_opts {
 	int force;
 	int detach;
+	int checkout;
 	const char *new_branch;
 	int force_new_branch;
 };

@@ -284,11 +285,13 @@ static int add_worktree(const char *path, const char *refname,
 	if (ret)
 		goto done;
 
-	cp.argv = NULL;
-	argv_array_clear(&cp.args);
-	argv_array_pushl(&cp.args, "reset", "--hard", NULL);
-	cp.env = child_env.argv;
-	ret = run_command(&cp);
+	if (opts->checkout) {
+		cp.argv = NULL;
+		argv_array_clear(&cp.args);
+		argv_array_pushl(&cp.args, "reset", "--hard", NULL);
+		cp.env = child_env.argv;
+		ret = run_command(&cp);
+	}
 	if (!ret) {
 		is_junk = 0;
 		free(junk_work_tree);

@@ -320,10 +323,12 @@ static int add(int ac, const char **av, const char *prefix)
 		OPT_STRING('B', NULL, &new_branch_force, N_("branch"),
 			   N_("create or reset a branch")),
 		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_END()
 	};
 
 	memset(&opts, 0, sizeof(opts));
+	opts.checkout = 1;
 	ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
 	if (!!opts.detach + !!opts.new_branch + !!new_branch_force > 1)
 		die(_("-b, -B, and --detach are mutually exclusive"));

diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index cbfa41e..601f963 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh

@@ -213,4 +213,9 @@ test_expect_success 'local clone from linked checkout' '
 	( cd here-clone && git fsck )
 '
 
+test_expect_success '"add" worktree without a checkout' '
+	git worktree add --no-checkout -b swamp swamp &&
+	( cd swamp && git reset --hard && git fsck)
+'
+
 test_done

--

https://github.com/git/git/pull/217

On Thu, Mar 24, 2016 at 1:07 PM, Ray Zhang [off-list ref] wrote:

By adding this option which defaults to true, we can use the
corresponding --no-checkout to make some customizations before
the checkout, like sparse checkout, etc.

I think we can follow git-clone and use '-n' for this. But if it's
sparse checkout that's you're after, be warned that it's not fully
supported (you either enable sparse chekcuot for all worktrees, or
none).
-- 
Duy

hi Duy,
My PATCH v1 did follow git-clone -n, however, Junio C Hamano and Eric Sunshine
suggested that we should avoid doing so , as --no-no-checkout could be
confusing.

Yes, core.sparsecheckout is the global switch for all worktrees, but
every worktree
can have its own info/sparse-checkout.

2016-03-24 17:16 GMT+08:00 Duy Nguyen [off-list ref]:

On Thu, Mar 24, 2016 at 1:07 PM, Ray Zhang [off-list ref] wrote:

quoted

By adding this option which defaults to true, we can use the
corresponding --no-checkout to make some customizations before
the checkout, like sparse checkout, etc.

I think we can follow git-clone and use '-n' for this. But if it's
sparse checkout that's you're after, be warned that it's not fully
supported (you either enable sparse chekcuot for all worktrees, or
none).
--
Duy

On Thu, Mar 24, 2016 at 2:07 AM, Ray Zhang [off-list ref] wrote:

By adding this option which defaults to true, we can use the
corresponding --no-checkout to make some customizations before
the checkout, like sparse checkout, etc.

This version of the patch looks better. Thanks. A few comments below...

Signed-off-by: Ray Zhang <redacted>
---

Here, below the "---" line is a good place to explain to reviewers
what changed since the previous version of the patch. It's also
helpful to provide a link to the previous version, like this[1].

[1]: http://thread.gmane.org/gmane.comp.version-control.git/289659

We can make it more clear that this is a boolean option by formatting
it either like this:

    --[no-]checkout::

or this:

    --checkout::
    --no-checkout::

I don't have a strong preference, and existing documentation uses either form.

+       Default option with `add`, populate the new working tree. Use
+       `--no-checkout` to skip the checkout.

It's subjective, but "Default option with `add`" doesn't quite convey
to me that this is the default behavior of "add". Also, readers would
likely benefit from some explanation of why they might ever want to
use this option. Perhaps it could be rewritten something like this:

    By default, `add` checks out HEAD, however, `--no-checkout` can
    be used to suppress checkout in order to make customizations,
    such as configuring sparse-checkout (see ...).

To match the style of the test immediately above this one, you'd want
a space before the closing ')'.

However, I'm not convinced that reset+fsck is is really telling you
much, as fsck is about checking the object database (which was already
the subject of earlier tests in the script) and doesn't say anything
about the working directory which is the point of --no-checkout. Much
more interesting would be to verify that no files were checked out.
There are many ways to do so; here's one:

    git worktree add --no-checkout -b swamp swamp &&
    ls swamp >actual &&
    test_line_count = 0 actual

+'

Finally, it wouldn't hurt to also add a test to verify that --checkout
works as expected (because the tests should check expected *behavior*,
not *implementation*).

[please respond inline rather than top-posting]

On Thu, Mar 24, 2016 at 5:52 AM, Zhang Lei [off-list ref] wrote:

2016-03-24 17:16 GMT+08:00 Duy Nguyen [off-list ref]:

quoted

I think we can follow git-clone and use '-n' for this. [...]

My PATCH v1 did follow git-clone -n, however, Junio C Hamano and Eric Sunshine
suggested that we should avoid doing so , as --no-no-checkout could be
confusing.

My impression was that Duy was suggesting only that -n be recognized
as shorthand for --no-checkout, however, git-worktree already
recognizes -n as shorthand for --dry-run (as a consequence of using
OPT__DRY_RUN), so -n as shorthand for --no-checkout is a no-go.

On Thu, Mar 24, 2016 at 9:22 PM, Eric Sunshine [off-list ref] wrote:

On Thu, Mar 24, 2016 at 5:52 AM, Zhang Lei [off-list ref] wrote:

quoted

2016-03-24 17:16 GMT+08:00 Duy Nguyen [off-list ref]:

quoted

I think we can follow git-clone and use '-n' for this. [...]

My PATCH v1 did follow git-clone -n, however, Junio C Hamano and Eric Sunshine
suggested that we should avoid doing so , as --no-no-checkout could be
confusing.

My impression was that Duy was suggesting only that -n be recognized
as shorthand for --no-checkout, however, git-worktree already
recognizes -n as shorthand for --dry-run (as a consequence of using
OPT__DRY_RUN), so -n as shorthand for --no-checkout is a no-go.

Ignore this. It's only 'prune' which recognizes -n, so it's possible
that 'add' could recognize it for an alternate meaning (though the
documentation would want to make this very clear).

On Fri, Mar 25, 2016 at 8:29 AM, Eric Sunshine [off-list ref] wrote:

On Thu, Mar 24, 2016 at 9:22 PM, Eric Sunshine [off-list ref] wrote:

quoted

On Thu, Mar 24, 2016 at 5:52 AM, Zhang Lei [off-list ref] wrote:

quoted

2016-03-24 17:16 GMT+08:00 Duy Nguyen [off-list ref]:

quoted

I think we can follow git-clone and use '-n' for this. [...]

My PATCH v1 did follow git-clone -n, however, Junio C Hamano and Eric Sunshine
suggested that we should avoid doing so , as --no-no-checkout could be
confusing.

My impression was that Duy was suggesting only that -n be recognized
as shorthand for --no-checkout, however, git-worktree already
recognizes -n as shorthand for --dry-run (as a consequence of using
OPT__DRY_RUN), so -n as shorthand for --no-checkout is a no-go.

Ignore this. It's only 'prune' which recognizes -n, so it's possible
that 'add' could recognize it for an alternate meaning (though the
documentation would want to make this very clear).

To make it clear, I don't feel strongly about '-n'. Yes muscle memory
may count. But if '-n' may become a new confusion source in
git-worktree then perhaps we should avoid it and go with
--[no-]checkout
-- 
Duy

Thanks for the clarification.
By the way, Duy, another unrelated question: why worktree name under
.git/worktrees is being named
after the working tree path basename? I think branch name is more
reasonable since we don't allow checking out
the same branch twice.

2016-03-25 9:49 GMT+08:00 Duy Nguyen [off-list ref]:

On Fri, Mar 25, 2016 at 8:29 AM, Eric Sunshine [off-list ref] wrote:

quoted

On Thu, Mar 24, 2016 at 9:22 PM, Eric Sunshine [off-list ref] wrote:

quoted

On Thu, Mar 24, 2016 at 5:52 AM, Zhang Lei [off-list ref] wrote:

quoted

2016-03-24 17:16 GMT+08:00 Duy Nguyen [off-list ref]:

quoted

I think we can follow git-clone and use '-n' for this. [...]

My PATCH v1 did follow git-clone -n, however, Junio C Hamano and Eric Sunshine
suggested that we should avoid doing so , as --no-no-checkout could be
confusing.

My impression was that Duy was suggesting only that -n be recognized
as shorthand for --no-checkout, however, git-worktree already
recognizes -n as shorthand for --dry-run (as a consequence of using
OPT__DRY_RUN), so -n as shorthand for --no-checkout is a no-go.

Ignore this. It's only 'prune' which recognizes -n, so it's possible
that 'add' could recognize it for an alternate meaning (though the
documentation would want to make this very clear).

To make it clear, I don't feel strongly about '-n'. Yes muscle memory
may count. But if '-n' may become a new confusion source in
git-worktree then perhaps we should avoid it and go with
--[no-]checkout
--
Duy

By adding this option which defaults to true, we can use the
corresponding --no-checkout to make some customizations before
the checkout, like sparse checkout, etc.

Signed-off-by: Ray Zhang <redacted>
---
1. reword on `--no-checkout` in Documentation/git-worktree.txt
2. update the test for `--no-checkout`
3. add a test for `--checkout`
Previous version of this patch:[v2]

[v2]: http://article.gmane.org/gmane.comp.version-control.git/289713
---
 Documentation/git-worktree.txt |  8 +++++++-
 builtin/worktree.c             | 15 ++++++++++-----
 t/t2025-worktree-add.sh        | 14 ++++++++++++++
 3 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 62c76c1..c2796bb 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt

@@ -9,7 +9,7 @@ git-worktree - Manage multiple working trees
 SYNOPSIS
 --------
 [verse]
-'git worktree add' [-f] [--detach] [-b <new-branch>] <path> [<branch>]
+'git worktree add' [-f] [--detach] [--checkout] [-b <new-branch>] <path> [<branch>]
 'git worktree prune' [-n] [-v] [--expire <expire>]
 'git worktree list' [--porcelain]
 

@@ -87,6 +87,12 @@ OPTIONS
 	With `add`, detach HEAD in the new working tree. See "DETACHED HEAD"
 	in linkgit:git-checkout[1].
 
+--[no-]checkout::
+	By default, `add` checks out HEAD, however, `--no-checkout` can
+	be used to suppress checkout in order to make customizations,
+	such as configuring sparse-checkout. See "Sparse checkout"
+	in linkgit:git-read-tree[1].
+
 -n::
 --dry-run::
 	With `prune`, do not remove anything; just report what it would

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 38b5609..e677cd7 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c

@@ -21,6 +21,7 @@ static const char * const worktree_usage[] = {
 struct add_opts {
 	int force;
 	int detach;
+	int checkout;
 	const char *new_branch;
 	int force_new_branch;
 };

@@ -284,11 +285,13 @@ static int add_worktree(const char *path, const char *refname,
 	if (ret)
 		goto done;
 
-	cp.argv = NULL;
-	argv_array_clear(&cp.args);
-	argv_array_pushl(&cp.args, "reset", "--hard", NULL);
-	cp.env = child_env.argv;
-	ret = run_command(&cp);
+	if (opts->checkout) {
+		cp.argv = NULL;
+		argv_array_clear(&cp.args);
+		argv_array_pushl(&cp.args, "reset", "--hard", NULL);
+		cp.env = child_env.argv;
+		ret = run_command(&cp);
+	}
 	if (!ret) {
 		is_junk = 0;
 		free(junk_work_tree);

@@ -320,10 +323,12 @@ static int add(int ac, const char **av, const char *prefix)
 		OPT_STRING('B', NULL, &new_branch_force, N_("branch"),
 			   N_("create or reset a branch")),
 		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_END()
 	};
 
 	memset(&opts, 0, sizeof(opts));
+	opts.checkout = 1;
 	ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
 	if (!!opts.detach + !!opts.new_branch + !!new_branch_force > 1)
 		die(_("-b, -B, and --detach are mutually exclusive"));

diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index cbfa41e..1ff96af 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh

@@ -213,4 +213,18 @@ test_expect_success 'local clone from linked checkout' '
 	( cd here-clone && git fsck )
 '
 
+test_expect_success '"add" worktree with --no-checkout' '
+	git worktree add --no-checkout -b swamp swamp &&
+	ls swamp >actual && test_line_count = 0 actual &&
+	(
+		cd swamp && git reset --hard &&
+		test_cmp ../init.t init.t
+	)
+'
+
+test_expect_success '"add" worktree with --checkout' '
+	git worktree add --checkout -b swmap2 swamp2 &&
+	( cd swamp2 && test_cmp ../init.t init.t )
+'
+
 test_done

--

https://github.com/git/git/pull/217

On Fri, Mar 25, 2016 at 6:31 PM, Zhang Lei [off-list ref] wrote:

By the way, Duy, another unrelated question: why worktree name under
.git/worktrees is being named
after the working tree path basename? I think branch name is more
reasonable since we don't allow checking out
the same branch twice.

Because branch name is not always available (e.g. detached HEAD) and
checkout branch can be switched later on. And normally you'll get
branch name there anyway with "git worktree add something" because the
branch "something" is automatically created. I've been wondering if
it's worth supporting "git worktree -b abc ./" where we create
worktree "./abc" based on branch name too.
-- 
Duy

Yes, path basename makes sense.
I am asking this question because we have some legacy code requires
that working tree
called something like 'src', as a result, multiple branch would have
src1 src2 src3 under .git/worktrees
which could not be easy to maintain.
I agreed with you, we should give users such option.


2016-03-25 19:41 GMT+08:00 Duy Nguyen [off-list ref]:

On Fri, Mar 25, 2016 at 6:31 PM, Zhang Lei [off-list ref] wrote:

quoted

By the way, Duy, another unrelated question: why worktree name under
.git/worktrees is being named
after the working tree path basename? I think branch name is more
reasonable since we don't allow checking out
the same branch twice.

Because branch name is not always available (e.g. detached HEAD) and
checkout branch can be switched later on. And normally you'll get
branch name there anyway with "git worktree add something" because the
branch "something" is automatically created. I've been wondering if
it's worth supporting "git worktree -b abc ./" where we create
worktree "./abc" based on branch name too.
--
Duy

Please don't top-post.

On Fri, Mar 25, 2016 at 7:06 PM, Zhang Lei [off-list ref] wrote:

Yes, path basename makes sense.
I am asking this question because we have some legacy code requires
that working tree
called something like 'src', as a result, multiple branch would have
src1 src2 src3 under .git/worktrees
which could not be easy to maintain.

If you really need to care about these names, I think a new option to
let you control the naming explicitly would be better.

I agreed with you, we should give users such option.


2016-03-25 19:41 GMT+08:00 Duy Nguyen [off-list ref]:

quoted

On Fri, Mar 25, 2016 at 6:31 PM, Zhang Lei [off-list ref] wrote:

quoted

By the way, Duy, another unrelated question: why worktree name under
.git/worktrees is being named
after the working tree path basename? I think branch name is more
reasonable since we don't allow checking out
the same branch twice.

Because branch name is not always available (e.g. detached HEAD) and
checkout branch can be switched later on. And normally you'll get
branch name there anyway with "git worktree add something" because the
branch "something" is automatically created. I've been wondering if
it's worth supporting "git worktree -b abc ./" where we create
worktree "./abc" based on branch name too.
--
Duy

-- 
Duy

On Fri, Mar 25, 2016 at 7:41 AM, Duy Nguyen [off-list ref] wrote:

On Fri, Mar 25, 2016 at 6:31 PM, Zhang Lei [off-list ref] wrote:

quoted

By the way, Duy, another unrelated question: why worktree name under
.git/worktrees is being named
after the working tree path basename? I think branch name is more
reasonable since we don't allow checking out
the same branch twice.

Because branch name is not always available (e.g. detached HEAD) and
checkout branch can be switched later on. And normally you'll get
branch name there anyway with "git worktree add something" because the
branch "something" is automatically created. I've been wondering if
it's worth supporting "git worktree -b abc ./" where we create
worktree "./abc" based on branch name too.

You can switch to any other branch in a worktree.  Consider that you
could switch branches in
worktrees such that you could eventually end up having the branches
swapped from original
worktree setup.

On Fri, Mar 25, 2016 at 11:25:37AM +0000, Ray Zhang wrote:

By adding this option which defaults to true, we can use the
corresponding --no-checkout to make some customizations before
the checkout, like sparse checkout, etc.

Signed-off-by: Ray Zhang <redacted>
---
1. reword on `--no-checkout` in Documentation/git-worktree.txt
2. update the test for `--no-checkout`
3. add a test for `--checkout`

Thanks, this version of the patch looks good and is:

    Reviewed-by: Eric Sunshine [off-list ref]

with or without the minor suggestions below. (If you do re-roll, feel
free to add my Reviewed-by: if you include these suggestions but not
if you make other major changes.)

I realize that this description is just a verbatim copy of what I
suggested during review[1], but in retrospect, I think
s/HEAD/`<branch>`/ would be clearer and more consistent:

    By default, `add` checks out `<branch>`, however, ...

[1]: http://article.gmane.org/gmane.comp.version-control.git/289840

Style: One statement per line, so this should be split over two
lines.

+	(
+		cd swamp && git reset --hard &&
+		test_cmp ../init.t init.t
+	)

No need for the subshell. This can be written more simply as:

    git -C swamp reset --hard &&
    test_cmp init.t swamp/init.t

+'
+
+test_expect_success '"add" worktree with --checkout' '
+	git worktree add --checkout -b swmap2 swamp2 &&
+	( cd swamp2 && test_cmp ../init.t init.t )

Likewise, you can drop the subshell:

    test_cmp init.t swamp2/init.t

+'
+
 test_done

For convenience, the above suggestions would look like this (applied
atop your patch). Feel free to fold them in if you re-roll.

--- 8< ---
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index c2796bb..c622345 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt

@@ -88,7 +88,7 @@ OPTIONS
 	in linkgit:git-checkout[1].
 
 --[no-]checkout::
-	By default, `add` checks out HEAD, however, `--no-checkout` can
+	By default, `add` checks out `<branch>`, however, `--no-checkout` can
 	be used to suppress checkout in order to make customizations,
 	such as configuring sparse-checkout. See "Sparse checkout"
 	in linkgit:git-read-tree[1].

diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index 1ff96af..472b811 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh

@@ -215,16 +215,15 @@ test_expect_success 'local clone from linked checkout' '
 
 test_expect_success '"add" worktree with --no-checkout' '
 	git worktree add --no-checkout -b swamp swamp &&
-	ls swamp >actual && test_line_count = 0 actual &&
-	(
-		cd swamp && git reset --hard &&
-		test_cmp ../init.t init.t
-	)
+	ls swamp >actual &&
+	test_line_count = 0 actual &&
+	git -C swamp reset --hard &&
+	test_cmp init.t swamp/init.t
 '
 
 test_expect_success '"add" worktree with --checkout' '
 	git worktree add --checkout -b swmap2 swamp2 &&
-	( cd swamp2 && test_cmp ../init.t init.t )
+	test_cmp init.t swamp2/init.t
 '
 
 test_done
--- 8< ---

By adding this option which defaults to true, we can use the
corresponding --no-checkout to make some customizations before
the checkout, like sparse checkout, etc.

Helped-by: Eric Sunshine [off-list ref]
Helped-by: Junio C Hamano [off-list ref]
Reviewed-by: Eric Sunshine <redacted>
Signed-off-by: Ray Zhang <redacted>
---
Changes since last version of this patch[v3]:
	Documentation/git-worktree.txt: HEAD --> `<branch>`
	t/t2025-worktree-add.sh: fix style

[v3]: http://article.gmane.org/gmane.comp.version-control.git/289877
[v2]: http://article.gmane.org/gmane.comp.version-control.git/289713
[v1]: http://article.gmane.org/gmane.comp.version-control.git/289659
---
 Documentation/git-worktree.txt |  8 +++++++-
 builtin/worktree.c             | 15 ++++++++++-----
 t/t2025-worktree-add.sh        | 13 +++++++++++++
 3 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 62c76c1..c622345 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt

@@ -9,7 +9,7 @@ git-worktree - Manage multiple working trees
 SYNOPSIS
 --------
 [verse]
-'git worktree add' [-f] [--detach] [-b <new-branch>] <path> [<branch>]
+'git worktree add' [-f] [--detach] [--checkout] [-b <new-branch>] <path> [<branch>]
 'git worktree prune' [-n] [-v] [--expire <expire>]
 'git worktree list' [--porcelain]
 

@@ -87,6 +87,12 @@ OPTIONS
 	With `add`, detach HEAD in the new working tree. See "DETACHED HEAD"
 	in linkgit:git-checkout[1].
 
+--[no-]checkout::
+	By default, `add` checks out `<branch>`, however, `--no-checkout` can
+	be used to suppress checkout in order to make customizations,
+	such as configuring sparse-checkout. See "Sparse checkout"
+	in linkgit:git-read-tree[1].
+
 -n::
 --dry-run::
 	With `prune`, do not remove anything; just report what it would

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 38b5609..e677cd7 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c

@@ -21,6 +21,7 @@ static const char * const worktree_usage[] = {
 struct add_opts {
 	int force;
 	int detach;
+	int checkout;
 	const char *new_branch;
 	int force_new_branch;
 };

@@ -284,11 +285,13 @@ static int add_worktree(const char *path, const char *refname,
 	if (ret)
 		goto done;
 
-	cp.argv = NULL;
-	argv_array_clear(&cp.args);
-	argv_array_pushl(&cp.args, "reset", "--hard", NULL);
-	cp.env = child_env.argv;
-	ret = run_command(&cp);
+	if (opts->checkout) {
+		cp.argv = NULL;
+		argv_array_clear(&cp.args);
+		argv_array_pushl(&cp.args, "reset", "--hard", NULL);
+		cp.env = child_env.argv;
+		ret = run_command(&cp);
+	}
 	if (!ret) {
 		is_junk = 0;
 		free(junk_work_tree);

@@ -320,10 +323,12 @@ static int add(int ac, const char **av, const char *prefix)
 		OPT_STRING('B', NULL, &new_branch_force, N_("branch"),
 			   N_("create or reset a branch")),
 		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_END()
 	};
 
 	memset(&opts, 0, sizeof(opts));
+	opts.checkout = 1;
 	ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
 	if (!!opts.detach + !!opts.new_branch + !!new_branch_force > 1)
 		die(_("-b, -B, and --detach are mutually exclusive"));

diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index cbfa41e..472b811 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh

@@ -213,4 +213,17 @@ test_expect_success 'local clone from linked checkout' '
 	( cd here-clone && git fsck )
 '
 
+test_expect_success '"add" worktree with --no-checkout' '
+	git worktree add --no-checkout -b swamp swamp &&
+	ls swamp >actual &&
+	test_line_count = 0 actual &&
+	git -C swamp reset --hard &&
+	test_cmp init.t swamp/init.t
+'
+
+test_expect_success '"add" worktree with --checkout' '
+	git worktree add --checkout -b swmap2 swamp2 &&
+	test_cmp init.t swamp2/init.t
+'
+
 test_done

--

https://github.com/git/git/pull/217

By adding this option which defaults to true, we can use the
corresponding --no-checkout to make some customizations before
the checkout, like sparse checkout, etc.

Helped-by: Eric Sunshine [off-list ref]
Helped-by: Junio C Hamano [off-list ref]
Reviewed-by: Eric Sunshine <redacted>
Signed-off-by: Ray Zhang <redacted>
---
Changes since last version of this patch[v4]:
	t/t2025-worktree-add.sh: use test -e to test file existence.
	builtin/worktree.c: refactor the code a little bit.

[v4]: http://article.gmane.org/gmane.comp.version-control.git/290030
[v3]: http://article.gmane.org/gmane.comp.version-control.git/289877
[v2]: http://article.gmane.org/gmane.comp.version-control.git/289713
[v1]: http://article.gmane.org/gmane.comp.version-control.git/289659
---
 Documentation/git-worktree.txt |  8 +++++++-
 builtin/worktree.c             | 29 ++++++++++++++++++-----------
 t/t2025-worktree-add.sh        | 12 ++++++++++++
 3 files changed, 37 insertions(+), 12 deletions(-)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 62c76c1..c622345 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt

@@ -9,7 +9,7 @@ git-worktree - Manage multiple working trees
 SYNOPSIS
 --------
 [verse]
-'git worktree add' [-f] [--detach] [-b <new-branch>] <path> [<branch>]
+'git worktree add' [-f] [--detach] [--checkout] [-b <new-branch>] <path> [<branch>]
 'git worktree prune' [-n] [-v] [--expire <expire>]
 'git worktree list' [--porcelain]
 

@@ -87,6 +87,12 @@ OPTIONS
 	With `add`, detach HEAD in the new working tree. See "DETACHED HEAD"
 	in linkgit:git-checkout[1].
 
+--[no-]checkout::
+	By default, `add` checks out `<branch>`, however, `--no-checkout` can
+	be used to suppress checkout in order to make customizations,
+	such as configuring sparse-checkout. See "Sparse checkout"
+	in linkgit:git-read-tree[1].
+
 -n::
 --dry-run::
 	With `prune`, do not remove anything; just report what it would

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 38b5609..d8e3795 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c

@@ -21,6 +21,7 @@ static const char * const worktree_usage[] = {
 struct add_opts {
 	int force;
 	int detach;
+	int checkout;
 	const char *new_branch;
 	int force_new_branch;
 };

@@ -284,18 +285,22 @@ static int add_worktree(const char *path, const char *refname,
 	if (ret)
 		goto done;
 
-	cp.argv = NULL;
-	argv_array_clear(&cp.args);
-	argv_array_pushl(&cp.args, "reset", "--hard", NULL);
-	cp.env = child_env.argv;
-	ret = run_command(&cp);
-	if (!ret) {
-		is_junk = 0;
-		free(junk_work_tree);
-		free(junk_git_dir);
-		junk_work_tree = NULL;
-		junk_git_dir = NULL;
+	if (opts->checkout) {
+		cp.argv = NULL;
+		argv_array_clear(&cp.args);
+		argv_array_pushl(&cp.args, "reset", "--hard", NULL);
+		cp.env = child_env.argv;
+		ret = run_command(&cp);
+		if (ret)
+			goto done;
 	}
+
+	is_junk = 0;
+	free(junk_work_tree);
+	free(junk_git_dir);
+	junk_work_tree = NULL;
+	junk_git_dir = NULL;
+
 done:
 	strbuf_reset(&sb);
 	strbuf_addf(&sb, "%s/locked", sb_repo.buf);

@@ -320,10 +325,12 @@ static int add(int ac, const char **av, const char *prefix)
 		OPT_STRING('B', NULL, &new_branch_force, N_("branch"),
 			   N_("create or reset a branch")),
 		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_END()
 	};
 
 	memset(&opts, 0, sizeof(opts));
+	opts.checkout = 1;
 	ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
 	if (!!opts.detach + !!opts.new_branch + !!new_branch_force > 1)
 		die(_("-b, -B, and --detach are mutually exclusive"));

diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index cbfa41e..3acb992 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh

@@ -213,4 +213,16 @@ test_expect_success 'local clone from linked checkout' '
 	( cd here-clone && git fsck )
 '
 
+test_expect_success '"add" worktree with --no-checkout' '
+	git worktree add --no-checkout -b swamp swamp &&
+	! test -e swamp/init.t &&
+	git -C swamp reset --hard &&
+	test_cmp init.t swamp/init.t
+'
+
+test_expect_success '"add" worktree with --checkout' '
+	git worktree add --checkout -b swmap2 swamp2 &&
+	test_cmp init.t swamp2/init.t
+'
+
 test_done

--

https://github.com/git/git/pull/217

On Tue, Mar 29, 2016 at 10:11:01AM +0000, Ray Zhang wrote:

This should be:

--checkout::
--no-checkout::

(see for example --progress in Documentation/merge-options.txt).

On Tue, Mar 29, 2016 at 6:54 AM, John Keeping [off-list ref] wrote:

On Tue, Mar 29, 2016 at 10:11:01AM +0000, Ray Zhang wrote:

quoted

      With `add`, detach HEAD in the new working tree. See "DETACHED HEAD"
      in linkgit:git-checkout[1].

+--[no-]checkout::

This should be:

--checkout::
--no-checkout::

(see for example --progress in Documentation/merge-options.txt).

[1] suggested either form without stating a preference since existing
Git documentation uses a mixture of the two. See, for instance,
git-format-patch.txt. However, I see now that --[no-]-option is the
minority.

[1]: http://article.gmane.org/gmane.comp.version-control.git/289840

On Tue, Mar 29, 2016 at 6:11 AM, Ray Zhang [off-list ref] wrote:

By adding this option which defaults to true, we can use the
corresponding --no-checkout to make some customizations before
the checkout, like sparse checkout, etc.

Reviewed-by: Eric Sunshine <redacted>
Signed-off-by: Ray Zhang <redacted>
---
Changes since last version of this patch[v4]:
        t/t2025-worktree-add.sh: use test -e to test file existence.
        builtin/worktree.c: refactor the code a little bit.

Thanks, this version is still:

    Reviewed-by: Eric Sunshine [off-list ref]

A couple comments below...

Doing the goto-dance and outdenting the "freeing" code as suggested as
a possible improvement by [1] probably should have been done as a
separate preparatory patch since the result in this patch is fairly
noisy and more difficult to review. However, it's probably not worth
the patch churn to do so now.

I realize that this was suggested by [2], however, a more modern way
to state this would be:

    test_path_is_missing swamp/init.t &&

but, as also mentioned in [2], it's probably not worth the patch churn
to change it now.

+       git -C swamp reset --hard &&
+       test_cmp init.t swamp/init.t
+'
+
+test_expect_success '"add" worktree with --checkout' '
+       git worktree add --checkout -b swmap2 swamp2 &&
+       test_cmp init.t swamp2/init.t
+'
+
 test_done

[1]: http://git.661346.n2.nabble.com/PATCH-add-option-n-no-checkout-to-git-worktree-add-tp7651385p7651884.html

[2]: http://article.gmane.org/gmane.comp.version-control.git/290050

On Tue, Mar 29, 2016 at 02:04:38PM -0400, Eric Sunshine wrote:

On Tue, Mar 29, 2016 at 6:54 AM, John Keeping [off-list ref] wrote:

quoted

On Tue, Mar 29, 2016 at 10:11:01AM +0000, Ray Zhang wrote:

quoted

      With `add`, detach HEAD in the new working tree. See "DETACHED HEAD"
      in linkgit:git-checkout[1].

+--[no-]checkout::

This should be:

--checkout::
--no-checkout::

(see for example --progress in Documentation/merge-options.txt).

[1] suggested either form without stating a preference since existing
Git documentation uses a mixture of the two. See, for instance,
git-format-patch.txt. However, I see now that --[no-]-option is the
minority.

[1]: http://article.gmane.org/gmane.comp.version-control.git/289840

I tend to skim the mailing list so I didn't register that at the time.

Having gone looking, I can't find a reference but I for some reason I
was convinced the separate version was preferred in the option
descriptions.  Note that AsciiDoc does handle this specially, at least
when outputting troff (HTML seems to show both on separate lines).

Thanks for the review.
Sorry for the patch churn, I wasn't quite familiar with working with
mailing list.

2016-03-30 3:20 GMT+08:00 Eric Sunshine [off-list ref]:

On Tue, Mar 29, 2016 at 6:11 AM, Ray Zhang [off-list ref] wrote:

quoted

By adding this option which defaults to true, we can use the
corresponding --no-checkout to make some customizations before
the checkout, like sparse checkout, etc.

Reviewed-by: Eric Sunshine <redacted>
Signed-off-by: Ray Zhang <redacted>
---
Changes since last version of this patch[v4]:
        t/t2025-worktree-add.sh: use test -e to test file existence.
        builtin/worktree.c: refactor the code a little bit.

Thanks, this version is still:

    Reviewed-by: Eric Sunshine [off-list ref]

A couple comments below...

quoted

---

diff --git a/builtin/worktree.c b/builtin/worktree.c

@@ -284,18 +285,22 @@ static int add_worktree(const char *path, const char *refname,
        if (ret)
                goto done;

-       cp.argv = NULL;
-       argv_array_clear(&cp.args);
-       argv_array_pushl(&cp.args, "reset", "--hard", NULL);
-       cp.env = child_env.argv;
-       ret = run_command(&cp);
-       if (!ret) {
-               is_junk = 0;
-               free(junk_work_tree);
-               free(junk_git_dir);
-               junk_work_tree = NULL;
-               junk_git_dir = NULL;
+       if (opts->checkout) {
+               cp.argv = NULL;
+               argv_array_clear(&cp.args);
+               argv_array_pushl(&cp.args, "reset", "--hard", NULL);
+               cp.env = child_env.argv;
+               ret = run_command(&cp);
+               if (ret)
+                       goto done;
        }
+
+       is_junk = 0;
+       free(junk_work_tree);
+       free(junk_git_dir);
+       junk_work_tree = NULL;
+       junk_git_dir = NULL;

Doing the goto-dance and outdenting the "freeing" code as suggested as
a possible improvement by [1] probably should have been done as a
separate preparatory patch since the result in this patch is fairly
noisy and more difficult to review. However, it's probably not worth
the patch churn to do so now.

quoted

 done:
        strbuf_reset(&sb);
        strbuf_addf(&sb, "%s/locked", sb_repo.buf);

diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
+test_expect_success '"add" worktree with --no-checkout' '
+       git worktree add --no-checkout -b swamp swamp &&
+       ! test -e swamp/init.t &&

I realize that this was suggested by [2], however, a more modern way
to state this would be:

    test_path_is_missing swamp/init.t &&

but, as also mentioned in [2], it's probably not worth the patch churn
to change it now.

quoted

+       git -C swamp reset --hard &&
+       test_cmp init.t swamp/init.t
+'
+
+test_expect_success '"add" worktree with --checkout' '
+       git worktree add --checkout -b swmap2 swamp2 &&
+       test_cmp init.t swamp2/init.t
+'
+
 test_done

[1]: http://git.661346.n2.nabble.com/PATCH-add-option-n-no-checkout-to-git-worktree-add-tp7651385p7651884.html

[2]: http://article.gmane.org/gmane.comp.version-control.git/290050

[please don't top-post]

On Tue, Mar 29, 2016 at 11:11 PM, Zhang Lei [off-list ref] wrote:

Thanks for the review.
Sorry for the patch churn, I wasn't quite familiar with working with
mailing list.

No need to apologize. Reviewers understand what it is like being a
newcomer and provide additional review comments to help get up to
speed. Thanks for working on this enhancement.