git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: David Aguilar <davvid@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: John Keeping <john@keeping.me.uk>,
	Bernhard Kirchen <bernhard.kirchen@rwth-aachen.de>,
	Tim Henigan <tim.henigan@gmail.com>,
	Git Mailing List <git@vger.kernel.org>
Subject: [PATCH 2/3] difftool: avoid $GIT_DIR and $GIT_WORK_TREE
Date: Mon, 18 Jul 2016 20:57:55 -0700	[thread overview]
Message-ID: <20160719035756.24961-2-davvid@gmail.com> (raw)
In-Reply-To: <20160719035756.24961-1-davvid@gmail.com>

Environment variables are global and hard to reason about.
Use the `--git-dir` and `--work-tree` arguments when invoking `git`
instead of relying on the environment.

Add a test to ensure that difftool's dir-diff feature works when these
variables are present in the environment.

Signed-off-by: David Aguilar <davvid@gmail.com>
---
 git-difftool.perl   | 27 ++++++---------------------
 t/t7800-difftool.sh | 16 ++++++++++++++++
 2 files changed, 22 insertions(+), 21 deletions(-)

diff --git a/git-difftool.perl b/git-difftool.perl
index c9d3ef8..bc2267f 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -83,20 +83,17 @@ sub changed_files
 {
 	my ($repo_path, $index, $worktree) = @_;
 	$ENV{GIT_INDEX_FILE} = $index;
-	$ENV{GIT_WORK_TREE} = $worktree;
-	my $must_unset_git_dir = 0;
-	if (not defined($ENV{GIT_DIR})) {
-		$must_unset_git_dir = 1;
-		$ENV{GIT_DIR} = $repo_path;
-	}
 
-	my @refreshargs = qw/update-index --really-refresh -q --unmerged/;
-	my @gitargs = qw/diff-files --name-only -z/;
+	my @gitargs = ('--git-dir', $repo_path, '--work-tree', $worktree);
+	my @refreshargs = (
+		@gitargs, 'update-index',
+		'--really-refresh', '-q', '--unmerged');
+	my @diffargs = (@gitargs, 'diff-files', '--name-only', '-z');
 	try {
 		Git::command_oneline(@refreshargs);
 	} catch Git::Error::Command with {};
 
-	my $line = Git::command_oneline(@gitargs);
+	my $line = Git::command_oneline(@diffargs);
 	my @files;
 	if (defined $line) {
 		@files = split('\0', $line);
@@ -105,8 +102,6 @@ sub changed_files
 	}
 
 	delete($ENV{GIT_INDEX_FILE});
-	delete($ENV{GIT_WORK_TREE});
-	delete($ENV{GIT_DIR}) if ($must_unset_git_dir);
 
 	return map { $_ => 1 } @files;
 }
@@ -204,15 +199,6 @@ EOF
 	mkpath($ldir) or exit_cleanup($tmpdir, 1);
 	mkpath($rdir) or exit_cleanup($tmpdir, 1);
 
-	# If $GIT_DIR is not set prior to calling 'git update-index' and
-	# 'git checkout-index', then those commands will fail if difftool
-	# is called from a directory other than the repo root.
-	my $must_unset_git_dir = 0;
-	if (not defined($ENV{GIT_DIR})) {
-		$must_unset_git_dir = 1;
-		$ENV{GIT_DIR} = $repo_path;
-	}
-
 	# Populate the left and right directories based on each index file
 	my ($inpipe, $ctx);
 	$ENV{GIT_INDEX_FILE} = "$tmpdir/lindex";
@@ -241,7 +227,6 @@ EOF
 
 	# If $GIT_DIR was explicitly set just for the update/checkout
 	# commands, then it should be unset before continuing.
-	delete($ENV{GIT_DIR}) if ($must_unset_git_dir);
 	delete($ENV{GIT_INDEX_FILE});
 
 	# Changes in the working tree need special treatment since they are
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index 42a2929..fa43c24 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -412,6 +412,22 @@ run_dir_diff_test 'difftool --dir-diff from subdirectory' '
 	)
 '
 
+run_dir_diff_test 'difftool --dir-diff from subdirectory with GIT_DIR set' '
+	(
+		GIT_DIR=$(pwd)/.git &&
+		export GIT_DIR &&
+		GIT_WORK_TREE=$(pwd) &&
+		export GIT_WORK_TREE &&
+		cd sub &&
+		git difftool --dir-diff $symlinks --extcmd ls \
+			branch -- sub >output &&
+		sane_unset GIT_WORK_TREE &&
+		sane_unset GIT_DIR &&
+		grep sub output &&
+		! grep file output
+	)
+'
+
 run_dir_diff_test 'difftool --dir-diff when worktree file is missing' '
 	test_when_finished git reset --hard &&
 	rm file2 &&
-- 
2.9.2.280.g385e27a


  reply	other threads:[~2016-07-19  3:58 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-19  3:57 [PATCH 1/3] difftool: fix argument handling in subdirs David Aguilar
2016-07-19  3:57 ` David Aguilar [this message]
2016-07-19 18:19   ` [PATCH 2/3] difftool: avoid $GIT_DIR and $GIT_WORK_TREE Junio C Hamano
2016-07-19 21:06     ` Junio C Hamano
2016-07-20 22:25       ` David Aguilar
2016-07-19  3:57 ` [PATCH 3/3] difftool: use Git::* functions instead of passing around state David Aguilar
2016-07-19 18:29   ` Junio C Hamano
2016-07-19 18:15 ` [PATCH 1/3] difftool: fix argument handling in subdirs Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160719035756.24961-2-davvid@gmail.com \
    --to=davvid@gmail.com \
    --cc=bernhard.kirchen@rwth-aachen.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=john@keeping.me.uk \
    --cc=tim.henigan@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).