When 'difftool' is called to compare a range of commits that modify
more than one file, it opens a separate instance of the diff tool for
each file that changed.
The new '--dir-diff' option copies all the modified files to a temporary
location and runs a directory diff on them in a single instance of the
diff tool.
Signed-off-by: Tim Henigan <redacted>
---
This replaces commit 4d6efe0 in the 'th/difftool-diffall' branch based
on pu. This version corrects some problems with submodule support in
difftool.
Changes in v11:
- Fix handling of submodules in subdirectories.
If the diff includes a submodule that lives in a subdirectory of
the project, the script fails with "No such file or directory".
For example, if a submodule is located at '<repo root>/lib/sub',
the script fails when it tries to write the submodule SHA1 to
'/tmp/.../lib/sub' because the 'lib' directory does not exist.
This commit teaches difftool to create the subdirectory path in
the tmp location prior to writing the submodule file.
- Append "dirty" to submodule SHA1 when needed
If a submodule is dirty (i.e. has uncommitted changes) then
'git diff' reports "<SHA1 of parent>-dirty"
while
'git diff --raw' reports "<SHA1 of parent>"
difftool uses the '--raw' diff format, so the "dirty" flag is lost.
To make the output more closely match that of 'git diff', teach
difftool to append "-dirty" to the submodule SHA1 when needed.
Documentation/git-difftool.txt | 6 ++
git-difftool--helper.sh | 19 +++--
git-difftool.perl | 170 +++++++++++++++++++++++++++++++++++-----
t/t7800-difftool.sh | 39 +++++++++
4 files changed, 209 insertions(+), 25 deletions(-)
diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt
index fe38f66..aba5e76 100644
--- a/Documentation/git-difftool.txt
+++ b/Documentation/git-difftool.txt
@@ -19,6 +19,12 @@ linkgit:git-diff[1].
OPTIONS
-------
+-d::
+--dir-diff::
+ Copy the modified files to a temporary location and perform
+ a directory diff on them. This mode never prompts before
+ launching the diff tool.
+
-y::
--no-prompt::
Do not prompt before launching a diff tool.
diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh
index e6558d1..3d0fe0c 100755
--- a/git-difftool--helper.sh
+++ b/git-difftool--helper.sh
@@ -73,9 +73,16 @@ then
fi
fi
-# Launch the merge tool on each path provided by 'git diff'
-while test $# -gt 6
-do
- launch_merge_tool "$1" "$2" "$5"
- shift 7
-done
+if test -n "$GIT_DIFFTOOL_DIRDIFF"
+then
+ LOCAL="$1"
+ REMOTE="$2"
+ run_merge_tool "$merge_tool" false
+else
+ # Launch the merge tool on each path provided by 'git diff'
+ while test $# -gt 6
+ do
+ launch_merge_tool "$1" "$2" "$5"
+ shift 7
+ done
+fi
diff --git a/git-difftool.perl b/git-difftool.perl
index d4fe998..5bb01e1 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -1,21 +1,29 @@
#!/usr/bin/env perl
# Copyright (c) 2009, 2010 David Aguilar
+# Copyright (c) 2012 Tim Henigan
#
# This is a wrapper around the GIT_EXTERNAL_DIFF-compatible
# git-difftool--helper script.
#
# This script exports GIT_EXTERNAL_DIFF and GIT_PAGER for use by git.
-# GIT_DIFFTOOL_NO_PROMPT, GIT_DIFFTOOL_PROMPT, and GIT_DIFF_TOOL
-# are exported for use by git-difftool--helper.
+# GIT_DIFFTOOL_NO_PROMPT, GIT_DIFFTOOL_PROMPT, GIT_DIFFTOOL_DIRDIFF,
+# and GIT_DIFF_TOOL are exported for use by git-difftool--helper.
#
# Any arguments that are unknown to this script are forwarded to 'git diff'.
use 5.008;
use strict;
use warnings;
+use File::Basename qw(dirname);
+use File::Copy;
+use File::stat;
+use File::Path qw(mkpath);
+use File::Temp qw(tempdir);
use Getopt::Long qw(:config pass_through);
use Git;
+my @working_tree;
+
sub usage
{
my $exitcode = shift;@@ -24,15 +32,121 @@ usage: git difftool [-t|--tool=<tool>]
[-x|--extcmd=<cmd>]
[-g|--gui] [--no-gui]
[--prompt] [-y|--no-prompt]
+ [-d|--dir-diff]
['git diff' options]
USAGE
exit($exitcode);
}
+sub setup_dir_diff
+{
+ # Run the diff; exit immediately if no diff found
+ my $repo = Git->repository();
+ my $diffrtn = $repo->command_oneline(['diff', '--raw', '--no-abbrev', '-z', @ARGV]);
+ exit(0) if (length($diffrtn) == 0);
+
+ # Setup temp directories
+ my $tmpdir = tempdir('git-diffall.XXXXX', CLEANUP => 1, TMPDIR => 1);
+ my $ldir = "$tmpdir/left";
+ my $rdir = "$tmpdir/right";
+ mkpath($ldir) or die $!;
+ mkpath($rdir) or die $!;
+
+ # Build index info for left and right sides of the diff
+ my $submodule_mode = "160000";
+ my $null_mode = 0 x 6;
+ my $null_sha1 = 0 x 40;
+ my $lindex = "";
+ my $rindex = "";
+ my %submodule;
+ my @rawdiff = split('\0', $diffrtn);
+
+ for (my $i=0; $i<@rawdiff; $i+=2) {
+ my ($lmode, $rmode, $lsha1, $rsha1, $status) = split(' ', substr($rawdiff[$i], 1));
+ my $path = $rawdiff[$i + 1];
+
+ if (($lmode eq $submodule_mode) or ($rmode eq $submodule_mode)) {
+ $submodule{$path}{left} = $lsha1;
+ if ($lsha1 ne $rsha1) {
+ $submodule{$path}{right} = $rsha1;
+ } else {
+ $submodule{$path}{right} = "$rsha1-dirty";
+ }
+ next;
+ }
+
+ if ($lmode ne $null_mode) {
+ $lindex .= "$lmode $lsha1\t$path\0";
+ }
+
+ if ($rmode ne $null_mode) {
+ if ($rsha1 ne $null_sha1) {
+ $rindex .= "$rmode $rsha1\t$path\0";
+ } else {
+ push(@working_tree, $path);
+ }
+ }
+ }
+
+ # Populate the left and right directories based on each index file
+ my ($inpipe, $ctx);
+ $ENV{GIT_DIR} = $repo->repo_path();
+ $ENV{GIT_INDEX_FILE} = "$tmpdir/lindex";
+ ($inpipe, $ctx) = $repo->command_input_pipe(qw/update-index -z --index-info/);
+ print($inpipe $lindex);
+ $repo->command_close_pipe($inpipe, $ctx);
+ system(('git', 'checkout-index', '--all', "--prefix=$ldir/"));
+
+ $ENV{GIT_INDEX_FILE} = "$tmpdir/rindex";
+ ($inpipe, $ctx) = $repo->command_input_pipe(qw/update-index -z --index-info/);
+ print($inpipe $rindex);
+ $repo->command_close_pipe($inpipe, $ctx);
+ system(('git', 'checkout-index', '--all', "--prefix=$rdir/"));
+
+ # Changes in the working tree need special treatment since they are
+ # not part of the index
+ my $workdir = $repo->repo_path() . "/..";
+ for (@working_tree) {
+ my $dir = dirname($_);
+ unless (-d "$rdir/$dir") {
+ mkpath("$rdir/$dir") or die $!;
+ }
+ copy("$workdir/$_", "$rdir/$_") or die $!;
+ chmod(stat("$workdir/$_")->mode, "$rdir/$_") or die $!;
+ }
+
+ # Changes to submodules require special treatment. This loop writes a
+ # temporary file to both the left and right directories to show the
+ # change in the recorded SHA1 for the submodule.
+ foreach my $path (keys %submodule) {
+ if (defined $submodule{$path}{left}) {
+ my $dir = dirname($path);
+ unless (-d "$ldir/$dir") {
+ mkpath("$ldir/$dir") or die $!;
+ }
+ open(my $fh, ">", "$ldir/$path") or die $!;
+ print($fh "Subproject commit $submodule{$path}{left}");
+ close($fh);
+ }
+ if (defined $submodule{$path}{right}) {
+ my $dir = dirname($path);
+ unless (-d "$rdir/$dir") {
+ mkpath("$rdir/$dir") or die $!;
+ }
+ open(my $fh, ">", "$rdir/$path") or die $!;
+ print($fh "Subproject commit $submodule{$path}{right}");
+ close($fh);
+ }
+ }
+
+ return ($ldir, $rdir);
+}
+
# parse command-line options. all unrecognized options and arguments
# are passed through to the 'git diff' command.
-my ($difftool_cmd, $extcmd, $gui, $help, $prompt);
+my ($difftool_cmd, $dirdiff, $extcmd, $gui, $help, $prompt);
GetOptions('g|gui!' => \$gui,
+ 'd|dir-diff' => \$dirdiff,
'h' => \$help,
'prompt!' => \$prompt,
'y' => sub { $prompt = 0; },@@ -65,22 +179,40 @@ if ($gui) {
$ENV{GIT_DIFF_TOOL} = $guitool;
}
}
-if (defined($prompt)) {
- if ($prompt) {
- $ENV{GIT_DIFFTOOL_PROMPT} = 'true';
+
+# In directory diff mode, 'git-difftool--helper' is called once
+# to compare the a/b directories. In file diff mode, 'git diff'
+# will invoke a separate instance of 'git-difftool--helper' for
+# each file that changed.
+if (defined($dirdiff)) {
+ my ($a, $b) = setup_dir_diff();
+ if (defined($extcmd)) {
+ system(($extcmd, $a, $b));
} else {
- $ENV{GIT_DIFFTOOL_NO_PROMPT} = 'true';
+ $ENV{GIT_DIFFTOOL_DIRDIFF} = 'true';
+ system(('git', 'difftool--helper', $a, $b));
+ }
+
+ # If the diff including working copy files and those
+ # files were modified during the diff, then the changes
+ # should be copied back to the working tree
+ my $repo = Git->repository();
+ my $workdir = $repo->repo_path() . "/..";
+ for (@working_tree) {
+ copy("$b/$_", "$workdir/$_") or die $!;
+ chmod(stat("$b/$_")->mode, "$workdir/$_") or die $!;
+ }
+} else {
+ if (defined($prompt)) {
+ if ($prompt) {
+ $ENV{GIT_DIFFTOOL_PROMPT} = 'true';
+ } else {
+ $ENV{GIT_DIFFTOOL_NO_PROMPT} = 'true';
+ }
}
-}
-$ENV{GIT_PAGER} = '';
-$ENV{GIT_EXTERNAL_DIFF} = 'git-difftool--helper';
-my @command = ('git', 'diff', @ARGV);
-
-# ActiveState Perl for Win32 does not implement POSIX semantics of
-# exec* system call. It just spawns the given executable and finishes
-# the starting program, exiting with code 0.
-# system will at least catch the errors returned by git diff,
-# allowing the caller of git difftool better handling of failures.
-my $rc = system(@command);
-exit($rc | ($rc >> 8));
+ $ENV{GIT_PAGER} = '';
+ $ENV{GIT_EXTERNAL_DIFF} = 'git-difftool--helper';
+ my $rc = system(('git', 'diff', @ARGV));
+ exit($rc | ($rc >> 8));
+}diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index e716d06..478c1be 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -319,4 +319,43 @@ test_expect_success PERL 'say no to the second file' '
echo "$diff" | stdin_doesnot_contain br2
'
+test_expect_success PERL 'setup change in subdirectory' '
+ git checkout master &&
+ mkdir sub &&
+ echo master >sub/sub &&
+ git add sub/sub &&
+ git commit -m "added sub/sub" &&
+ echo test >>file &&
+ echo test >>sub/sub &&
+ git add . &&
+ git commit -m "modified both"
+'
+
+test_expect_success PERL 'difftool -d' '
+ diff=$(git difftool -d --extcmd ls branch) &&
+ echo "$diff" | stdin_contains sub &&
+ echo "$diff" | stdin_contains file
+'
+
+test_expect_success PERL 'difftool --dir-diff' '
+ diff=$(git difftool --dir-diff --extcmd ls branch) &&
+ echo "$diff" | stdin_contains sub &&
+ echo "$diff" | stdin_contains file
+'
+
+test_expect_success PERL 'difftool --dir-diff ignores --prompt' '
+ diff=$(git difftool --dir-diff --prompt --extcmd ls branch) &&
+ echo "$diff" | stdin_contains sub &&
+ echo "$diff" | stdin_contains file
+'
+
+test_expect_success PERL 'difftool --dir-diff from subdirectory' '
+ (
+ cd sub &&
+ diff=$(git difftool --dir-diff --extcmd ls branch) &&
+ echo "$diff" | stdin_contains sub &&
+ echo "$diff" | stdin_contains file
+ )
+'
+
test_done
--
1.7.10.rc2.22.ga52d4
On Wed, Apr 4, 2012 at 12:21 PM, Tim Henigan [off-list ref] wrote:
quoted hunk
diff --git a/git-difftool.perl b/git-difftool.perl
index d4fe998..5bb01e1 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
#!/usr/bin/env perl
# Copyright (c) 2009, 2010 David Aguilar
+# Copyright (c) 2012 Tim Henigan
#
# This is a wrapper around the GIT_EXTERNAL_DIFF-compatible
# git-difftool--helper script.
#
# This script exports GIT_EXTERNAL_DIFF and GIT_PAGER for use by git.
-# GIT_DIFFTOOL_NO_PROMPT, GIT_DIFFTOOL_PROMPT, and GIT_DIFF_TOOL
-# are exported for use by git-difftool--helper.
+# GIT_DIFFTOOL_NO_PROMPT, GIT_DIFFTOOL_PROMPT, GIT_DIFFTOOL_DIRDIFF,
+# and GIT_DIFF_TOOL are exported for use by git-difftool--helper.
What if we punt on enumerating each variable and reword this to:
# This script exports GIT_EXTERNAL_DIFF and GIT_PAGER for use by git.
# The GIT_DIFF* variables are exported for use by git-difftool--helper.
I also think we should change the shebang line to #!/usr/bin/perl.
quoted hunk
@@ -24,15 +32,121 @@ usage: git difftool [-t|--tool=<tool>]
[-x|--extcmd=<cmd>]
[-g|--gui] [--no-gui]
[--prompt] [-y|--no-prompt]
+ [-d|--dir-diff]
['git diff' options]
USAGE
exit($exitcode);
}
+sub setup_dir_diff
+{
+ # Run the diff; exit immediately if no diff found
+ my $repo = Git->repository();
+ my $diffrtn = $repo->command_oneline(['diff', '--raw', '--no-abbrev', '-z', @ARGV]);
+ exit(0) if (length($diffrtn) == 0);
+
+ # Setup temp directories
+ my $tmpdir = tempdir('git-diffall.XXXXX', CLEANUP => 1, TMPDIR => 1);
+ my $ldir = "$tmpdir/left";
+ my $rdir = "$tmpdir/right";
+ mkpath($ldir) or die $!;
+ mkpath($rdir) or die $!;
+
+ # Build index info for left and right sides of the diff
+ my $submodule_mode = "160000";
+ my $null_mode = 0 x 6;
+ my $null_sha1 = 0 x 40;
I know Perl allows it, but my eyes get confused.
How about writing "0" x 6 and "0" x 40?
That way we can visually see that the result is a string.
+ my $lindex = "";
+ my $rindex = "";
+ my %submodule;
+ my @rawdiff = split('\0', $diffrtn);
+
+ for (my $i=0; $i<@rawdiff; $i+=2) {
We use $i + 1 to grab list elements, so how about $#rawdiff instead of @rawdiff?
It doesn't matter in practice, though...
+ my ($lmode, $rmode, $lsha1, $rsha1, $status) = split(' ', substr($rawdiff[$i], 1));
+ my $path = $rawdiff[$i + 1];
+
+ if (($lmode eq $submodule_mode) or ($rmode eq $submodule_mode)) {
+ $submodule{$path}{left} = $lsha1;
+ if ($lsha1 ne $rsha1) {
+ $submodule{$path}{right} = $rsha1;
+ } else {
+ $submodule{$path}{right} = "$rsha1-dirty";
+ }
+ next;
+ }
+
+ if ($lmode ne $null_mode) {
+ $lindex .= "$lmode $lsha1\t$path\0";
+ }
+
+ if ($rmode ne $null_mode) {
+ if ($rsha1 ne $null_sha1) {
+ $rindex .= "$rmode $rsha1\t$path\0";
+ } else {
+ push(@working_tree, $path);
+ }
+ }
+ }
+
+ # Populate the left and right directories based on each index file
+ my ($inpipe, $ctx);
+ $ENV{GIT_DIR} = $repo->repo_path();
+ $ENV{GIT_INDEX_FILE} = "$tmpdir/lindex";
+ ($inpipe, $ctx) = $repo->command_input_pipe(qw/update-index -z --index-info/);
+ print($inpipe $lindex);
+ $repo->command_close_pipe($inpipe, $ctx);
+ system(('git', 'checkout-index', '--all', "--prefix=$ldir/"));
Please drop the extra parens. Perl ignores them.
We should also check the return value from system() here.
+
+ $ENV{GIT_INDEX_FILE} = "$tmpdir/rindex";
+ ($inpipe, $ctx) = $repo->command_input_pipe(qw/update-index -z --index-info/);
+ print($inpipe $rindex);
+ $repo->command_close_pipe($inpipe, $ctx);
+ system(('git', 'checkout-index', '--all', "--prefix=$rdir/"));
+
+ # Changes in the working tree need special treatment since they are
+ # not part of the index
+ my $workdir = $repo->repo_path() . "/..";
+ for (@working_tree) {
Please use the "for my $foo (@list)" form so we can say $foo instead of $_.
+ my $dir = dirname($_);
+ unless (-d "$rdir/$dir") {
+ mkpath("$rdir/$dir") or die $!;
+ }
+ copy("$workdir/$_", "$rdir/$_") or die $!;
+ chmod(stat("$workdir/$_")->mode, "$rdir/$_") or die $!;
+ }
+
+ # Changes to submodules require special treatment. This loop writes a
+ # temporary file to both the left and right directories to show the
+ # change in the recorded SHA1 for the submodule.
+ foreach my $path (keys %submodule) {
I think it's better to use "for" instead of "foreach" since we do not
modify $path.
+ if (defined $submodule{$path}{left}) {
In some places we write "defined(...)" but here it's "defined ...".
We should be consistent...
quoted hunk
@@ -65,22 +179,40 @@ if ($gui) { $ENV{GIT_DIFF_TOOL} = $guitool;
}
}
-if (defined($prompt)) {
- if ($prompt) {
- $ENV{GIT_DIFFTOOL_PROMPT} = 'true';
+
+# In directory diff mode, 'git-difftool--helper' is called once
+# to compare the a/b directories. In file diff mode, 'git diff'
+# will invoke a separate instance of 'git-difftool--helper' for
+# each file that changed.
+if (defined($dirdiff)) {
+ my ($a, $b) = setup_dir_diff();
+ if (defined($extcmd)) {
+ system(($extcmd, $a, $b));
} else {
- $ENV{GIT_DIFFTOOL_NO_PROMPT} = 'true';
+ $ENV{GIT_DIFFTOOL_DIRDIFF} = 'true';
+ system(('git', 'difftool--helper', $a, $b));
+ }
+
+ # If the diff including working copy files and those
+ # files were modified during the diff, then the changes
+ # should be copied back to the working tree
+ my $repo = Git->repository();
+ my $workdir = $repo->repo_path() . "/..";
Does this work when $GIT_WORK_TREE / core.worktree are defined?
+ for (@working_tree) {
+ copy("$b/$_", "$workdir/$_") or die $!;
+ chmod(stat("$b/$_")->mode, "$workdir/$_") or die $!;
+ }
"for my ..."
+} else {
+ if (defined($prompt)) {
+ if ($prompt) {
+ $ENV{GIT_DIFFTOOL_PROMPT} = 'true';
+ } else {
+ $ENV{GIT_DIFFTOOL_NO_PROMPT} = 'true';
+ }
}
-}
-$ENV{GIT_PAGER} = '';
-$ENV{GIT_EXTERNAL_DIFF} = 'git-difftool--helper';
-my @command = ('git', 'diff', @ARGV);
-
-# ActiveState Perl for Win32 does not implement POSIX semantics of
-# exec* system call. It just spawns the given executable and finishes
-# the starting program, exiting with code 0.
-# system will at least catch the errors returned by git diff,
-# allowing the caller of git difftool better handling of failures.
-my $rc = system(@command);
-exit($rc | ($rc >> 8));
+ $ENV{GIT_PAGER} = '';
+ $ENV{GIT_EXTERNAL_DIFF} = 'git-difftool--helper';
+ my $rc = system(('git', 'diff', @ARGV));
+ exit($rc | ($rc >> 8));
+}
We went back and forth a few times on this section,
eventually landing back on using system().
Should we retain this comment to help future readers from
having to re-learn it the hard way again?
We could also link to the ML threads, if you think that's helpful.
--
David
Hi David,
Thank you for the review. I am working on the changes you suggested,
but want to discuss one of them:
On Mon, Apr 9, 2012 at 8:14 AM, David Aguilar [off-list ref] wrote:
On Wed, Apr 4, 2012 at 12:21 PM, Tim Henigan [off-list ref] wrote:
quoted
+
+ # If the diff including working copy files and those
+ # files were modified during the diff, then the changes
+ # should be copied back to the working tree
+ my $repo = Git->repository();
+ my $workdir = $repo->repo_path() . "/..";
Does this work when $GIT_WORK_TREE / core.worktree are defined?
I had not considered this case and the existing code will certainly
ignore alternate work tree locations. To fix the problem, I plan to
add something like this:
+sub find_worktree
+{
+ my $worktree;
+ my $env_worktree = $ENV{GIT_WORK_TREE};
+ my $core_worktree = Git::config('core.worktree');
+
+ if (length($env_worktree) > 0) {
+ $worktree = $env_worktree;
+ } elsif (length($core_worktree) > 0) {
+ $worktree = $core_worktree;
+ } else {
+ my $repo = Git->repository();
+ $worktree = $repo->repo_path() . "/..";
+ }
+
+ return $worktree;
+}
Does this look like a reasonable solution? I don't typically use
'$GIT_WORK_TREE' or 'core.worktree'. The above function works for me,
but I would appreciate some feedback before I send the next revision
of the patch (v12!).
Thanks,
Tim
On Mon, Apr 9, 2012 at 8:14 AM, David Aguilar [off-list ref] wrote:
On Wed, Apr 4, 2012 at 12:21 PM, Tim Henigan [off-list ref] wrote:
quoted
diff --git a/git-difftool.perl b/git-difftool.perl
index d4fe998..5bb01e1 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
...snip...
quoted
+ $ENV{GIT_DIR} = $repo->repo_path();
+ $ENV{GIT_INDEX_FILE} = "$tmpdir/lindex";
+ ($inpipe, $ctx) = $repo->command_input_pipe(qw/update-index -z --index-info/);
+ print($inpipe $lindex);
+ $repo->command_close_pipe($inpipe, $ctx);
+ system(('git', 'checkout-index', '--all', "--prefix=$ldir/"));
...snip...
quoted
+ # If the diff including working copy files and those
+ # files were modified during the diff, then the changes
+ # should be copied back to the working tree
+ my $repo = Git->repository();
+ my $workdir = $repo->repo_path() . "/..";
Does this work when $GIT_WORK_TREE / core.worktree are defined?
Should I also be concerned that the existing code always overrides $GIT_DIR?
For example, should I squash in the following before calling 'git update-index'?
- $ENV{GIT_DIR} = $repo->repo_path();
+ if (not defined($ENV{GIT_DIR})) {
+ $ENV{GIT_DIR} = $repo->repo_path();
+ }
This seems like the right thing to do, but again I have not used
$GIT_DIR except for the implementation of this script.
On Tue, Apr 10, 2012 at 10:24 AM, Tim Henigan [off-list ref] wrote:
On Mon, Apr 9, 2012 at 8:14 AM, David Aguilar [off-list ref] wrote:
quoted
On Wed, Apr 4, 2012 at 12:21 PM, Tim Henigan [off-list ref] wrote:
quoted
diff --git a/git-difftool.perl b/git-difftool.perl
index d4fe998..5bb01e1 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
...snip...
quoted
quoted
+ $ENV{GIT_DIR} = $repo->repo_path();
+ $ENV{GIT_INDEX_FILE} = "$tmpdir/lindex";
+ ($inpipe, $ctx) = $repo->command_input_pipe(qw/update-index -z --index-info/);
+ print($inpipe $lindex);
+ $repo->command_close_pipe($inpipe, $ctx);
+ system(('git', 'checkout-index', '--all', "--prefix=$ldir/"));
...snip...
quoted
quoted
+ # If the diff including working copy files and those
+ # files were modified during the diff, then the changes
+ # should be copied back to the working tree
+ my $repo = Git->repository();
+ my $workdir = $repo->repo_path() . "/..";
Does this work when $GIT_WORK_TREE / core.worktree are defined?
Should I also be concerned that the existing code always overrides $GIT_DIR?
For example, should I squash in the following before calling 'git update-index'?
- $ENV{GIT_DIR} = $repo->repo_path();
+ if (not defined($ENV{GIT_DIR})) {
+ $ENV{GIT_DIR} = $repo->repo_path();
+ }
I think the right thing to do would be to not override GIT_DIR at all.
I haven't read it deeply enough to know whether it was being
overridden for a specific reason, but I think it should be safe to
leave it as-is.
Git.pm ends up overriding these variables itself anyways when calling commands.
The GIT_WORK_TREE check should use $repo->wc_path(). Git.pm's already
done all the hard work ;-)
--
David
On Wed, Apr 11, 2012 at 5:02 AM, David Aguilar [off-list ref] wrote:
On Tue, Apr 10, 2012 at 10:24 AM, Tim Henigan [off-list ref] wrote:
quoted
On Mon, Apr 9, 2012 at 8:14 AM, David Aguilar [off-list ref] wrote:
quoted
On Wed, Apr 4, 2012 at 12:21 PM, Tim Henigan [off-list ref] wrote:
I think the right thing to do would be to not override GIT_DIR at all.
I haven't read it deeply enough to know whether it was being
overridden for a specific reason, but I think it should be safe to
leave it as-is.
Git.pm ends up overriding these variables itself anyways when calling commands.
I tried to avoid setting $GIT_DIR in earlier versions of the patch.
However as discussed here [1], either 'git update-index' or 'git
checkout-index' did not work as expected without explicitly setting
$GIT_DIR.
If $GIT_DIR is not set, 'update-index' and 'checkout-index' will only
work if 'difftool' is called from the repo root. If 'difftool' is
called from a subdirectory, then one of the commands fails.
I suspect that when $GIT_INDEX_FILE is set but $GIT_DIR is not, then
$GIT_DIR is assumed to be 'pwd'. However, I was not able to prove
that.
The GIT_WORK_TREE check should use $repo->wc_path(). Git.pm's already
done all the hard work ;-)
I also tried this in an earlier version of the patch. As noted here
[2], I found that when 'difftool' was run from a subdirectory of the
repo root, '$repo->wc_path()' returned the subdirectory rather than
the repo root.
Thinking about this again now, I realize it was a side-effect of
$GIT_DIR being set in the script. The man page for git config states
that:
If --git-dir or GIT_DIR is specified but none of --work-tree, GIT_WORK_TREE
and core.worktree is specified, the current working directory is regarded as
the top level of your working tree.
So, if I explicitly set $GIT_DIR just for the 'update-index' and
'checkout-index' commands, I need to unset it afterwards. This should
allow '$repo->wc_path()' to behave as expected.
[1]: http://thread.gmane.org/gmane.comp.version-control.git/193296/focus=193302
[2]: http://thread.gmane.org/gmane.comp.version-control.git/193601/focus=193603
On Wed, Apr 11, 2012 at 9:46 AM, Tim Henigan [off-list ref] wrote:
On Wed, Apr 11, 2012 at 5:02 AM, David Aguilar [off-list ref] wrote:
quoted
On Tue, Apr 10, 2012 at 10:24 AM, Tim Henigan [off-list ref] wrote:
quoted
On Mon, Apr 9, 2012 at 8:14 AM, David Aguilar [off-list ref] wrote:
quoted
On Wed, Apr 4, 2012 at 12:21 PM, Tim Henigan [off-list ref] wrote:
I think the right thing to do would be to not override GIT_DIR at all.
I haven't read it deeply enough to know whether it was being
overridden for a specific reason, but I think it should be safe to
leave it as-is.
Git.pm ends up overriding these variables itself anyways when calling commands.
I tried to avoid setting $GIT_DIR in earlier versions of the patch.
However as discussed here [1], either 'git update-index' or 'git
checkout-index' did not work as expected without explicitly setting
$GIT_DIR.
If $GIT_DIR is not set, 'update-index' and 'checkout-index' will only
work if 'difftool' is called from the repo root. If 'difftool' is
called from a subdirectory, then one of the commands fails.
I suspect that when $GIT_INDEX_FILE is set but $GIT_DIR is not, then
$GIT_DIR is assumed to be 'pwd'. However, I was not able to prove
that.
quoted
The GIT_WORK_TREE check should use $repo->wc_path(). Git.pm's already
done all the hard work ;-)
I also tried this in an earlier version of the patch. As noted here
[2], I found that when 'difftool' was run from a subdirectory of the
repo root, '$repo->wc_path()' returned the subdirectory rather than
the repo root.
Thinking about this again now, I realize it was a side-effect of
$GIT_DIR being set in the script. The man page for git config states
that:
If --git-dir or GIT_DIR is specified but none of --work-tree, GIT_WORK_TREE
and core.worktree is specified, the current working directory is regarded as
the top level of your working tree.
So, if I explicitly set $GIT_DIR just for the 'update-index' and
'checkout-index' commands, I need to unset it afterwards. This should
allow '$repo->wc_path()' to behave as expected.
[1]: http://thread.gmane.org/gmane.comp.version-control.git/193296/focus=193302
[2]: http://thread.gmane.org/gmane.comp.version-control.git/193601/focus=193603
That's right. If we set it for one command we have to remember to
unset it afterwards else it won't work as expected.
If we're setting GIT_DIR then we should probably set GIT_WORK_TREE
too. It seems like one way would be to call repo_path() and wc_path()
early, set the variables with the returned values, and then worry
about managing GIT_WORK_TREE before+after calling checkout-index.
That might work.
And in case the code needs it, there's a GIT_PREFIX variable that is
set when the current directory is a subdir beneath the repo root.
--
David