From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Matthieu Moy" <Matthieu.Moy@grenoble-inp.fr>,
"Kirill Smelkov" <kirr@landau.phys.spbu.ru>,
git@vger.kernel.org, "Axel Bonnet" <axel.bonnet@ensimag.imag.fr>,
"Clément Poulain" <clement.poulain@ensimag.imag.fr>,
"Diane Gasselin" <diane.gasselin@ensimag.imag.fr>
Subject: Re: [BUG, PATCH 0/3] Fix {blame,cat-file} --textconv for cases with symlinks
Date: Mon, 20 Sep 2010 14:00:46 -0400 [thread overview]
Message-ID: <20100920180046.GA1790@sigill.intra.peff.net> (raw)
In-Reply-To: <7vpqwa254i.fsf@alter.siamese.dyndns.org>
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
next prev parent reply other threads:[~2010-09-20 18:00 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-09-18 17:25 [BUG, PATCH 0/3] Fix {blame,cat-file} --textconv for cases with symlinks Kirill Smelkov
2010-09-18 17:25 ` [PATCH 1/3] tests: Prepare --textconv tests for correctly-failing conversion program Kirill Smelkov
2010-09-18 19:14 ` Matthieu Moy
2010-09-18 17:25 ` [PATCH 2/3] blame,cat-file: Demonstrate --textconv is wrongly running converter on symlinks Kirill Smelkov
2010-09-18 19:26 ` Matthieu Moy
2010-09-18 17:25 ` [PATCH 3/3] RFC: blame,cat-file --textconv: Don't assume mode is ``S_IFREF | 0664'' Kirill Smelkov
2010-09-18 19:04 ` Matthieu Moy
2010-09-20 18:21 ` Jeff King
2010-09-20 20:35 ` [PATCH 1/3] tests: Prepare --textconv tests for correctly-failing conversion program Kirill Smelkov
2010-09-20 21:03 ` Matthieu Moy
2010-09-21 18:39 ` Kirill Smelkov
2010-09-20 21:01 ` [PATCH] sha1_name.c: update comment to mention :/foo syntax Matthieu Moy
2010-09-21 18:02 ` Junio C Hamano
2010-09-21 20:06 ` Matthieu Moy
2010-09-21 23:29 ` Junio C Hamano
2010-09-24 16:43 ` [PATCH] update comment and documentation for " Matthieu Moy
2010-09-18 18:08 ` [BUG, PATCH 0/3] Fix {blame,cat-file} --textconv for cases with symlinks Matthieu Moy
2010-09-18 20:01 ` Junio C Hamano
2010-09-19 8:58 ` Matthieu Moy
2010-09-19 18:17 ` Junio C Hamano
2010-09-20 18:00 ` Jeff King [this message]
2010-09-20 20:18 ` Johannes Sixt
2010-09-21 17:57 ` Junio C Hamano
2010-09-21 18:42 ` Jeff King
2010-09-21 18:56 ` Jeff King
2010-09-21 20:59 ` [PATCH 0/2] better userdiff behavior for symlinks Jeff King
2010-09-21 21:01 ` [PATCH 1/2] diff: don't use pathname-based diff drivers " Jeff King
2010-09-22 5:40 ` Matthieu Moy
2010-09-22 5:50 ` Jeff King
2010-09-21 21:13 ` [PATCH 2/2] diff: add a special SYMLINK user-diff driver Jeff King
2010-09-22 0:12 ` Ævar Arnfjörð Bjarmason
2010-09-22 0:30 ` Jeff King
2010-09-22 0:39 ` Ævar Arnfjörð Bjarmason
2010-09-22 5:53 ` Matthieu Moy
2010-09-22 16:59 ` Matthieu Moy
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20100920180046.GA1790@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=Matthieu.Moy@grenoble-inp.fr \
--cc=axel.bonnet@ensimag.imag.fr \
--cc=clement.poulain@ensimag.imag.fr \
--cc=diane.gasselin@ensimag.imag.fr \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=kirr@landau.phys.spbu.ru \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).