git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 0/7] nd/worktree-move reboot

From
Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Date
Feb 12, 2018, 09:49 UTC
Message-ID
<20180212094940.23834-1-pclouds@gmail.com>
In-Reply-To
<20180124095357.19645-1-pclouds@gmail.com>

v2 basically fixes lots of comments from Eric (many thanks!): memory leak, typos, document updates, tests, corner case fixes.

Interdiff:
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 6f83723d9a..d322acbc67 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -36,10 +36,6 @@ 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, 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
 being pruned by issuing the `git worktree lock` command, optionally
@@ -211,7 +207,7 @@ 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
+If you manually 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
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 8ce86aef0e..f77ef994c4 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -653,21 +653,19 @@ static int move_worktree(int ac, const char **av, const char *prefix)
 		die(_("'%s' is not a working tree"), av[0]);
 	if (is_main_worktree(wt))
 		die(_("'%s' is a main working tree"), av[0]);
-
-	validate_no_submodules(wt);
-
 	if (is_directory(dst.buf)) {
 		const char *sep = find_last_dir_sep(wt->path);
 
 		if (!sep)
 			die(_("could not figure out destination name from '%s'"),
 			    wt->path);
+		strbuf_trim_trailing_dir_sep(&dst);
 		strbuf_addstr(&dst, sep);
-		if (file_exists(dst.buf))
-			die(_("target '%s' already exists"), dst.buf);
-	} else if (file_exists(dst.buf)) {
-		die(_("target '%s' already exists"), av[1]);
 	}
+	if (file_exists(dst.buf))
+		die(_("target '%s' already exists"), dst.buf);
+
+	validate_no_submodules(wt);
 
 	reason = is_worktree_locked(wt);
 	if (reason) {
@@ -677,7 +675,7 @@ static int move_worktree(int ac, const char **av, const char *prefix)
 		die(_("cannot move a locked working tree"));
 	}
 	if (validate_worktree(wt, &errmsg, 0))
-		die(_("validation failed, cannot move working tree:\n%s"),
+		die(_("validation failed, cannot move working tree: %s"),
 		    errmsg.buf);
 	strbuf_release(&errmsg);
 
@@ -686,6 +684,8 @@ static int move_worktree(int ac, const char **av, const char *prefix)
 
 	update_worktree_location(wt, dst.buf);
 
+	strbuf_release(&dst);
+	free_worktrees(worktrees);
 	return 0;
 }
 
