Re: [PATCH 39/47] setup: limit get_git_work_tree()'s to explicit setup case only

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Jan 18, 2011, 07:44 UTC
Message-ID: <20110118074400.GA4185@burratino>
In-Reply-To: <1290785563-15339-40-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy wrote:

Show 5 quoted lines

> get_git_work_tree() takes input as core.worktree, core.bare,
> GIT_WORK_TREE and decides correct worktree setting.
>
> Unfortunately it does not do its job well. core.worktree and
> GIT_WORK_TREE should only be taken into account, if GIT_DIR is set

As advertised, before this change

	GIT_WORK_TREE=$path git add .

paid attention to $GIT_WORK_TREE (and set GIT_DIR=.git), and afterwards the GIT_WORK_TREE setting has been ignored when GIT_DIR is not set explicitly.

Unfortunately, that breaks some scripts. Example: using the "git import-dsc" utility from git-buildpackage to import a source package:

	debsnap libwpd 0.7.2-1
	git clone git://libwpd.git.sourceforge.net/gitroot/libwpd/libwpd
	git tag upstream/0.7.2 RELEASE-0-7-2^{commit}
	git reset --keep origin/master
	git import-dsc ../source-libwpd/libwpd_0.7.2-1.dsc 
	echo $?
	git show --raw
	echo done.

With git v1.7.4-rc2:

	HEAD is now at d183cc6 Imported Debian patch 0.7.2-1
	gbp:info: Everything imported under 'libwpd'
	0
	commit d183cc6c4b8dcaf22b1b0875aad0655846ed3b1b
	Author: Masayuki Hatta (mhatta) <mhatta@debian.org>
	Date:   Tue Aug 10 00:37:47 2004 +0900

	    Imported Debian patch 0.7.2-1

	done

versus v1.7.3.2:

	HEAD is now at 735a8f2 Imported Debian patch 0.7.2-1
	gbp:info: Everything imported under 'libwpd'
	0
	commit 735a8f21ea817417ecc035817f777bd2943d4efc
	Author: Masayuki Hatta (mhatta) <mhatta@debian.org>
	Date:   Tue Aug 10 00:37:47 2004 +0900

	    Imported Debian patch 0.7.2-1

	:100644 000000 038cd09... 0000000... D  .gitignore
	:000000 100644 0000000... 0189791... A  CHANGES
	[...]

git-buildpackage uses the equivalent of

	GIT_INDEX_FILE=$(pwd)/.git/gbp_index \
	GIT_WORK_TREE=$unpack_dir \
	git add -f .

when creating the commit, so with v1.7.4-rc0 and later it is taking its content from the cwd, producing an incorrect result without complaint.

How about something like this patch? It only concerns the GIT_WORK_TREE variable and --work-tree command line option; the semantics of core.worktree are unaffected.

(Sorry, no real patch yet. Updating the test to reflect the change is a pain in the neck.)

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---

diff --git a/setup.c b/setup.c
index 3d73269..28b2fef 100644
--- a/setup.c
+++ 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);
+	}
+
 	/* #0, #1, #5, #8, #9, #12, #13 */
 	set_git_work_tree(".");
 	if (strcmp(gitdir, DEFAULT_GIT_DIR_ENVIRONMENT))
@@ -443,6 +448,11 @@ static const char *setup_bare_git_dir(char *cwd, int offset, int len, int *nongi
 	if (check_repository_format_gently(".", nongit_ok))
 		return NULL;
 
+	if (getenv(GIT_WORK_TREE_ENVIRONMENT)) {
+		warning("GIT_WORK_TREE without explicit GIT_DIR is deprecated");
+		return setup_explicit_git_dir(".", cwd, offset, nongit_ok);
+	}
+
 	inside_git_dir = 1;
 	inside_work_tree = 0;
 	if (offset != len) {