On Tue, Aug 21, 2018 at 3:36 PM Jeff King <peff@xxxxxxxx> wrote: > The script does basically this to set up the temporary tree: > > test -d $tmp || git worktree add $tmp ... > > The script never cleans up the worktree (since its results can often be > reused between runs), but you may do so with "rm" or "git clean". That > creates an interesting situation if the script is run again before > "worktree prune" runs. Aside from the problems you enumerate below, leaving worktrees sitting around which the user did not create explicitly does seem a bit unfriendly, which leads me to think that worktrees may not be the best tool for this task. How about using "git clone --shared" instead? More below... > We identify the directory as a "new" worktree, > and add it to the list. So you may end up with several copies: > > $ git worktree list > [...] > /home/peff/compile/git/Documentation/tmp-doc-diff/worktree cc6237c051 (detached HEAD) > /home/peff/compile/git/Documentation/tmp-doc-diff/worktree e55de40950 (detached HEAD) > /home/peff/compile/git/Documentation/tmp-doc-diff/worktree e55de40950 (detached HEAD) > > If I then run "git worktree prune", those duplicates don't go away > (because the directory is still there; it just corresponds to only the > final entry). If I delete the tmp-doc-diff directory and then run "git > worktree prune", they do all go away. > > 1. Should the script be doing something else to indicate that the > worktree may be reused? I tried "git worktree remove", but it's > unhappy that the directory doesn't exist. Should it quietly handle > ignore that and remove any leftover cruft in $GIT_DIR/worktrees? That's a weird case. There are multiple entries in .git/worktrees/*/gitdir pointing at the same worktree directory, which I don't think was considered when the machinery was being designed. "git worktree remove" refusing to delete the worktree in this case seems a good safety measure since something is obviously askew in the bookkeeping and it doesn't want to lose potential work. The solution to this problem might be to upgrade "prune" as you describe in #3 and then ensure that that sort of aggressive pruning happens automatically at "git worktree add" time. > 2. Should "git worktree add" be more clever about realizing that an > existing entry in $GIT_DIR/worktrees points to this directory? That > would be fine for my use, but I wonder if there's some potential > for loss (e.g., you blew away the work tree but until you do a > "worktree prune", the refs are still there, objects reachable, > etc). In the case that you've already blown away the directory, then having "git worktree add" prune away the old worktree bookkeeping would make sense and wouldn't lose anything (you've already thrown it away manually). However, it could be lossy for the case when the directory is only temporarily missing (because it's on removable media or a network share). In this case, it might make sense for "git worktree add" to refuse to operate if an existing worktree entry still points at the directory that you're trying to add. That should prevent those duplicate worktree entries you saw. > 3. Should "git worktree prune" be more clever about dropping > duplicates? I think it should be easy to identify them: they are > entries in $GIT_DIR/worktrees for which: > > - the directory in $entry/gitdir does exist, but > - $(cat $entry/gitdir)/.git does not point back to $entry Seems a sensible improvement to the pruning logic. However, upon further consideration, any of the proposed "fixes" could potentially be lossy. Consider a case like this: % git worktree add foo ... make some changes in 'foo' ... % mv foo bar # (fogetting to do "git worktree move foo bar") % git worktree add foo As currently implemented, one can "correct" the situation by manually fixing the bookkeeping file in .git/worktrees for the worktree created first. If it gets pruned automatically, then the state of those changes in "bar" (nee "foo") could be lost. So, I'm not sure what, if any, fix is appropriate. Such uncertainty also further argues in favor of "git clone --shared" for your particular use-case, I think.
