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

Re: [PATCH 3/3] setup: always honor GIT_WORK_TREE and core.worktree

From
Junio C Hamano <gitster@pobox.com>
Date
Jan 21, 2011, 22:02 UTC
Message-ID
<7vtyh1oqy8.fsf@alter.siamese.dyndns.org>
In-Reply-To
<7v39omotxg.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
Show 9 quoted lines
> I was re-reading this thread, and changed my mind; I think we should have
> this series to avoid unnecessary regression, with or without clarifying
> (5), before 1.7.4 final.
>
> Even if some scripts you had trouble with started using GIT_WORK_TREE
> without specifying GIT_DIR because they misunderstood what these are
> designed to do, as long as the combination has been working consistently
> with the expectation of these scripts, ans as long as we can keep the same
> behaviour, I don't see a reason to change it.

... and that leads me to suggest that we may not even want to issue a warning in these cases.

Perhaps squash this into, or apply on top of, your 3/3?
-- >8 --
Subject: setup: officially support --work-tree without --git-dir

The original intention of --work-tree was to allow people to work in a subdirectory of their working tree that does not have an embedded .git directory. Because their working tree, which their $cwd was in, did not have an embedded .git, they needed to use $GIT_DIR to specify where it is, and because this meant there was no way to discover where the root level of the working tree was, so we needed to add $GIT_WORK_TREE to tell git where it was.

However, this facility has long been (mis)used by people's scripts to start git from a working tree _with_ an embedded .git directory, let git find .git directory, and then pretend as if an unrelated directory were the associated working tree of the .git directory found by the discovery process. It happens to work in simple cases, and is not worth causing "regression" to these scripts.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 setup.c               |    8 +-----
 t/t1510-repo-setup.sh |   59 +++++++++++++++++++++++++-----------------------
 2 files changed, 33 insertions(+), 34 deletions(-)
