v3worktree: add 'list' command

Differences from [v2](http://www.mail-archive.com/git@vger.kernel.org/msg75467.html)
   - removed unintended whitespace changes
   - cleanup based on comments from v2

Michael Rappazzo (1):
  worktree: add 'list' command

 Documentation/git-worktree.txt |  6 +++-
 builtin/worktree.c             | 67 ++++++++++++++++++++++++++++++++++++++++++
 t/t2027-worktree-list.sh       | 30 +++++++++++++++++++
 3 files changed, 102 insertions(+), 1 deletion(-)
 create mode 100755 t/t2027-worktree-list.sh

'git worktree list' will list the main worktree followed by any linked worktrees which were created using 'git worktree add'.

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

Michael Rappazzo <rappazzo@gmail.com> writes:

Hmm, main-only is still there?

Asterisks stick to the variable names, not types.

Please do not introduce decl-after-stmt.

Hmm, interesting.

Because .git/config is shared, core.bare read from that tells us if the "main" one is bare, even if you start this command from one of its linked worktrees. So in that sense, this test of is_bare correctly tells if "main" one is a bare repository.

But that by itself feels wrong. Doesn't the presense of a working tree mean that you should not get "is_bare==true" in such a case (i.e. your "main" one is bare, you are in a linked worktree of it that has the index and the working tree)?

Duy?  Eric?  What do you guys think?

There are many codepaths that change their behaviour (e.g. if we create reflogs by default) based on the return value of is_bare_repository(). If I am reading this correctly, I _think_ a new working area that was prepared with "git worktree add" out of a bare repository would not work well, as these operations behave as if we do not have a working tree. Perhaps is_bare_repository() in such a working area _should_ say "No", even if core.bare in the shared bare one is set to true.

In any case, what is that stripping of "/." about? Who is adding that extra trailing string?

What I am getting at is (1) perhaps it shouldn't be adding that in the first place, and (2) if some other code is randomly adding "/." at the end, what guarantees you that you would need to strip it only once here---if the other code added that twice, don't you have to repeatedly remove "/." from the end?

On Mon, Aug 10, 2015 at 6:10 PM, Junio C Hamano <gitster@pobox.com> wrote:

Sorry, I missed that.

Since I reused this value below, I thought it would be acceptable.

Is is even correct for a bare repo to be included in the list? I think that is part of what you are asking here.

In the case of a common dir, the value returned is just '.'. I wanted to resolve that to the full path, so I called `absolute_path(common_dir)`. Hence the trailing "/.". Similarly, in the main repo, the common dir is just ".git", and I handled that by using `get_git_work_tree()`.

Mike Rappazzo <rappazzo@gmail.com> writes:

Use of a new variable is fine. "Do not declare one in a block after you already wrote statement" is what "decl-after-stmt not allowed" means. In your patch:

+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); + + struct strbuf main_path = STRBUF_INIT; + const char* common_dir = get_git_common_dir(); + int is_bare = is_bare_repository();

Three variables, main_path, common_dir and is_bare are declared here after statements such as a call to parse_options(). Don't.

+static int list(int ac, const char **av, const char *prefix) +{ + int main_only = 0; + struct strbuf main_path = STRBUF_INIT; + const char *common_dir; + int is_bare; + 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); + + common_dir = get_git_common_dir(); + int is_bare = is_bare_repository();

On Mon, 2015-08-10 at 16:53 -0400, Michael Rappazzo wrote:

I think it would be useful to break this loop out into a for_each_worktree function.

While looking into per-worktree ref stuff, I have just noticed that git prune will delete objects that are only referenced in a different worktree's detached HEAD. To fix this, git prune will need to walk over each worktree, looking at that worktree's HEAD (and other per-worktree refs). It would be useful to be able to reuse some of this code for that task.

On Mon, Aug 10, 2015 at 10:55 PM, David Turner <dturner@twopensource.com> wrote:

I agree, but I will save that for another round.

David Turner <dturner@twopensource.com> writes:

Very good point.