David Aguilar <davvid@xxxxxxxxx> writes:
> 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@xxxxxx>
> Signed-off-by: David Aguilar <davvid@xxxxxxxxx>
> ---
> Only 3/3 was re-sent; the rest are the same.
OK, so in short you did the get_symlink() thing that was brought up
in the previous round of review and everything else fell out as a
natural consequence? That is wonderful ;-)
> @@ -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;
It still makes me wonder why the new !S_ISLNK() is done only for the
right hand side, though. In fact, the processing done for the left
hand side and the right hand side are vastly different even without
this patch.
I suspect this is probably because the code is not prepared to drive
the underlying "diff" when given the "-R" option (if I am reading
the code correctly, argv[] that came from the end-user is appended
to "diff --raw --no-abbrev -z", so the user could ask "difftool
--dir-diff -R ..."), in which case you would see the working tree
files as the left hand side of the diff. In the dir-diff mode,
because you want to make only the working-tree side writable (and
reflect whatever edit the user made back to the working-tree side),
the choices you have to fix it would either be forbid "-R" (which is
less preferrable as it is a more brittle solution between the two)
or read the "diff --raw" output and swap the sides when you notice
that LHS has 0{40} with non 0 mode, which is a sign that that side
represents the working tree.
Having said all that, let's focus on the "symlink" stuff in this
series.
Thanks.