From: Matthieu Moy <Matthieu.Moy@grenoble-inp•fr>
To: Kirill Smelkov <kirr@landau•phys.spbu.ru>
Cc: "Junio C Hamano" <gitster@pobox•com>,
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>,
"Jeff King" <peff@peff•net>
Subject: Re: [PATCH 1/3] tests: Prepare --textconv tests for correctly-failing conversion program
Date: Sat, 18 Sep 2010 21:14:57 +0200 [thread overview]
Message-ID: <vpqiq22vp72.fsf@bauges.imag.fr> (raw)
In-Reply-To: <26d0544dac2515e76bee0608881cfd8c23bf1ebf.1284830388.git.kirr@landau.phys.spbu.ru> (Kirill Smelkov's message of "Sat\, 18 Sep 2010 21\:25\:04 +0400")
Kirill Smelkov <kirr@landau•phys.spbu.ru> writes:
> Recently I've spot a bug
We usually avoid the first person in commit messages. The cover letter
is a good place to tell about your personal story, but the commit
message is what will remain, what people will read after a blame or
bisect. They won't care whether you've "recently" found a bug, or in
which circumstances you've found it.
I'd write stg like (which would probably go to PATCH 2/3 instead of
here):
-----8<----
git blame --textconv is wrongly calling the textconv filter on
symlinks: symlinks are stored as blobs whose content is the target of
the link, and blame calls the textconv filter on a temporary file
filled-in with the content of this blob.
For example:
> $ git blame -C -C regular-file.pdf
> Error: May not be a PDF file (continuing anyway)
> Error: PDF file is damaged - attempting to reconstruct xref table...
> Error: Couldn't find trailer dictionary
> Error: Couldn't read xref table
> Warning: program returned non-zero exit code #1
> fatal: unable to read files to diff
-----8<----
> So git-blame is wrong here, and I'm going to write problem-demonstration
> tests + try to fix it, but first we have to convert existing textconv
> converter, so it will mimic pdftext behaviour -- if there is no '^bin:'
> in input -- it's not a "binary" file and helper exits with error.
What's interesting here is not that you mimick pdftext behavior, but
that you allow to easily distinguish file content and symlink target.
Here's my try:
-----8<----
The textconv filter is sometimes incorrectly ran on a temporary file
whose content is the target of a symbolic link, instead of actual file
content. Prepare to test this by marking the content of the file to
convert with "bin:", and let the helper die if "bin:" is not found in
the file content.
-----8<----
> No other semantic changes at this stage.
Otherwise, the code looks OK.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
next prev parent reply other threads:[~2010-09-18 19:20 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 [this message]
2010-09-20 20:35 ` Kirill Smelkov
2010-09-20 21:03 ` Matthieu Moy
2010-09-21 18:39 ` Kirill Smelkov
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 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
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=vpqiq22vp72.fsf@bauges.imag.fr \
--to=matthieu.moy@grenoble-inp$(echo .)fr \
--cc=axel.bonnet@ensimag$(echo .)imag.fr \
--cc=clement.poulain@ensimag$(echo .)imag.fr \
--cc=diane.gasselin@ensimag$(echo .)imag.fr \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=kirr@landau$(echo .)phys.spbu.ru \
--cc=peff@peff$(echo .)net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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