Git development
 help / color / mirror / Atom feed
* [PATCH/RFC 0/3] setup: stop ignoring GIT_WORK_TREE (when GIT_DIR is unset)
From: Jonathan Nieder @ 2011-01-19 12:37 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nguyễn Thái Ngọc Duy, git
In-Reply-To: <7v1v4aknij.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:

>> +++ b/setup.c
>> @@ -419,6 +419,11 @@ static const char *setup_discovered_git_dir(const char *gitdir,
>>  		return NULL;
>>  	}
>>  
>> +	if (getenv(GIT_WORK_TREE_ENVIRONMENT)) {
>> +		warning("GIT_WORK_TREE without explicit GIT_DIR is deprecated");
>> +		return setup_explicit_git_dir(gitdir, cwd, offset, nongit_ok);
>> +	}
>> +
>
> My knee-jerk reaction is that calling this "deprecated" is probably
> confusing. git merely failed to notice misconfiguration

After staring at test results, I agree.  The rule, before nd/setup,
seems to be something like this:

 - if GIT_DIR is set, you're safe
 - if GIT_DIR is unset and .git is in the current directory,
   GIT_WORK_TREE will work by accident
 - otherwise, git errors out.

The patch below should be more robust.  It prints a warning:

	warning: pretending GIT_DIR was supplied alongside GIT_WORK_TREE

As the night winds on I am less sure that that warning is a good idea.
The .git discovery behavior seems relatively safe.  What is more
worrisome for the future of the use of GIT_WORK_TREE independent of
cwd is the interaction with pathspecs.

Jonathan Nieder (3):
  tests: cosmetic improvements to the repo-setup test
  tests: make the setup tests briefer
  setup: stop ignoring GIT_WORK_TREE and core.worktree

 setup.c               |   27 +-
 t/t1510-repo-setup.sh | 5166 +++++++------------------------------------------
 2 files changed, 729 insertions(+), 4464 deletions(-)

^ permalink raw reply

* [PATCH 1/3] tests: cosmetic improvements to the repo-setup test
From: Jonathan Nieder @ 2011-01-19 12:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nguyễn Thái Ngọc Duy, git
In-Reply-To: <20110119123732.GA23222@burratino>

Give an overview in "sh t1510-repo-setup.sh --help" output.
Waste some vertical and horizontal space for clearer code.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 t/t1510-repo-setup.sh |   94 ++++++++++++++++++++++++++-----------------------
 1 files changed, 50 insertions(+), 44 deletions(-)

diff --git a/t/t1510-repo-setup.sh b/t/t1510-repo-setup.sh
index e9c451c..f42f206 100755
--- a/t/t1510-repo-setup.sh
+++ b/t/t1510-repo-setup.sh
@@ -1,53 +1,59 @@
 #!/bin/sh
 
-test_description='Tests of cwd/prefix/worktree/gitdir setup in all cases'
+test_description="Tests of cwd/prefix/worktree/gitdir setup in all cases
 
+A few rules for repo setup:
+
+1. GIT_DIR is relative to user's cwd. --git-dir is equivalent to
+   GIT_DIR.
+
+2. .git file is relative to parent directory. .git file is basically
+   symlink in disguise. The directory where .git file points to will
+   become new git_dir.
+
+3. core.worktree is relative to git_dir.
+
+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 effective if GIT_DIR is set
+   Uneffective worktree settings should be warned.
+
+6. Effective GIT_WORK_TREE overrides core.worktree and core.bare
+
+7. Effective core.worktree conflicts with core.bare
+
+8. If GIT_DIR is set but neither worktree nor bare setting is given,
+   original cwd becomes worktree.
+
+9. If .git discovery is done inside a repo, the repo becomes a bare
+   repo. .git discovery is performed if GIT_DIR is not set.
+
+10. If no worktree is available, cwd remains unchanged, prefix is
+    NULL.
+
+11. When user's cwd is outside worktree, cwd remains unchanged,
+    prefix is NULL.
+"
 . ./test-lib.sh
 
-#
-# A few rules for repo setup:
-#
-# 1. GIT_DIR is relative to user's cwd. --git-dir is equivalent to
-#    GIT_DIR.
-#
-# 2. .git file is relative to parent directory. .git file is basically
-#    symlink in disguise. The directory where .git file points to will
-#    become new git_dir.
-#
-# 3. core.worktree is relative to git_dir.
-#
-# 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 effective if GIT_DIR is set
-#    Uneffective worktree settings should be warned.
-#
-# 6. Effective GIT_WORK_TREE overrides core.worktree and core.bare
-#
-# 7. Effective core.worktree conflicts with core.bare
-#
-# 8. If GIT_DIR is set but neither worktree nor bare setting is given,
-#    original cwd becomes worktree.
-#
-# 9. If .git discovery is done inside a repo, the repo becomes a bare
-#    repo. .git discovery is performed if GIT_DIR is not set.
-#
-# 10. If no worktree is available, cwd remains unchanged, prefix is
-#     NULL.
-#
-# 11. When user's cwd is outside worktree, cwd remains unchanged,
-#     prefix is NULL.
-#
-
-test_repo() {
+test_repo () {
 	(
-	cd "$1" &&
-	if test -n "$2"; then GIT_DIR="$2" && export GIT_DIR; fi &&
-	if test -n "$3"; then GIT_WORK_TREE="$3" && export GIT_WORK_TREE; fi &&
-	rm -f trace &&
-	GIT_TRACE="`pwd`/trace" git symbolic-ref HEAD >/dev/null &&
-	grep '^setup: ' trace >result &&
-	test_cmp expected result
+		cd "$1" &&
+		if test -n "$2"
+		then
+			GIT_DIR="$2" &&
+			export GIT_DIR
+		fi &&
+		if test -n "$3"
+		then
+			GIT_WORK_TREE="$3" &&
+			export GIT_WORK_TREE
+		fi &&
+		rm -f trace &&
+		GIT_TRACE="$(pwd)/trace" git symbolic-ref HEAD >/dev/null &&
+		grep '^setup: ' trace >result &&
+		test_cmp expected result
 	)
 }
 
-- 
1.7.4.rc2

^ permalink raw reply related

* [PATCH 3/3] setup: always honor GIT_WORK_TREE and core.worktree
From: Jonathan Nieder @ 2011-01-19 12:42 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nguyễn Thái Ngọc Duy, git
In-Reply-To: <20110119123732.GA23222@burratino>

In the usual case, git commands are run from within a git worktree.
The toplevel directory and associated .git dir are found by chdir-ing
to .. repeatedly until .git is a valid repository.

That behavior can be overridden in two ways.

 (1) By setting GIT_DIR (through the environment or the --git-dir
     command line option), one can keep the repository somewhere
     unrelated to the worktree.

 (2) By setting GIT_WORK_TREE (through the environment, --work-tree
     command line option, or [core] worktree configuration) in
     addition to GIT_DIR, one can use a directory other than the cwd
     as toplevel of the worktree.

Unfortunately the existence of GIT_WORK_TREE makes it tempting to
use without setting GIT_DIR.  Until v1.7.4-rc0~4^2~9 (setup: limit
get_git_work_tree()'s to explicit setup case only, 2010-11-26), that
only happened to work, sometimes:

 - if .git is in the current directory, it might work;
 - otherwise, git would fail to discover the .git directory.

Ouch.  It would be better to forbid setting a work tree without git
dir entirely, except that due to this accidental support some scripts
(e.g., git-buildpackage, gitolite) learned to use rely on it over the
years.

Instead (1) first run discovery as though within a worktree trying to
find the .git dir, and (2) only once discovery is finished, check the
GIT_WORK_TREE setting to see if the worktree is declared to be
somewhere else entirely.

Also print a warning when GIT_WORK_TREE is set and GIT_DIR not, to
prevent nonportable scripts relying on this new behavior.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 setup.c               |   27 +++++++-
 t/t1510-repo-setup.sh |  199 ++++++++++++++++++++++++++----------------------
 2 files changed, 134 insertions(+), 92 deletions(-)

diff --git a/setup.c b/setup.c
index 3d73269..c0f5846 100644
--- a/setup.c
+++ b/setup.c
@@ -411,6 +411,16 @@ 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. */
+	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))
+			die_errno("Could not come back to cwd");
+		return setup_explicit_git_dir(gitdir, cwd, len, nongit_ok);
+	}
+
 	/* #16.2, #17.2, #20.2, #21.2, #24, #25, #28, #29 (see t1510) */
 	if (is_bare_repository_cfg > 0) {
 		set_git_dir(offset == len ? gitdir : make_absolute_path(gitdir));
@@ -443,6 +453,19 @@ 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.
+	 */
+	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");
+		return setup_explicit_git_dir(gitdir, cwd, len, nongit_ok);
+	}
+
 	inside_git_dir = 1;
 	inside_work_tree = 0;
 	if (offset != len) {
@@ -509,8 +532,10 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)
 	 * validation.
 	 */
 	gitdirenv = getenv(GIT_DIR_ENVIRONMENT);
