[PATCH v2] worktree: list operation

[PATCH v2] worktree: list operation

[Michael Rappazzo]

I am attempting to add the 'git worktree list' command.  

I don't have a lot of c experience, so please double check that I am 
not missing something important.

Sorry about publishing the first version too soon.

Michael Rappazzo (1):
  worktree:  list operation

 Documentation/git-worktree.txt |  9 ++++-
 builtin/worktree.c             | 80 ++++++++++++++++++++++++++++++++++++++----
 t/t2027-worktree-list.sh       | 68 +++++++++++++++++++++++++++++++++++
 3 files changed, 150 insertions(+), 7 deletions(-)
 create mode 100755 t/t2027-worktree-list.sh

-- 
2.5.0

[PATCH v2] worktree: list operation

[Michael Rappazzo]

'git worktree list' will list the main worktree followed by any linked
worktrees which were created using 'git worktree add'.  The option
'--main-only' will restrict the list to only the main worktree.
---
 Documentation/git-worktree.txt |  9 ++++-
 builtin/worktree.c             | 84 ++++++++++++++++++++++++++++++++++++++----
 t/t2027-worktree-list.sh       | 68 ++++++++++++++++++++++++++++++++++
 3 files changed, 152 insertions(+), 9 deletions(-)
 create mode 100755 t/t2027-worktree-list.sh

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 3387e2f..2b6b543 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -11,6 +11,7 @@ SYNOPSIS
 [verse]
 'git worktree add' [-f] [--detach] [-b <new-branch>] <path> [<branch>]
 'git worktree prune' [-n] [-v] [--expire <expire>]
+'git worktree list' [--main-only]
 
 DESCRIPTION
 -----------
@@ -59,6 +60,10 @@ prune::
 
 Prune working tree information in $GIT_DIR/worktrees.
 
+list::
+
+List the main worktree followed by all of the linked worktrees.
+
 OPTIONS
 -------
 
@@ -86,6 +91,9 @@ OPTIONS
 	With `prune`, do not remove anything; just report what it would
 	remove.
 
+--main-only::
+	With `list`, only list the main worktree.
+
 -v::
 --verbose::
 	With `prune`, report all removals.
@@ -167,7 +175,6 @@ performed manually, such as:
 - `remove` to remove a linked worktree and its administrative files (and
   warn if the worktree is dirty)
 - `mv` to move or rename a worktree and update its administrative files
-- `list` to list linked worktrees
 - `lock` to prevent automatic pruning of administrative files (for instance,
   for a worktree on a portable device)
 
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 6a264ee..8c4a82a 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -10,6 +10,7 @@
 static const char * const worktree_usage[] = {
 	N_("git worktree add [<options>] <path> <branch>"),
 	N_("git worktree prune [<options>]"),
+	N_("git worktree list [<options>]"),
 	NULL
 };
 
@@ -36,7 +37,7 @@ static int prune_worktree(const char *id, struct strbuf *reason)
 	fd = open(git_path("worktrees/%s/gitdir", id), O_RDONLY);
 	if (fd < 0) {
 		strbuf_addf(reason, _("Removing worktrees/%s: unable to read gitdir file (%s)"),
-			    id, strerror(errno));
+			id, strerror(errno));
 		return 1;
 	}
 	len = st.st_size;
