Bug report: .gitattributes: -diff Unset causes files to be reported as binaries

Bug report: .gitattributes: -diff Unset causes files to be reported as binaries

[Frans Englich]

[-- Attachment #1: Type: text/plain, Size: 580 bytes --]


Hi,

Applying -diff Unset to a file using .gittattributes causes "git diff" to 
state that the file is a binary even though it isn't, or have been instructed 
to be treated as one. See attached script for reproducing.

This causes confusion since the file isn't a binary, it's only the diff output 
that has been suppressed. For instance, an auto generated cpp file has -diff 
Unset applied to it to suppress the meaningless output, but people contact 
you to point out the file is binary.

Perhaps this is easy to fix, a question of changing message strings.


Cheers,

		Frans

[-- Attachment #2: reproduceDiffBinary.sh --]
[-- Type: application/x-shellscript, Size: 365 bytes --]

Re: Bug report: .gitattributes: -diff Unset causes files to be reported as binaries

[Jeff King]

On Wed, Jul 01, 2009 at 12:08:35PM +0200, Frans Englich wrote:

> Applying -diff Unset to a file using .gittattributes causes "git diff"
> to state that the file is a binary even though it isn't, or have been
> instructed to be treated as one. See attached script for reproducing.

I think you are a little confused by the syntax. Each line of the
gitattributes file has a filename pattern and a set of attributes. Each
attribute is either set, unset, set to a value, or unspecified. For your
example (file.txt and the "diff" attribute), they look like:

  Set:
    file.txt diff
  Unset:
    file.txt -diff
  Set to a value:
    file.txt diff=foo
  Unspecified:
    file.txt

So the word "Unset" is unnecessary in "-diff Unset" (and syntactically
means "set the attribute named "Unset", not any sort of modifier on the
diff attribute). This is described in the first section of "git help
attributes".

All of that being said, your example does end up, in fact, making the
diff attribute "unset" for you (because it uses "-diff"). And the effect
of doing so is to mark the file as binary (i.e., not to be diffed). From
"git help attributes", section "Generating diff text":

  The attribute `diff` affects how 'git' generates diffs for particular
  files. It can tell git whether to generate a textual patch for the
  path or to treat the path as a binary file.
  [...]
  Unset
      A path to which the diff attribute is unset will generate Binary
      files differ.

So as far as I can see, git is behaving exactly as it is supposed to.
Maybe you can be more specific about what effect you were trying to
achieve by setting gitattributes in the first place?

-Peff

Re: Bug report: .gitattributes: -diff Unset causes files to be reported as binaries

[Frans Englich]

On Thursday 02 July 2009 07:35:34 Jeff King wrote:
> On Wed, Jul 01, 2009 at 12:08:35PM +0200, Frans Englich wrote:
> > Applying -diff Unset to a file using .gittattributes causes "git diff"
> > to state that the file is a binary even though it isn't, or have been
> > instructed to be treated as one. See attached script for reproducing.
>
> I think you are a little confused by the syntax. Each line of the
> gitattributes file has a filename pattern and a set of attributes. Each
> attribute is either set, unset, set to a value, or unspecified. For your
> example (file.txt and the "diff" attribute), they look like:

Perhaps that should be considered another bug; that invalid syntax is 
accepted, instead of being communicated to the user.

[...]
> So as far as I can see, git is behaving exactly as it is supposed to.
> Maybe you can be more specific about what effect you were trying to
> achieve by setting gitattributes in the first place?

To exclude it in diffs, such as from `git show`. Take the case where you have 
a grammar file for a parser and generate a source file from it(or any similar 
scenario); the diff for the generated source file is not of interest and is 
just noisy when read as part of a patch. This applies to all kinds of 
generated files. However, this doesn't mean that the file should be treated 
as a binary, and what practicalities that implies.

If -diff affects whether a file is treated as a binary, as opposed whether 
it's diff'ed, it would imo make sense to call it -binary.


Cheers,

		Frans

Re: Bug report: .gitattributes: -diff Unset causes files to be reported as binaries

[Paolo Bonzini]

>> So as far as I can see, git is behaving exactly as it is supposed to.
>> Maybe you can be more specific about what effect you were trying to
>> achieve by setting gitattributes in the first place?
>
> To exclude it in diffs, such as from `git show`. Take the case where you have
> a grammar file for a parser and generate a source file from it(or any similar
> scenario); the diff for the generated source file is not of interest and is
> just noisy when read as part of a patch. This applies to all kinds of
> generated files. However, this doesn't mean that the file should be treated
> as a binary, and what practicalities that implies.

I am not sure it is a good idea, but you can do this with

FILE diff=/bin/true

> If -diff affects whether a file is treated as a binary, as opposed whether
> it's diff'ed, it would imo make sense to call it -binary.

No, diff affects how a file is diffed.  The particular setting "-diff" 
diffs the file as if it was binary.

Paolo

Re: Bug report: .gitattributes: -diff Unset causes files to be reported as binaries

[Frans Englich]

On Thursday 02 July 2009 13:12:28 Paolo Bonzini wrote:
> >> So as far as I can see, git is behaving exactly as it is supposed to.
> >> Maybe you can be more specific about what effect you were trying to
> >> achieve by setting gitattributes in the first place?
> >
> > To exclude it in diffs, such as from `git show`. Take the case where you
> > have a grammar file for a parser and generate a source file from it(or
> > any similar scenario); the diff for the generated source file is not of
> > interest and is just noisy when read as part of a patch. This applies to
> > all kinds of generated files. However, this doesn't mean that the file
> > should be treated as a binary, and what practicalities that implies.
>
> I am not sure it is a good idea, but you can do this with
>
> FILE diff=/bin/true
>
> > If -diff affects whether a file is treated as a binary, as opposed
> > whether it's diff'ed, it would imo make sense to call it -binary.
>
> No, diff affects how a file is diffed.  The particular setting "-diff"
> diffs the file as if it was binary.

Aha, then we're maybe at conclusion; if -diff doesn't cause the files to be 
treated as binaries, then the phrase "Binary files a/file.txt and b/file.txt 
differ" is wrong, it shouldn't read "Binary files", but maybe rather "The 
files".


Cheers,

		Frans

Re: Bug report: .gitattributes: -diff Unset causes files to be reported as binaries

[Paolo Bonzini]

>>> If -diff affects whether a file is treated as a binary, as opposed
>>> whether it's diff'ed, it would imo make sense to call it -binary.
>> No, diff affects how a file is diffed.  The particular setting "-diff"
>> diffs the file as if it was binary.
>
> Aha, then we're maybe at conclusion; if -diff doesn't cause the files to be
                                                 ^^^^^^^

???

> treated as binaries

Reread carefully.

1) There is no "-diff" attribute.  There is a "diff" attribute that 
affects *how* (not *whether*) files are diff.

2) A "-" in front of an attribute unsets the attribute.

