difftool: handle modified symlinks in dir-diff mode
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>
David Aguilar committed
Mar 15, 2017 at 02:31 UTC
18ec800512eb0634a0bf5e86b36ed156fbee73f3
2 files changed
+106
-5
builtin/difftool.c
+46
-5
@@ -254,6 +254,49 @@ static int ensure_leading_directories(char *path)
254
}
255
}
256
257
+/*
258
+ * Unconditional writing of a plain regular file is what
259
+ * "git difftool --dir-diff" wants to do for symlinks. We are preparing two
260
+ * temporary directories to be fed to a Git-unaware tool that knows how to
261
+ * show a diff of two directories (e.g. "diff -r A B").
262
+ *
263
+ * Because the tool is Git-unaware, if a symbolic link appears in either of
264
+ * these temporary directories, it will try to dereference and show the
265
+ * difference of the target of the symbolic link, which is not what we want,
266
+ * as the goal of the dir-diff mode is to produce an output that is logically
267
+ * equivalent to what "git diff" produces.
268
+ *
269
+ * Most importantly, we want to get textual comparison of the result of the
270
+ * readlink(2). get_symlink() provides that---it returns the contents of
271
+ * the symlink that gets written to a regular file to force the external tool
272
+ * to compare the readlink(2) result as text, even on a filesystem that is
273
+ * capable of doing a symbolic link.
274
+ */
275
+static char *get_symlink(const struct object_id *oid, const char *path)
276
+{
277
+ char *data;
278
+ if (is_null_oid(oid)) {
279
+ /* The symlink is unknown to Git so read from the filesystem */
280
+ struct strbuf link = STRBUF_INIT;
281
+ if (has_symlinks) {
282
+ if (strbuf_readlink(&link, path, strlen(path)))
283
+ die(_("could not read symlink %s"), path);
284
+ } else if (strbuf_read_file(&link, path, 128))
285
+ die(_("could not read symlink file %s"), path);
286
+
287
+ data = strbuf_detach(&link, NULL);
288
+ } else {
289
+ enum object_type type;
290
+ unsigned long size;
291
+ data = read_sha1_file(oid->hash, &type, &size);
292
+ if (!data)
293
+ die(_("could not read object %s for symlink %s"),
294
+ oid_to_hex(oid), path);
295
+ }
296
+
297
+ return data;
298
+}
299
+
300
static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
301
int argc, const char **argv)
302
{
@@ -270,8 +313,6 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
313
struct hashmap working_tree_dups, submodules, symlinks2;
314
struct hashmap_iter iter;
315
struct pair_entry *entry;
273
- enum object_type type;
274
- unsigned long size;
316
struct index_state wtindex;
317
struct checkout lstate, rstate;
318
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,
418
}
419
420
if (S_ISLNK(lmode)) {
380
- char *content = read_sha1_file(loid.hash, &type, &size);
421
+ char *content = get_symlink(&loid, src_path);
422
add_left_or_right(&symlinks2, src_path, content, 0);
423
free(content);
424
}
425
426
if (S_ISLNK(rmode)) {
386
- char *content = read_sha1_file(roid.hash, &type, &size);
427
+ char *content = get_symlink(&roid, dst_path);
428
add_left_or_right(&symlinks2, dst_path, content, 1);
429
free(content);
430
}
@@ -397,7 +438,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
438
return error("could not write '%s'", src_path);
439
}
440
400
- if (rmode) {
441
+ if (rmode && !S_ISLNK(rmode)) {
442
struct working_tree_entry *entry;
443
444
/* Avoid duplicate working_tree entries */
t/t7800-difftool.sh
+60
@@ -626,4 +626,64 @@ test_expect_success SYMLINKS 'difftool --dir-diff symlinked directories' '
626
)
627
'
628
629
+test_expect_success SYMLINKS 'difftool --dir-diff handles modified symlinks' '
630
+ test_when_finished git reset --hard &&
631
+ touch b &&
632
+ ln -s b c &&
633
+ git add b c &&
634
+ test_tick &&
635
+ git commit -m initial &&
636
+ touch d &&
637
+ rm c &&
638
+ ln -s d c &&
639
+ cat >expect <<-EOF &&
640
+ b
641
+ c
642
+
643
+ c
644
+ EOF
645
+ git difftool --symlinks --dir-diff --extcmd ls >output &&
646
+ grep -v ^/ output >actual &&
647
+ test_cmp expect actual &&
648
+
649
+ git difftool --no-symlinks --dir-diff --extcmd ls >output &&
650
+ grep -v ^/ output >actual &&
651
+ test_cmp expect actual &&
652
+
653
+ # The left side contains symlink "c" that points to "b"
654
+ test_config difftool.cat.cmd "cat \$LOCAL/c" &&
655
+ printf "%s\n" b >expect &&
656
+
657
+ git difftool --symlinks --dir-diff --tool cat >actual &&
658
+ test_cmp expect actual &&
659
+
660
+ git difftool --symlinks --no-symlinks --dir-diff --tool cat >actual &&
661
+ test_cmp expect actual &&
662
+
663
+ # The right side contains symlink "c" that points to "d"
664
+ test_config difftool.cat.cmd "cat \$REMOTE/c" &&
665
+ printf "%s\n" d >expect &&
666
+
667
+ git difftool --symlinks --dir-diff --tool cat >actual &&
668
+ test_cmp expect actual &&
669
+
670
+ git difftool --no-symlinks --dir-diff --tool cat >actual &&
671
+ test_cmp expect actual &&
672
+
673
+ # Deleted symlinks
674
+ rm -f c &&
675
+ cat >expect <<-EOF &&
676
+ b
677
+ c
678
+
679
+ EOF
680
+ git difftool --symlinks --dir-diff --extcmd ls >output &&
681
+ grep -v ^/ output >actual &&
682
+ test_cmp expect actual &&
683
+
684
+ git difftool --no-symlinks --dir-diff --extcmd ls >output &&
685
+ grep -v ^/ output >actual &&
686
+ test_cmp expect actual
687
+'
688
+
689
test_done