@@ -59,7 +60,7 @@ static int prune_worktree(const char *id, struct strbuf *reason)
 		 * accessed since?
 		 */
 		if (!stat(git_path("worktrees/%s/link", id), &st_link) &&
-		    st_link.st_nlink > 1)
+			st_link.st_nlink > 1)
 			return 0;
 		if (st.st_mtime <= expire) {
 			strbuf_addf(reason, _("Removing worktrees/%s: gitdir file points to non-existent location"), id);
@@ -187,11 +188,11 @@ static int add_worktree(const char *path, const char **child_argv)
 
 	name = worktree_basename(path, &len);
 	strbuf_addstr(&sb_repo,
-		      git_path("worktrees/%.*s", (int)(path + len - name), name));
+				git_path("worktrees/%.*s", (int)(path + len - name), name));
 	len = sb_repo.len;
 	if (safe_create_leading_directories_const(sb_repo.buf))
 		die_errno(_("could not create leading directories of '%s'"),
-			  sb_repo.buf);
+			sb_repo.buf);
 	while (!stat(sb_repo.buf, &st)) {
 		counter++;
 		strbuf_setlen(&sb_repo, len);
@@ -218,14 +219,14 @@ static int add_worktree(const char *path, const char **child_argv)
 	strbuf_addf(&sb_git, "%s/.git", path);
 	if (safe_create_leading_directories_const(sb_git.buf))
 		die_errno(_("could not create leading directories of '%s'"),
-			  sb_git.buf);
+			sb_git.buf);
 	junk_work_tree = xstrdup(path);
 
 	strbuf_reset(&sb);
 	strbuf_addf(&sb, "%s/gitdir", sb_repo.buf);
 	write_file(sb.buf, 1, "%s\n", real_path(sb_git.buf));
 	write_file(sb_git.buf, 1, "gitdir: %s/worktrees/%s\n",
-		   real_path(get_git_common_dir()), name);
+			real_path(get_git_common_dir()), name);
 	/*
 	 * This is to keep resolve_ref() happy. We need a valid HEAD
 	 * or is_git_directory() will reject the directory. Moreover, HEAD
@@ -280,9 +281,9 @@ static int add(int ac, const char **av, const char *prefix)
 	struct option options[] = {
 		OPT__FORCE(&force, N_("checkout <branch> even if already checked out in other worktree")),
 		OPT_STRING('b', NULL, &new_branch, N_("branch"),
-			   N_("create a new branch")),
+				N_("create a new branch")),
 		OPT_STRING('B', NULL, &new_branch_force, N_("branch"),
-			   N_("create or reset a branch")),
+				N_("create or reset a branch")),
 		OPT_BOOL(0, "detach", &detach, N_("detach HEAD at named commit")),
 		OPT_END()
 	};
@@ -316,6 +317,71 @@ static int add(int ac, const char **av, const char *prefix)
 	return add_worktree(path, cmd.argv);
 }
 
+static int list(int ac, const char **av, const char *prefix)
+{
+	int main_only = 0;
+	struct option options[] = {
+		OPT_BOOL(0, "main-only", &main_only, N_("only list the main worktree")),
+		OPT_END()
+	};
+
+	ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
+	if (ac)
+		usage_with_options(worktree_usage, options);
+
+	const char *work_tree;
+	work_tree = get_git_work_tree();
+	if (!work_tree)
+		die("This operation must be run in a work tree");
+
+	struct strbuf worktree_git_path = STRBUF_INIT;
+	strbuf_addf(&worktree_git_path, _("%s/.git"), work_tree);
+
+	struct strbuf main_work_tree = STRBUF_INIT;
+	if (is_directory(worktree_git_path.buf)) {
+		/* This is the main tree */
+		strbuf_addstr(&main_work_tree, work_tree);
+	} else {
+		const char *git_dir = get_git_dir();
+		strbuf_addf(&main_work_tree, "%.*s", (int)(strstr(git_dir, "/.git/") - git_dir), git_dir);
+	}
+	printf("%s\n", main_work_tree.buf);
+
+	if (!main_only) {
+		chdir( main_work_tree.buf );
+		if ( is_directory(git_path("worktrees")) ) {
+			DIR *dir = opendir( git_path("worktrees") );
+			if (dir != NULL) {
+				struct dirent *d;
+				struct stat st;
+				char *path;
+				int fd, len;
+				while ((d = readdir(dir)) != NULL) {
+					if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
+						continue;
+					if (stat(git_path("worktrees/%s/gitdir", d->d_name), &st))
+						continue;
+					fd = open(git_path("worktrees/%s/gitdir", d->d_name), O_RDONLY);
+					if (fd < 0)
+						continue;
+
+					len = st.st_size;
+					path = xmalloc(len + 1);
+					read_in_full(fd, path, len);
+					close(fd);
+
+					printf("%.*s\n", (int)(strstr(path, "/.git") - path), path);
+					free(path);
+				}
+			}
+			closedir(dir);
+		}
+	}
+	strbuf_release(&main_work_tree);
+	strbuf_release(&worktree_git_path);
+	return 0;
+}
+
 int cmd_worktree(int ac, const char **av, const char *prefix)
 {
 	struct option options[] = {
@@ -328,5 +394,7 @@ int cmd_worktree(int ac, const char **av, const char *prefix)
 		return add(ac - 1, av + 1, prefix);
 	if (!strcmp(av[1], "prune"))
 		return prune(ac - 1, av + 1, prefix);
+	if (!strcmp(av[1], "list"))
+		return list(ac - 1, av + 1, prefix);
 	usage_with_options(worktree_usage, options);
 }
diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh
new file mode 100755
index 0000000..998b34f
--- /dev/null
+++ b/t/t2027-worktree-list.sh
@@ -0,0 +1,68 @@
+#!/bin/sh
+
+test_description='test git worktree list'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	test_commit init
+'
+
+
+test_expect_success '"list" all worktrees from main' '
+	orig_path=$PWD &&
+	git rev-parse --show-toplevel >expect &&
+	git worktree add --detach here master &&
+	(
+		cd here &&
+		git rev-parse --show-toplevel >>"$orig_path/expect" &&
+		cd "$orig_path" &&
+		git worktree list >actual &&
+		test_cmp expect actual &&
+		rm -rf here &&
+		git worktree prune
+	)
+'
+test_expect_success '"list" all worktrees from linked' '
+	orig_path=$PWD &&
+	git rev-parse --show-toplevel >expect &&
+	git worktree add --detach here master &&
+	(
+		cd here &&
+		git rev-parse --show-toplevel >>"$orig_path/expect" &&
+		git worktree list >actual &&
+		test_cmp "$orig_path/expect" actual &&
+		cd "$orig_path" &&
+		rm -rf here &&
+		git worktree prune
+	)
+'
+
+test_expect_success '"list" main worktree from main' '
+	orig_path=$PWD &&
+	git rev-parse --show-toplevel >expect &&
+	git worktree add --detach here master &&
+	(
+		cd here &&
+		cd "$orig_path" &&
+		git worktree list --main-only >actual &&
+		test_cmp expect actual &&
+		rm -rf here &&
+		git worktree prune
+	)
+'
+test_expect_success '"list" main worktree from linked' '
+	orig_path=$PWD &&
+	git rev-parse --show-toplevel >expect &&
+	git worktree add --detach here master &&
+	(
+		cd here &&
+		git worktree list --main-only >actual &&
+		test_cmp "$orig_path/expect" actual &&
+		cd "$orig_path" &&
+		rm -rf here &&
+		git worktree prune
+	)
+'
+
+test_done
-- 
2.5.0

Re: [PATCH v2] worktree: list operation

[Andreas Schwab]

Michael Rappazzo <rappazzo@gmail.com> writes:

> @@ -36,7 +37,7 @@ static int prune_worktree(const char *id, struct strbuf *reason)
>  	fd = open(git_path("worktrees/%s/gitdir", id), O_RDONLY);
>  	if (fd < 0) {
>  		strbuf_addf(reason, _("Removing worktrees/%s: unable to read gitdir file (%s)"),
> -			    id, strerror(errno));
> +			id, strerror(errno));

There are still a lot of spurious whitespace changes.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH v2] worktree: list operation