3) The "diff" attribute, when unset, treats the file as binary.

ergo

4) Using "-diff" in .gitattributes treats the final as binary.

Paolo

Re: Bug report: .gitattributes: -diff Unset causes files to be reported as binaries

[Jakub Narebski]

Frans Englich <fenglich@fastmail.fm> writes:
> On Thursday 02 July 2009 07:35:34 Jeff King wrote:
> > On Wed, Jul 01, 2009 at 12:08:35PM +0200, Frans Englich wrote:

> > > Applying -diff Unset to a file using .gittattributes causes "git diff"
> > > to state that the file is a binary even though it isn't, or have been
> > > instructed to be treated as one. See attached script for reproducing.
> >
> > I think you are a little confused by the syntax. Each line of the
> > gitattributes file has a filename pattern and a set of attributes. Each
> > attribute is either set, unset, set to a value, or unspecified. For your
> > example (file.txt and the "diff" attribute), they look like:
> 
> Perhaps that should be considered another bug; that invalid syntax is 
> accepted, instead of being communicated to the user.

This is not a bug, this is a feature.  

You can add extra attributes which are unknown to Git (like "Unset"),
just like you can add config variables in $GIT_DIR/config file which
are unknown to Git, to be used by some wrapper, tool or porcelain.  An
example for unknown config variables is e.g. stgit.sender or
stgit.editor (for StGIT patch management interface).
 
> [...]
> > So as far as I can see, git is behaving exactly as it is supposed to.
> > Maybe you can be more specific about what effect you were trying to
> > achieve by setting gitattributes in the first place?
> 
> To exclude it in diffs, such as from `git show`. Take the case where
> you have a grammar file for a parser and generate a source file from
> it(or any similar scenario); the diff for the generated source file
> is not of interest and is just noisy when read as part of a
> patch. This applies to all kinds of generated files. However, this
> doesn't mean that the file should be treated as a binary, and what
> practicalities that implies.
> 
> If -diff affects whether a file is treated as a binary, as opposed
> whether it's diff'ed, it would imo make sense to call it -binary.