-	if (gitdirenv)
+	if (gitdirenv) {
+		trace_printf("trace: gitdirenv = %s\n", gitdirenv);
 		return setup_explicit_git_dir(gitdirenv, cwd, len, nongit_ok);
+	}
 
 	ceil_offset = longest_ancestor_length(cwd, env_ceiling_dirs);
 	if (ceil_offset < 0 && has_dos_drive_prefix(cwd))
diff --git a/t/t1510-repo-setup.sh b/t/t1510-repo-setup.sh
index 4f8f976..b8a1b02 100755
--- a/t/t1510-repo-setup.sh
+++ b/t/t1510-repo-setup.sh
@@ -16,8 +16,9 @@ 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 effective if GIT_DIR is set
-   Uneffective worktree settings should be warned.
+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.
 
 6. Effective GIT_WORK_TREE overrides core.worktree and core.bare
 
@@ -224,13 +225,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/
+		.git "$here/0" "$here/0" sub/ 2>messages &&
+	! grep "warning:.*GIT_DIR.*GIT_WORK_TREE" messages
 '
 
-test_expect_success '#1: GIT_WORK_TREE without explicit GIT_DIR is ignored' '
-	try_repo 1 non-existent unset unset "" unset \
-		.git "$here/1" "$here/1" "(null)" \
-		.git "$here/1" "$here/1" sub/
+test_expect_success '#1: GIT_WORK_TREE without explicit GIT_DIR is reluctantly 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_expect_success '#2: worktree defaults to cwd with explicit GIT_DIR' '
@@ -251,17 +255,24 @@ test_expect_success '#3: setup' '
 '
 run_wt_tests 3
 
-test_expect_success '#4: core.worktree without GIT_DIR set is ignored' '
-	try_repo 4 unset unset non-existent "" unset \
-		.git "$here/4" "$here/4" "(null)" \
-		.git "$here/4" "$here/4" sub/
+test_expect_success '#4: core.worktree without GIT_DIR set is reluctantly 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_expect_success '#5: core.worktree + GIT_WORK_TREE is still ignored' '
+test_expect_success '#5: core.worktree + GIT_WORK_TREE is reluctantly accepted' '
 	# or: you cannot intimidate away the lack of GIT_DIR setting
-	try_repo 5 non-existent-too unset non-existent "" unset \
-		.git "$here/5" "$here/5" "(null)" \
-		.git "$here/5" "$here/5" sub/
+	try_repo 5 "$here" unset "$here/5" "" unset \
+		"$here/5/.git" "$here" "$here" 5/ \
+		"$here/5/.git" "$here" "$here" 5/sub/ 2>message &&
+	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_expect_success '#6: setting GIT_DIR brings core.worktree to life' '
@@ -353,10 +364,12 @@ test_expect_success '#8: gitfile, easy case' '
 		"$here/8.git" "$here/8" "$here/8" sub/
 '
 
-test_expect_success '#9: GIT_WORK_TREE ignored even with gitfile' '
-	try_repo 9 non-existent unset unset gitfile unset \
-		"$here/9.git" "$here/9" "$here/9" "(null)" \
-		"$here/9.git" "$here/9" "$here/9" sub/
+test_expect_success '#9: GIT_WORK_TREE reluctantly 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_expect_success '#10: GIT_DIR can point to gitfile' '
@@ -378,17 +391,19 @@ test_expect_success '#11: setup' '
 '
 run_wt_tests 11 gitfile
 
-test_expect_success '#12: core.worktree with gitfile is still ignored' '
-	try_repo 12 unset unset non-existent gitfile unset \
+test_expect_success '#12: core.worktree with gitfile is reluctantly 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/
+		"$here/12.git" "$here/12" "$here/12" sub/ 2>message &&
+	grep "warning:.*GIT_DIR.*GIT_WORK_TREE" message
 '
 
-test_expect_success '#13: core.worktree+GIT_WORK_TREE ignored (with gitfile)' '
+test_expect_success '#13: core.worktree+GIT_WORK_TREE relucantly 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" "$here/13" "(null)" \
-		"$here/13.git" "$here/13" "$here/13" sub/
+		"$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
 '
 
 # case #14.
@@ -499,30 +514,32 @@ 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 ignored (bare case)' '
+test_expect_success '#17: GIT_WORK_TREE without explicit GIT_DIR is reluctantly accepted (bare case)' '
 	# Just like #16.
 	setup_repo 17a unset "" true &&
 	setup_repo 17b unset "" true &&
 	mkdir -p 17a/.git/wt/sub &&
 	mkdir -p 17b/.git/wt/sub &&
 
-	try_case 17a/.git non-existent unset \
-		. "(null)" "$here/17a/.git" "(null)" &&
-	try_case 17a/.git/wt non-existent unset \
-		"$here/17a/.git" "(null)" "$here/17a/.git/wt" "(null)" &&
-	try_case 17a/.git/wt/sub non-existent unset \
-		"$here/17a/.git" "(null)" "$here/17a/.git/wt/sub" "(null)" &&
+	try_case 17a/.git "$here/17a" unset \
+		"$here/17a/.git" "$here/17a" "$here/17a" .git/ \
+		2>message &&
+	try_case 17a/.git/wt "$here/17a" unset \
+		"$here/17a/.git" "$here/17a" "$here/17a" .git/wt/ &&
+	try_case 17a/.git/wt/sub "$here/17a" unset \
+		"$here/17a/.git" "$here/17a" "$here/17a" .git/wt/sub/ &&
 
-	try_case 17b/.git non-existent unset \
-		. "(null)" "$here/17b/.git" "(null)" &&
-	try_case 17b/.git/wt non-existent unset \
-		"$here/17b/.git" "(null)" "$here/17b/.git/wt" "(null)" &&
-	try_case 17b/.git/wt/sub non-existent unset \
-		"$here/17b/.git" "(null)" "$here/17b/.git/wt/sub" "(null)" &&
+	try_case 17b/.git "$here/17b" unset \
+		"$here/17b/.git" "$here/17b" "$here/17b" .git/ &&
+	try_case 17b/.git/wt "$here/17b" unset \
+		"$here/17b/.git" "$here/17b" "$here/17b" .git/wt/ &&
+	try_case 17b/.git/wt/sub "$here/17b" unset \
+		"$here/17b/.git" "$here/17b" "$here/17b" .git/wt/sub/ &&
 
