From: Ben Peart <peartben@gmail.com>
To: git@vger.kernel.org
Cc: benpeart@microsoft.com, christian.couder@gmail.com,
larsxschneider@gmail.com, David Aguilar <davvid@gmail.com>,
Junio C Hamano <gitster@pobox.com>
Subject: [PATCH v2 3/4] difftool: handle modified symlinks in dir-diff mode
Date: Fri, 24 Mar 2017 11:27:25 -0400 [thread overview]
Message-ID: <20170324152726.14632-6-benpeart@microsoft.com> (raw)
In-Reply-To: <20170324152726.14632-1-benpeart@microsoft.com>
From: David Aguilar <davvid@gmail.com>
Detect the null object ID for symlinks in dir-diff so that difftool can
detect when symlinks are modified in the worktree.
Previously, a null symlink object ID would crash difftool.
Handle null object IDs as unknown content that must be read from
the worktree.
Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: David Aguilar <davvid@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Ben Peart <benpeart@microsoft.com>
---
builtin/difftool.c | 51 ++++++++++++++++++++++++++++++++++++++++-----
t/t7800-difftool.sh | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 106 insertions(+), 5 deletions(-)
diff --git a/builtin/difftool.c b/builtin/difftool.c
index d13350ce83..25e54ad3ed 100644
--- a/builtin/difftool.c
+++ b/builtin/difftool.c
@@ -254,6 +254,49 @@ static int ensure_leading_directories(char *path)
}
}
+/*
+ * Unconditional writing of a plain regular file is what
+ * "git difftool --dir-diff" wants to do for symlinks. We are preparing two
+ * temporary directories to be fed to a Git-unaware tool that knows how to
+ * show a diff of two directories (e.g. "diff -r A B").
+ *
+ * Because the tool is Git-unaware, if a symbolic link appears in either of
+ * these temporary directories, it will try to dereference and show the
+ * difference of the target of the symbolic link, which is not what we want,
+ * as the goal of the dir-diff mode is to produce an output that is logically
+ * equivalent to what "git diff" produces.
+ *
+ * Most importantly, we want to get textual comparison of the result of the
+ * readlink(2). get_symlink() provides that---it returns the contents of
+ * the symlink that gets written to a regular file to force the external tool
+ * to compare the readlink(2) result as text, even on a filesystem that is
+ * capable of doing a symbolic link.
+ */
+static char *get_symlink(const struct object_id *oid, const char *path)
+{
+ char *data;
+ if (is_null_oid(oid)) {
+ /* The symlink is unknown to Git so read from the filesystem */
+ struct strbuf link = STRBUF_INIT;
+ if (has_symlinks) {
+ if (strbuf_readlink(&link, path, strlen(path)))
+ die(_("could not read symlink %s"), path);
+ } else if (strbuf_read_file(&link, path, 128))
+ die(_("could not read symlink file %s"), path);
+
+ data = strbuf_detach(&link, NULL);
+ } else {
+ enum object_type type;
+ unsigned long size;
+ data = read_sha1_file(oid->hash, &type, &size);
+ if (!data)
+ die(_("could not read object %s for symlink %s"),
+ oid_to_hex(oid), path);
+ }
+
+ return data;
+}
+
static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
int argc, const char **argv)
{
@@ -270,8 +313,6 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
struct hashmap working_tree_dups, submodules, symlinks2;
struct hashmap_iter iter;
struct pair_entry *entry;
- enum object_type type;
- unsigned long size;
struct index_state wtindex;
struct checkout lstate, rstate;
int rc, flags = RUN_GIT_CMD, err = 0;
@@ -377,13 +418,13 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
}
if (S_ISLNK(lmode)) {
- char *content = read_sha1_file(loid.hash, &type, &size);
+ char *content = get_symlink(&loid, src_path);
add_left_or_right(&symlinks2, src_path, content, 0);
free(content);
}
if (S_ISLNK(rmode)) {
- char *content = read_sha1_file(roid.hash, &type, &size);
+ char *content = get_symlink(&roid, dst_path);
add_left_or_right(&symlinks2, dst_path, content, 1);
free(content);
}
@@ -397,7 +438,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
return error("could not write '%s'", src_path);
}
- if (rmode) {
+ if (rmode && !S_ISLNK(rmode)) {
struct working_tree_entry *entry;
/* Avoid duplicate working_tree entries */
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index e0e65df8de..0e7f30db2d 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -626,4 +626,64 @@ test_expect_success SYMLINKS 'difftool --dir-diff symlinked directories' '
)
'
+test_expect_success SYMLINKS 'difftool --dir-diff handles modified symlinks' '
+ test_when_finished git reset --hard &&
+ touch b &&
+ ln -s b c &&
+ git add b c &&
+ test_tick &&
+ git commit -m initial &&
+ touch d &&
+ rm c &&
+ ln -s d c &&
+ cat >expect <<-EOF &&
+ b
+ c
+
+ c
+ EOF
+ git difftool --symlinks --dir-diff --extcmd ls >output &&
+ grep -v ^/ output >actual &&
+ test_cmp expect actual &&
+
+ git difftool --no-symlinks --dir-diff --extcmd ls >output &&
+ grep -v ^/ output >actual &&
+ test_cmp expect actual &&
+
+ # The left side contains symlink "c" that points to "b"
+ test_config difftool.cat.cmd "cat \$LOCAL/c" &&
+ printf "%s\n" b >expect &&
+
+ git difftool --symlinks --dir-diff --tool cat >actual &&
+ test_cmp expect actual &&
+
+ git difftool --symlinks --no-symlinks --dir-diff --tool cat >actual &&
+ test_cmp expect actual &&
+
+ # The right side contains symlink "c" that points to "d"
+ test_config difftool.cat.cmd "cat \$REMOTE/c" &&
+ printf "%s\n" d >expect &&
+
+ git difftool --symlinks --dir-diff --tool cat >actual &&
+ test_cmp expect actual &&
+
+ git difftool --no-symlinks --dir-diff --tool cat >actual &&
+ test_cmp expect actual &&
+
+ # Deleted symlinks
+ rm -f c &&
+ cat >expect <<-EOF &&
+ b
+ c
+
+ EOF
+ git difftool --symlinks --dir-diff --extcmd ls >output &&
+ grep -v ^/ output >actual &&
+ test_cmp expect actual &&
+
+ git difftool --no-symlinks --dir-diff --extcmd ls >output &&
+ grep -v ^/ output >actual &&
+ test_cmp expect actual
+'
+
test_done
--
2.12.0.gvfs.1.43.g876ba2a
next prev parent reply other threads:[~2017-03-24 15:28 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-24 15:27 [PATCH v2 0/2] Refactor the filter process code into a reusable module Ben Peart
2017-03-24 15:27 ` [PATCH v2 1/2] pkt-line: add packet_writel() and packet_read_line_gently() Ben Peart
2017-03-25 5:47 ` Torsten Bögershausen
2017-03-27 22:19 ` Ben Peart
2017-03-30 14:04 ` Torsten Bögershausen
2017-03-30 16:01 ` Ben Peart
2017-03-30 17:01 ` Torsten Bögershausen
2017-03-24 15:27 ` [PATCH v2 1/4] t7800: remove whitespace before redirect Ben Peart
2017-03-24 16:21 ` Ben Peart
2017-03-24 15:27 ` [PATCH v2 2/2] sub-process: refactor the filter process code into a reusable module Ben Peart
2017-03-27 18:59 ` Jonathan Tan
2017-03-27 23:54 ` Ben Peart
2017-03-24 15:27 ` [PATCH v2 2/4] t7800: cleanup cruft left behind by tests Ben Peart
2017-03-24 15:27 ` Ben Peart [this message]
2017-03-24 15:27 ` [PATCH v2 4/4] Git 2.12.1 Ben Peart
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=20170324152726.14632-6-benpeart@microsoft.com \
--to=peartben@gmail.com \
--cc=benpeart@microsoft.com \
--cc=christian.couder@gmail.com \
--cc=davvid@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=larsxschneider@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).