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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Kirill Smelkov <kirr@xxxxxxxxxxxxxxxxxxx> 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/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]