-	try_repo 17c non-existent unset unset "" true \
-		.git "(null)" "$here/17c" "(null)" \
-		"$here/17c/.git" "(null)" "$here/17c/sub" "(null)"
+	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_expect_success '#18: bare .git named by GIT_DIR has no worktree' '
@@ -541,56 +558,43 @@ test_expect_success '#19: setup' '
 '
 run_wt_tests 19
 
-test_expect_success '#20a: core.worktree without GIT_DIR ignored (inside .git)' '
-	# Just like case #16a.
-	setup_repo 20a non-existent "" unset &&
+test_expect_success '#20a: core.worktree without GIT_DIR reluctantly accepted (inside .git)' '
+	# Unlike case #16a.
+	setup_repo 20a "$here/20a" "" unset &&
 	mkdir -p 20a/.git/wt/sub &&
 	try_case 20a/.git unset unset \
-		. "(null)" "$here/20a/.git" "(null)" &&
+		"$here/20a/.git" "$here/20a" "$here/20a" .git/ 2>message &&
 	try_case 20a/.git/wt unset unset \
-		"$here/20a/.git" "(null)" "$here/20a/.git/wt" "(null)" &&
+		"$here/20a/.git" "$here/20a" "$here/20a" .git/wt/ &&
 	try_case 20a/.git/wt/sub unset unset \
-		"$here/20a/.git" "(null)" "$here/20a/.git/wt/sub" "(null)"
+		"$here/20a/.git" "$here/20a" "$here/20a" .git/wt/sub/ &&
+	grep "warning:.*GIT_DIR.*GIT_WORK_TREE" message
 '
 
-test_expect_success '#20b/c: core.worktree without GIT_DIR ignored (bare repository)' '
-	# Just like case #16b/c.
+test_expect_success '#20b/c: core.worktree and core.bare conflict' '
 	setup_repo 20b non-existent "" true &&
 	mkdir -p 20b/.git/wt/sub &&
-	try_case 20b/.git unset unset \
-		. "(null)" "$here/20b/.git" "(null)" &&
-	try_case 20b/.git/wt unset unset \
-		"$here/20b/.git" "(null)" "$here/20b/.git/wt" "(null)" &&
-	try_case 20b/.git/wt/sub unset unset \
-		"$here/20b/.git" "(null)" "$here/20b/.git/wt/sub" "(null)" &&
-	try_repo 20c unset unset non-existent "" true \
-		.git "(null)" "$here/20c" "(null)" \
-		"$here/20c/.git" "(null)" "$here/20c/sub" "(null)"
+	(
+		cd 20b/.git &&
+		test_must_fail git symbolic-ref HEAD >/dev/null
+	) 2>message &&
+	grep "core.bare and core.worktree" message
 '
 
-test_expect_success '#21: core.worktree+GIT_WORK_TREE without GIT_DIR ignored (bare cases)' '
-	setup_repo 21a non-existent "" unset &&
-	mkdir -p 21a/.git/wt/sub &&
-	try_case 21a/.git non-existent-too unset \
-		. "(null)" "$here/21a/.git" "(null)" &&
-	try_case 21a/.git/wt non-existent-too unset \
-		"$here/21a/.git" "(null)" "$here/21a/.git/wt" "(null)" &&
-	try_case 21a/.git/wt/sub non-existent-too unset \
-		"$here/21a/.git" "(null)" "$here/21a/.git/wt/sub" "(null)" &&
+# Case #21: core.worktree/GIT_WORK_TREE reluctantly 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 &&
+	(
+		cd 21/.git &&
+		GIT_WORK_TREE="$here/21" &&
+		export GIT_WORK_TREE &&
+		git symbolic-ref HEAD >/dev/null
+	) 2>message &&
+	grep "warning:.*GIT_DIR.*GIT_WORK_TREE" message
 
-	setup_repo 21b non-existent "" true &&
-	mkdir -p 21b/.git/wt/sub &&
-	try_case 21b/.git non-existent-too unset \
-		. "(null)" "$here/21b/.git" "(null)" &&
-	try_case 21b/.git/wt non-existent-too unset \
-		"$here/21b/.git" "(null)" "$here/21b/.git/wt" "(null)" &&
-	try_case 21b/.git/wt/sub non-existent-too unset \
-		"$here/21b/.git" "(null)" "$here/21b/.git/wt/sub" "(null)" &&
-
-	try_repo 21c non-existent-too unset non-existent "" true \
-		.git "(null)" "$here/21c" "(null)" \
-		"$here/21c/.git" "(null)" "$here/21c/sub" "(null)"
 '
+run_wt_tests 21
 
 test_expect_success '#22a: core.worktree = GIT_DIR = .git dir' '
 	# like case #6.
@@ -699,10 +703,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 ignored if GIT_DIR unset (bare gitfile case)' '
-	try_repo 25 non-existent unset unset gitfile true \
-		"$here/25.git" "(null)" "$here/25" "(null)" \
-		"$here/25.git" "(null)" "$here/25/sub" "(null)"
+test_expect_success '#25: GIT_WORK_TREE accepted reluctantly 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_expect_success '#26: bare repo has no worktree (GIT_DIR -> gitfile case)' '
@@ -721,17 +726,29 @@ test_expect_success '#27: setup' '
 '
 run_wt_tests 27 gitfile
 
-test_expect_success '#28: core.worktree ignored if GIT_DIR unset (bare gitfile case)' '
-	try_repo 28 unset unset non-existent gitfile true \
-		"$here/28.git" "(null)" "$here/28" "(null)" \
-		"$here/28.git" "(null)" "$here/28/sub" "(null)"
+test_expect_success '#28: core.worktree and core.bare conflict (gitfile case)' '
+	setup_repo 28 "$here/28" gitfile true &&
+	(
+		cd 28 &&
+		test_must_fail git symbolic-ref HEAD
+	) 2>message &&
+	grep "warning:.*GIT_DIR.*GIT_WORK_TREE" message &&
+	grep "core.bare and core.worktree" message
 '
 
-test_expect_success '#29: GIT_WORK_TREE+core.worktree ignored if GIT_DIR unset (bare gitfile case)' '
-	try_repo 29 non-existent-too unset non-existent gitfile true \
-		"$here/29.git" "(null)" "$here/29" "(null)" \
-		"$here/29.git" "(null)" "$here/29/sub" "(null)"
+# Case #29: GIT_WORK_TREE(+core.worktree) reluctantly 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
+	(
+		cd 29 &&
+		GIT_WORK_TREE="$here/29" &&
+		export GIT_WORK_TREE &&
+		git symbolic-ref HEAD >/dev/null
+	) 2>message &&
+	grep "warning:.*GIT_DIR.*GIT_WORK_TREE" message
 '
+run_wt_tests 29 gitfile
 
 test_expect_success '#30: core.worktree and core.bare conflict (gitfile version)' '
 	# Just like case #22.
-- 
1.7.4.rc2

^ permalink raw reply related

* Re: [PATCH/RFC 0/3] setup: stop ignoring GIT_WORK_TREE (when GIT_DIR is unset)
From: Jonathan Nieder @ 2011-01-19 13:05 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nguyễn Thái Ngọc Duy, git
In-Reply-To: <20110119123732.GA23222@burratino>

Jonathan Nieder wrote:

>   tests: cosmetic improvements to the repo-setup test
>   tests: make the setup tests briefer
>   setup: stop ignoring GIT_WORK_TREE and core.worktree
> 
>  setup.c               |   27 +-
>  t/t1510-repo-setup.sh | 5166 +++++++------------------------------------------
>  2 files changed, 729 insertions(+), 4464 deletions(-)

It seems that the second patch was too long to send by mail.  The
series can be retrieved by git at

 git://repo.or.cz/git/jrn.git worktree-with-implicit-git-dir

Patches are based on js/test-windows to avoid semantically
uninteresting conflicts.

