[PATCH] D124606: Use `-text` git attribute instead of `text eol=...'

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 28 07:18:14 PDT 2022


ilya-biryukov added a comment.

In D124606#3480012 <https://reviews.llvm.org/D124606#3480012>, @aaronpuchert wrote:

> I think this change broke again what D97625 <https://reviews.llvm.org/D97625> was trying to fix. These are text files and we want them to keep the specified line endings regardless of the local git configuration. See also @smeenai's comment D124563#3478625 <https://reviews.llvm.org/D124563#3478625>.

This change should be doing exactly that.
The behaviour in the mentioned comment should apply when the text attribute is unspecified, not when it's explicitly unset.
>From git docs <https://git-scm.com/docs/gitattributes>:

  Unset
  Unsetting the text attribute on a path tells Git not to attempt any end-of-line conversion upon checkin or checkout.
  
  Unspecified
  If the text attribute is unspecified, Git uses the core.autocrlf configuration variable to determine if the file should be converted.



================
Comment at: clang-tools-extra/test/.gitattributes:7-15
+clang-apply-replacements/ClangRenameClassReplacements.cpp -text
+clang-apply-replacements/Inputs/basic/basic.h -text
+clang-apply-replacements/Inputs/format/no.cpp -text
+clang-apply-replacements/Inputs/format/yes.cpp -text
+clang-tidy/infrastructure/export-diagnostics.cpp -text
 
 # These test input files rely on two-byte Windows (CRLF) line endings.
----------------
labath wrote:
> aaronpuchert wrote:
> > My understanding is that `-text` removes an attribute, but what attribute is there to remove? There is no global `.gitattributes` that would set it in the first place. This change just reverts to the status quo before D97625, or rather before that file was moved.
> Are you sure about that? Git seems to distinguish between "unset" and "unspecified" values of an attribute:
> ```
>        Each attribute can be in one of these states for a given path:
> 
>        Set
>            The path has the attribute with special value "true"; this is specified by listing only the
>            name of the attribute in the attribute list.
> 
>        Unset
>            The path has the attribute with special value "false"; this is specified by listing the
>            name of the attribute prefixed with a dash - in the attribute list.
> 
>        Set to a value
>            The path has the attribute with specified string value; this is specified by listing the
>            name of the attribute followed by an equal sign = and its value in the attribute list.
> 
>        Unspecified
>            No pattern matches the path, and nothing says if the path has or does not have the
>            attribute, the attribute for the path is said to be Unspecified.
> 
> ```
> 
> And then it says this about the `text` attribute in particular:
> ```
>            Set
>                Setting the text attribute on a path enables end-of-line normalization and marks the
>                path as a text file. End-of-line conversion takes place without guessing the content
>                type.
> 
>            Unset
>                Unsetting the text attribute on a path tells Git not to attempt any end-of-line
>                conversion upon checkin or checkout.
> 
>            Set to string value "auto"
>                When text is set to "auto", the path is marked for automatic end-of-line conversion. If
>                Git decides that the content is text, its line endings are converted to LF on checkin.
>                When the file has been committed with CRLF, no conversion is done.
> 
>            Unspecified
>                If the text attribute is unspecified, Git uses the core.autocrlf configuration variable
>                to determine if the file should be converted.
> 
> ```
The [[ https://git-scm.com/docs/gitattributes | git docs ]] seem to be pretty clear that unsetting the attribute should do the right thing:
```
Unset
Unsetting the text attribute on a path tells Git not to attempt any end-of-line conversion upon checkin or checkout.
```

It seems git is supposed to do the right thing for us here, unless I misunderstand something.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124606/new/

https://reviews.llvm.org/D124606



More information about the cfe-commits mailing list