{"thread":{"id":"42876","subject":"[PATCH 1/3] difftool: fix argument handling in subdirs","startedAt":"2016-07-19T03:58:06Z","lastAt":"2016-07-20T22:25:32Z","messageCount":8,"participants":["David Aguilar","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"291701","messageId":"20160719035756.24961-1-davvid@gmail.com","threadId":"42876","inReplyTo":null,"subject":"[PATCH 1/3] difftool: fix argument handling in subdirs","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2016-07-19T03:57:54Z","receivedAt":"2016-07-19T03:58:06Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"From: John Keeping <john@keeping.me.uk>\n\nWhen in a subdirectory of a repository, path arguments should be\ninterpreted relative to the current directory not the root of the\nworking tree.\n\nThe Git::repository object passed into setup_dir_diff() is configured to\nhandle this correctly but we create a new Git::repository here without\nsetting the WorkingSubdir argument.  By simply using the existing\nrepository, path arguments are handled relative to the current\ndirectory.\n\nReported-by: Bernhard Kirchen <bernhard.kirchen@rwth-aachen.de>\nSigned-off-by: John Keeping <john@keeping.me.uk>\nAcked-by: David Aguilar <davvid@gmail.com>\n---\nThis patch is unchanged from John's version but also includes\nReported-by and Acked-by lines.\n\n git-difftool.perl | 13 +++----------\n 1 file changed, 3 insertions(+), 10 deletions(-)\n\ndiff --git a/git-difftool.perl b/git-difftool.perl\nindex ebd13ba..c9d3ef8 100755\n--- a/git-difftool.perl\n+++ b/git-difftool.perl\n@@ -115,16 +115,9 @@ sub setup_dir_diff\n {\n \tmy ($repo, $workdir, $symlinks) = @_;\n \n-\t# Run the diff; exit immediately if no diff found\n-\t# 'Repository' and 'WorkingCopy' must be explicitly set to insure that\n-\t# if $GIT_DIR and $GIT_WORK_TREE are set in ENV, they are actually used\n-\t# by Git->repository->command*.\n \tmy $repo_path = $repo->repo_path();\n-\tmy %repo_args = (Repository => $repo_path, WorkingCopy => $workdir);\n-\tmy $diffrepo = Git->repository(%repo_args);\n-\n \tmy @gitargs = ('diff', '--raw', '--no-abbrev', '-z', @ARGV);\n-\tmy $diffrtn = $diffrepo->command_oneline(@gitargs);\n+\tmy $diffrtn = $repo->command_oneline(@gitargs);\n \texit(0) unless defined($diffrtn);\n \n \t# Build index info for left and right sides of the diff\n@@ -176,12 +169,12 @@ EOF\n \n \t\tif ($lmode eq $symlink_mode) {\n \t\t\t$symlink{$src_path}{left} =\n-\t\t\t\t$diffrepo->command_oneline('show', \"$lsha1\");\n+\t\t\t\t$repo->command_oneline('show', \"$lsha1\");\n \t\t}\n \n \t\tif ($rmode eq $symlink_mode) {\n \t\t\t$symlink{$dst_path}{right} =\n-\t\t\t\t$diffrepo->command_oneline('show', \"$rsha1\");\n+\t\t\t\t$repo->command_oneline('show', \"$rsha1\");\n \t\t}\n \n \t\tif ($lmode ne $null_mode and $status !~ /^C/) {\n-- \n2.9.2.280.g385e27a\n\n"},{"id":"291702","messageId":"20160719035756.24961-2-davvid@gmail.com","threadId":"42876","inReplyTo":"20160719035756.24961-1-davvid@gmail.com","subject":"[PATCH 2/3] difftool: avoid $GIT_DIR and $GIT_WORK_TREE","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2016-07-19T03:57:55Z","receivedAt":"2016-07-19T03:58:12Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"Environment variables are global and hard to reason about.\nUse the `--git-dir` and `--work-tree` arguments when invoking `git`\ninstead of relying on the environment.\n\nAdd a test to ensure that difftool's dir-diff feature works when these\nvariables are present in the environment.\n\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\n git-difftool.perl   | 27 ++++++---------------------\n t/t7800-difftool.sh | 16 ++++++++++++++++\n 2 files changed, 22 insertions(+), 21 deletions(-)\n\ndiff --git a/git-difftool.perl b/git-difftool.perl\nindex c9d3ef8..bc2267f 100755\n--- a/git-difftool.perl\n+++ b/git-difftool.perl\n@@ -83,20 +83,17 @@ sub changed_files\n {\n \tmy ($repo_path, $index, $worktree) = @_;\n \t$ENV{GIT_INDEX_FILE} = $index;\n-\t$ENV{GIT_WORK_TREE} = $worktree;\n-\tmy $must_unset_git_dir = 0;\n-\tif (not defined($ENV{GIT_DIR})) {\n-\t\t$must_unset_git_dir = 1;\n-\t\t$ENV{GIT_DIR} = $repo_path;\n-\t}\n \n-\tmy @refreshargs = qw/update-index --really-refresh -q --unmerged/;\n-\tmy @gitargs = qw/diff-files --name-only -z/;\n+\tmy @gitargs = ('--git-dir', $repo_path, '--work-tree', $worktree);\n+\tmy @refreshargs = (\n+\t\t@gitargs, 'update-index',\n+\t\t'--really-refresh', '-q', '--unmerged');\n+\tmy @diffargs = (@gitargs, 'diff-files', '--name-only', '-z');\n \ttry {\n \t\tGit::command_oneline(@refreshargs);\n \t} catch Git::Error::Command with {};\n \n-\tmy $line = Git::command_oneline(@gitargs);\n+\tmy $line = Git::command_oneline(@diffargs);\n \tmy @files;\n \tif (defined $line) {\n \t\t@files = split('\\0', $line);\n@@ -105,8 +102,6 @@ sub changed_files\n \t}\n \n \tdelete($ENV{GIT_INDEX_FILE});\n-\tdelete($ENV{GIT_WORK_TREE});\n-\tdelete($ENV{GIT_DIR}) if ($must_unset_git_dir);\n \n \treturn map { $_ => 1 } @files;\n }\n@@ -204,15 +199,6 @@ EOF\n \tmkpath($ldir) or exit_cleanup($tmpdir, 1);\n \tmkpath($rdir) or exit_cleanup($tmpdir, 1);\n \n-\t# If $GIT_DIR is not set prior to calling 'git update-index' and\n-\t# 'git checkout-index', then those commands will fail if difftool\n-\t# is called from a directory other than the repo root.\n-\tmy $must_unset_git_dir = 0;\n-\tif (not defined($ENV{GIT_DIR})) {\n-\t\t$must_unset_git_dir = 1;\n-\t\t$ENV{GIT_DIR} = $repo_path;\n-\t}\n-\n \t# Populate the left and right directories based on each index file\n \tmy ($inpipe, $ctx);\n \t$ENV{GIT_INDEX_FILE} = \"$tmpdir/lindex\";\n@@ -241,7 +227,6 @@ EOF\n \n \t# If $GIT_DIR was explicitly set just for the update/checkout\n \t# commands, then it should be unset before continuing.\n-\tdelete($ENV{GIT_DIR}) if ($must_unset_git_dir);\n \tdelete($ENV{GIT_INDEX_FILE});\n \n \t# Changes in the working tree need special treatment since they are\ndiff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\nindex 42a2929..fa43c24 100755\n--- a/t/t7800-difftool.sh\n+++ b/t/t7800-difftool.sh\n@@ -412,6 +412,22 @@ run_dir_diff_test 'difftool --dir-diff from subdirectory' '\n \t)\n '\n \n+run_dir_diff_test 'difftool --dir-diff from subdirectory with GIT_DIR set' '\n+\t(\n+\t\tGIT_DIR=$(pwd)/.git &&\n+\t\texport GIT_DIR &&\n+\t\tGIT_WORK_TREE=$(pwd) &&\n+\t\texport GIT_WORK_TREE &&\n+\t\tcd sub &&\n+\t\tgit difftool --dir-diff $symlinks --extcmd ls \\\n+\t\t\tbranch -- sub >output &&\n+\t\tsane_unset GIT_WORK_TREE &&\n+\t\tsane_unset GIT_DIR &&\n+\t\tgrep sub output &&\n+\t\t! grep file output\n+\t)\n+'\n+\n run_dir_diff_test 'difftool --dir-diff when worktree file is missing' '\n \ttest_when_finished git reset --hard &&\n \trm file2 &&\n-- \n2.9.2.280.g385e27a\n\n"},{"id":"291703","messageId":"20160719035756.24961-3-davvid@gmail.com","threadId":"42876","inReplyTo":"20160719035756.24961-1-davvid@gmail.com","subject":"[PATCH 3/3] difftool: use Git::* functions instead of passing around state","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2016-07-19T03:57:56Z","receivedAt":"2016-07-19T03:58:16Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"Call Git::command() and friends directly wherever possible.\nThis makes it clear that these operations can be invoked directly\nwithout needing to manage the current directory and related GIT_*\nenvironment variables.\n\nEliminate find_repository() since we can now use wc_path() and\nnot worry about side-effects involving environment variables.\n\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\n git-difftool.perl | 54 ++++++++++++++++++++++--------------------------------\n 1 file changed, 22 insertions(+), 32 deletions(-)\n\ndiff --git a/git-difftool.perl b/git-difftool.perl\nindex bc2267f..a5790d0 100755\n--- a/git-difftool.perl\n+++ b/git-difftool.perl\n@@ -37,14 +37,6 @@ USAGE\n \texit($exitcode);\n }\n \n-sub find_worktree\n-{\n-\t# Git->repository->wc_path() does not honor changes to the working\n-\t# tree location made by $ENV{GIT_WORK_TREE} or the 'core.worktree'\n-\t# config variable.\n-\treturn Git::command_oneline('rev-parse', '--show-toplevel');\n-}\n-\n sub print_tool_help\n {\n \t# See the comment at the bottom of file_diff() for the reason behind\n@@ -67,14 +59,14 @@ sub exit_cleanup\n \n sub use_wt_file\n {\n-\tmy ($repo, $workdir, $file, $sha1) = @_;\n+\tmy ($workdir, $file, $sha1) = @_;\n \tmy $null_sha1 = '0' x 40;\n \n \tif (-l \"$workdir/$file\" || ! -e _) {\n \t\treturn (0, $null_sha1);\n \t}\n \n-\tmy $wt_sha1 = $repo->command_oneline('hash-object', \"$workdir/$file\");\n+\tmy $wt_sha1 = Git::command_oneline('hash-object', \"$workdir/$file\");\n \tmy $use = ($sha1 eq $null_sha1) || ($sha1 eq $wt_sha1);\n \treturn ($use, $wt_sha1);\n }\n@@ -88,11 +80,11 @@ sub changed_files\n \tmy @refreshargs = (\n \t\t@gitargs, 'update-index',\n \t\t'--really-refresh', '-q', '--unmerged');\n-\tmy @diffargs = (@gitargs, 'diff-files', '--name-only', '-z');\n \ttry {\n \t\tGit::command_oneline(@refreshargs);\n \t} catch Git::Error::Command with {};\n \n+\tmy @diffargs = (@gitargs, 'diff-files', '--name-only', '-z');\n \tmy $line = Git::command_oneline(@diffargs);\n \tmy @files;\n \tif (defined $line) {\n@@ -108,11 +100,9 @@ sub changed_files\n \n sub setup_dir_diff\n {\n-\tmy ($repo, $workdir, $symlinks) = @_;\n-\n-\tmy $repo_path = $repo->repo_path();\n+\tmy ($workdir, $symlinks) = @_;\n \tmy @gitargs = ('diff', '--raw', '--no-abbrev', '-z', @ARGV);\n-\tmy $diffrtn = $repo->command_oneline(@gitargs);\n+\tmy $diffrtn = Git::command_oneline(@gitargs);\n \texit(0) unless defined($diffrtn);\n \n \t# Build index info for left and right sides of the diff\n@@ -164,12 +154,12 @@ EOF\n \n \t\tif ($lmode eq $symlink_mode) {\n \t\t\t$symlink{$src_path}{left} =\n-\t\t\t\t$repo->command_oneline('show', \"$lsha1\");\n+\t\t\t\tGit::command_oneline('show', $lsha1);\n \t\t}\n \n \t\tif ($rmode eq $symlink_mode) {\n \t\t\t$symlink{$dst_path}{right} =\n-\t\t\t\t$repo->command_oneline('show', \"$rsha1\");\n+\t\t\t\tGit::command_oneline('show', $rsha1);\n \t\t}\n \n \t\tif ($lmode ne $null_mode and $status !~ /^C/) {\n@@ -181,8 +171,8 @@ EOF\n \t\t\tif ($working_tree_dups{$dst_path}++) {\n \t\t\t\tnext;\n \t\t\t}\n-\t\t\tmy ($use, $wt_sha1) = use_wt_file($repo, $workdir,\n-\t\t\t\t\t\t\t  $dst_path, $rsha1);\n+\t\t\tmy ($use, $wt_sha1) =\n+\t\t\t\tuse_wt_file($workdir, $dst_path, $rsha1);\n \t\t\tif ($use) {\n \t\t\t\tpush @working_tree, $dst_path;\n \t\t\t\t$wtindex .= \"$rmode $wt_sha1\\t$dst_path\\0\";\n@@ -203,27 +193,27 @@ EOF\n \tmy ($inpipe, $ctx);\n \t$ENV{GIT_INDEX_FILE} = \"$tmpdir/lindex\";\n \t($inpipe, $ctx) =\n-\t\t$repo->command_input_pipe(qw(update-index -z --index-info));\n+\t\tGit::command_input_pipe('update-index', '-z', '--index-info');\n \tprint($inpipe $lindex);\n-\t$repo->command_close_pipe($inpipe, $ctx);\n+\tGit::command_close_pipe($inpipe, $ctx);\n \n \tmy $rc = system('git', 'checkout-index', '--all', \"--prefix=$ldir/\");\n \texit_cleanup($tmpdir, $rc) if $rc != 0;\n \n \t$ENV{GIT_INDEX_FILE} = \"$tmpdir/rindex\";\n \t($inpipe, $ctx) =\n-\t\t$repo->command_input_pipe(qw(update-index -z --index-info));\n+\t\tGit::command_input_pipe('update-index', '-z', '--index-info');\n \tprint($inpipe $rindex);\n-\t$repo->command_close_pipe($inpipe, $ctx);\n+\tGit::command_close_pipe($inpipe, $ctx);\n \n \t$rc = system('git', 'checkout-index', '--all', \"--prefix=$rdir/\");\n \texit_cleanup($tmpdir, $rc) if $rc != 0;\n \n \t$ENV{GIT_INDEX_FILE} = \"$tmpdir/wtindex\";\n \t($inpipe, $ctx) =\n-\t\t$repo->command_input_pipe(qw(update-index --info-only -z --index-info));\n+\t\tGit::command_input_pipe('update-index', '--info-only', '-z', '--index-info');\n \tprint($inpipe $wtindex);\n-\t$repo->command_close_pipe($inpipe, $ctx);\n+\tGit::command_close_pipe($inpipe, $ctx);\n \n \t# If $GIT_DIR was explicitly set just for the update/checkout\n \t# commands, then it should be unset before continuing.\n@@ -393,9 +383,9 @@ sub dir_diff\n \tmy $rc;\n \tmy $error = 0;\n \tmy $repo = Git->repository();\n-\tmy $workdir = find_worktree();\n-\tmy ($a, $b, $tmpdir, @worktree) =\n-\t\tsetup_dir_diff($repo, $workdir, $symlinks);\n+\tmy $repo_path = $repo->repo_path();\n+\tmy $workdir = $repo->wc_path();\n+\tmy ($a, $b, $tmpdir, @worktree) = setup_dir_diff($workdir, $symlinks);\n \n \tif (defined($extcmd)) {\n \t\t$rc = system($extcmd, $a, $b);\n@@ -421,10 +411,10 @@ sub dir_diff\n \t\tnext if ! -f \"$b/$file\";\n \n \t\tif (!$indices_loaded) {\n-\t\t\t%wt_modified = changed_files($repo->repo_path(),\n-\t\t\t\t\"$tmpdir/wtindex\", \"$workdir\");\n-\t\t\t%tmp_modified = changed_files($repo->repo_path(),\n-\t\t\t\t\"$tmpdir/wtindex\", \"$b\");\n+\t\t\t%wt_modified = changed_files(\n+\t\t\t\t$repo_path, \"$tmpdir/wtindex\", $workdir);\n+\t\t\t%tmp_modified = changed_files(\n+\t\t\t\t$repo_path, \"$tmpdir/wtindex\", $b);\n \t\t\t$indices_loaded = 1;\n \t\t}\n \n-- \n2.9.2.280.g385e27a\n\n"},{"id":"291741","messageId":"xmqq37n5h4ub.fsf@gitster.mtv.corp.google.com","threadId":"42876","inReplyTo":"20160719035756.24961-1-davvid@gmail.com","subject":"Re: [PATCH 1/3] difftool: fix argument handling in subdirs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-19T18:15:24Z","receivedAt":"2016-07-19T18:15:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Aguilar <davvid@gmail.com> writes:\n\n> From: John Keeping <john@keeping.me.uk>\n>\n> When in a subdirectory of a repository, path arguments should be\n> ...\n> directory.\n>\n> Reported-by: Bernhard Kirchen <bernhard.kirchen@rwth-aachen.de>\n> Signed-off-by: John Keeping <john@keeping.me.uk>\n> Acked-by: David Aguilar <davvid@gmail.com>\n> ---\n> This patch is unchanged from John's version but also includes\n> Reported-by and Acked-by lines.\n\nThanks; will replace.\n"},{"id":"291742","messageId":"xmqqy44xfq3n.fsf@gitster.mtv.corp.google.com","threadId":"42876","inReplyTo":"20160719035756.24961-2-davvid@gmail.com","subject":"Re: [PATCH 2/3] difftool: avoid $GIT_DIR and $GIT_WORK_TREE","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-19T18:19:08Z","receivedAt":"2016-07-19T18:19:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Aguilar <davvid@gmail.com> writes:\n\n> Environment variables are global and hard to reason about.\n> Use the `--git-dir` and `--work-tree` arguments when invoking `git`\n> instead of relying on the environment.\n>\n> Add a test to ensure that difftool's dir-diff feature works when these\n> variables are present in the environment.\n\nYes, that is a very good idea.  I was wondering what would happen\nwith that \"must-not-unset-nor-smudge if it already is set\" thing.\n\n> diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\n> index 42a2929..fa43c24 100755\n> --- a/t/t7800-difftool.sh\n> +++ b/t/t7800-difftool.sh\n> @@ -412,6 +412,22 @@ run_dir_diff_test 'difftool --dir-diff from subdirectory' '\n>  \t)\n>  '\n>  \n> +run_dir_diff_test 'difftool --dir-diff from subdirectory with GIT_DIR set' '\n> +\t(\n> +\t\tGIT_DIR=$(pwd)/.git &&\n> +\t\texport GIT_DIR &&\n> +\t\tGIT_WORK_TREE=$(pwd) &&\n> +\t\texport GIT_WORK_TREE &&\n> +\t\tcd sub &&\n> +\t\tgit difftool --dir-diff $symlinks --extcmd ls \\\n> +\t\t\tbranch -- sub >output &&\n> +\t\tsane_unset GIT_WORK_TREE &&\n> +\t\tsane_unset GIT_DIR &&\n\nIt is not wrong per-se, but as you are in a subshell, you do not\nhave to unset these, I would think.  Not worth a reroll, but unless\nI am overlooking something (in which case please holler) I'm\ninclined to remove these two lines myself while queuing the series.\n\nThanks.\n\n> +\t\tgrep sub output &&\n> +\t\t! grep file output\n> +\t)\n> +'\n"},{"id":"291744","messageId":"xmqqtwflfpn6.fsf@gitster.mtv.corp.google.com","threadId":"42876","inReplyTo":"20160719035756.24961-3-davvid@gmail.com","subject":"Re: [PATCH 3/3] difftool: use Git::* functions instead of passing around state","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-19T18:29:01Z","receivedAt":"2016-07-19T18:29:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Aguilar <davvid@gmail.com> writes:\n\n> Call Git::command() and friends directly wherever possible.\n> This makes it clear that these operations can be invoked directly\n> without needing to manage the current directory and related GIT_*\n> environment variables.\n\nGood.\n\n> @@ -88,11 +80,11 @@ sub changed_files\n>  \tmy @refreshargs = (\n>  \t\t@gitargs, 'update-index',\n>  \t\t'--really-refresh', '-q', '--unmerged');\n> -\tmy @diffargs = (@gitargs, 'diff-files', '--name-only', '-z');\n>  \ttry {\n>  \t\tGit::command_oneline(@refreshargs);\n>  \t} catch Git::Error::Command with {};\n>  \n> +\tmy @diffargs = (@gitargs, 'diff-files', '--name-only', '-z');\n>  \tmy $line = Git::command_oneline(@diffargs);\n>  \tmy @files;\n>  \tif (defined $line) {\n\nThis hunk looks like \"oops, the @diffargs variable logically belongs\nhere, not there\", correcting an mistake in an earlier step.\n"},{"id":"291769","messageId":"xmqqvb01e3s4.fsf@gitster.mtv.corp.google.com","threadId":"42876","inReplyTo":"xmqqy44xfq3n.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/3] difftool: avoid $GIT_DIR and $GIT_WORK_TREE","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-19T21:06:35Z","receivedAt":"2016-07-19T21:06:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> It is not wrong per-se, but as you are in a subshell, you do not\n> have to unset these, I would think.  Not worth a reroll, but unless\n> I am overlooking something (in which case please holler) I'm\n> inclined to remove these two lines myself while queuing the series.\n\nI propose to squashing the following to 2/3 (and adjusting 3/3 as\nneeded).  No need to resend if you agree it is a good idea, as it is\npart of what I've queued on 'pu'.\n\nThanks.\n\n git-difftool.perl   | 2 +-\n t/t7800-difftool.sh | 2 --\n 2 files changed, 1 insertion(+), 3 deletions(-)\n\ndiff --git a/git-difftool.perl b/git-difftool.perl\nindex bc2267f..c81cbe4 100755\n--- a/git-difftool.perl\n+++ b/git-difftool.perl\n@@ -88,11 +88,11 @@ sub changed_files\n \tmy @refreshargs = (\n \t\t@gitargs, 'update-index',\n \t\t'--really-refresh', '-q', '--unmerged');\n-\tmy @diffargs = (@gitargs, 'diff-files', '--name-only', '-z');\n \ttry {\n \t\tGit::command_oneline(@refreshargs);\n \t} catch Git::Error::Command with {};\n \n+\tmy @diffargs = (@gitargs, 'diff-files', '--name-only', '-z');\n \tmy $line = Git::command_oneline(@diffargs);\n \tmy @files;\n \tif (defined $line) {\ndiff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\nindex afdf370..cb25480 100755\n--- a/t/t7800-difftool.sh\n+++ b/t/t7800-difftool.sh\n@@ -421,8 +421,6 @@ run_dir_diff_test 'difftool --dir-diff from subdirectory with GIT_DIR set' '\n \t\tcd sub &&\n \t\tgit difftool --dir-diff $symlinks --extcmd ls \\\n \t\t\tbranch -- sub >output &&\n-\t\tsane_unset GIT_WORK_TREE &&\n-\t\tsane_unset GIT_DIR &&\n \t\tgrep sub output &&\n \t\t! grep file output\n \t)\n-- \n2.9.2-581-g77f0ffb\n\n"},{"id":"291890","messageId":"20160720222520.GA22966@gmail.com","threadId":"42876","inReplyTo":"xmqqvb01e3s4.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/3] difftool: avoid $GIT_DIR and $GIT_WORK_TREE","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2016-07-20T22:25:20Z","receivedAt":"2016-07-20T22:25:32Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Tue, Jul 19, 2016 at 02:06:35PM -0700, Junio C Hamano wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n> \n> > It is not wrong per-se, but as you are in a subshell, you do not\n> > have to unset these, I would think.  Not worth a reroll, but unless\n> > I am overlooking something (in which case please holler) I'm\n> > inclined to remove these two lines myself while queuing the series.\n> \n> I propose to squashing the following to 2/3 (and adjusting 3/3 as\n> needed).  No need to resend if you agree it is a good idea, as it is\n> part of what I've queued on 'pu'.\n> \n> Thanks.\n\n\nI had originally meant to squash that in but it slipped through.\nIt looks great.\n\nThank you!\n\n\n>  git-difftool.perl   | 2 +-\n>  t/t7800-difftool.sh | 2 --\n>  2 files changed, 1 insertion(+), 3 deletions(-)\n> \n> diff --git a/git-difftool.perl b/git-difftool.perl\n> index bc2267f..c81cbe4 100755\n> --- a/git-difftool.perl\n> +++ b/git-difftool.perl\n> @@ -88,11 +88,11 @@ sub changed_files\n>  \tmy @refreshargs = (\n>  \t\t@gitargs, 'update-index',\n>  \t\t'--really-refresh', '-q', '--unmerged');\n> -\tmy @diffargs = (@gitargs, 'diff-files', '--name-only', '-z');\n>  \ttry {\n>  \t\tGit::command_oneline(@refreshargs);\n>  \t} catch Git::Error::Command with {};\n>  \n> +\tmy @diffargs = (@gitargs, 'diff-files', '--name-only', '-z');\n>  \tmy $line = Git::command_oneline(@diffargs);\n>  \tmy @files;\n>  \tif (defined $line) {\n> diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\n> index afdf370..cb25480 100755\n> --- a/t/t7800-difftool.sh\n> +++ b/t/t7800-difftool.sh\n> @@ -421,8 +421,6 @@ run_dir_diff_test 'difftool --dir-diff from subdirectory with GIT_DIR set' '\n>  \t\tcd sub &&\n>  \t\tgit difftool --dir-diff $symlinks --extcmd ls \\\n>  \t\t\tbranch -- sub >output &&\n> -\t\tsane_unset GIT_WORK_TREE &&\n> -\t\tsane_unset GIT_DIR &&\n>  \t\tgrep sub output &&\n>  \t\t! grep file output\n>  \t)\n> -- \n> 2.9.2-581-g77f0ffb\n> \n\n-- \nDavid\n"}]}