^ permalink raw reply

* Re: git bisect problems/ideas
From: Christian Couder @ 2011-01-19 13:15 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Aaron S. Meurer, git@vger.kernel.org
In-Reply-To: <7vd3nukqn8.fsf@alter.siamese.dyndns.org>

On Tue, Jan 18, 2011 at 7:34 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Christian Couder <christian.couder@gmail.com> writes:
>
>> Well, bugs are usually fixed within days after they have been
>> reported. Otherwise they are usually documented in the code or in the
>> documentation or in the test suite (with test_expect_failure).
>>
>> For the rest we rely on people remembering what happened and on
>> people's mailing list searching skills ;-)
>
> Not really.
>
> What we do is to take advantage of the fact that issues people do care
> about are important ones, and others that nobody cares about are not worth
> pursuing.
>
> In a sense, "people forgetting" is a lot more important than "people
> remembering" to filter unimportant issues (issues that are so unimportant
> that even the original complainer does not bother to come back and
> re-raise it).

Thanks for the clarification, now I feel much better about how well I
am doing regarding the mailing list :-)
Christian.

^ permalink raw reply

* [PATCH] Sanity-ckeck config variable names
From: Libor Pechacek @ 2011-01-19 14:11 UTC (permalink / raw)
  To: git; +Cc: Jeff King
In-Reply-To: <20110119100105.GB8034@fm.suse.cz>

Sanity-ckeck config variable names when adding and retrieving them.

As a side effect code duplication between git_config_set_multivar and get_value
(in builtin/config.c) was removed and the common functionality was placed in
git_config_parse_key.

The regular expression patterns are left intact when using --get-regexp option.

Signed-off-by: Libor Pechacek <lpechacek@suse.cz>
Cc: Jeff King <peff@peff.net>
---
 builtin/config.c |    8 ++---
 cache.h          |    1 +
 config.c         |  106 ++++++++++++++++++++++++++++++++++--------------------
 3 files changed, 71 insertions(+), 44 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index ca4a0db..068ef76 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -153,7 +153,6 @@ static int show_config(const char *key_, const char *value_, void *cb)
 static int get_value(const char *key_, const char *regex_)
 {
 	int ret = -1;
-	char *tl;
 	char *global = NULL, *repo_config = NULL;
 	const char *system_wide = NULL, *local;
 
@@ -168,10 +167,6 @@ static int get_value(const char *key_, const char *regex_)
 	}
 
 	key = xstrdup(key_);
-	for (tl=key+strlen(key)-1; tl >= key && *tl != '.'; --tl)
-		*tl = tolower(*tl);
-	for (tl=key; *tl && *tl != '.'; ++tl)
-		*tl = tolower(*tl);
 
 	if (use_key_regexp) {
 		key_regexp = (regex_t*)xmalloc(sizeof(regex_t));
@@ -179,6 +174,9 @@ static int get_value(const char *key_, const char *regex_)
 			fprintf(stderr, "Invalid key pattern: %s\n", key_);
 			goto free_strings;
 		}
+	} else {
+		if (git_config_parse_key(key_, &key, NULL))
+			goto free_strings;
 	}
 
 	if (regex_) {
diff --git a/cache.h b/cache.h
index d83d68c..1e32d63 100644
--- a/cache.h
+++ b/cache.h
@@ -997,6 +997,7 @@ extern int git_config_maybe_bool(const char *, const char *);
 extern int git_config_string(const char **, const char *, const char *);
 extern int git_config_pathname(const char **, const char *, const char *);
 extern int git_config_set(const char *, const char *);
+extern int git_config_parse_key(const char *, char **, int *);
 extern int git_config_set_multivar(const char *, const char *, const char *, int);
 extern int git_config_rename_section(const char *, const char *);
 extern const char *git_etc_gitconfig(void);
diff --git a/config.c b/config.c
index 625e051..205282f 100644
--- a/config.c
+++ b/config.c
@@ -1098,6 +1098,72 @@ int git_config_set(const char *key, const char *value)
 	return git_config_set_multivar(key, value, NULL, 0);
 }
 
+/* Auxiliary function to sanity-check and split the key into the section
+ * identifier and variable name.
+ *
+ * Returns 0 on success, 1 when there is an invalid character in the key and 2
+ * if there is no section name in the key.
+ *
+ * store_key - pointer to char* which will hold a copy of the key with
+ *             lowercase section and variable name, can be NULL
+ * baselen - pointer to int which will hold the length of the
+ *           section + subsection part, can be NULL
+ */
+int git_config_parse_key(const char *key, char **store_key, int *baselen_)
+{
+	int i, dot, baselen;
+	const char *last_dot = strrchr(key, '.');
+
+	/*
+	 * Since "key" actually contains the section name and the real
+	 * key name separated by a dot, we have to know where the dot is.
+	 */
+
+	if (last_dot == NULL) {
+		error("key does not contain a section: %s", key);
+		return 2;
+	}
+
+	baselen = last_dot - key;
+	if (baselen_)
+		*baselen_ = baselen;
+
+	/*
+	 * Validate the key and while at it, lower case it for matching.
+	 */
+	if (store_key)
+		*store_key = xmalloc(strlen(key) + 1);
+
+	dot = 0;
+	for (i = 0; key[i]; i++) {
+		unsigned char c = key[i];
+		if (c == '.')
+			dot = 1;
+		/* Leave the extended basename untouched.. */
+		if (!dot || i > baselen) {
+			if (!iskeychar(c) || (i == baselen+1 && !isalpha(c))) {
+				error("invalid key: %s", key);
+				goto out_free_ret_1;
+			}
+			c = tolower(c);
+		} else if (c == '\n') {
+			error("invalid key (newline): %s", key);
+			goto out_free_ret_1;
+		}
+		if (store_key)
+			(*store_key)[i] = c;
+	}
+	if (store_key)
+		(*store_key)[i] = 0;
+
+	return 0;
+
+out_free_ret_1:
+	if (store_key)
+		free(*store_key);
+	return 1;
+}
+
 /*
  * If value==NULL, unset in (remove from) config,
  * if value_regex!=NULL, disregard key/value pairs where value does not match.
@@ -1124,59 +1190,21 @@ int git_config_set(const char *key, const char *value)
 int git_config_set_multivar(const char *key, const char *value,
 	const char *value_regex, int multi_replace)
 {
-	int i, dot;
 	int fd = -1, in_fd;
 	int ret;
 	char *config_filename;
 	struct lock_file *lock = NULL;
-	const char *last_dot = strrchr(key, '.');
 
 	if (config_exclusive_filename)
 		config_filename = xstrdup(config_exclusive_filename);
 	else
 		config_filename = git_pathdup("config");
 
-	/*
-	 * Since "key" actually contains the section name and the real
-	 * key name separated by a dot, we have to know where the dot is.
-	 */
-
-	if (last_dot == NULL) {
-		error("key does not contain a section: %s", key);
-		ret = 2;
+	if ((ret = git_config_parse_key(key, &store.key, &store.baselen)))
 		goto out_free;
-	}
-	store.baselen = last_dot - key;
 
 	store.multi_replace = multi_replace;
 
