Thread (1 message) 1 message, 1 author, 2016-06-15

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

From: Matthieu Moy <hidden>
Date: 2016-06-15 22:49:34

Kirill Smelkov [off-list ref] 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/
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help