diff --git a/setup.c b/setup.c
index e08cdf2..dadc666 100644
--- a/setup.c
+++ b/setup.c
@@ -411,9 +411,8 @@ static const char *setup_discovered_git_dir(const char *gitdir,
 	if (check_repository_format_gently(gitdir, nongit_ok))
 		return NULL;
 
-	/* Accept --work-tree to support old scripts that played with fire. */
+	/* --work-tree is set without --git-dir; use discovered one */
 	if (getenv(GIT_WORK_TREE_ENVIRONMENT) || git_work_tree_cfg) {
-		warning("pretending GIT_DIR was supplied alongside GIT_WORK_TREE");
 		if (offset != len && !is_absolute_path(gitdir))
 			gitdir = xstrdup(make_absolute_path(gitdir));
 		if (chdir(cwd))
@@ -453,13 +452,10 @@ static const char *setup_bare_git_dir(char *cwd, int offset, int len, int *nongi
 	if (check_repository_format_gently(".", nongit_ok))
 		return NULL;
 
-	/*
-	 * Accept --work-tree, reluctantly.
-	 */
+	/* --work-tree is set without --git-dir; use discovered one */
 	if (getenv(GIT_WORK_TREE_ENVIRONMENT) || git_work_tree_cfg) {
 		const char *gitdir;
 
-		warning("pretending GIT_DIR was supplied alongside GIT_WORK_TREE");
 		gitdir = offset == len ? "." : xmemdupz(cwd, offset);
 		if (chdir(cwd))
 			die_errno("Could not come back to cwd");
diff --git a/t/t1510-repo-setup.sh b/t/t1510-repo-setup.sh
index b8a1b02..dcc0f86 100755
--- a/t/t1510-repo-setup.sh
+++ b/t/t1510-repo-setup.sh
@@ -16,9 +16,12 @@ A few rules for repo setup:
 4. GIT_WORK_TREE is relative to user's cwd. --work-tree is
    equivalent to GIT_WORK_TREE.
 
-5. GIT_WORK_TREE/core.worktree is only meant to work if GIT_DIR is set.
-   Otherwise there is a warning and a best effort is made to follow
-   historical behavior.
+5. GIT_WORK_TREE/core.worktree was originally meant to work only if
+   GIT_DIR is set, but earlier git didn't enforce it, and some scripts
+   depend on the implementation that happened to first discover .git by
+   going up from the users $cwd and then using the specified working tree
+   that may or may not have any relation to where .git was found in.  This
+   historical behaviour must be kept.
 
 6. Effective GIT_WORK_TREE overrides core.worktree and core.bare
 
@@ -225,16 +228,16 @@ try_repo () {
 test_expect_success '#0: nonbare repo, no explicit configuration' '
 	try_repo 0 unset unset unset "" unset \
 		.git "$here/0" "$here/0" "(null)" \
-		.git "$here/0" "$here/0" sub/ 2>messages &&
-	! grep "warning:.*GIT_DIR.*GIT_WORK_TREE" messages
+		.git "$here/0" "$here/0" sub/ 2>message &&
+	! test -s message
 '
 
-test_expect_success '#1: GIT_WORK_TREE without explicit GIT_DIR is reluctantly accepted' '
+test_expect_success '#1: GIT_WORK_TREE without explicit GIT_DIR is accepted' '
 	mkdir -p wt &&
 	try_repo 1 "$here" unset unset "" unset \
 		"$here/1/.git" "$here" "$here" 1/ \
 		"$here/1/.git" "$here" "$here" 1/sub/ 2>message &&
-	grep "warning:.*GIT_DIR.*GIT_WORK_TREE" message
+	! test -s message
 '
 
 test_expect_success '#2: worktree defaults to cwd with explicit GIT_DIR' '
@@ -255,16 +258,16 @@ test_expect_success '#3: setup' '
 '
 run_wt_tests 3
 
-test_expect_success '#4: core.worktree without GIT_DIR set is reluctantly accepted' '
+test_expect_success '#4: core.worktree without GIT_DIR set is accepted' '
 	setup_repo 4 ../sub "" unset &&
 	mkdir -p 4/sub sub &&
 	try_case 4 unset unset \
 		.git "$here/4/sub" "$here/4" "(null)" \
 		"$here/4/.git" "$here/4/sub" "$here/4/sub" "(null)" 2>message &&
-	grep "warning:.*GIT_DIR.*GIT_WORK_TREE" message
+	! test -s message
 '
 
-test_expect_success '#5: core.worktree + GIT_WORK_TREE is reluctantly accepted' '
+test_expect_success '#5: core.worktree + GIT_WORK_TREE is accepted' '
 	# or: you cannot intimidate away the lack of GIT_DIR setting
 	try_repo 5 "$here" unset "$here/5" "" unset \
 		"$here/5/.git" "$here" "$here" 5/ \
@@ -272,7 +275,7 @@ test_expect_success '#5: core.worktree + GIT_WORK_TREE is reluctantly accepted'
 	try_repo 5a .. unset "$here/5a" "" unset \
 		"$here/5a/.git" "$here" "$here" 5a/ \
 		"$here/5a/.git" "$here/5a" "$here/5a" sub/ &&
-	grep "warning:.*GIT_DIR.*GIT_WORK_TREE" message
+	! test -s message
 '
 
 test_expect_success '#6: setting GIT_DIR brings core.worktree to life' '
@@ -364,12 +367,12 @@ test_expect_success '#8: gitfile, easy case' '
 		"$here/8.git" "$here/8" "$here/8" sub/
 '
 
-test_expect_success '#9: GIT_WORK_TREE reluctantly accepted with gitfile' '
+test_expect_success '#9: GIT_WORK_TREE accepted with gitfile' '
 	mkdir -p 9/wt &&
 	try_repo 9 wt unset unset gitfile unset \
 		"$here/9.git" "$here/9/wt" "$here/9" "(null)" \
 		"$here/9.git" "$here/9/sub/wt" "$here/9/sub" "(null)" 2>message &&
-	grep "warning:.*GIT_DIR.*GIT_WORK_TREE" message
+	! test -s message
 '
 
 test_expect_success '#10: GIT_DIR can point to gitfile' '
@@ -391,19 +394,19 @@ test_expect_success '#11: setup' '
 '
 run_wt_tests 11 gitfile
 
-test_expect_success '#12: core.worktree with gitfile is reluctantly accepted' '
+test_expect_success '#12: core.worktree with gitfile is accepted' '
 	try_repo 12 unset unset "$here/12" gitfile unset \
 		"$here/12.git" "$here/12" "$here/12" "(null)" \
 		"$here/12.git" "$here/12" "$here/12" sub/ 2>message &&
-	grep "warning:.*GIT_DIR.*GIT_WORK_TREE" message
+	! test -s message
 '
 
-test_expect_success '#13: core.worktree+GIT_WORK_TREE relucantly accepted (with gitfile)' '
+test_expect_success '#13: core.worktree+GIT_WORK_TREE accepted (with gitfile)' '
 	# or: you cannot intimidate away the lack of GIT_DIR setting
 	try_repo 13 non-existent-too unset non-existent gitfile unset \
 		"$here/13.git" "$here/13/non-existent-too" "$here/13" "(null)" \
 		"$here/13.git" "$here/13/sub/non-existent-too" "$here/13/sub" "(null)" 2>message &&
-	grep "warning:.*GIT_DIR.*GIT_WORK_TREE" message
+	! test -s message
 '
 
 # case #14.
@@ -514,7 +517,7 @@ test_expect_success '#16c: bare .git has no worktree' '
 		"$here/16c/.git" "(null)" "$here/16c/sub" "(null)"
 '
 
-test_expect_success '#17: GIT_WORK_TREE without explicit GIT_DIR is reluctantly accepted (bare case)' '
+test_expect_success '#17: GIT_WORK_TREE without explicit GIT_DIR is accepted (bare case)' '
 	# Just like #16.
 	setup_repo 17a unset "" true &&
 	setup_repo 17b unset "" true &&
@@ -539,7 +542,7 @@ test_expect_success '#17: GIT_WORK_TREE without explicit GIT_DIR is reluctantly
 	try_repo 17c "$here/17c" unset unset "" true \
 		.git "$here/17c" "$here/17c" "(null)" \
 		"$here/17c/.git" "$here/17c" "$here/17c" sub/ 2>message &&
-	grep "warning:.*GIT_DIR.*GIT_WORK_TREE" message
+	! test -s message
 '
 
 test_expect_success '#18: bare .git named by GIT_DIR has no worktree' '
@@ -558,7 +561,7 @@ test_expect_success '#19: setup' '
 '
 run_wt_tests 19
 
-test_expect_success '#20a: core.worktree without GIT_DIR reluctantly accepted (inside .git)' '
+test_expect_success '#20a: core.worktree without GIT_DIR accepted (inside .git)' '
 	# Unlike case #16a.
 	setup_repo 20a "$here/20a" "" unset &&
 	mkdir -p 20a/.git/wt/sub &&
@@ -568,7 +571,7 @@ test_expect_success '#20a: core.worktree without GIT_DIR reluctantly accepted (i
 		"$here/20a/.git" "$here/20a" "$here/20a" .git/wt/ &&
 	try_case 20a/.git/wt/sub unset unset \
 		"$here/20a/.git" "$here/20a" "$here/20a" .git/wt/sub/ &&
-	grep "warning:.*GIT_DIR.*GIT_WORK_TREE" message
+	! test -s message
 '
 
 test_expect_success '#20b/c: core.worktree and core.bare conflict' '
@@ -581,7 +584,7 @@ test_expect_success '#20b/c: core.worktree and core.bare conflict' '
 	grep "core.bare and core.worktree" message
 '
 
-# Case #21: core.worktree/GIT_WORK_TREE reluctantly overrides core.bare' '
+# Case #21: core.worktree/GIT_WORK_TREE overrides core.bare' '
 test_expect_success '#21: setup, core.worktree warns before overriding core.bare' '
 	setup_repo 21 non-existent "" unset &&
 	mkdir -p 21/.git/wt/sub &&
@@ -591,7 +594,7 @@ test_expect_success '#21: setup, core.worktree warns before overriding core.bare
 		export GIT_WORK_TREE &&
 		git symbolic-ref HEAD >/dev/null
 	) 2>message &&
-	grep "warning:.*GIT_DIR.*GIT_WORK_TREE" message
+	! test -s message
 
 '
 run_wt_tests 21
@@ -703,11 +706,11 @@ test_expect_success '#24: bare repo has no worktree (gitfile case)' '
 		"$here/24.git" "(null)" "$here/24/sub" "(null)"
 '
 
-test_expect_success '#25: GIT_WORK_TREE accepted reluctantly if GIT_DIR unset (bare gitfile case)' '
+test_expect_success '#25: GIT_WORK_TREE accepted if GIT_DIR unset (bare gitfile case)' '
 	try_repo 25 "$here/25" unset unset gitfile true \
 		"$here/25.git" "$here/25" "$here/25" "(null)"  \
 		"$here/25.git" "$here/25" "$here/25" "sub/" 2>message &&
-	grep "warning:.*GIT_DIR.*GIT_WORK_TREE" message
+	! test -s message
 '
 
 test_expect_success '#26: bare repo has no worktree (GIT_DIR -> gitfile case)' '
@@ -732,11 +735,11 @@ test_expect_success '#28: core.worktree and core.bare conflict (gitfile case)' '
 		cd 28 &&
 		test_must_fail git symbolic-ref HEAD
 	) 2>message &&
-	grep "warning:.*GIT_DIR.*GIT_WORK_TREE" message &&
+	! grep "^warning:" message &&
 	grep "core.bare and core.worktree" message
 '
 
-# Case #29: GIT_WORK_TREE(+core.worktree) reluctantly overries core.bare (gitfile case).
+# Case #29: GIT_WORK_TREE(+core.worktree) overries core.bare (gitfile case).
 test_expect_success '#29: setup' '
 	setup_repo 29 non-existent gitfile true &&
 	mkdir -p 29/sub/sub 29/wt/sub
@@ -746,7 +749,7 @@ test_expect_success '#29: setup' '
 		export GIT_WORK_TREE &&
 		git symbolic-ref HEAD >/dev/null
 	) 2>message &&
-	grep "warning:.*GIT_DIR.*GIT_WORK_TREE" message
+	! test -s message
 '
 run_wt_tests 29 gitfile
Previous: Junio C HamanoNext: Jonathan Nieder
Message 63 of 87 in “nd/setup updates on pu”
  1. 00/47 nd/setup updates on puNguyễn Thái Ngọc Duy, Nov 26, 2010
  2. 01/47 builtins: print setup info if repo is foundNguyễn Thái Ngọc Duy, Nov 26, 2010
  3. 0/3 trace: omit noisy repository discovery reportJonathan Nieder, Jan 26, 2011
  4. 1/3 setup: do not expose tracing codeJonathan Nieder, Jan 26, 2011
  5. 2/3 trace: omit repository discovery reportJonathan Nieder, Jan 26, 2011
  6. Sverre RabbelierJan 26, 2011
  7. Jonathan NiederJan 26, 2011
  8. Nguyen Thai Ngoc DuyJan 26, 2011
  9. 3/3 tests: avoid unnecessary use of GIT_TRACE in repo-setup testsJonathan Nieder, Jan 26, 2011
  10. Nguyen Thai Ngoc DuyJan 26, 2011
  11. Jeff KingJan 26, 2011
  12. 02/47 Add t1510 and basic rules that run repo setupNguyễn Thái Ngọc Duy, Nov 26, 2010
  13. 03/47 t1510: setup case #0Nguyễn Thái Ngọc Duy, Nov 26, 2010
  14. 04/47 t1510: setup case #1Nguyễn Thái Ngọc Duy, Nov 26, 2010
  15. 05/47 t1510: setup case #2Nguyễn Thái Ngọc Duy, Nov 26, 2010
  16. 06/47 t1510: setup case #3Nguyễn Thái Ngọc Duy, Nov 26, 2010
  17. 07/47 t1510: setup case #4Nguyễn Thái Ngọc Duy, Nov 26, 2010
  18. 08/47 t1510: setup case #5Nguyễn Thái Ngọc Duy, Nov 26, 2010
  19. 09/47 t1510: setup case #6Nguyễn Thái Ngọc Duy, Nov 26, 2010
  20. 10/47 t1510: setup case #7Nguyễn Thái Ngọc Duy, Nov 26, 2010
  21. 11/47 t1510: setup case #8Nguyễn Thái Ngọc Duy, Nov 26, 2010
  22. 12/47 t1510: setup case #9Nguyễn Thái Ngọc Duy, Nov 26, 2010
  23. 13/47 t1510: setup case #10Nguyễn Thái Ngọc Duy, Nov 26, 2010
  24. 14/47 t1510: setup case #11Nguyễn Thái Ngọc Duy, Nov 26, 2010
  25. 15/47 t1510: setup case #12Nguyễn Thái Ngọc Duy, Nov 26, 2010
  26. 16/47 t1510: setup case #13Nguyễn Thái Ngọc Duy, Nov 26, 2010
  27. 17/47 t1510: setup case #14Nguyễn Thái Ngọc Duy, Nov 26, 2010
  28. 18/47 t1510: setup case #15Nguyễn Thái Ngọc Duy, Nov 26, 2010
  29. 19/47 t1510: setup case #16Nguyễn Thái Ngọc Duy, Nov 26, 2010
  30. 20/47 t1510: setup case #17Nguyễn Thái Ngọc Duy, Nov 26, 2010
  31. 21/47 t1510: setup case #18Nguyễn Thái Ngọc Duy, Nov 26, 2010
  32. 22/47 t1510: setup case #19Nguyễn Thái Ngọc Duy, Nov 26, 2010
  33. 23/47 t1510: setup case #20Nguyễn Thái Ngọc Duy, Nov 26, 2010
  34. 24/47 t1510: setup case #21Nguyễn Thái Ngọc Duy, Nov 26, 2010
  35. 25/47 t1510: setup case #22Nguyễn Thái Ngọc Duy, Nov 26, 2010
  36. 26/47 t1510: setup case #23Nguyễn Thái Ngọc Duy, Nov 26, 2010
  37. 27/47 t1510: setup case #24Nguyễn Thái Ngọc Duy, Nov 26, 2010
  38. 28/47 t1510: setup case #25Nguyễn Thái Ngọc Duy, Nov 26, 2010
  39. 29/47 t1510: setup case #26Nguyễn Thái Ngọc Duy, Nov 26, 2010
  40. 30/47 t1510: setup case #27Nguyễn Thái Ngọc Duy, Nov 26, 2010
  41. 31/47 t1510: setup case #28Nguyễn Thái Ngọc Duy, Nov 26, 2010
  42. 32/47 t1510: setup case #29Nguyễn Thái Ngọc Duy, Nov 26, 2010
  43. 33/47 t1510: setup case #30Nguyễn Thái Ngọc Duy, Nov 26, 2010
  44. 34/47 t1510: setup case #31Nguyễn Thái Ngọc Duy, Nov 26, 2010
  45. 35/47 git-rev-parse.txt: clarify --git-dirNguyễn Thái Ngọc Duy, Nov 26, 2010
  46. 36/47 rev-parse: prints --git-dir relative to user's cwdNguyễn Thái Ngọc Duy, Nov 26, 2010
  47. Junio C HamanoDec 22, 2010
  48. Nguyen Thai Ngoc DuyDec 22, 2010
  49. 37/47 Add git_config_early()Nguyễn Thái Ngọc Duy, Nov 26, 2010
  50. 38/47 Use git_config_early() instead of git_config() during repo setupNguyễn Thái Ngọc Duy, Nov 26, 2010
  51. 39/47 setup: limit get_git_work_tree()'s to explicit setup case onlyNguyễn Thái Ngọc Duy, Nov 26, 2010
  52. Jonathan NiederJan 18, 2011
  53. Nguyen Thai Ngoc DuyJan 18, 2011
  54. Junio C HamanoJan 18, 2011
  55. Nguyen Thai Ngoc DuyJan 19, 2011
  56. 0/3 setup: stop ignoring GIT_WORK_TREE (when GIT_DIR is unset)Jonathan Nieder, Jan 19, 2011
  57. 1/3 tests: cosmetic improvements to the repo-setup testJonathan Nieder, Jan 19, 2011
  58. 3/3 setup: always honor GIT_WORK_TREE and core.worktreeJonathan Nieder, Jan 19, 2011
  59. Nguyen Thai Ngoc DuyJan 19, 2011
  60. Jonathan NiederJan 19, 2011
  61. Junio C HamanoJan 19, 2011
  62. Junio C HamanoJan 21, 2011
  63. Junio C HamanoJan 21, 2011
  64. Jonathan NiederJan 21, 2011
  65. Junio C HamanoJan 21, 2011
  66. Nguyen Thai Ngoc DuyJan 22, 2011
  67. Junio C HamanoJan 23, 2011
  68. Jonathan NiederJan 24, 2011
  69. MaaartinJan 19, 2011
  70. Junio C HamanoJan 19, 2011
  71. MaaartinJan 19, 2011
  72. checkout to other directory (Re: [PATCH 3/3] setup: always honor GIT_WORK_TREE and core.worktree)Jonathan Nieder, Jan 19, 2011
  73. Jonathan NiederJan 19, 2011
  74. Junio C HamanoJan 19, 2011
  75. Jonathan NiederJan 19, 2011
  76. 40/47 setup: clean up setup_bare_git_dir()Nguyễn Thái Ngọc Duy, Nov 26, 2010
  77. 41/47 t1020-subdirectory: test alias expansion in a subdirectoryNguyễn Thái Ngọc Duy, Nov 26, 2010
  78. 42/47 setup: clean up setup_discovered_git_dir()Nguyễn Thái Ngọc Duy, Nov 26, 2010
  79. 43/47 setup: rework setup_explicit_git_dir()Nguyễn Thái Ngọc Duy, Nov 26, 2010
  80. 44/47 Remove all logic from get_git_work_tree()Nguyễn Thái Ngọc Duy, Nov 26, 2010
  81. Junio C HamanoDec 22, 2010
  82. Nguyen Thai Ngoc DuyDec 22, 2010
  83. Junio C HamanoDec 22, 2010
  84. 45/47 t0001: test git init when run via an aliasNguyễn Thái Ngọc Duy, Nov 26, 2010
  85. 46/47 Revert "Documentation: always respect core.worktree if set"Nguyễn Thái Ngọc Duy, Nov 26, 2010
  86. 47/47 git.txt: correct where --work-tree path is relative toNguyễn Thái Ngọc Duy, Nov 26, 2010
  87. Junio C HamanoNov 29, 2010

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.