-	/*
-	 * Validate the key and while at it, lower case it for matching.
-	 */
-	store.key = xmalloc(strlen(key) + 1);
-	dot = 0;
-	for (i = 0; key[i]; i++) {
-		unsigned char c = key[i];
-		if (c == '.')
-			dot = 1;
-		/* Leave the extended basename untouched.. */
-		if (!dot || i > store.baselen) {
-			if (!iskeychar(c) || (i == store.baselen+1 && !isalpha(c))) {
-				error("invalid key: %s", key);
-				free(store.key);
-				ret = 1;
-				goto out_free;
-			}
-			c = tolower(c);
-		} else if (c == '\n') {
-			error("invalid key (newline): %s", key);
-			free(store.key);
-			ret = 1;
-			goto out_free;
-		}
-		store.key[i] = c;
-	}
-	store.key[i] = 0;
 
 	/*
 	 * The lock serves a purpose in addition to locking: the new
-- 
1.7.4.rc2.2.gb4f4f

^ permalink raw reply related

* [PATCH] Documentation fixes in git-config
From: Libor Pechacek @ 2011-01-19 14:14 UTC (permalink / raw)
  To: git; +Cc: Jeff King
In-Reply-To: <20110119100105.GB8034@fm.suse.cz>

Variable names must start with an alphabetic character, regexp config key
matching is case sensitive.

Signed-off-by: Libor Pechacek <lpechacek@suse.cz>
Cc: Jeff King <peff@peff.net>
---
 Documentation/config.txt     |   12 +++++++-----
 Documentation/git-config.txt |    4 +++-
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index ff7c225..0f23bc7 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -12,8 +12,9 @@ The configuration variables are used by both the git plumbing
 and the porcelains. The variables are divided into sections, wherein
 the fully qualified variable name of the variable itself is the last
 dot-separated segment and the section name is everything before the last
-dot. The variable names are case-insensitive and only alphanumeric
-characters are allowed. Some variables may appear multiple times.
+dot. The variable names are case-insensitive, only alphanumeric
+characters and '-' are allowed and must start with an alphabetic character.
+Some variables may appear multiple times.
 
 Syntax
 ~~~~~~
@@ -53,9 +54,10 @@ All the other lines (and the remainder of the line after the section
 header) are recognized as setting variables, in the form
 'name = value'.  If there is no equal sign on the line, the entire line
 is taken as 'name' and the variable is recognized as boolean "true".
-The variable names are case-insensitive and only alphanumeric
-characters and `-` are allowed.  There can be more than one value
-for a given variable; we say then that variable is multivalued.
+The variable names are case-insensitive, only alphanumeric
+characters and `-` are allowed and must start with an alphabetic character.
+There can be more than one value for a given variable; we say then that
+variable is multivalued.
 
 Leading and trailing whitespace in a variable value is discarded.
 Internal whitespace within a variable value is retained verbatim.
diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 543dd64..6966ed6 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -84,7 +84,9 @@ OPTIONS
 
 --get-regexp::
 	Like --get-all, but interprets the name as a regular expression.
-	Also outputs the key names.
+	Regular expression matching is case sensitive in all parts of the key,
+	therefore make sure your pattern matches lower case letters in section
+	and variable names.  Also outputs the key names.
 
 --global::
 	For writing options: write to global ~/.gitconfig file rather than
-- 
1.7.4.rc2.2.gb4f4f

^ permalink raw reply related

* Re: filter-branch --env-filter GIT_AUTHOR_DATE
From: Tuncer Ayaz @ 2011-01-19 14:30 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git
In-Reply-To: <4D36BDD9.4050108@viscovery.net>

On Wed, Jan 19, 2011 at 11:32 AM, Johannes Sixt wrote:
> Am 1/19/2011 11:08, schrieb Tuncer Ayaz:
>> On Wed, Jan 19, 2011 at 8:01 AM, Johannes Sixt wrote:
>>> Am 1/18/2011 17:43, schrieb Tuncer Ayaz:
>>>> To fix invalid timezone info in a repo I ran
>>>> git filter-branch --env-filter '
>>>>   GIT_AUTHOR_DATE=`echo ${GIT_AUTHOR_DATE}|sed s/+0000/-0800/`' HEAD
>>>>
>>>> This fixed the invalid entries but the new timezone is -0700
>>>> instead of -0800. Is this expected?
>>>
>>> Parse error. You fixed it, but it is not fixed? So what?
>>
>> Fixed because it is not +0000 anymore. Surprised because the new
>> timezone is -0700 and not -0800.
>>
>>> What do you mean by "the new timezone is"? Do you mean "...is reported
>>> as"? If so, reported by which tools?
>>
>> git log
>> git cat-file $REV
>
> $ git filter-branch -f --env-filter 'echo; echo "$GIT_AUTHOR_DATE";
> GIT_AUTHOR_DATE=`echo ${GIT_AUTHOR_DATE}|sed s/+0100/-0800/`;
> echo "$GIT_AUTHOR_DATE"' -- -1
> Rewrite 6fb5ec91707a4433628eae5d9d68153ca8b819fe (1/1)
> 1292311163 +0100
> 1292311163 -0800
>
> Ref 'refs/heads/master' was rewritten
> $ git cat-file commit HEAD
> tree 43554f2216bbcfc96385db0641ae212409f26f21
> parent 942f54790453970cfffbfedf29e47ac27b9ba995
> author Johannes Sixt <j.sixt@viscovery.net> 1292311163 -0800
> committer Johannes Sixt <j.sixt@viscovery.net> 1292311163 +0100
>
> master
>
> *Shrug*

Cannot reproduce anymore. Works as expected.

Successfully rewrote all GIT_AUTHOR_DATE timezone entries:
  -0700 -> -0800
  -0800 -> +0000
  +0000 -> -0800

git version 1.7.4.rc2.3.g60a2e

^ permalink raw reply

* Re: [PATCH 3/3] setup: always honor GIT_WORK_TREE and core.worktree
From: Nguyen Thai Ngoc Duy @ 2011-01-19 14:48 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Junio C Hamano, git
In-Reply-To: <20110119124230.GD23222@burratino>

2011/1/19 Jonathan Nieder <jrnieder@gmail.com>:
> @@ -411,6 +411,16 @@ 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. */
> +       if (getenv(GIT_WORK_TREE_ENVIRONMENT) || git_work_tree_cfg) {

Can we leave git_work_tree_cfg out? If this code is to support misused
scripts, then $GIT_WORK_TREE alone ought to be enough. I don't think
any scripts would use core.worktree. Most of worktree headache comes
from core.worktree, not $GIT_WORK_TREE. Granted though the situation
is better now that we don't set worktree in setup_git_directory().

> +               warning("pretending GIT_DIR was supplied alongside GIT_WORK_TREE");

What if core.worktree is set, not $GIT_WORK_TREE? In my opinion

> +               if (offset != len && !is_absolute_path(gitdir))
> +                       gitdir = xstrdup(make_absolute_path(gitdir));

The behavior regarding relative $GIT_WORK_TREE before nd/setup series
is inconsistent. If setup_git_directory() is used, work_tree is
relative to user's cwd. In other cases, when get_git_work_tree() is
called, work_tree is made absolute relative to _current_ cwd (usually
at discovered work_tree root). Which way do you want to keep?
-- 
Duy

^ permalink raw reply

* pushing to a remote branch that one does not own
From: iani @ 2011-01-19 18:16 UTC (permalink / raw)
  To: git


Hello, 
I want to contribute via "push" to a branch which I have previously cloned
from a remote repository. There is a problem: 

If i create a new commit on branch "master", which coincides with
origin/master, then I can push this commit simply with
    git push
However, if i switch to a different branch of the repository which was
cloned, for example by doing:
    git checkout origin/lilt 
Then I get the message: 
"You are in 'detached HEAD' state. You can look around, make experimental
... (etc)"
So I HAVE to create my own new branch based on the one downloaded in order
to start committing and pushing changes. My question is therefore: 

Am I strictly limited to committing only on the master / origin/master
branched, and forced to make a new branch for every branch that I cloned
from the remote repo, or is there a way of making the heads of the remote
branches visible as local too? 

Thanks, 

Iannis Zannos


-- 
View this message in context: http://git.661346.n2.nabble.com/pushing-to-a-remote-branch-that-one-does-not-own-tp5940751p5940751.html
Sent from the git mailing list archive at Nabble.com.

^ permalink raw reply

* Re: git submodule vs git mergetool
From: Jens Lehmann @ 2011-01-19 18:40 UTC (permalink / raw)
  To: Mathieu Malaterre; +Cc: git
In-Reply-To: <AANLkTi=jS1LQY0kfSD_=o0PZ9BAF7k=06QFT+agkvGNa@mail.gmail.com>

Am 18.01.2011 10:04, schrieb Mathieu Malaterre:
> $ git mergetool
> merge tool candidates: meld opendiff kdiff3 tkdiff xxdiff
> tortoisemerge gvimdiff diffuse ecmerge p4merge araxis emerge vimdiff
> Merging the files: Testing/Data
> 
> mv: cannot stat `Testing/Data': No such file or directory
> cp: cannot stat `./Testing/Data.BACKUP.5251': No such file or directory
> error: git checkout-index: cannot create temporary subproject
> error: git checkout-index: cannot create temporary subproject
> Normal merge conflict for 'Testing/Data':
>   {local}: created
>   {remote}: created
> Hit return to start merge resolution tool (gvimdiff):
> 3 files to edit
> Testing/Data seems unchanged.
> Was the merge successful? [y/n] y
> fatal: unable to stat 'Testing/Data': No such file or directory
> 
> I guess this is a minor issue, but I thought I should report it here.

I assume this happens when you do the merge in a work tree where this
submodule does not exist? Right now git does not populate submodules
that show up while switching branches or doing a merge. I am working
on that feature, but progress is not as swift as I hoped ...

^ permalink raw reply

* Re: [PATCH 3/3] setup: always honor GIT_WORK_TREE and core.worktree
From: Maaartin @ 2011-01-19 18:51 UTC (permalink / raw)
  To: Jonathan Nieder
  Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy, git
In-Reply-To: <20110119124230.GD23222@burratino>

On 11-01-19 13:42, Jonathan Nieder wrote:
> Unfortunately the existence of GIT_WORK_TREE makes it tempting to
> use without setting GIT_DIR.

Maybe I'm asking nonsense, but why should I always use both? On the
command line, I either cd to my (alternate) working tree and use GIT_DIR
only or the other way round. I used both several times when I wanted to
compare the (normal) working tree with another commit using something
better than git-diff (git-difftool didn't work for me on cygwin).

^ permalink raw reply

* Re: [PATCH 3/3] setup: always honor GIT_WORK_TREE and core.worktree
From: Jonathan Nieder @ 2011-01-19 19:03 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nguyễn Thái Ngọc Duy, git
In-Reply-To: <20110119124230.GD23222@burratino>

Jonathan Nieder wrote:

> --- a/setup.c
> +++ b/setup.c
> @@ -509,8 +532,10 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)
>  	 * validation.
>  	 */
>  	gitdirenv = getenv(GIT_DIR_ENVIRONMENT);
> -	if (gitdirenv)
> +	if (gitdirenv) {
> +		trace_printf("trace: gitdirenv = %s\n", gitdirenv);
>  		return setup_explicit_git_dir(gitdirenv, cwd, len, nongit_ok);
> +	}
>  

The above hunk is a debugging remnant.  I suppose it was foolish to
send out the series so late.  Probably not such a big deal since the
problem Maaartin mentioned remains anyway (that is, the warning is
still confusing).

-- 8< --
Subject: fixup! setup: always honor GIT_WORK_TREE and core.worktree

Remove noisy debug code.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
diff --git b/setup.c a/setup.c
index c0f5846..e08cdf2 100644
--- b/setup.c
+++ a/setup.c
@@ -532,10 +532,8 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)
 	 * validation.
 	 */
 	gitdirenv = getenv(GIT_DIR_ENVIRONMENT);
