[PATCH 0/2 v4] worktree: for_each_worktree and list

[PATCH 0/2 v4] worktree: for_each_worktree and list

[Michael Rappazzo]

This patch series adds a for_each_worktree function and then uses it to implement 
the `git worktree list` command.

Differences from [v3](http://www.mail-archive.com/git@vger.kernel.org/msg75599.html)
  - create the for_each_worktree function
  - uses the function in the list
  - a bare repo is NOT included in the iterated worktrees


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

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

-- 
2.5.0

[PATCH 1/2 v4] 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 | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 84 insertions(+)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 430b51e..a43e360 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;
@@ -358,6 +366,82 @@ 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 main_path = STRBUF_INIT;
+	struct dirent *d;
+	struct stat st;
+	struct strbuf worktree_path = STRBUF_INIT;
+	struct strbuf worktree_git = STRBUF_INIT;
+	const char *common_dir;
+	DIR *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(&main_path, get_git_work_tree());
+		} else {
+			strbuf_addstr(&main_path, common_dir);
+		}
+	} else {
+		strbuf_addstr(&main_path, common_dir);
+		strbuf_strip_suffix(&main_path, "/.git");
+		/* If the common_dir DOES NOT end with '/.git', then it is bare */
+		main_is_bare = !strcmp(common_dir, main_path.buf);
+	}
+
+	if (!main_is_bare) {
+		strbuf_addstr(&worktree_path, main_path.buf);
+
+		strbuf_addstr(&main_path, "/.git");
+		strbuf_addstr(&worktree_git, main_path.buf);
+
+		ret = fn(worktree_path.buf, worktree_git.buf, cb_data);
+		if (ret)
+			goto done;
+	}
+	strbuf_addstr(&main_path, "/worktrees");
+
+	if (is_directory(main_path.buf)) {
+		dir = opendir(main_path.buf);
+		if (dir) {
+			while ((d = readdir(dir)) != NULL) {
+				if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
+					continue;
+
+				strbuf_reset(&worktree_git);
+				strbuf_addf(&worktree_git, "%s/%s/gitdir", main_path.buf, d->d_name);
+
+				if (stat(worktree_git.buf, &st))
+					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);
+	strbuf_release(&main_path);
+	return ret;
+}
 
 int cmd_worktree(int ac, const char **av, const char *prefix)
 {
-- 
2.5.0

[PATCH 2/2 v4] 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       | 51 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 116 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 a43e360..b39ecbd 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
 };
 
@@ -443,6 +444,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[] = {
@@ -455,5 +508,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..220f98e
--- /dev/null
+++ b/t/t2027-worktree-list.sh
@@ -0,0 +1,51 @@
+#!/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_done
-- 
2.5.0

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

[David Turner]

On Thu, 2015-08-13 at 14:32 -0400, Michael Rappazzo wrote:
> 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`.

Thanks!  This will be super-useful!  I particularly appreciate the
detailed function documentation.

More comments below.

> 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 | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 84 insertions(+)
> 
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> index 430b51e..a43e360 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;
> @@ -358,6 +366,82 @@ static int add(int ac, const char **av, const char *prefix)
>  
>  	return add_worktree(path, branch, &opts);
>  }
> +/*

nit: newline above this line

> + * 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 main_path = STRBUF_INIT;
> +	struct dirent *d;


The scope of d can be smaller; move it down to the place I've marked
below

> +	struct stat st;
> +	struct strbuf worktree_path = STRBUF_INIT;
> +	struct strbuf worktree_git = STRBUF_INIT;
> +	const char *common_dir;
> +	DIR *dir;

ditto

> +	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(&main_path, get_git_work_tree());
> +		} else {
> +			strbuf_addstr(&main_path, common_dir);
> +		}
> +	} else {
> +		strbuf_addstr(&main_path, common_dir);
> +		strbuf_strip_suffix(&main_path, "/.git");
> +		/* If the common_dir DOES NOT end with '/.git', then it is bare */
> +		main_is_bare = !strcmp(common_dir, main_path.buf);

strbuf_strip_suffix returns 1 if the suffix was stripped and 0
otherwise, so there is no need for this strcmp.

I'm a little worried about this path manipulation; it looks like the
config setting core.bare is the actual thing to check?  (Along with
maybe the GIT_WORK_TREE environment var; not sure how that interacts
with worktrees).

> +	}
> +
> +	if (!main_is_bare) {
> +		strbuf_addstr(&worktree_path, main_path.buf);
> +
> +		strbuf_addstr(&main_path, "/.git");
> +		strbuf_addstr(&worktree_git, main_path.buf);
> +
> +		ret = fn(worktree_path.buf, worktree_git.buf, cb_data);
> +		if (ret)
> +			goto done;
> +	}
> +	strbuf_addstr(&main_path, "/worktrees");

Earlier, you said:
		strbuf_addstr(&main_path, common_dir);
		strbuf_strip_suffix(&main_path, "/.git");

Now you are adding /worktrees.  Doesn't this mean, in the non-bare case,
that you'll be looking in $common_dir/worktrees instead of
$common_dir/.git/worktrees ?

> +	if (is_directory(main_path.buf)) {

declare dir here...

> +		dir = opendir(main_path.buf);
> +		if (dir) {

... and d here.

> +			while ((d = readdir(dir)) != NULL) {
> +				if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
> +					continue;
> +
> +				strbuf_reset(&worktree_git);
> +				strbuf_addf(&worktree_git, "%s/%s/gitdir", main_path.buf, d->d_name);

tioli: main_path never changes, so no need to keep chopping it off and
adding it back on; just truncate worktree_git to main_path.len + 1 here
and then add d->name.

> +				if (stat(worktree_git.buf, &st))
> +					continue;

Do we want to report errors other than ENOENT? (genuine question)

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

[David Turner]

On Thu, 2015-08-13 at 14:32 -0400, Michael Rappazzo wrote:
> 'git worktree list' uses the for_each_worktree function to iterate,
> and outputs in the format: '<worktree>  (<short-ref>)'

I'm not sure I'm going to have time to review the whole thing, but I
think we ought to have tests with both bare and non-bare main repos.

> Signed-off-by: Michael Rappazzo <rappazzo@gmail.com>
> ---
>  Documentation/git-worktree.txt | 11 ++++++++-
>  builtin/worktree.c             | 55 ++++++++++++++++++++++++++++++++++++++++++
>  t/t2027-worktree-list.sh       | 51 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 116 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 a43e360..b39ecbd 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
>  };
>  
> @@ -443,6 +444,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[] = {
> @@ -455,5 +508,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..220f98e
> --- /dev/null
> +++ b/t/t2027-worktree-list.sh
> @@ -0,0 +1,51 @@
> +#!/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_done

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

[Mike Rappazzo]

I will reroll this series with adjustments as you suggested, and I
will add the extra tests for bare repos.

On Thu, Aug 13, 2015 at 3:23 PM, David Turner <dturner@twopensource.com> wrote:
> The scope of d can be smaller; move it down to the place I've marked
> below

I have adjusted the scoping.  I misunderstood the meaning of some
comments from the v3 patch, but your statements have helped me
understand.


>
> strbuf_strip_suffix returns 1 if the suffix was stripped and 0
> otherwise, so there is no need for this strcmp.

Done.

>
> I'm a little worried about this path manipulation; it looks like the
> config setting core.bare is the actual thing to check?  (Along with
> maybe the GIT_WORK_TREE environment var; not sure how that interacts
> with worktrees).

As Junio pointed out in a previous version of this patch, the
core.bare will always be 'true' since they share the config.  He also
suggested that this could be the cause for concern.  Therefore, I can
use core.bare to check if the main is a bare repo.  I guess that with
the inclusion of tests using a bare repo, that will catch it if things
change in the future.

>
>> +     }
>> +
>> +     if (!main_is_bare) {
>> +             strbuf_addstr(&worktree_path, main_path.buf);
>> +
>> +             strbuf_addstr(&main_path, "/.git");
>> +             strbuf_addstr(&worktree_git, main_path.buf);
>> +
>> +             ret = fn(worktree_path.buf, worktree_git.buf, cb_data);
>> +             if (ret)
>> +                     goto done;
>> +     }
>> +     strbuf_addstr(&main_path, "/worktrees");
>
> Earlier, you said:
>                 strbuf_addstr(&main_path, common_dir);
>                 strbuf_strip_suffix(&main_path, "/.git");
>
> Now you are adding /worktrees.  Doesn't this mean, in the non-bare case,
> that you'll be looking in $common_dir/worktrees instead of
> $common_dir/.git/worktrees ?

I read the gitdir file from the common dir.

>> +                     while ((d = readdir(dir)) != NULL) {
>> +                             if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
>> +                                     continue;
>> +
>> +                             strbuf_reset(&worktree_git);
>> +                             strbuf_addf(&worktree_git, "%s/%s/gitdir", main_path.buf, d->d_name);
>
> tioli: main_path never changes, so no need to keep chopping it off and
> adding it back on; just truncate worktree_git to main_path.len + 1 here
> and then add d->name.

I will try to clean it up a bit.  I am not very experienced with C,
but I will do my best.