Re: [PATCH 1/3] tests: Prepare --textconv tests for correctly-failing conversion program

From: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date: Sep 18, 2010, 19:14 UTC
Message-ID: <vpqiq22vp72.fsf@bauges.imag.fr>
In-Reply-To: <26d0544dac2515e76bee0608881cfd8c23bf1ebf.1284830388.git.kirr@landau.phys.spbu.ru>

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:

Show 7 quoted lines

>     $ 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/