[Eric Sunshine]

Thanks for the patch. Comments below...

On Sat, Aug 8, 2015 at 8:19 PM, Michael Rappazzo <rappazzo@gmail.com> wrote:
> worktree: list operation

Imperative mood:

    worktree: add 'list' command

> 'git worktree list' will list the main worktree followed by any linked
> worktrees which were created using 'git worktree add'.  The option
> '--main-only' will restrict the list to only the main worktree.

Missing sign-off.

> ---
> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
> index 3387e2f..2b6b543 100644
> --- a/Documentation/git-worktree.txt
> +++ b/Documentation/git-worktree.txt
> @@ -59,6 +60,10 @@ prune::
> +list::
> +
> +List the main worktree followed by all of the linked worktrees.
> +
>  OPTIONS
>  -------
> @@ -86,6 +91,9 @@ OPTIONS
> +--main-only::
> +       With `list`, only list the main worktree.

Considering that the main worktree is always listed first, I wonder
how much value the --main-only option has. That is (Windows aside),
the same can easily be achieved via:

    git worktree list | head -1

The more options we have, the more we have to document, test, and
support, so I'm feeling skeptical about --main-only since it can be
easily handled externally (for instance, via "head").

>  -v::
>  --verbose::
>         With `prune`, report all removals.
> @@ -36,7 +37,7 @@ static int prune_worktree(const char *id, struct strbuf *reason)
>         fd = open(git_path("worktrees/%s/gitdir", id), O_RDONLY);
>         if (fd < 0) {
>                 strbuf_addf(reason, _("Removing worktrees/%s: unable to read gitdir file (%s)"),
> -                           id, strerror(errno));
> +                       id, strerror(errno));