Actually there exists already "binary" gitattribute, or to be more
exact "binary" built-in attribute macro.  "binary" means "-diff -crlf",
as you can see at the bottom of gitattributes(5) manpage.

The message "Binary files differ" is taken from GNU diff, I think.

-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: Bug report: .gitattributes: -diff Unset causes files to be reported as binaries

[René Scharfe]

Frans Englich schrieb:
> Hi,
> 
> Applying -diff Unset to a file using .gittattributes causes "git diff" to 
> state that the file is a binary even though it isn't, or have been instructed 
> to be treated as one. See attached script for reproducing.
> 
> This causes confusion since the file isn't a binary, it's only the diff output 
> that has been suppressed. For instance, an auto generated cpp file has -diff 
> Unset applied to it to suppress the meaningless output, but people contact 
> you to point out the file is binary.

Generated files normally don't need to be tracked, as long as all their
sources are tracked and the tools to generate them are readily available
to your target audience.  So perhaps ceasing to track the files and
adding them to the file .gitignore instead is an option?

René

Re: Bug report: .gitattributes: -diff Unset causes files to be reported as binaries

[Jeff King]

On Thu, Jul 02, 2009 at 10:14:06AM +0200, Frans Englich wrote:

> > I think you are a little confused by the syntax. Each line of the
> > gitattributes file has a filename pattern and a set of attributes. Each
> > attribute is either set, unset, set to a value, or unspecified. For your
> > example (file.txt and the "diff" attribute), they look like:
> 
> Perhaps that should be considered another bug; that invalid syntax is 
> accepted, instead of being communicated to the user.

It's not invalid syntax; it just doesn't do what you thought it did. And
even though it is _semantically_ useless to git, it is a feature of
gitattributes (and the git config) to allow unknown keys, so that they
can be used by other programs and scripts built on top of git.

> > So as far as I can see, git is behaving exactly as it is supposed to.
> > Maybe you can be more specific about what effect you were trying to
> > achieve by setting gitattributes in the first place?
> 
> To exclude it in diffs, such as from `git show`. Take the case where
> you have a grammar file for a parser and generate a source file from
> it(or any similar scenario); the diff for the generated source file is
> not of interest and is just noisy when read as part of a patch. This
> applies to all kinds of generated files. However, this doesn't mean
> that the file should be treated as a binary, and what practicalities
> that implies.

The usual workflow is not to check in generated files at all. You can
mark them with '.gitignore' to make sure they are not actually added.

However, I can imagine a case where your workflow required checking in
generated files (because, e.g., the tools to generate them were
difficult to use or not available to all developers), but most of the
time seeing their diffs is just clutter. So the instruction you want to
give to git is "when diffing these, generate nothing". And that is:

  $ echo file.txt diff=invisible >>.gitattributes
  $ git config diff.invisible.command /bin/true

Or even some other custom script of your choosing that might show a more
condensed diff.

> If -diff affects whether a file is treated as a binary, as opposed
> whether it's diff'ed, it would imo make sense to call it -binary.

I'm not sure I agree, but even if I did, it is far too late to change
the semantics of the "diff" and "binary" attributes; you would be
breaking the existing setup of many other users.

-Peff

Re: Bug report: .gitattributes: -diff Unset causes files to be reported as binaries

[Jeff King]

On Thu, Jul 02, 2009 at 01:12:28PM +0200, Paolo Bonzini wrote:

> >To exclude it in diffs, such as from `git show`. Take the case where you have
> >a grammar file for a parser and generate a source file from it(or any similar
> >scenario); the diff for the generated source file is not of interest and is
> >just noisy when read as part of a patch. This applies to all kinds of
> >generated files. However, this doesn't mean that the file should be treated
> >as a binary, and what practicalities that implies.
> 
> I am not sure it is a good idea, but you can do this with
> 
> FILE diff=/bin/true

This is a good idea Paolo (and I stole it when responding elsewhere in
the thread ;) ), but it should actually be:

  FILE diff=invisible

and then putting

   [diff "invisible"]
     command = /bin/true

in your config.

-Peff