[PATCH v5 0/2] Worktree: for-each function and list command

[PATCH v5 0/2] Worktree: for-each function and list command

[Michael Rappazzo]

Changes since v4:
 - reduced memory usage by reusing string buffer variables
 - re-scoped variables in the for-each function
 - added tests for 'worktree list' with bare repos

Notes from previous discussion:
 - The following snippet:

> +	/* If the common_dir DOES NOT end with '/.git', then it is bare */
> +	main_is_bare = !strbuf_strip_suffix(&worktree_path, "/.git");

This code is run when the current dir is in a linked worktree (not the primary 
worktree).  In that context, the git config _does_ report that 'core.bare' is
true when the primary repo is bare, however `is_bare_repository()` returns
false.  The worktree_path also needs to the "/.git" removed from it.  
Therefore, I opted to keep the code like this.


Michael Rappazzo (2):
  worktree: add 'for_each_worktree' function
  worktree: add 'list' command

 Documentation/git-worktree.txt |  11 +++-
 builtin/worktree.c             | 138 +++++++++++++++++++++++++++++++++++++++++
 t/t2027-worktree-list.sh       |  81 ++++++++++++++++++++++++
 3 files changed, 229 insertions(+), 1 deletion(-)
 create mode 100755 t/t2027-worktree-list.sh

-- 
2.5.0

[PATCH v5 1/2] worktree: add 'for_each_worktree' function

[Michael Rappazzo]

for_each_worktree iterates through each worktree and invokes a callback
function.  The main worktree (if not bare) is always encountered first,
followed by worktrees created by `git worktree add`.

If the callback function returns a non-zero value, iteration stops, and
the callback's return value is returned from the for_each_worktree
function.

Signed-off-by: Michael Rappazzo <rappazzo@gmail.com>
---
 builtin/worktree.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 83 insertions(+)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 430b51e..7b3cb96 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -26,6 +26,14 @@ static int show_only;
 static int verbose;
 static unsigned long expire;
 
+/*
+ * The signature for the callback function for the for_each_worktree()
+ * function below.  The memory pointed to by the callback arguments
+ * is only guaranteed to be valid for the duration of a single
+ * callback invocation.
+ */
+typedef int each_worktree_fn(const char *path, const char *git_dir, void *cb_data);
+
 static int prune_worktree(const char *id, struct strbuf *reason)
 {
 	struct stat st;
@@ -359,6 +367,81 @@ static int add(int ac, const char **av, const char *prefix)
 	return add_worktree(path, branch, &opts);
 }
 
