Re: [PATCH] setup: do not create $X/gitdir unnecessarily when accessing git file $X

Duy Nguyen [off-list ref] writes:

The whole prune strategy is a bit messy trying to cover all cases
while still keeping out of the user's way. Perhaps if we implement
"git worktree mv", or even "worktree fixup" so the user can do it
manually (back when the prune strategy commit was implemented, there
was no git-worktree), then we don't need this magic any more.

So, which way to go?

I'd prefer to see "conservative and minimal first and carefully
build up" instead of "snapping something together quickly and having
to patch it up in rapid succession before people get hurt." and that
is not limited to the "worktree" topic.

I think "if you move, you are on your own, do not do it." would be a
good starting point.  The user education material would say: if you
create worktree B out of the primary A, and if you do not like the
location B, you "rm -fr B" and then create a new C out of the
primary A at a desired place, and do not reuse location B for any
other purpose.  The list of worktrees kept somewhere in A would then
name the old location B, and it is OK to notice the staleness and
remove it, but we do not have to.  At that point, the magic can and
should go.

After setting the user expectation that way, it is fine to design
how we would give "worktree rm" and "worktree mv".  As long as
users' initial expectation is set to "you do not mv, ever", these
can be introduced without hurting their established use pattern that
would involve no 'mv'.

The current update_linked_gitdir() has a bug that can create "gitdir"
file in non-multi-worktree setup. Instead of fixing this, we step back a
bit. The original design was probably not well thought out. For now, if
the user manually moves a worktree, they have to fix up "gitdir" file
manually or the worktree will get pruned. In future, we probably will
add "git worktree mv" to support this use case.

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 Documentation/git-worktree.txt |  6 ++----
 setup.c                        | 12 ------------
 2 files changed, 2 insertions(+), 16 deletions(-)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 5b9ad04..4814f48 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt

@@ -33,10 +33,8 @@ The working tree's administrative files in the repository (see
 clean up any stale administrative files.
 
 If you move a linked working tree 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 linked working tree
-(e.g. `git status`) in order to update its administrative files in the
-repository so that they do not get automatically pruned.
+within a file system that does not support hard links, you need to update
+$GIT_DIR/worktrees/<id>/gitdir so that they do not get automatically pruned.
 
 If a linked working tree is stored on a portable device or network share
 which is not always mounted, you can prevent its administrative files from

diff --git a/setup.c b/setup.c
index d343725..6ee2b23 100644
--- a/setup.c
+++ b/setup.c

@@ -434,17 +434,6 @@ static int check_repository_format_gently(const char *gitdir, int *nongit_ok)
 	return ret;
 }
 