-	if (gitdirenv) {
-		trace_printf("trace: gitdirenv = %s\n", gitdirenv);
+	if (gitdirenv)
 		return setup_explicit_git_dir(gitdirenv, cwd, len, nongit_ok);
-	}
 
 	ceil_offset = longest_ancestor_length(cwd, env_ceiling_dirs);
 	if (ceil_offset < 0 && has_dos_drive_prefix(cwd))

^ permalink raw reply related

* Re: [PATCH 3/3] setup: always honor GIT_WORK_TREE and core.worktree
From: Junio C Hamano @ 2011-01-19 19:13 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Nguyễn Thái Ngọc Duy, git
In-Reply-To: <20110119124230.GD23222@burratino>

Jonathan Nieder <jrnieder@gmail.com> writes:

> In the usual case, git commands are run from within a git worktree.
> The toplevel directory and associated .git dir are found by chdir-ing
> to .. repeatedly until .git is a valid repository.
>
> That behavior can be overridden in two ways.

Hmm, I wouldn't call them "two ways".

We originally had GIT_DIR and people who wanted to use a working tree
without an embedded .git (hence having to use GIT_DIR) complained that
they cannot work from subdirectories while cd'ing around; GIT_WORK_TREE
was added to augment the mechanism to allow them to specify where their
root is.  Saying "two ways" sounds as if it is sane to do one without
doing the other, which is not the case here.

For the same reason (another tangent), although t1510 seems to have
complex rules about "relative to", it does not make much sense to specify
a path in GIT_WORK_TREE relative to user's $cwd.  The mechanism is to
allow people to chdir around inside their working tree that does not have
an embedded .git directory to begin with.

> diff --git a/setup.c b/setup.c
> index 3d73269..c0f5846 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -411,6 +411,16 @@ 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. */
> +	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))
> +			die_errno("Could not come back to cwd");
> +		return setup_explicit_git_dir(gitdir, cwd, len, nongit_ok);
> +	}
> +
>  	/* #16.2, #17.2, #20.2, #21.2, #24, #25, #28, #29 (see t1510) */
>  	if (is_bare_repository_cfg > 0) {
>  		set_git_dir(offset == len ? gitdir : make_absolute_path(gitdir));
> @@ -443,6 +453,19 @@ 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.
> +	 */
> +	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");
> +		return setup_explicit_git_dir(gitdir, cwd, len, nongit_ok);
> +	}
> +
>  	inside_git_dir = 1;
>  	inside_work_tree = 0;
>  	if (offset != len) {

This seems to be a good workaround to resurrect the historical buggy
behaviour, but it makes the already ugly code even uglier. Can we do
something about it?

I wonder if it would make it more readable if we restructure
setup_git_directory_gently_1() slightly, perhaps by making the loop to
discover the .git directory/file into a separate helper function that
returns:

 - the path to the discovered .git;
 - if the .git was found as an ancestor of the original $cwd (to handle "bare");
 - if the discovery was aborted due to ceiling (or hitting root);

and make the caller decide what kind of setup to call outside the loop.

But that is a minor point.

^ permalink raw reply

* Re: git bisect problems/ideas
From: Aaron S. Meurer @ 2011-01-19 19:15 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Christian Couder, git
In-Reply-To: <7vd3nukqn8.fsf@alter.siamese.dyndns.org>

So if I care about this issue I should keep bumping it until it gets fixed?

Aaron Meurer

On Jan 18, 2011, at 11:34 AM, Junio C Hamano wrote:

> Christian Couder <christian.couder@gmail.com> writes:
> 
>> Well, bugs are usually fixed within days after they have been
>> reported. Otherwise they are usually documented in the code or in the
>> documentation or in the test suite (with test_expect_failure).
>> 
>> For the rest we rely on people remembering what happened and on
>> people's mailing list searching skills ;-)
> 
> Not really.
> 
> What we do is to take advantage of the fact that issues people do care
> about are important ones, and others that nobody cares about are not worth
> pursuing.
> 
> In a sense, "people forgetting" is a lot more important than "people
> remembering" to filter unimportant issues (issues that are so unimportant
> that even the original complainer does not bother to come back and
> re-raise it).

