Matthieu Moy [off-list ref] writes:
Kirill Smelkov [off-list ref] writes:
quoted
Recently I've spot a bug in git blame --textconv, which was wrongly
calling pdftotext (my *.pdf conversion program) on a symlink.pdf,
No time for a detailed review now, but textconv is also called by diff
(that's the original implementation). Does "git diff" work properly on
symlinks in the same case?
diff knows symlinks and regular files are different, and produces "delete
old then add new" if you changed a regular file to a symlink.
That said, if you changed a symlink from pointing at A to pointing at B,
it does run the textual diff between the string we get from readlink(3).
I happen to think that textconv, if specified, for such a path should be
honored, so that people can keep doing whatever munging they have been
doing in their existing textconv filters.
I didn't look at the thread or problem description, but are we running the
textconv filter on the file that symlink points at, instead of the
pathname stored in the symlink? If so I'd call that a bug.
On the other hand, if we are running the textconv filter on the pathname,
then I don't think we are doing anything wrong. If you have a filter that
is meant to munge a PDF file to some other format, and if you do not want
to apply that filter to munge a pathname a symlink that happens to be
named "foo.pdf", either the filter itself or the attributes pattern you
are using to choose what paths to apply that filter might want to be
written more carefully, that's all.
On Sat, Sep 18, 2010 at 01:01:17PM -0700, Junio C Hamano wrote:
diff knows symlinks and regular files are different, and produces "delete
old then add new" if you changed a regular file to a symlink.
That said, if you changed a symlink from pointing at A to pointing at B,
it does run the textual diff between the string we get from readlink(3).
I happen to think that textconv, if specified, for such a path should be
honored, so that people can keep doing whatever munging they have been
doing in their existing textconv filters.
I think you came to the conclusion later in the thread that this is a
bad idea, if only because it is not how "git diff" works, but I wanted
to make one additional point.
I think that git, being symlink aware, needs to behave similarly to
"lstat". That is, we should never dereference symlinks transparently
when diffing or analyzing content, because otherwise there is no way to
actually look at the symlink data itself. It is the user's
responsibility to dereference symlinks in their diffs (e.g., I can get
either the symlink data _or_ the actual file data by doing "git diff
symlink-to-foo.bin" or "git diff foo.bin". If git dereferenced for me, I
would get file data for _both_). Not to mention that we can't always
dereference anyway because of broken links or links outside the repo, as
Matthieu pointed out.
So doing anything but a straight text diff for the pathnames in symlink
blobs is, IMHO, a bug.
The one thing this does not enable is using a special diff driver on the
_pathnames_ of symlinks. Since these are by-definition text, I don't
know why anyone would want to do that. But it is an orthogonal problem,
anyway. We would need some way in the .gitattributes or the .gitconfig
to say "this is the diff driver to use not based on pathname matching,
but based on the file's mode". E.g., a special "SYMLINK" diff driver
like:
[diff "SYMLINK"]
textconv = pointless-munge
But again, I have no idea why anyone would want such a feature, so it is
not worth thinking too hard about it.
-Peff
On Montag, 20. September 2010, Jeff King wrote:
[diff "SYMLINK"]
textconv = pointless-munge
But again, I have no idea why anyone would want such a feature, so it is
not worth thinking too hard about it.
Some people dislike "\ No newline at end of file" for symlinks. This would be
the opportunity to hook a suitable diff driver.
-- Hannes