+/*
+ * Iterate through each worktree and invoke the callback function.  If
+ * the callback function returns non-zero, the iteration stops, and
+ * this function returns that return value
+ */
+static int for_each_worktree(each_worktree_fn fn, void *cb_data)
+{
+	struct strbuf worktree_path = STRBUF_INIT;
+	struct strbuf worktree_git = STRBUF_INIT;
+	const char *common_dir;
+	int main_is_bare = 0;
+	int ret = 0;
+
+	common_dir = get_git_common_dir();
+	if (!strcmp(common_dir, get_git_dir())) {
+		/* simple case - this is the main repo */
+		main_is_bare = is_bare_repository();
+		if (!main_is_bare) {
+			strbuf_addstr(&worktree_path, get_git_work_tree());
+		} else {
+			strbuf_addstr(&worktree_path, common_dir);
+		}
+	} else {
+		strbuf_addstr(&worktree_path, common_dir);
+		/* If the common_dir DOES NOT end with '/.git', then it is bare */
+		main_is_bare = !strbuf_strip_suffix(&worktree_path, "/.git");
+	}
+	strbuf_addstr(&worktree_git, worktree_path.buf);
+
+	if (!main_is_bare) {
+		strbuf_addstr(&worktree_git, "/.git");
+
+		ret = fn(worktree_path.buf, worktree_git.buf, cb_data);
+		if (ret)
+			goto done;
+	}
+	strbuf_addstr(&worktree_git, "/worktrees");
+
+	if (is_directory(worktree_git.buf)) {
+		DIR *dir = opendir(worktree_git.buf);
+		if (dir) {
+			struct stat st;
+			struct dirent *d;
+			size_t base_path_len = worktree_git.len;
+
+			while ((d = readdir(dir)) != NULL) {
+				if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
+					continue;
+
+				strbuf_setlen(&worktree_git, base_path_len);
+				strbuf_addf(&worktree_git, "/%s/gitdir", d->d_name);
+
+				if (stat(worktree_git.buf, &st)) {
+					fprintf(stderr, "Unable to read worktree git dir: %s\n", worktree_git.buf);
+					continue;
+				}
+
+				strbuf_reset(&worktree_path);
+				strbuf_read_file(&worktree_path, worktree_git.buf, st.st_size);
+				strbuf_strip_suffix(&worktree_path, "/.git\n");
+
+				strbuf_strip_suffix(&worktree_git, "/gitdir");
+				ret = fn(worktree_path.buf, worktree_git.buf, cb_data);
+				if (ret)
+					break;
+			}
+		}
+		closedir(dir);
+	}
+done:
+	strbuf_release(&worktree_git);
+	strbuf_release(&worktree_path);
+	return ret;
+}
+
 int cmd_worktree(int ac, const char **av, const char *prefix)
 {
 	struct option options[] = {
-- 
2.5.0

[PATCH v5 2/2] worktree: add 'list' command

[Michael Rappazzo]

'git worktree list' uses the for_each_worktree function to iterate,
and outputs in the format: '<worktree>  (<short-ref>)'

Signed-off-by: Michael Rappazzo <rappazzo@gmail.com>
---
 Documentation/git-worktree.txt | 11 +++++-
 builtin/worktree.c             | 55 ++++++++++++++++++++++++++++
 t/t2027-worktree-list.sh       | 81 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 146 insertions(+), 1 deletion(-)
 create mode 100755 t/t2027-worktree-list.sh

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index fb68156..e953b4e 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' [--path-only]
 
 DESCRIPTION
 -----------
@@ -59,6 +60,12 @@ prune::
 
 Prune working tree information in $GIT_DIR/worktrees.
 
+list::
+
+List the main worktree followed by all of the linked worktrees.  The default
+format of the list includes the full path to the worktree and the branch or
+revision that the head of that worktree is currently pointing to.
+
 OPTIONS
 -------
 
@@ -93,6 +100,9 @@ OPTIONS
 --expire <time>::
 	With `prune`, only expire unused working trees older than <time>.
 
+--path-only
+	With `list`, only show the worktree path
+
 DETAILS
 -------
 Each linked working tree has a private sub-directory in the repository's
@@ -167,7 +177,6 @@ performed manually, such as:
 - `remove` to remove a linked working tree and its administrative files (and
   warn if the working tree is dirty)
 - `mv` to move or rename a working tree and update its administrative files
-- `list` to list linked working trees
 - `lock` to prevent automatic pruning of administrative files (for instance,
   for a working tree on a portable device)
 
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 7b3cb96..673f292 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -12,6 +12,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
 };
 
@@ -442,6 +443,58 @@ done:
 	return ret;
 }
 