Unintended whitespace change here (and 7 places below)? The
indentation looks fine as-is, so I think you don't want to include
these changes.

>                 return 1;
>         }
>         len = st.st_size;
> @@ -316,6 +317,71 @@ static int add(int ac, const char **av, const char *prefix)
> +static int list(int ac, const char **av, const char *prefix)
> +{
> +       int main_only = 0;
> +       struct option options[] = {
> +               OPT_BOOL(0, "main-only", &main_only, N_("only list the main worktree")),
> +               OPT_END()
> +       };
> +
> +       ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
> +       if (ac)
> +               usage_with_options(worktree_usage, options);
> +
> +       const char *work_tree;
> +       work_tree = get_git_work_tree();
> +       if (!work_tree)
> +               die("This operation must be run in a work tree");

Why this restriction? Other git-worktree operations support a bare
repository, so this one ought to also.

(In fact, when dealing with a bare repository, the "main worktree" is
really the bare repository, so the term "worktree" is a bit of a
misnomer.)

> +       struct strbuf worktree_git_path = STRBUF_INIT;
> +       strbuf_addf(&worktree_git_path, _("%s/.git"), work_tree);
> +
> +       struct strbuf main_work_tree = STRBUF_INIT;
> +       if (is_directory(worktree_git_path.buf)) {

This is probably not a valid test. Even in the main worktree, ".git"
may not be a directory; it could be a symref to the actual repository.

> +               /* This is the main tree */
> +               strbuf_addstr(&main_work_tree, work_tree);
> +       } else {
> +               const char *git_dir = get_git_dir();
> +               strbuf_addf(&main_work_tree, "%.*s", (int)(strstr(git_dir, "/.git/") - git_dir), git_dir);
> +       }
> +       printf("%s\n", main_work_tree.buf);

This can probably all be done more simply by taking a hint from the
code which tests if a branch is already checked out in the main or a
linked worktree. For the main worktree, it just uses
get_git_common_dir() and strips the trailing ".git" and prints that.
For instance:

    strbuf_addstr(&sb, get_git_common_dir());
    strbuf_strip_suffix(&sb, ".git");
    printf(...);

should probably suffice in place of all the above code.

> +       if (!main_only) {
> +               chdir( main_work_tree.buf );

Style: Drop spaces inside parentheses.

I realize that the program exits after printing the list of worktrees,
but this chdir() makes me uncomfortable, partly because it's not
necessary, and partly because it could negatively impact code which
someone later adds to extend "list", if that new code expects the
current working directory to be the top of the worktree (as most git
code assumes).

> +               if ( is_directory(git_path("worktrees")) ) {
> +                       DIR *dir = opendir( git_path("worktrees") );

Style: Drop spaces inside parentheses (both lines).

The opendir() will fail if the directory doesn't exist anyhow, so the
'if (is_directory(...))' is superfluous and can be dropped altogether.

Taking a hint from the code which tests if a branch is already checked
out elsewhere:

    strbuf_addf(&sb, "%s/worktrees", get_git_common_dir());
    dir = opendir(sb.buf);

> +                       if (dir != NULL) {

Style: if (dir) {

> +                               struct dirent *d;
> +                               struct stat st;
> +                               char *path;
> +                               int fd, len;
> +                               while ((d = readdir(dir)) != NULL) {
> +                                       if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
> +                                               continue;

This is at least the third bit of functionality which needs to iterate
over the linked worktrees and glean information about them. As such,
it would make sense to factor out that logic so that it can be
re-used, as one or more preparatory patches if you feel so inclined
(though is not a requirement by any means).

> +                                       if (stat(git_path("worktrees/%s/gitdir", d->d_name), &st))
> +                                               continue;
> +                                       fd = open(git_path("worktrees/%s/gitdir", d->d_name), O_RDONLY);
> +                                       if (fd < 0)
> +                                               continue;
> +
> +                                       len = st.st_size;
> +                                       path = xmalloc(len + 1);
> +                                       read_in_full(fd, path, len);
> +                                       close(fd);

I realize that this code was pretty much copied from elsewhere in
worktree.c, but it could be simplified considerably by taking
advantage of strbuf_read_file() to slurp the entire file into a
strbuf, thus allowing you to drop the stat(), open(), xmalloc(),
read_in_full(), and close() calls:

    if (strbuf_read_file(&sb, "whatever/.../gitdir", 0) < 0)
        continue;
     strbuf_rtrim(&sb);
     strbuf_strip_suffix(&sb, ".git");

> +                                       printf("%.*s\n", (int)(strstr(path, "/.git") - path), path);

For git-worktree commands such as "lock", "mv", "remove", it likely
will be nice to allow people to specify the linked worktree not only
by path, but also by tag, and possibly even by $(basename $path) if
not ambiguous. Therefore, to support such usage, at minimum, I think
you also want to show the worktree's tag (d->d_name) in addition to
the path.

Other information which would be nice to display for each worktree
(possibly controlled by a --verbose flag):

   * the checked out branch or detached head
   * whether it is locked
        - the lock reason (if available)
        - and whether the worktree is currently accessible
    * whether it can be pruned
        - and the prune reason if so

The prune reason could be obtained by factoring out the
reason-determination code from worktree.c:prune_worktree() to make it
re-usable.

For scripters, a --porcelain option might be useful.

None of this additional functionality is an immediate requirement, and
wouldn't belong in this patch anyhow, but can be added via follow-up
patches (if you or someone else is interested in the task).

> +                                       free(path);
> +                               }
> +                       }
> +                       closedir(dir);
> +               }
> +       }
> +       strbuf_release(&main_work_tree);
> +       strbuf_release(&worktree_git_path);
> +       return 0;
> +}
> diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh
> new file mode 100755
> index 0000000..998b34f
> --- /dev/null
> +++ b/t/t2027-worktree-list.sh
> @@ -0,0 +1,68 @@
> +#!/bin/sh
> +
> +test_description='test git worktree list'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'setup' '
> +       test_commit init
> +'
> +
> +

Drop the extra blank line.

> +test_expect_success '"list" all worktrees from main' '
> +       orig_path=$PWD &&
> +       git rev-parse --show-toplevel >expect &&
> +       git worktree add --detach here master &&
> +       (
> +               cd here &&
> +               git rev-parse --show-toplevel >>"$orig_path/expect" &&
> +               cd "$orig_path" &&

The normal and automatic way to return to the original directory is
simply to end the subshell; that is, have the closing ')' at this
point. No need for 'orig_path'.

In fact, in this case, you don't even need a subshell at all because,
these days, you don't need to 'cd here'. Instead, take advantage of
-C:

    git -C here rev-parse --show-toplevel >>expect &&

> +               git worktree list >actual &&
> +               test_cmp expect actual &&
> +               rm -rf here &&
> +               git worktree prune
> +       )
> +'
> +test_expect_success '"list" all worktrees from linked' '
> +       orig_path=$PWD &&
> +       git rev-parse --show-toplevel >expect &&
> +       git worktree add --detach here master &&
> +       (
> +               cd here &&
> +               git rev-parse --show-toplevel >>"$orig_path/expect" &&
> +               git worktree list >actual &&
> +               test_cmp "$orig_path/expect" actual &&
> +               cd "$orig_path" &&

Ditto. Just end the subshell to return to the original directory. Or,
better, ditch the subshell entirely and use -C:

    git rev-parse -C here --show-toplevel >>expect &&
    git -C here worktree list >actual &&

> +               rm -rf here &&
> +               git worktree prune
> +       )
> +'
> +
> +test_expect_success '"list" main worktree from main' '
> +       orig_path=$PWD &&
> +       git rev-parse --show-toplevel >expect &&
> +       git worktree add --detach here master &&
> +       (
> +               cd here &&
> +               cd "$orig_path" &&
> +               git worktree list --main-only >actual &&
> +               test_cmp expect actual &&
> +               rm -rf here &&
> +               git worktree prune
> +       )
> +'
> +test_expect_success '"list" main worktree from linked' '
> +       orig_path=$PWD &&
> +       git rev-parse --show-toplevel >expect &&
> +       git worktree add --detach here master &&
> +       (
> +               cd here &&
> +               git worktree list --main-only >actual &&
> +               test_cmp "$orig_path/expect" actual &&
> +               cd "$orig_path" &&
> +               rm -rf here &&
> +               git worktree prune
> +       )
> +'
> +
> +test_done
> --
> 2.5.0