Skip to content

Commit 18ec800

Browse files
davvidgitster
davvid
authored and
gitster
committed
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>
1 parent 98fde5e commit 18ec800

File tree

2 files changed

+106
-5
lines changed

2 files changed

+106
-5
lines changed

‎builtin/difftool.c‎

Lines changed: 46 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,49 @@ static int ensure_leading_directories(char *path)
254254
}
255255
}
256256

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+
257300
static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
258301
int argc, const char **argv)
259302
{
@@ -270,8 +313,6 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
270313
struct hashmap working_tree_dups, submodules, symlinks2;
271314
struct hashmap_iter iter;
272315
struct pair_entry *entry;
273-
enum object_type type;
274-
unsigned long size;
275316
struct index_state wtindex;
276317
struct checkout lstate, rstate;
277318
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,
377418
}
378419

379420
if (S_ISLNK(lmode)) {
380-
char *content = read_sha1_file(loid.hash, &type, &size);
421+
char *content = get_symlink(&loid, src_path);
381422
add_left_or_right(&symlinks2, src_path, content, 0);
382423
free(content);
383424
}
384425

385426
if (S_ISLNK(rmode)) {
386-
char *content = read_sha1_file(roid.hash, &type, &size);
427+
char *content = get_symlink(&roid, dst_path);
387428
add_left_or_right(&symlinks2, dst_path, content, 1);
388429
free(content);
389430
}
@@ -397,7 +438,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
397438
return error("could not write '%s'", src_path);
398439
}
399440

400-
if (rmode) {
441+
if (rmode && !S_ISLNK(rmode)) {
401442
struct working_tree_entry *entry;
402443

403444
/* Avoid duplicate working_tree entries */

‎t/t7800-difftool.sh‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -626,4 +626,64 @@ test_expect_success SYMLINKS 'difftool --dir-diff symlinked directories' '
626626
)
627627
'
628628

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+
629689
test_done

0 commit comments

Comments
 (0)