git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
To: Junio C Hamano <gitster@pobox.com>
Cc: "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: Sun, 19 Sep 2010 10:58:55 +0200	[thread overview]
Message-ID: <vpq8w2yt8hc.fsf@bauges.imag.fr> (raw)
In-Reply-To: <7vpqwa254i.fsf@alter.siamese.dyndns.org> (Junio C. Hamano's message of "Sat\, 18 Sep 2010 13\:01\:17 -0700")

Junio C Hamano <gitster@pobox.com> writes:

> 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).

Yes, and my question was to make sure we don't run the textconv filter
on these strings.

I've just checked, and from my little tests, git diff doesn't try to
textconv the pathnames, it runs the textual diff without textconv,
which is the expected behavior.

> 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.

No, we don't do that. And yes, it's good not to do that: AFAIK, Git
never looks at the file symlinks points to (which allows one to store
broken symlinks or symlinks pointing outside the repository).

> 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.

If a symlink points to "foo.pdf", then you really want to call the
string "foo.pdf" a pathname, not a "content", although it happens to
be stored in a blob content.

If you have a program that can textconv PDF files, then you really
want to say

*.pdf textconv=the-driver-for-that-program

in your .gitattributes file, and let Git call the program on _files_,
not pathnames, and hence not symlink target (i.e. result of readlink).

If you think Git should call the textconv filter on pathnames, and let
the driver decide whether to call it depending on whether it's a file
content or a symlink target, then you should 1) give a use-case where
it's usefull to textconv symlink targets (I really don't see any, and
even on highly artificial examples, I can't imagine one where the same
driver should textconv file content and pathnames), and 2) tell us the
syntax to use in .gitattributes or in the driver to call it only on
file content (I don't know any, and the .gitattribute manpage doesn't
containt either "link" or "mode", so I don't think there is actually
any).

So, I'm pretty sure either I misunderstood you or you misunderstood
something ;-).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

  reply	other threads:[~2010-09-19  9: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 [this message]
2010-09-19 18:17       ` Junio C Hamano
2010-09-20 18:00     ` Jeff King
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=vpq8w2yt8hc.fsf@bauges.imag.fr \
    --to=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).