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: Tue, 21 Sep 2010 14:42:41 -0400 [thread overview]
Message-ID: <20100921184241.GA28567@sigill.intra.peff.net> (raw)
In-Reply-To: <7vaanbuggr.fsf@alter.siamese.dyndns.org>
On Tue, Sep 21, 2010 at 10:57:56AM -0700, Junio C Hamano 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.
>
> I agree with you; pointless-munge would just be 'printf "%s\n"' ;-)
Almost. That would print the name of the tempfile; the actual pathname
is the contents of the tempfile (unless you are proposing totally
alternate semantics for the symlink diff section. :) ).
Just for fun, the patch to make this work is as tiny as:
diff --git a/diff.c b/diff.c
index 6fb18ae..58c4477 100644
--- a/diff.c
+++ b/diff.c
@@ -1771,8 +1771,14 @@ static void emit_binary_diff(FILE *file, mmfile_t *one, mmfile_t *two, char *pre
static void diff_filespec_load_driver(struct diff_filespec *one)
{
- if (!one->driver)
+ if (one->driver)
+ return;
+
+ if (S_ISLNK(one->mode))
+ one->driver = userdiff_find_by_name("SYMLINK");
+ else
one->driver = userdiff_find_by_path(one->path);
+
if (!one->driver)
one->driver = userdiff_find_by_name("default");
}
@@ -1820,7 +1826,7 @@ struct userdiff_driver *get_textconv(struct diff_filespec *one)
{
if (!DIFF_FILE_VALID(one))
return NULL;
- if (!S_ISREG(one->mode))
+ if (!S_ISREG(one->mode) && !S_ISLNK(one->mode))
return NULL;
diff_filespec_load_driver(one);
if (!one->driver->textconv)
after which I successfully tested with:
git init repo && cd repo &&
echo content >file.txt &&
ln -s file.txt link.txt &&
echo '*.txt diff=txt' >.gitattributes &&
git add . && git commit -m foo &&
git config diff.txt.textconv "sed 's/^/converted: /'" &&
git config diff.SYMLINK.textconv "perl -pe 's/$/\n/'" &&
git show
It works with the whole range of diff config, so you could do something
as awesomely stupid as:
$ git config diff.SYMLINK.binary true
$ git show link.txt
...
diff --git a/link.txt b/link.txt
new file mode 120000
index 0000000..4c33073
Binary files /dev/null and b/link.txt differ
If you really wanted the "dereference my symlinks" behavior, you could
do:
git config diff.SYMLINK.textconv 'sh -c "cat `cat $1`" -'
but that is not quite right; you would actually need to dereference
relative symlinks with respect to the link itself, which textconv never
gets (plus you would probably want to handle broken links more
gracefully).
Anyway, as I said at the beginning, for me this was just for fun. I find
the intended use rather silly, but maybe somebody else is interested. I
do think it's the right way of implementing such a feature, because we
already turn off textconv when making patches that are meant to be
applied rather than viewed. However, I didn't do any testing or give
much thought to whether this would affect any unintended code paths.
-Peff
next prev parent reply other threads:[~2010-09-21 18:42 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
2010-09-20 20:18 ` Johannes Sixt
2010-09-21 17:57 ` Junio C Hamano
2010-09-21 18:42 ` Jeff King [this message]
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=20100921184241.GA28567@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).