+/* list callback data */
+struct list_opts {
+	int path_only;
+};
+
+static int print_worktree_details(const char *path, const char *git_dir, void *cb_data)
+{
+	struct strbuf head_file = STRBUF_INIT;
+	struct strbuf head_ref = STRBUF_INIT;
+	struct stat st;
+	struct list_opts *opts = cb_data;
+	const char *ref_prefix = "ref: refs/heads/";
+
+	strbuf_addf(&head_file, "%s/HEAD", git_dir);
+	if (!opts->path_only && !stat(head_file.buf, &st)) {
+		strbuf_read_file(&head_ref, head_file.buf, st.st_size);
+		strbuf_strip_suffix(&head_ref, "\n");
+
+		if (starts_with(head_ref.buf, ref_prefix)) {
+			/* branch checked out */
+			strbuf_remove(&head_ref, 0, strlen(ref_prefix));
+		/* } else {
+		 *  headless -- no-op
+		 */
+		}
+		printf("%s  (%s)\n", path, head_ref.buf);
+	} else {
+		printf("%s\n", path);
+	}
+
+	strbuf_release(&head_ref);
+	strbuf_release(&head_file);
+	return 0;
+}
+
+static int list(int ac, const char **av, const char *prefix)
+{
+	struct list_opts opts;
+	struct option options[] = {
+		OPT_BOOL(0, "path-only", &opts.path_only, N_("only show the path of the worktree")),
+		OPT_END()
+	};
+
+	opts.path_only = 0;
+
+	ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
+	if (ac)
+		usage_with_options(worktree_usage, options);
+
+	return for_each_worktree(&print_worktree_details, &opts);
+}
+
 int cmd_worktree(int ac, const char **av, const char *prefix)
 {
 	struct option options[] = {
@@ -454,5 +507,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..66a635a
--- /dev/null
+++ b/t/t2027-worktree-list.sh
@@ -0,0 +1,81 @@
+#!/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' '
+	echo "$(git rev-parse --show-toplevel)  ($(git symbolic-ref --short HEAD))" >expect &&
+	git worktree add --detach here master &&
+	echo "$(git -C here rev-parse --show-toplevel)  ($(git -C here rev-parse HEAD))" >>expect &&
+	git worktree list >actual &&
+	test_cmp expect actual &&
+	rm -rf here &&
+	git worktree prune
+'
+
+test_expect_success '"list" all worktrees from linked' '
+	echo "$(git rev-parse --show-toplevel)  ($(git symbolic-ref --short HEAD))" >expect &&
+	git worktree add --detach here master &&
+	echo "$(git -C here rev-parse --show-toplevel)  ($(git -C here rev-parse HEAD))" >>expect &&
+	git -C here worktree list >actual &&
+	test_cmp expect actual &&
+	rm -rf here &&
+	git worktree prune
+'
+
+test_expect_success '"list" all worktrees from main --path-only' '
+	git rev-parse --show-toplevel >expect &&
+	git worktree add --detach here master &&
+	git -C here rev-parse --show-toplevel >>expect &&
+	git worktree list --path-only >actual &&
+	test_cmp expect actual &&
+	rm -rf here &&
+	git worktree prune
+'
+
+test_expect_success '"list" all worktrees from linked --path-only' '
+	git rev-parse --show-toplevel >expect &&
+	git worktree add --detach here master &&
+	git -C here rev-parse --show-toplevel >>expect &&
+	git -C here worktree list --path-only >actual &&
+	test_cmp expect actual &&
+	rm -rf here &&
+	git worktree prune
+'
+
+test_expect_success 'bare repo setup' '
+	git init --bare bare1 &&
+	echo "data" > file1 &&
+	git add file1 &&
+	git commit -m"File1: add data" &&
+	git push bare1 master &&
+	git reset --hard HEAD^
+'
+
+test_expect_success '"list" all worktrees from bare main' '
+	git -C bare1 worktree add --detach ../there master &&
+	echo "$(git -C there rev-parse --show-toplevel)  ($(git -C there rev-parse HEAD))" >expect &&
+	git -C bare1 worktree list >actual &&
+	test_cmp expect actual &&
+	rm -rf there &&
+	git -C bare1 worktree prune
+'
+test_expect_success '"list" all worktrees from worktree with a bare main' '
+	git -C bare1 worktree add --detach ../there master &&
+	echo "$(git -C there rev-parse --show-toplevel)  ($(git -C there rev-parse HEAD))" >expect &&
+	git -C there worktree list >actual &&
+	test_cmp expect actual &&
+	rm -rf there &&
+	git -C bare1 worktree prune
+'
+
+test_expect_success 'bare repo cleanup' '
+	rm -rf bare1
+'
+
+test_done
-- 
2.5.0

Re: [PATCH v5 2/2] worktree: add 'list' command

[Junio C Hamano]

Michael Rappazzo <rappazzo@gmail.com> writes:

> +static int print_worktree_details(const char *path, const char *git_dir, void *cb_data)
> +{
> +	struct strbuf head_file = STRBUF_INIT;
> +	struct strbuf head_ref = STRBUF_INIT;
> +	struct stat st;
> +	struct list_opts *opts = cb_data;
> +	const char *ref_prefix = "ref: refs/heads/";
> +
> +	strbuf_addf(&head_file, "%s/HEAD", git_dir);
> +	if (!opts->path_only && !stat(head_file.buf, &st)) {
> +		strbuf_read_file(&head_ref, head_file.buf, st.st_size);

This does not work for traditional "symlinked HEAD", no?

I'd prefer to see the callback functions of for_each_worktree() not
to duplicate the logic we already have elsewhere in the system.
Such an incomplete attempt to duplication (as we see here) will lead
to unnecessary bugs (as we see here).

Conceptually, for-each-worktree should give us the worktree root
(i.e. equivalent to $GIT_WORK_TREE in the pre-multi-worktree world)
and the git directory (i.e. equivalent to $GIT_DIR in the
pre-multi-worktree world), and the callbacks should be able to do an
equivalent of

    system("git --work-tree=$GIT_WORK_TREE --git-dir=$GIT_DIR cmd")

where in this particular case "cmd" may be "symbolic-ref HEAD" or
something, no?

> +		strbuf_strip_suffix(&head_ref, "\n");
> +
> +		if (starts_with(head_ref.buf, ref_prefix)) {
> +			/* branch checked out */
> +			strbuf_remove(&head_ref, 0, strlen(ref_prefix));
> +		/* } else {
> +		 *  headless -- no-op
> +		 */
> +		}
> +		printf("%s  (%s)\n", path, head_ref.buf);

Is this new command meant to be a Porcelain?  This would not work as
a plumbing that produces a machine-parseable stable output.

I am not saying that it _should_; I do not know if we even need a
'list' command that is driven from an end-user script that gives
anything more than "where the work trees are".

My inclination is to suggest dropping the "which branch" code
altogether and only give "path_only" behaviour.

Thanks.

Re: [PATCH v5 2/2] worktree: add 'list' command

[Eric Sunshine]

On Mon, Aug 24, 2015 at 2:05 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Michael Rappazzo <rappazzo@gmail.com> writes:
>> +             strbuf_strip_suffix(&head_ref, "\n");
>> +
>> +             if (starts_with(head_ref.buf, ref_prefix)) {
>> +                     /* branch checked out */
>> +                     strbuf_remove(&head_ref, 0, strlen(ref_prefix));
>> +             /* } else {
>> +              *  headless -- no-op
>> +              */
>> +             }
>> +             printf("%s  (%s)\n", path, head_ref.buf);
>
> Is this new command meant to be a Porcelain?  This would not work as
> a plumbing that produces a machine-parseable stable output.
>
> I am not saying that it _should_; I do not know if we even need a
> 'list' command that is driven from an end-user script that gives
> anything more than "where the work trees are".
>
> My inclination is to suggest dropping the "which branch" code
> altogether and only give "path_only" behaviour.

The "which branch" was probably added in response to this [1] review,
which suggested that at some point, we might want to provide the user
with interesting information about each worktree, such as
branch/detached head, tag, locked status (plus lock reason and whether
currently accessible), prune-able status (plus reason). This could
optionally be controlled by --verbose or some other extended
formatting option.

The same review also suggested a --porcelain option for script writers.

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

Re: [PATCH v5 2/2] worktree: add 'list' command

[Mikael Magnusson]

On Sat, Aug 22, 2015 at 11:51 PM, Michael Rappazzo <rappazzo@gmail.com> wrote:
> 'git worktree list' uses the for_each_worktree function to iterate,
> and outputs in the format: '<worktree>  (<short-ref>)'
>
> Signed-off-by: Michael Rappazzo <rappazzo@gmail.com>
> ---
>  Documentation/git-worktree.txt | 11 +++++-
>  builtin/worktree.c             | 55 ++++++++++++++++++++++++++++
>  t/t2027-worktree-list.sh       | 81 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 146 insertions(+), 1 deletion(-)
>  create mode 100755 t/t2027-worktree-list.sh
>
> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
> index fb68156..e953b4e 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' [--path-only]
>
>  DESCRIPTION
>  -----------
> @@ -59,6 +60,12 @@ prune::
>
>  Prune working tree information in $GIT_DIR/worktrees.
>
> +list::
> +
> +List the main worktree followed by all of the linked worktrees.  The default
> +format of the list includes the full path to the worktree and the branch or
> +revision that the head of that worktree is currently pointing to.

Maybe just "and the branch or revision currently checked out in that worktree."?

-- 
Mikael Magnusson