git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: John Keeping <john@keeping.me.uk>
To: Matt McClure <matthewlmcclure@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	David Aguilar <davvid@gmail.com>,
	"git@vger.kernel.org" <git@vger.kernel.org>,
	Tim Henigan <tim.henigan@gmail.com>
Subject: Re: difftool -d symlinks, under what conditions
Date: Wed, 13 Mar 2013 00:17:59 +0000	[thread overview]
Message-ID: <20130313001758.GH2317@serenity.lan> (raw)
In-Reply-To: <3222724986386016520@unknownmsgid>

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?

-- >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

  reply	other threads:[~2013-03-13  0:18 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-26 20:23 difftool -d symlinks, under what conditions Matt McClure
2012-11-27  6:20 ` David Aguilar
2012-11-27 21:10   ` Matt McClure
     [not found]   ` <CAJELnLEL8y0G3MBGkW+YDKtVxX4n4axJG7p0oPsXsV4_FRyGDg@mail.gmail.com>
2013-03-12 18:12     ` Matt McClure
2013-03-12 19:09       ` John Keeping
2013-03-12 19:23         ` David Aguilar
2013-03-12 19:43           ` John Keeping
2013-03-12 20:39             ` Junio C Hamano
2013-03-12 21:06               ` John Keeping
2013-03-12 21:26                 ` Junio C Hamano
2013-03-12 21:43                 ` Matt McClure
2013-03-12 22:11                   ` Matt McClure
2013-03-12 22:16                   ` Junio C Hamano
2013-03-12 22:48                     ` Matt McClure
2013-03-13  0:17                       ` John Keeping [this message]
2013-03-13  0:56                         ` Matt McClure
2013-03-13  8:24                         ` David Aguilar
2013-03-13 15:30                           ` Junio C Hamano
2013-03-13 16:45                             ` Junio C Hamano
2013-03-13 17:08                               ` John Keeping
2013-03-13 17:40                                 ` Junio C Hamano
2013-03-13 18:01                                   ` John Keeping
2013-03-13 19:28                                     ` Junio C Hamano
2013-03-13 20:33                                       ` [PATCH 0/2] difftool --dir-diff: symlink all files matching the working tree John Keeping
2013-03-13 20:33                                         ` [PATCH 1/2] git-difftool(1): fix formatting of --symlink description John Keeping
2013-03-13 20:33                                         ` [PATCH 2/2] difftool --dir-diff: symlink all files matching the working tree John Keeping
2013-03-14  3:41                                           ` David Aguilar
2013-03-14  9:36                                             ` John Keeping
2013-03-14 15:18                                           ` Junio C Hamano
2013-03-14 20:19                                         ` [PATCH v2 0/3] " John Keeping
2013-03-14 20:19                                           ` [PATCH v2 1/3] git-difftool(1): fix formatting of --symlink description John Keeping
2013-03-14 20:19                                           ` [PATCH v2 2/3] difftool: avoid double slashes in symlink targets John Keeping
2013-03-14 21:19                                             ` Junio C Hamano
2013-03-14 20:19                                           ` [PATCH v2 3/3] difftool --dir-diff: symlink all files matching the working tree John Keeping
2013-03-14 21:28                                             ` Junio C Hamano
2013-03-14 22:24                                               ` John Keeping
2013-03-14 22:31                                                 ` Junio C Hamano
2013-03-14  9:43                               ` difftool -d symlinks, under what conditions John Keeping
2013-03-14 17:25                                 ` John Keeping
2013-03-14 17:33                                   ` Junio C Hamano
2013-03-14 20:00                                     ` [PATCH 0/2] checkout-index: fix .gitattributes handling with --prefix John Keeping
2013-03-14 20:00                                       ` [PATCH 1/2] t2003: modernize style John Keeping
2013-03-14 20:00                                       ` [PATCH 2/2] entry: fix filter lookup John Keeping
2013-03-14 21:50                                         ` Junio C Hamano
2013-03-12 20:38           ` difftool -d symlinks, under what conditions Junio C Hamano
2013-03-12 19:24         ` [PATCH] difftool: Make directory diff symlink work tree John Keeping
2013-03-12 23:12           ` Matt McClure
2013-03-12 23:40             ` John Keeping
2013-03-13  0:26           ` Matt McClure
2013-03-13  9:11             ` John Keeping

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130313001758.GH2317@serenity.lan \
    --to=john@keeping.me.uk \
    --cc=davvid@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=matthewlmcclure@gmail.com \
    --cc=tim.henigan@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).