@@ -708,6 +708,10 @@ static void check_clean_worktree(struct worktree *wt,
 	char buf[1];
 	int ret;
 
+	/*
+	 * Until we sort this out, all submodules are "dirty" and
+	 * will abort this function.
+	 */
 	validate_no_submodules(wt);
 
 	argv_array_pushf(&child_env, "%s=%s/.git",
@@ -724,7 +728,7 @@ static void check_clean_worktree(struct worktree *wt,
 	cp.out = -1;
 	ret = start_command(&cp);
 	if (ret)
-		die_errno(_("failed to run git-status on '%s'"),
+		die_errno(_("failed to run 'git status' on '%s'"),
 			  original_path);
 	ret = xread(cp.out, buf, sizeof(buf));
 	if (ret)
@@ -733,7 +737,7 @@ static void check_clean_worktree(struct worktree *wt,
 	close(cp.out);
 	ret = finish_command(&cp);
 	if (ret)
-		die_errno(_("failed to run git-status on '%s', code %d"),
+		die_errno(_("failed to run 'git status' on '%s', code %d"),
 			  original_path, ret);
 }
 
@@ -748,7 +752,6 @@ static int delete_git_work_tree(struct worktree *wt)
 		ret = -1;
 	}
 	strbuf_release(&sb);
-
 	return ret;
 }
 
@@ -797,7 +800,7 @@ static int remove_worktree(int ac, const char **av, const char *prefix)
 		die(_("cannot remove a locked working tree"));
 	}
 	if (validate_worktree(wt, &errmsg, WT_VALIDATE_WORKTREE_MISSING_OK))
-		die(_("validation failed, cannot remove working tree:\n%s"),
+		die(_("validation failed, cannot remove working tree: %s"),
 		    errmsg.buf);
 	strbuf_release(&errmsg);
 
diff --git a/strbuf.c b/strbuf.c
index 1df674e919..46930ad027 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -95,6 +95,7 @@ void strbuf_trim(struct strbuf *sb)
 	strbuf_rtrim(sb);
 	strbuf_ltrim(sb);
 }
+
 void strbuf_rtrim(struct strbuf *sb)
 {
 	while (sb->len > 0 && isspace((unsigned char)sb->buf[sb->len - 1]))
@@ -102,6 +103,13 @@ void strbuf_rtrim(struct strbuf *sb)
 	sb->buf[sb->len] = '\0';
 }
 
+void strbuf_trim_trailing_dir_sep(struct strbuf *sb)
+{
+	while (sb->len > 0 && is_dir_sep((unsigned char)sb->buf[sb->len - 1]))
+		sb->len--;
+	sb->buf[sb->len] = '\0';
+}
+
 void strbuf_ltrim(struct strbuf *sb)
 {
 	char *b = sb->buf;
diff --git a/strbuf.h b/strbuf.h
index 14c8c10d66..e6cae5f439 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -179,6 +179,9 @@ extern void strbuf_trim(struct strbuf *);
 extern void strbuf_rtrim(struct strbuf *);
 extern void strbuf_ltrim(struct strbuf *);
 
+/* Strip trailing directory separators */
+extern void strbuf_trim_trailing_dir_sep(struct strbuf *);
+
 /**
  * Replace the contents of the strbuf with a reencoded form.  Returns -1
  * on error, 0 on success.
diff --git a/t/t2028-worktree-move.sh b/t/t2028-worktree-move.sh
index 459f676683..082368d8c6 100755
--- a/t/t2028-worktree-move.sh
+++ b/t/t2028-worktree-move.sh
@@ -66,21 +66,16 @@ test_expect_success 'move non-worktree' '
 
 test_expect_success 'move locked worktree' '
 	git worktree lock source &&
-	test_must_fail git worktree move source destination &&
-	git worktree unlock source
+	test_when_finished "git worktree unlock source" &&
+	test_must_fail git worktree move source destination
 '
 
 test_expect_success 'move worktree' '
 	toplevel="$(pwd)" &&
 	git worktree move source destination &&
 	test_path_is_missing source &&
-	git worktree list --porcelain | grep "^worktree" >actual &&
-	cat <<-EOF >expected &&
-	worktree $toplevel
-	worktree $toplevel/destination
-	worktree $toplevel/elsewhere
-	EOF
-	test_cmp expected actual &&
+	git worktree list --porcelain | grep "^worktree.*/destination" &&
+	! git worktree list --porcelain | grep "^worktree.*/source" >empty &&
 	git -C destination log --format=%s >actual2 &&
 	echo init >expected2 &&
 	test_cmp expected2 actual2
@@ -90,23 +85,38 @@ test_expect_success 'move main worktree' '
 	test_must_fail git worktree move . def
 '
 
+test_expect_success 'move worktree to another dir' '
+	toplevel="$(pwd)" &&
+	mkdir some-dir &&
+	git worktree move destination some-dir &&
+	test_path_is_missing source &&
+	git worktree list --porcelain | grep "^worktree.*/some-dir/destination" &&
+	git -C some-dir/destination log --format=%s >actual2 &&
+	echo init >expected2 &&
+	test_cmp expected2 actual2
+'
+
 test_expect_success 'remove main worktree' '
 	test_must_fail git worktree remove .
 '
 
+test_expect_success 'move some-dir/destination back' '
+	git worktree move some-dir/destination destination
+'
+
 test_expect_success 'remove locked worktree' '
 	git worktree lock destination &&
-	test_must_fail git worktree remove destination &&
-	git worktree unlock destination
+	test_when_finished "git worktree unlock destination" &&
+	test_must_fail git worktree remove destination
 '
 
 test_expect_success 'remove worktree with dirty tracked file' '
 	echo dirty >>destination/init.t &&
+	test_when_finished "git -C destination checkout init.t" &&
 	test_must_fail git worktree remove destination
 '
 
 test_expect_success 'remove worktree with untracked file' '
-	git -C destination checkout init.t &&
 	: >destination/untracked &&
 	test_must_fail git worktree remove destination
 '
@@ -124,4 +134,13 @@ test_expect_success 'remove missing worktree' '
 	test_path_is_missing .git/worktrees/to-be-gone
 '
 
+test_expect_success 'NOT remove missing-but-locked worktree' '
+	git worktree add gone-but-locked &&
+	git worktree lock gone-but-locked &&
+	test -d .git/worktrees/gone-but-locked &&
+	mv gone-but-locked really-gone-now &&
+	test_must_fail git worktree remove gone-but-locked &&
+	test_path_is_dir .git/worktrees/gone-but-locked
+'
+
 test_done
diff --git a/worktree.c b/worktree.c
index 542196f0ad..28989cf06e 100644
--- a/worktree.c
+++ b/worktree.c
@@ -340,11 +340,10 @@ void update_worktree_location(struct worktree *wt, const char *path_)
 	if (is_main_worktree(wt))
 		die("BUG: can't relocate main worktree");
 
-	strbuf_add_absolute_path(&path, path_);
+	strbuf_realpath(&path, path_, 1);
 	if (fspathcmp(wt->path, path.buf)) {
-		write_file(git_common_path("worktrees/%s/gitdir",
-					   wt->id),
-			   "%s/.git", real_path(path.buf));
+		write_file(git_common_path("worktrees/%s/gitdir", wt->id),
+			   "%s/.git", path.buf);
 		free(wt->path);
 		wt->path = strbuf_detach(&path, NULL);
 	}
Previous: Junio C HamanoNext: Nguyễn Thái Ngọc Duy
Message 36 of 49 in “nd/worktree-move reboot”
  1. 0/7 nd/worktree-move rebootNguyễn Thái Ngọc Duy, Jan 24, 2018
  2. 1/7 worktree.c: add validate_worktree()Nguyễn Thái Ngọc Duy, Jan 24, 2018
  3. 2/7 worktree.c: add update_worktree_location()Nguyễn Thái Ngọc Duy, Jan 24, 2018
  4. Eric SunshineFeb 2, 2018
  5. Duy NguyenFeb 2, 2018
  6. 3/7 worktree move: new commandNguyễn Thái Ngọc Duy, Jan 24, 2018
  7. Eric SunshineFeb 2, 2018
  8. Eric SunshineFeb 2, 2018
  9. Duy NguyenFeb 5, 2018
  10. Jeff KingFeb 6, 2018
  11. Martin ÅgrenFeb 6, 2018
  12. Duy NguyenFeb 12, 2018
  13. Martin ÅgrenFeb 12, 2018
  14. Duy NguyenFeb 13, 2018
  15. Jeff KingFeb 14, 2018
  16. Duy NguyenFeb 14, 2018
  17. Junio C HamanoFeb 14, 2018
  18. t/known-leaky: add list of known-leaky test scriptsMartin Ågren, Feb 14, 2018
  19. Jeff KingFeb 19, 2018
  20. Martin ÅgrenFeb 20, 2018
  21. Jeff KingFeb 20, 2018
  22. Junio C HamanoFeb 21, 2018
  23. Jeff KingFeb 21, 2018
  24. Kaartic SivaraamFeb 25, 2018
  25. Martin ÅgrenFeb 26, 2018
  26. 4/7 worktree move: accept destination as directoryNguyễn Thái Ngọc Duy, Jan 24, 2018
  27. Eric SunshineFeb 2, 2018
  28. 5/7 worktree move: refuse to move worktrees with submodulesNguyễn Thái Ngọc Duy, Jan 24, 2018
  29. Eric SunshineFeb 2, 2018
  30. 6/7 worktree remove: new commandNguyễn Thái Ngọc Duy, Jan 24, 2018
  31. Eric SunshineFeb 2, 2018
  32. Duy NguyenFeb 12, 2018
  33. 7/7 worktree remove: allow it when $GIT_WORK_TREE is already goneNguyễn Thái Ngọc Duy, Jan 24, 2018
  34. Eric SunshineFeb 2, 2018
  35. Junio C HamanoJan 24, 2018
  36. 0/7 nd/worktree-move rebootNguyễn Thái Ngọc Duy, Feb 12, 2018
  37. 1/7 worktree.c: add validate_worktree()Nguyễn Thái Ngọc Duy, Feb 12, 2018
  38. 2/7 worktree.c: add update_worktree_location()Nguyễn Thái Ngọc Duy, Feb 12, 2018
  39. 4/7 worktree move: accept destination as directoryNguyễn Thái Ngọc Duy, Feb 12, 2018
  40. 5/7 worktree move: refuse to move worktrees with submodulesNguyễn Thái Ngọc Duy, Feb 12, 2018
  41. 6/7 worktree remove: new commandNguyễn Thái Ngọc Duy, Feb 12, 2018
  42. 7/7 worktree remove: allow it when $GIT_WORK_TREE is already goneNguyễn Thái Ngọc Duy, Feb 12, 2018
  43. 3/7 worktree move: new commandNguyễn Thái Ngọc Duy, Feb 12, 2018
  44. t2028: fix minor error and issues in newly-added "worktree move" testsEric Sunshine, Mar 4, 2018
  45. Duy NguyenMar 5, 2018
  46. SZEDER GáborMar 5, 2018
  47. Junio C HamanoMar 5, 2018
  48. Eric SunshineMar 5, 2018
  49. Eric SunshineMar 4, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.