^ permalink raw reply

* Re: [PATCH 3/3] setup: always honor GIT_WORK_TREE and core.worktree
From: Junio C Hamano @ 2011-01-19 19:24 UTC (permalink / raw)
  To: Maaartin; +Cc: Jonathan Nieder, Nguyễn Thái Ngọc Duy, git
In-Reply-To: <4D373296.6030101@seznam.cz>

Maaartin <grajcar1@seznam.cz> writes:

> On 11-01-19 13:42, Jonathan Nieder wrote:
>> Unfortunately the existence of GIT_WORK_TREE makes it tempting to
>> use without setting GIT_DIR.
>
> Maybe I'm asking nonsense, but why should I always use both? On the
> command line, I either cd to my (alternate) working tree and use GIT_DIR
> only or the other way round.

As long as you were at the root level of these two sets of "working trees",
you don't need GIT_WORK_TREE at all.

We originally had only GIT_DIR and people who wanted to use a working tree
without an embedded .git (hence having to use GIT_DIR) complained that
they cannot work from subdirectories while cd'ing around, because you
declare that you are at the root of your working tree by using GIT_DIR
(naturally, there is no discovery of .git so we won't know where the root
is).  GIT_WORK_TREE was added to augment the mechanism to allow them to
specify where their root is, so that they can set both and then chdir
around inside their working tree.

^ permalink raw reply

* Re: pushing to a remote branch that one does not own
From: Maaartin @ 2011-01-19 19:25 UTC (permalink / raw)
  To: iani; +Cc: git
In-Reply-To: <1295461011273-5940751.post@n2.nabble.com>

On 11-01-19 19:16, iani wrote:
>
> Hello,
> I want to contribute via "push" to a branch which I have previously cloned
> from a remote repository. There is a problem:
>
> If i create a new commit on branch "master", which coincides with
> origin/master, then I can push this commit simply with
>     git push
> However, if i switch to a different branch of the repository which was
> cloned, for example by doing:
>     git checkout origin/lilt
> Then I get the message:
> "You are in 'detached HEAD' state. You can look around, make experimental
> ... (etc)"
> So I HAVE to create my own new branch based on the one downloaded in order
> to start committing and pushing changes. My question is therefore:
>
> Am I strictly limited to committing only on the master / origin/master

AFAIK, there's nothing special about origin, except that it's already
set up to track origin/master.

You can't commit to origin/lilt just like you can't commit to
origin/master. Those remote branches are there to mirror corresponding
branches in the remote repo, so you can do things like
git diff master origin/master

> branched, and forced to make a new branch for every branch that I cloned
> from the remote repo, or is there a way of making the heads of the remote
> branches visible as local too?

AFAIK, using
git branch lilt origin/lilt
you create a local branch tracking the remote one. Then you can work
with it just like you work with master. You may want to have a look at
.git/config for [branch <branchname>] sections or not. I'm quite a
newbie, but this works for me.

^ permalink raw reply

* Re: git bisect problems/ideas
From: Junio C Hamano @ 2011-01-19 19:29 UTC (permalink / raw)
  To: Aaron S. Meurer; +Cc: Christian Couder, git
In-Reply-To: <0F4E18EC-E67D-4C3E-BB5C-C8D8BA326C1D@gmail.com>

"Aaron S. Meurer" <asmeurer@gmail.com> writes:

> So if I care about this issue I should keep bumping it until it gets fixed?

Depends on your definition of "bumping".  If it means "whining repeatedly
without adding anything to help resolving the issue", then no.

I thought the previous discussion was stuck after a small step by Dscho in
the right direction because nobody followed through?

^ permalink raw reply

* Re: [PATCH 3/3] setup: always honor GIT_WORK_TREE and core.worktree
From: Jonathan Nieder @ 2011-01-19 19:31 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: Junio C Hamano, git, Maaartin
In-Reply-To: <AANLkTinE5gNZM_HDJq31qs5ARJn-DrO9HW66cszTayPa@mail.gmail.com>

Nguyen Thai Ngoc Duy wrote:
> 2011/1/19 Jonathan Nieder <jrnieder@gmail.com>:

