Re: difftool -d symlinks, under what conditions
- From
David Aguilar <davvid@gmail.com>
- Date
- Mar 13, 2013, 08:24 UTC
- Message-ID
- <CAJDDKr7ZU16XWtCfYX9-RMzcpKa_FF80Od+mUMG4n8dUKeLsvw@mail.gmail.com>
- In-Reply-To
- <20130313001758.GH2317@serenity.lan>
On Tue, Mar 12, 2013 at 5:17 PM, John Keeping <john@keeping.me.uk> wrote:
Show 34 quoted lines
> On Tue, Mar 12, 2013 at 04:48:16PM -0600, Matt McClure wrote: >> On Mar 12, 2013, at 4:16 PM, Junio C Hamano <gitster@pobox.com> wrote: >> >> > Matt McClure <matthewlmcclure@gmail.com> writes: >> > >> > * If you are comparing two trees, and especially if your RHS is not >> > HEAD, you will send everything to a temporary without >> > symlinks. Any edit made by the user will be lost. >> >> I think you're suggesting to use a symlink any time the content of any >> given RHS revision is the same as the working tree. >> >> I imagine that might confuse me as a user. It would create >> circumstances where some files are symlinked and others aren't for >> reasons that won't be straightforward. >> >> I imagine solving that case, I might instead implement a copy back to >> the working tree with conflict detection/resolution. Some earlier >> iterations of the directory diff feature used copy back without >> conflict detection and created situations where I clobbered my own >> changes by finishing a directory diff after making edits concurrently. > > The code to copy back working tree files is already there, it just > triggers using the same logic as the creation of symlinks in the first > place and doesn't attempt any conflict detection. I suspect that any > more comprehensive solution will need to restrict the use of "git > difftool -d" whenever the index contains unmerged entries or when there > are both staged and unstaged changes, since the merge resolution will > cause these states to be lost. > > The implementation of Junio's suggestion is relatively straightforward > (this is untested, although t7800 passes, and can probably be improved > by someone better versed in Perl). Does this work for your original > scenario?
This is a nice straightforward approach.
As Junio mentioned, a good next step would be this patch in combination with making the truly temporary files created by dir-diff readonly.
Will that need a win32 platform check? Does anyone want to take this and whip it into a proper patch?
Show 44 quoted lines
> -- >8 --
> diff --git a/git-difftool.perl b/git-difftool.perl
> index 0a90de4..5f093ae 100755
> --- a/git-difftool.perl
> +++ b/git-difftool.perl
> @@ -83,6 +83,21 @@ sub exit_cleanup
> exit($status | ($status >> 8));
> }
>
> +sub use_wt_file
> +{
> + my ($repo, $workdir, $file, $sha1, $symlinks) = @_;
> + my $null_sha1 = '0' x 40;
> +
> + if ($sha1 eq $null_sha1) {
> + return 1;
> + } elsif (not $symlinks) {
> + return 0;
> + }
> +
> + my $wt_sha1 = $repo->command_oneline('hash-object', "$workdir/$file");
> + return $sha1 eq $wt_sha1;
> +}
> +
> sub setup_dir_diff
> {
> my ($repo, $workdir, $symlinks) = @_;
> @@ -159,10 +174,10 @@ EOF
> }
>
> if ($rmode ne $null_mode) {
> - if ($rsha1 ne $null_sha1) {
> - $rindex .= "$rmode $rsha1\t$dst_path\0";
> - } else {
> + if (use_wt_file($repo, $workdir, $dst_path, $rsha1, $symlinks)) {
> push(@working_tree, $dst_path);
> + } else {
> + $rindex .= "$rmode $rsha1\t$dst_path\0";
> }
> }
> }
> --
> 1.8.2.rc2.4.g7799588
>-- David