Re: [PATCH] worktree: new place for "git prune --worktrees"

[Duy Nguyen <pclouds@gmail.com>]

On Mon, Jun 29, 2015 at 11:13 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> Commit 23af91d (prune: strategies for linked checkouts - 2014-11-30)
>> adds "--worktrees" to "git prune" without realizing that "git prune" is
>> for object database only. This patch moves the same functionality to a
>> new command "git worktree".
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> ---
>>  In future I probably move the big block of text in git-checkout.txt to
>>  git-worktree.txt and add "git worktree list". But let's start with
>>  something small and simple before "git prune --worktrees" is shipped
>>  out.
>
> Thanks.  I notice that after applying this, builtin/prune.c does not
> revert to the original before 23af91d.  It stops including "dir.h"
> and it adds an extra blank line.  We've been including a header that
> is not necessary even in v2.4.0, it seems.  We used to walk
> $GIT_DIR/objects ourselves to find loose object files and "dir.h"
> was needed for is_dot_or_dotdot(); for_each_loose_file_in_objdir()
> is what we use these days to hide the implementation details these
> days; 27e1e22 forgot to remove the inclusion when it did this.
>
> The C code part is mostly just \C-x \C-v and I found nothing
> questionable.

I was about to prepare v2, but the only change in the end was removing
this blank line, not worth another round.

>> diff --git a/command-list.txt b/command-list.txt
>> index b17c011..2a94137 100644
>> --- a/command-list.txt
>> +++ b/command-list.txt
>> @@ -148,4 +148,5 @@ git-verify-pack                         plumbinginterrogators
>>  git-verify-tag                          ancillaryinterrogators
>>  gitweb                                  ancillaryinterrogators
>>  git-whatchanged                         ancillaryinterrogators
>> +git-worktree                            mainporcelain
>
> I doubt that a helper that is primarily spawned from "gc" as its
> implementation detail is more mainporcelain than "git config" is:
>
>    git-config                              ancillarymanipulators
>
> I also wonder if "git worktree" command should have a mode that
> works in a way similar to how new-workdir (in contrib/workdir) does,
> instead of an option "checkout --to" that looks just out of place as
> "worktree prune" was out of place in "prune".  The feature is doing
> a lot more than what "checkout" normally does (somewhere in between
> "checkout" and "clone", I would say), and it may be cleaner to use
> an independent command "git worktree" to manage a separate worktree.
>
> And when that happens, the command should definitely be classified
> as a mainporcelain.

And because "git worktree add" is coming, I guess it's ok to stick to
mainporcelain here..

>> diff --git a/git.c b/git.c
>> index 44374b1..fa77bc9 100644
>> --- a/git.c
>> +++ b/git.c
>> @@ -483,6 +483,7 @@ static struct cmd_struct commands[] = {
>>       { "verify-tag", cmd_verify_tag, RUN_SETUP },
>>       { "version", cmd_version },
>>       { "whatchanged", cmd_whatchanged, RUN_SETUP },
>> +     { "worktree", cmd_worktree, RUN_SETUP },
>
> We do not NEED_WORK_TREE because we can create a new worktree
> off of a bare repository?

Yes.
-- 
Duy