Re: [PATCH v3 4/5] difftool: Use symlinks when diffing against the worktree
Tim Henigan writes: > I'm sorry I am so late to see and comment on this...I am just getting > caught up after a few busy weeks due to $dayjob and vacation. > > > On Mon, Jul 23, 2012 at 2:05 AM, David Aguilar wrote: >> >> diff --git a/git-difftool.perl b/git-difftool.perl >> index 2ae344c..a5b371f 100755 >> --- a/git-difftool.perl >> +++ b/git-difftool.perl >> >> @@ -271,6 +276,7 @@ sub main >> gui => undef, >> help => undef, >> prompt => undef, >> + symlinks => $^O ne 'MSWin32' && $^O ne 'msys', > > Should this test for cygwin as well? > > >> @@ -342,13 +350,18 @@ sub dir_diff >> >> # 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 >> - for my $file (@working_tree) { >> - if (-e "$b/$file" && compare("$b/$file", "$workdir/$file")) { >> + # should be copied back to the working tree. >> + # Do not copy back files when symlinks are used and the >> + # external tool did not replace the original link with a file. >> + for my $file (@worktree) { >> + next if $symlinks && -l "$b/$file"; >> + if (-f "$b/$file" && compare("$b/$file", "$workdir/$file")) { > > compare returns '-1' if an error is encountered while reading a file. > In this (unlikely) case, should it still overwrite the working copy > file? I think the answer is 'yes', but thought it was worth > mentioning. It probably is safer to report the error, not touch anything and let the user take an appropriate action. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/5] difftool: Use symlinks when diffing against the worktree
I'm sorry I am so late to see and comment on this...I am just getting caught up after a few busy weeks due to $dayjob and vacation. On Mon, Jul 23, 2012 at 2:05 AM, David Aguilar wrote: > > diff --git a/git-difftool.perl b/git-difftool.perl > index 2ae344c..a5b371f 100755 > --- a/git-difftool.perl > +++ b/git-difftool.perl > > @@ -271,6 +276,7 @@ sub main > gui => undef, > help => undef, > prompt => undef, > + symlinks => $^O ne 'MSWin32' && $^O ne 'msys', Should this test for cygwin as well? > @@ -342,13 +350,18 @@ sub dir_diff > > # 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 > - for my $file (@working_tree) { > - if (-e "$b/$file" && compare("$b/$file", "$workdir/$file")) { > + # should be copied back to the working tree. > + # Do not copy back files when symlinks are used and the > + # external tool did not replace the original link with a file. > + for my $file (@worktree) { > + next if $symlinks && -l "$b/$file"; > + if (-f "$b/$file" && compare("$b/$file", "$workdir/$file")) { compare returns '-1' if an error is encountered while reading a file. In this (unlikely) case, should it still overwrite the working copy file? I think the answer is 'yes', but thought it was worth mentioning. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/5] difftool: Use symlinks when diffing against the worktree
David Aguilar writes: > Teach difftool's --dir-diff mode to use symlinks to represent > files from the working copy, and make it the default behavior > for the non-Windows platforms. > > Using symlinks is simpler and safer since we do not need to > worry about copying files back into the worktree. > The old behavior is still available as --no-symlinks. > > Signed-off-by: David Aguilar > --- > Handles the case where an editor unlinks the original symlink, > replacing it with a file. Thanks; will replace. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 4/5] difftool: Use symlinks when diffing against the worktree
Teach difftool's --dir-diff mode to use symlinks to represent files from the working copy, and make it the default behavior for the non-Windows platforms. Using symlinks is simpler and safer since we do not need to worry about copying files back into the worktree. The old behavior is still available as --no-symlinks. Signed-off-by: David Aguilar --- Handles the case where an editor unlinks the original symlink, replacing it with a file. Documentation/git-difftool.txt |8 git-difftool.perl | 33 +++-- 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt index 31fc2e3..313d54e 100644 --- a/Documentation/git-difftool.txt +++ b/Documentation/git-difftool.txt @@ -66,6 +66,14 @@ of the diff post-image. `$MERGED` is the name of the file which is being compared. `$BASE` is provided for compatibility with custom merge tool commands and has the same value as `$MERGED`. +--symlinks:: +--no-symlinks:: + 'git difftool''s default behavior is create symlinks to the + working tree when run in `--dir-diff` mode. ++ + Specifying `--no-symlinks` instructs 'git difftool' to create + copies instead. `--no-symlinks` is the default on Windows. + --tool-help:: Print a list of diff tools that may be used with `--tool`. diff --git a/git-difftool.perl b/git-difftool.perl index 2ae344c..a5b371f 100755 --- a/git-difftool.perl +++ b/git-difftool.perl @@ -92,7 +92,7 @@ sub print_tool_help sub setup_dir_diff { - my ($repo, $workdir) = @_; + my ($repo, $workdir, $symlinks) = @_; # Run the diff; exit immediately if no diff found # 'Repository' and 'WorkingCopy' must be explicitly set to insure that @@ -209,8 +209,13 @@ sub setup_dir_diff unless (-d "$rdir/$dir") { mkpath("$rdir/$dir") or die $!; } - copy("$workdir/$file", "$rdir/$file") or die $!; - chmod(stat("$workdir/$file")->mode, "$rdir/$file") or die $!; + if ($symlinks) { + symlink("$workdir/$file", "$rdir/$file") or die $!; + } else { + copy("$workdir/$file", "$rdir/$file") or die $!; + my $mode = stat("$workdir/$file")->mode; + chmod($mode, "$rdir/$file") or die $!; + } } # Changes to submodules require special treatment. This loop writes a @@ -271,6 +276,7 @@ sub main gui => undef, help => undef, prompt => undef, + symlinks => $^O ne 'MSWin32' && $^O ne 'msys', tool_help => undef, ); GetOptions('g|gui!' => \$opts{gui}, @@ -278,6 +284,8 @@ sub main 'h' => \$opts{help}, 'prompt!' => \$opts{prompt}, 'y' => sub { $opts{prompt} = 0; }, + 'symlinks' => \$opts{symlinks}, + 'no-symlinks' => sub { $opts{symlinks} = 0; }, 't|tool:s' => \$opts{difftool_cmd}, 'tool-help' => \$opts{tool_help}, 'x|extcmd:s' => \$opts{extcmd}); @@ -316,7 +324,7 @@ sub main # will invoke a separate instance of 'git-difftool--helper' for # each file that changed. if (defined($opts{dirdiff})) { - dir_diff($opts{extcmd}); + dir_diff($opts{extcmd}, $opts{symlinks}); } else { file_diff($opts{prompt}); } @@ -324,13 +332,13 @@ sub main sub dir_diff { - my ($extcmd) = @_; + my ($extcmd, $symlinks) = @_; my $rc; my $repo = Git->repository(); my $workdir = find_worktree($repo); - my ($a, $b, @working_tree) = setup_dir_diff($repo, $workdir); + my ($a, $b, @worktree) = setup_dir_diff($repo, $workdir, $symlinks); if (defined($extcmd)) { $rc = system($extcmd, $a, $b); } else { @@ -342,13 +350,18 @@ sub dir_diff # 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 - for my $file (@working_tree) { - if (-e "$b/$file" && compare("$b/$file", "$workdir/$file")) { + # should be copied back to the working tree. + # Do not copy back files when symlinks are used and the + # external tool did not replace the original link with a file. + for my $file (@worktree) { + next if $symlinks && -l "$b/$file"; + if (-f "$b/$file" && compare("$b/$file", "$workdir/$file")) { copy("$b/$file", "$workdir/$file") or die $!; - chmod(stat("$b/$file")->mode, "$workdir/$file") or die $!; + my $mode = stat("$b/$file")->mode; + chmod($mode, "$workdir/$fil