-static void update_linked_gitdir(const char *gitfile, const char *gitdir)
-{
-	struct strbuf path = STRBUF_INIT;
-	struct stat st;
-
-	strbuf_addf(&path, "%s/gitdir", gitdir);
-	if (stat(path.buf, &st) || st.st_mtime + 24 * 3600 < time(NULL))
-		write_file(path.buf, "%s", gitfile);
-	strbuf_release(&path);
-}
-
 /*
  * Try to read the location of the git directory from the .git file,
  * return path to git directory if found.

@@ -514,7 +503,6 @@ const char *read_gitfile_gently(const char *path, int *return_error_code)
 		error_code = READ_GITFILE_ERR_NOT_A_REPO;
 		goto cleanup_return;
 	}
-	update_linked_gitdir(path, dir);
 	path = real_path(dir);
 
 cleanup_return:

-- 
2.3.0.rc1.137.g477eb31

On Sun, Dec 27, 2015 at 10:43:16AM +0700, Nguyễn Thái Ngọc Duy wrote:

Hmm, is this "hard links" feature implemented? If not, then this
documentation is a bit outdated.

+$GIT_DIR/worktrees/<id>/gitdir so that they do not get automatically pruned.

Following the example of af189b4 (Documentation/git-worktree: split
technical info from general description, 2015-07-06), it might be a
good idea to keep this high-level overview free of such low-level
details and instead mention $GIT_DIR/worktrees/<id>/gitdir in the
"DETAILS" section.

Perhaps something like this, on top of your patch (assuming that the
"hard links" feature is not implemented):

--- 8< ---
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 4814f48..62c76c1 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt

@@ -32,9 +32,9 @@ The working tree's administrative files in the repository (see
 `git worktree prune` in the main or any linked working tree to
 clean up any stale administrative files.
 
-If you move a linked working tree to another file system, or
-within a file system that does not support hard links, you need to update
-$GIT_DIR/worktrees/<id>/gitdir so that they do not get automatically pruned.
+If you move a linked working tree, you need to manually update the
+administrative files so that they do not get pruned automatically. See
+section "DETAILS" for more information.
 
 If a linked working tree is stored on a portable device or network share
 which is not always mounted, you can prevent its administrative files from

@@ -135,6 +135,13 @@ 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.
 
+If you move a linked working tree, you need to update the 'gitdir' file
+in the entry's directory. For example, if a linked working tree is moved
+to `/newpath/test-next` and its `.git` file points to
+`/path/main/.git/worktrees/test-next`, then update
+`/path/main/.git/worktrees/test-next/gitdir` to reference `/newpath/test-next`
+instead.
+
 To prevent a $GIT_DIR/worktrees entry from being pruned (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
--- 8< ---

On Mon, Dec 28, 2015 at 1:22 PM, Eric Sunshine [off-list ref] wrote:

On Sun, Dec 27, 2015 at 10:43:16AM +0700, Nguyễn Thái Ngọc Duy wrote:

quoted

The current update_linked_gitdir() has a bug that can create "gitdir"
file in non-multi-worktree setup. Instead of fixing this, we step back a
bit. The original design was probably not well thought out. For now, if
the user manually moves a worktree, they have to fix up "gitdir" file
manually or the worktree will get pruned. In future, we probably will
add "git worktree mv" to support this use case.

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt

@@ -33,10 +33,8 @@ The working tree's administrative files in the repository (see
 If you move a linked working tree 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 linked working tree
-(e.g. `git status`) in order to update its administrative files in the
-repository so that they do not get automatically pruned.
+within a file system that does not support hard links, you need to update

Hmm, is this "hard links" feature implemented? If not, then this
documentation is a bit outdated.

The prune logic is there. But this hard link is not created by
'worktree add'. I think calling link() was done at some point but then
it got dropped. Ah found it, it wasn't a big "no" so maybe we can
revive it at some point.

http://article.gmane.org/gmane.comp.version-control.git/243475

quoted

+$GIT_DIR/worktrees/<id>/gitdir so that they do not get automatically pruned.

Following the example of af189b4 (Documentation/git-worktree: split
technical info from general description, 2015-07-06), it might be a
good idea to keep this high-level overview free of such low-level
details and instead mention $GIT_DIR/worktrees/<id>/gitdir in the
"DETAILS" section.

Perhaps something like this, on top of your patch (assuming that the
"hard links" feature is not implemented):

Looks good.

How about something like this at the end of the last new paragraph?
"alternatively if your file system supports hard link and the worktree
and $GIT_DIR are on the same file system, you can create a hard link
named "link" back to the .git file. See gitrepository-layout.txt for
more information".

-- 
Duy

On Tue, Dec 29, 2015 at 8:55 AM, Duy Nguyen [off-list ref] wrote:

On Mon, Dec 28, 2015 at 1:22 PM, Eric Sunshine [off-list ref] wrote:

quoted

On Sun, Dec 27, 2015 at 10:43:16AM +0700, Nguyễn Thái Ngọc Duy wrote:

quoted

 If you move a linked working tree 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 linked working tree
-(e.g. `git status`) in order to update its administrative files in the
-repository so that they do not get automatically pruned.
+within a file system that does not support hard links, you need to update

Hmm, is this "hard links" feature implemented? If not, then this
documentation is a bit outdated.

The prune logic is there. But this hard link is not created by
'worktree add'. I think calling link() was done at some point but then
it got dropped. Ah found it, it wasn't a big "no" so maybe we can
revive it at some point.

http://article.gmane.org/gmane.comp.version-control.git/243475

Hmm, yes...

quoted

quoted

+$GIT_DIR/worktrees/<id>/gitdir so that they do not get automatically pruned.

Following the example of af189b4 (Documentation/git-worktree: split
technical info from general description, 2015-07-06), it might be a
good idea to keep this high-level overview free of such low-level
details and instead mention $GIT_DIR/worktrees/<id>/gitdir in the
"DETAILS" section.

Perhaps something like this, on top of your patch (assuming that the
"hard links" feature is not implemented):

Looks good.

How about something like this at the end of the last new paragraph?
"alternatively if your file system supports hard link and the worktree
and $GIT_DIR are on the same file system, you can create a hard link
named "link" back to the .git file. See gitrepository-layout.txt for
more information".

That makes sense, however...

If I understand correctly, while it's true that the 'link' file will
inhibit pruning, don't we still have the problem that "git worktree
list" will show an outdated path if the user fails to update 'gitdir'?
And doesn't the "branch already checked out in some other worktree"
interrogation also depend upon 'gitdir' being up-to-date?