Normalize to LF in the repository. New policy: files are binary unless #2323
Conversation
|
Python and Perl scripts should also be text. |
|
Possibly also Objective-C files (*.m) of which we have one for the OS X build. Given the problems last time I'd like to be extra-careful with this, which may require some coordination. I'd like sign-off from two people (the merger and one other) on the actual final commits. Preferably to check this: apply the gitattributes file yourself and do your own normalisation and check that you end up with the same tree as the PR. The merge should then be done as a fast-forward to guarantee no conflicts or weirdness, and so that master ends up at the exact state that has been signed-off. When we've confirmed that the gitattributes file itself is configured the way we want, then the commits will need to be remade on top of master to allow for the fast-forward merge. |
|
The marking and the normalization steps are separate. To avoid any confusion, I've made a list of files which do have CRs, using this command: These are the only files that can possibly need normalization. No file outside that list should appear in the normalization commit. Every file marked as text in .gitattributes and present in that list should appear. Here's the list: That pretty much covers the normalization part, which depends on what files are marked as text, that is, the contents of the .gitattributes file. Now there's an argument against *.vcproj, *.vcxproj, *.vcxproj.filters, *.sln and *.props files (collectively, textual VC files), on one hand, and against .obj and .DAE files (collectively, textual 3D files) being marked as text. I disagree with the arguments. Textual VC files matter little, as it's extremely unlikely that someone under an OS different than Windows will use or modify them, unless there's some sort of cross-compiler or something like that that uses/generates them. But the argument was that merge conflicts are almost impossible to resolve, and that happens either with binary or with text files, so that's not a reason to exclude them. Merge conflicts in that case should be handled by either overwriting or ditching the whole file, just as with binary files. The text attribute in .gitattributes only influences line endings, not conflict handling. That's an argument for using the merge attribute on them; it does not have to do with the text attribute. I haven't looked into the merge attribute in depth yet. As for textual 3D files, I don't think that they should be kept as binary. The idea of a file being text is that it is checked out from the repository in the OS's native line ending format. Currently, there are .obj files with CRs and .obj files without CRs in the repository, as well as .DAE files with CRs and .DAE files without CRs. In the current situation, if someone regenerates from Linux a .DAE or .obj file that was generated under Windows, git will interpret that every line in the whole file has changed, when possibly only a minuscule part did actually change. And same if a Linux-saved file is regenerated from Windows. So, by checking out the files in the OS's native format, git would work properly with them, minimizing the differences, without interfering with normal workflow. An argument can be made that an .obj file could potentially be a binary file if Intel OMF files are ever included in the repository (that's an unfortunate extension clash). It's highly unlikely that OMF files are ever included, but if that's to be prevented, .obj files could be marked as text=auto. Wavefront .obj files will surely be correctly interpreted by git as text, as there's little chance that a character outside the printable ASCII range or CR/LF appears there, not even an Unicode one. And Intel OMF .obj files have records with a type indicator that is always above 0x80, so it's highly unlikely to be ever interpreted as text. Now, as for what files should be marked as text, I've also made a list of the extensions excluding the same binary files as above, and the extensionless files present in the repository, using this command: (The first sed line removes the path; the second sed line removes everything before the first dot only if there's a dot. I've separated them just for clarity when reviewed.) Conservatively, that command removes up to the first dot, not up to the last. The list doesn't include dotfiles in the top dir, mainly to prevent find from recursing into .git. Here's the result: After reviewing them one by one, I think that every file in that list should be marked as text, excluding the dupes (.0.sln, .0.txt, .0.vcproj, .1.txt, .frag.glsl, .vert.glsl, .music.txt, .inc.h). The proposed .gitattributes would thus be: Followed by a normalization commit that includes the whole list of files listed at the beginning. [Edited for grammar] |
|
After an in-depth review of https://www.kernel.org/pub/software/scm/git/docs/gitattributes.html I think that the first line should instead say: which is equivalent to: * -text -diff -merge. The page also says that When more than one pattern matches the path, a later line overrides an earlier line. This overriding is done per attribute. Since overriding is done per attribute, and only the text attribute is declared, the above .gitattributes is wrong, as it doesn't include diff and merge. And to avoid marking each file as 'text diff merge', we should also make a custom attribute for our text files, which following the example in the DEFINING MACRO ATTRIBUTES section, should probably look like this: Then we can change the rest of lines to include pitext instead of text. If merge conflicts are a concern for textual VC files, their lines can be changed to: |
|
The PR is amended as described. The normalized files list, which was created by git following the procedure in http://stackoverflow.com/questions/1510798/trying-to-fix-line-endings-with-git-filter-branch-but-having-no-luck/1511273#1511273 , matches exactly the list of files having CRs obtained with rgrep (compared after some grep/sort/cut trickery). Note that that list was obtained from a fresh clone with core.autocrlf unset, so that no CRLF conversion had taken place. For reproducibility: is the list after committing the new .gitattributes file and after following the normalization steps except the commit itself, and yields the same file as the following command when run on a fresh clone: |
…attributes Modified as per discussion of PR pioneerspacesim#2323.
|
My own normalisation commit on top of I'm fine with the contents of I will update the build scripts to ensure that |
|
Done. In the end I went with a simpler attributes file, because I wasn't sure if we need anything more complicated. If that turns out to be the case, we can try the more complex attributes file presented here. I hope we can keep going forward now. |
explicitly marked as text in .gitattributes.