difftool: avoid $GIT_DIR and $GIT_WORK_TREE

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>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/git-difftool.perl b/git-difftool.perl
index c9d3ef887f9897c7b952e363706696abf37017ce..c81cbe4ffa73f5c19710268a26b9af916c72b93a 100755 (executable)
--- 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');
        try {
                Git::command_oneline(@refreshargs);
        } catch Git::Error::Command with {};
 
-       my $line = Git::command_oneline(@gitargs);
+       my @diffargs = (@gitargs, 'diff-files', '--name-only', '-z');
+       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 7ce4cd753eb0c50f42b68653da79f90b19edd764..cb25480e1f7282abf93d1dc2b4d3a914a3d8a40d 100755 (executable)
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -412,6 +412,20 @@ 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 &&
+               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 &&