On Sun, Sep 19 2021, David Aguilar wrote:
> The difftool dir-diff mode handles symlinks by replacing them with their
> readlink(2) values. This allows diff tools to see changes to symlinks
> as if they were regular text diffs with the old and new path values.
> This is analogous to what "git diff" displays when symlinks change.
>
> The temporary diff directories that are created initially contain
> symlinks because they get checked-out using a temporary index that
> retains the original symlinks as checked-in to the repository.
>
> A bug was introduced when difftool was rewritten in C that made
> difftool write the readlink(2) contents into the pointed-to file rather
> than the symlink itself. The write was going through the symlink and
> writing to its target rather than writing to the symlink path itself.
>
> Replace symlinks with raw text files by unlinking the symlink path
> before writing the readlink(2) content into them.
>
> When 18ec800512eb0634a0bf5e86b36ed156fbee73f3 added handling for
Nit: Please format commits like this: 18ec800512e (difftool: handle
modified symlinks in dir-diff mode, 2017-03-15)
See "git show -s --pretty=reference" in Documentation/SubmittingPatches.
> modified symlinks this bug got recorded in the test suite. The tests
> included the pointed-to symlink target paths. These paths were being
> reported because difftool was erroneously writing to them, but they
> should have never been reported nor written.
>
> Correct the modified-symlinks test cases by removing the target files
> from the expected output.
>
> Add a test to ensure that symlinks are written with the readlink(2)
> values and that the target files contain their original content.
>
> Reported-by: Alan Blotz <work@xxxxxxxxx>
> Helped-by: Đoàn Trần Công Danh <congdanhqx@xxxxxxxxx>
> Signed-off-by: David Aguilar <davvid@xxxxxxxxx>
> ---
> builtin/difftool.c | 2 ++
> t/t7800-difftool.sh | 68 +++++++++++++++++++++++++++++++++++++++++++--
> 2 files changed, 68 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/difftool.c b/builtin/difftool.c
> index bb9fe7245a..21e055d13a 100644
> --- a/builtin/difftool.c
> +++ b/builtin/difftool.c
> @@ -557,11 +557,13 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
> if (*entry->left) {
> add_path(&ldir, ldir_len, entry->path);
> ensure_leading_directories(ldir.buf);
> + unlink(ldir.buf);
> write_file(ldir.buf, "%s", entry->left);
> }
> if (*entry->right) {
> add_path(&rdir, rdir_len, entry->path);
> ensure_leading_directories(rdir.buf);
> + unlink(rdir.buf);
> write_file(rdir.buf, "%s", entry->right);
> }
> }
> diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
> index a173f564bc..07a52fb8e1 100755
> --- a/t/t7800-difftool.sh
> +++ b/t/t7800-difftool.sh
> @@ -674,7 +674,6 @@ test_expect_success SYMLINKS 'difftool --dir-diff handles modified symlinks' '
> rm c &&
> ln -s d c &&
> cat >expect <<-EOF &&
> - b
> c
>
> c
> @@ -710,7 +709,6 @@ test_expect_success SYMLINKS 'difftool --dir-diff handles modified symlinks' '
> # Deleted symlinks
> rm -f c &&
> cat >expect <<-EOF &&
> - b
> c
>
> EOF
> @@ -723,6 +721,72 @@ test_expect_success SYMLINKS 'difftool --dir-diff handles modified symlinks' '
> test_cmp expect actual
> '
>
> +test_expect_success SYMLINKS 'difftool --dir-diff writes symlinks as raw text' '
> + # Start out on a branch called "branch-init".
> + git init -b branch-init symlink-files &&
> + (
> + cd ./symlink-files &&
The "./" isn't needed
Neither of those small comments needs a re-roll I think, just reading
through & noting things I spotted...
I've omitted things that Eric mentioned in
<CAPig+cTBfP5_czsPiALcF3tODJmNfXvNkTjqVFRbHCS535d-ng@xxxxxxxxxxxxxx>.
