Re: [PATCH v2 23/32] prune: strategies for linked checkouts

[Marc Branchaud <marcnarc@xiplink.com>]

On 14-09-11 11:06 PM, Eric Sunshine wrote:
> On Thu, Sep 11, 2014 at 11:36 AM, Marc Branchaud <marcnarc@xiplink.com> wrote:
>> On 14-09-10 06:41 PM, Nguyễn Thái Ngọc Duy wrote:
>>> (alias R=$GIT_COMMON_DIR/worktrees/<id>)
>>>
>>>  - linked checkouts are supposed to keep its location in $R/gitdir up
>>>    to date. The use case is auto fixup after a manual checkout move.
>>>
>>>  - linked checkouts are supposed to update mtime of $R/gitdir. If
>>>    $R/gitdir's mtime is older than a limit, and it points to nowhere,
>>>    worktrees/<id> is to be pruned.
>>>
>>>  - If $R/locked exists, worktrees/<id> is not supposed to be pruned. If
>>>    $R/locked exists and $R/gitdir's mtime is older than a really long
>>>    limit, warn about old unused repo.
>>>
>>>  - "git checkout --to" is supposed to make a hard link named $R/link
>>>    pointing to the .git file on supported file systems to help detect
>>>    the user manually deleting the checkout. If $R/link exists and its
>>>    link count is greated than 1, the repo is kept.
>>>
>>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>>> ---
>>>  Documentation/git-checkout.txt             | 18 ++++++
>>>  Documentation/git-prune.txt                |  3 +
>>>  Documentation/gitrepository-layout.txt     | 19 ++++++
>>>  builtin/checkout.c                         | 19 +++++-
>>>  builtin/prune.c                            | 95 ++++++++++++++++++++++++++++++
>>>  setup.c                                    | 13 ++++
>>>  t/t2026-prune-linked-checkouts.sh (new +x) | 84 ++++++++++++++++++++++++++
>>>  7 files changed, 249 insertions(+), 2 deletions(-)
>>>  create mode 100755 t/t2026-prune-linked-checkouts.sh
>>>
>>> diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
>>> index bd0fc1d..a29748e 100644
>>> --- a/Documentation/git-checkout.txt
>>> +++ b/Documentation/git-checkout.txt
>>> @@ -431,6 +431,24 @@ thumb is do not make any assumption about whether a path belongs to
>>>  $GIT_DIR or $GIT_COMMON_DIR when you need to directly access something
>>>  inside $GIT_DIR. Use `git rev-parse --git-path` to get the final path.
>>>
>>> +When you are done, simply deleting the linked working directory would
>>> +suffice. $GIT_DIR/worktrees can be cleaned up using `git prune
>>> +--worktrees`.
>>
>> This needs a tweak or two so that it follows more naturally from the previous
>> paragraph.  How about:
>>
>>         When you are done with a linked working tree you can simply delete
>>         it.  You can clean up any stale $GIT_DIR/worktrees entries with
>>         `git prune --worktrees`.
> 
> Thanks for these rewrites; I was going to provide similar suggestions.
> 
> One minor addition for clarification would be to mention that the 'git
> prune --worktrees' invocation applies to the main worktree:
> 
>     When you are done with a linked working tree, you can simply delete
>     it. You can clean up any stale $GIT_DIR/worktrees entries via
>     `git prune --worktrees` in the main worktree.
> 
>> Then in commit 28, when you add worktrees pruning to gc, you should change
>> this paragraph again:
>>
>>         When you are done with a linked working tree you can simply delete
>>         it.  The working tree's entry in the repository's $GIT_DIR/worktrees
>>         directory will eventually be removed automatically (see
>>         `gc.pruneworktreesexpire` in linkgit::git-config[1]), or you can run
>>         `git prune --worktrees` to clean up any stale entries in
>>         $GIT_DIR/worktrees.
> 
> Ditto about qualifying 'git prune --worktrees' with "in the main work tree".
> 
>>> +After you move a linked working directory to another file system, or
>>> +on a file system that does not support hard link, execute any git
>>> +command (e.g. `git status`) in the new working directory so that it
>>> +could update its location in $GIT_DIR/worktrees and not be
>>> +accidentally pruned.
>>
>> It took me a couple of seconds to parse that.  How about:
>>
>>         If you move a linked working directory to another file system, or
>>         within a file system that does not support hard links, you need to
>>         run at least one git command inside the moved linked working
> 
> I trip over "moved linked" every time I read it. I think there's
> sufficient context in the 'if' clause leading to this that "moved" can
> be dropped.

Fine by me, as are your other suggestions.

		M.


>>         directory (e.g. `git status`) in order to update its entry in
>>         $GIT_DIR/worktrees so that it does not get automatically removed.
>>
>>> +To stop `git prune --worktrees` from deleting a specific working
>>> +directory (e.g. because it's on a portable device), you could add the
>>> +file 'locked' to $GIT_DIR/worktrees. For example, if `.git` file of
>>> +the new working directory points to `/path/main/worktrees/test-next`,
>>> +the full path of the 'locked' file would be
>>> +`/path/main/worktrees/test-next/locked`. See
>>> +linkgit:gitrepository-layout[5] for details.
>>
>> Sorry, I can't help rewriting this one too:
>>
>>         To prevent `git prune --worktrees` from deleting a
>>         $GIT_DIR/worktrees entry (which can be useful in some situations,
>>         such as when the entry's working tree is stored on a portable
>>         device), add a file named 'locked' to the entry's directory.  For
>>         example, if a linked working tree's `.git` file points to
>>         `/path/main/.git/worktrees/test-next` then a file named
>>         `/path/main/.git/worktrees/test-next/locked` will prevent the
>>         `test-next` entry from being pruned.  See
>>         linkgit:gitrepository-layout[5] for details.
> 
> Each time I read this (or Duy's original), the first question that
> pops into my head is "should 'locked' be empty or have content, and if
> content, what content?" gitrepository-layout.txt does explain the
> content (if the reader bothers to chase the link), but perhaps it is
> worth an explanatory sentence here?
> 
>> I also suggest this paragraph get updated in commit 28, but just change the
>> first clause, before "(which can be ...":
>>
>>         To prevent a $GIT_DIR/worktrees entry from being pruned (which ...
>>
>> Thanks for all this work!
>>
>>                 M.
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>