>> @@ -411,6 +411,16 @@ 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. */
>> +       if (getenv(GIT_WORK_TREE_ENVIRONMENT) || git_work_tree_cfg) {
>
> Can we leave git_work_tree_cfg out? If this code is to support misused
> scripts, then $GIT_WORK_TREE alone ought to be enough.

Sure, it would be easy to exclude git_work_tree_cfg here.  I guess the
relevant question is

Maaartin wrote:
> On 11-01-19 13:42, Jonathan Nieder wrote:

>> Unfortunately the existence of GIT_WORK_TREE makes it tempting to
>> use without setting GIT_DIR.
>
> Maybe I'm asking nonsense, but why should I always use both?

That is, why do we want to discourage setting the work tree without
GIT_DIR in the first place?

The issues seem to be:

1. Previously there was some confusion about what path the worktree
   is relative to.  Now setup_explicit_git_dir makes it clear:
   + GIT_WORK_TREE and --work-tree are relative to the original cwd;
   + the "[core] worktree" setting is relative to the gitdir.

2. Previously there was some confusion about the right order of
   operations --- move to worktree first, or find the git dir?
   Now the setup procedure is clearly "first find git dir, then
   act on worktree".

3. A global "[core] worktree" setting with relative path is
   nonsensical since there is not necessarily a gitdir for it to be
   relative to.

4. Likewise, setting GIT_WORK_TREE when outside any git repository
   has no clear meaning.

5. The interaction with core.bare and implicit bareness are not
   obvious.  Clearly core.bare should conflict with core.worktree,
   but can GIT_WORK_TREE override that?  Maybe
   check_repository_format_gently is the right place for this check
   (rather than the setup procedure).

(1) and (2) have been resolved by your work (nice!), (3) seems like
a case of "don't do that, then", and (4) out to error out in
setup_nongit (though my patch doesn't take care of that).  Given an
answer to (5) we could wholeheartedly and consistently support
worktree with implicit gitdir, as a new feature.

Is that a fair summary?

[...]
>> +               if (offset != len && !is_absolute_path(gitdir))
>> +                       gitdir = xstrdup(make_absolute_path(gitdir));
>
> The behavior regarding relative $GIT_WORK_TREE before nd/setup series
> is inconsistent. If setup_git_directory() is used, work_tree is
> relative to user's cwd. In other cases, when get_git_work_tree() is
> called, work_tree is made absolute relative to _current_ cwd (usually
> at discovered work_tree root). Which way do you want to keep?

The former.  But this is worrisome: scripts using cd_to_toplevel are
getting the latter behavior.  Maybe a warning for relative
GIT_WORK_TREE is in order, like you hinted before.

^ permalink raw reply

* Opinions on my GIT model
From: julia @ 2011-01-19 19:33 UTC (permalink / raw)
  To: git


Hi All,

Working on a GIT model for my projects.

1.  Three main repositories, bare public repo, maintainer repo, developer
repo
2.  Developer1 clones the public repo and makes feature1 branch, updates,
commits changes and pulls/pushes.
3.  Developer2 on their local repo does the same thing and pushes updates.
4.  Maintainer get the green light to rebase feature1 branch with master -
they pull the whole repo, rebase the feature1 branch with master and then
push feature1.
5.  Developer1 pulls feature1 brach again and continues to work, does a
pull/push to feature1 on public repo.

Now, main question is - given that in step 4 maintainer rebase the feature
branch with master then pushed, he is committing a cardinal sin by rebasing
a branch that has already been pushed to the public repo, so if anyone has
made any changes based on the commits originally pushed that now have been
rebased - developer who tries to push those changes will have issues.

Will these be solved by executing of a pull? 

Sorry if these are trivial questions...still trying to wrap my head around
GIT.

Thank you for your help.

Cheers,

Julia
-- 
View this message in context: http://git.661346.n2.nabble.com/Opinions-on-my-GIT-model-tp5941048p5941048.html
Sent from the git mailing list archive at Nabble.com.

^ permalink raw reply

* [PATCH] gitk: Update cherry-pick error message parsing
From: Anders Kaseorg @ 2011-01-19 19:45 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: git

Commit 981ff5c37ae20687c98d98c8689d5e89016026d2 changed the error
message from git cherry-pick from
    Automatic cherry-pick failed.  [...advice...]
to
    error: could not apply 7ab78c9... Do something neat.
    [...advice...]

Update gitk’s regex to match this, restoring the ability to launch git
citool to resolve conflicted cherry-picks.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
---
 gitk |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/gitk b/gitk
index 9cbc09d..a0f7816 100755
--- a/gitk
+++ b/gitk
@@ -9063,7 +9063,7 @@ proc cherrypick {} {
 			to file '%s'.\nPlease commit, reset or stash\
 			your changes and try again." $fname]
 	} elseif {[regexp -line \
-		       {^(CONFLICT \(.*\):|Automatic cherry-pick failed)} \
+		       {^(CONFLICT \(.*\):|Automatic cherry-pick failed|error: could not apply)} \
 		       $err]} {
 	    if {[confirm_popup [mc "Cherry-pick failed because of merge\
 			conflict.\nDo you wish to run git citool to\
-- 
1.7.4.rc1

^ permalink raw reply related

* Re: git bisect problems/ideas
From: Aaron S. Meurer @ 2011-01-19 19:44 UTC (permalink / raw)
  To: Christian Couder; +Cc: git, SZEDER Gábor, Jonathan Nieder
In-Reply-To: <AANLkTi=A2Twepg3Jo_VYxtvghkhx6ixcpRH3332BoRQo@mail.gmail.com>

>>> Yeah, many people find it difficult to reverse the meaning of "bad"
>>> and "good" when looking for a fix. There were some suggestions at some
>>> points to do something about it. Some of the suggestions were to use
>>> some aliases for "good" and "bad", but there was no agreement. Other
>>> suggestions had a patch attached but the patch was not good enough or
>>> something.
>>> 
>>> Anyway, the restriction is that the "bad" commit cannot be an ancestor
>>> of a "good" commit. But the "good" commits need not be all ancestors
>>> of the "bad" commit. For example if you have a "master" branch and a
>>> "dev" branch that was forked from the "master" branch at one point,
>>> like that:
>>> 
>>> A-B-C-D-E <-- master
>>>     \F-G <-- dev
>>> 
>> 
>> I don't understand how this can only be one way?  Isn't this symmetric?  In
>> other words, how is it different from
>> 
>> A-B-C-D-E <-- dev
>>    \F-G <-- master
>> 
>> as far as bisect is concerned? Or maybe I am not entirely clear on what you
>> are saying.
> 
> Yes, it is symmetric, so we cannot just automatically reverse the
> meanning because there is no "after" or "before" relationship between
> "dev" and "master".
> 
> Best regards,
> Christian.


I think I understand.  What if something works in A, gets broken in C, stays broken in E, but gets fixed in G?  Should it maybe be implied by whichever one is marked as good first, A or G, if you trying to find the fix or the break?

If no, I think --reverse is actually a suitable fix.

Aaron Meurer

^ permalink raw reply

* [PATCH 1/3] gitk: Remove unused $cdate array
From: Anders Kaseorg @ 2011-01-19 19:46 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: git

It was unused since commit 9f1afe05c3ab7228e21ba3666c6e35d693149b37.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
---
 gitk |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/gitk b/gitk
index a0f7816..bf68959 100755
--- a/gitk
+++ b/gitk
@@ -1621,7 +1621,7 @@ proc readcommit {id} {
 }
 
 proc parsecommit {id contents listed} {
-    global commitinfo cdate
+    global commitinfo
 
     set inhdr 1
     set comment {}
@@ -1671,9 +1671,6 @@ proc parsecommit {id contents listed} {
 	}
 	set comment $newcomment
     }
-    if {$comdate != {}} {
-	set cdate($id) $comdate
-    }
     set commitinfo($id) [list $headline $auname $audate \
 			     $comname $comdate $comment]
 }
-- 
1.7.4.rc1

^ permalink raw reply related

* [PATCH 2/3] gitk: Remember time zones from author and commit timestamps
From: Anders Kaseorg @ 2011-01-19 19:47 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: git
In-Reply-To: <alpine.DEB.2.02.1101191445130.23868@dr-wily.mit.edu>

When resolving a conflicted cherry-pick, this lets us pass
GIT_AUTHOR_DATE to git citool with the correct timezone.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
---
 gitk |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/gitk b/gitk
index bf68959..5c6058a 100755
--- a/gitk
+++ b/gitk
@@ -659,7 +659,7 @@ proc newvarc {view id} {
 	if {![info exists commitinfo($id)]} {
 	    parsecommit $id $commitdata($id) 1
 	}
-	set cdate [lindex $commitinfo($id) 4]
+	set cdate [lindex [lindex $commitinfo($id) 4] 0]
 	if {![string is integer -strict $cdate]} {
 	    set cdate 0
 	}
@@ -1641,10 +1641,10 @@ proc parsecommit {id contents listed} {
 	set line [split $line " "]
 	set tag [lindex $line 0]
 	if {$tag == "author"} {
-	    set audate [lindex $line end-1]
+	    set audate [lrange $line end-1 end]
 	    set auname [join [lrange $line 1 end-2] " "]
 	} elseif {$tag == "committer"} {
-	    set comdate [lindex $line end-1]
+	    set comdate [lrange $line end-1 end]
 	    set comname [join [lrange $line 1 end-2] " "]
 	}
     }
@@ -11018,7 +11018,7 @@ proc prefsok {} {
 proc formatdate {d} {
     global datetimeformat
     if {$d ne {}} {
-	set d [clock format $d -format $datetimeformat]
+	set d [clock format [lindex $d 0] -format $datetimeformat]
     }
     return $d
 }
-- 
1.7.4.rc1

^ permalink raw reply related

* [PATCH 3/3] gitk: Allow displaying time zones from author and commit timestamps
From: Anders Kaseorg @ 2011-01-19 19:47 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: git
In-Reply-To: <alpine.DEB.2.02.1101191445130.23868@dr-wily.mit.edu>

Now gitk can be configured to display author and commit dates in their
original timezone, by putting %z into datetimeformat in ~/.gitk.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
---
 gitk |   13 ++++++++++++-
 1 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/gitk b/gitk
index 5c6058a..a072593 100755
--- a/gitk
+++ b/gitk
@@ -11018,7 +11018,18 @@ proc prefsok {} {
 proc formatdate {d} {
     global datetimeformat
     if {$d ne {}} {
-	set d [clock format [lindex $d 0] -format $datetimeformat]
+	if {[string match {*%[zZ]*} $datetimeformat]} {
+	    if {[catch {set d [clock format [lindex $d 0] -timezone [lindex $d 1] -format $datetimeformat]}]} {
+		# Tcl < 8.5 does not support -timezone.
+		global env
+		set zone [lindex $d 1]
+		set env(TZ) "IDK[string range $zone 0 2]:[string range $zone 3 end]"
+		set d [clock format [lindex $d 0] -format $datetimeformat]
+		unset env(TZ)
+	    }
+	} else {
+	    set d [clock format [lindex $d 0] -format $datetimeformat]
+	}
     }
     return $d
 }
-- 
1.7.4.rc1

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox