Buildifier overwrites Windows line endings with Unix line endings

[grossag]

I built and ran buildifier on Windows and found that when I run it on .bzl files (e.g. BUILD.bzl) with Windows CRLF line endings, it rewrites the file with Unix line endings. It would be nice if buildifier remembered the original file line endings and outputted the file with those same line endings.

[laurentlb]

That's intended. The goal of Buildifier is to format files using a canonical formatting. I don't think it should have a different output depending on the platform.

Note that gofmt seems to do the same.

[grossag]

Thanks for the response! The problem I'm running into is that this interferes with developer workflows if their source control is set to use CRLF (e.g. git's "core.autocrlf=true" or Perforce's default line ending behavior on Windows). After running "bazel buildifier", Visual Studio warns that the line endings are inconsistent in the file.

[vladmos]

You can try creating a script that runs buildifier and then replaces \n with \r\n. It may be not correct if you have multiline string literals where the line ending matters, but I assume git's autocrlf does the same.

In theory it's not hard to add a flag to buildifier to make it use Windows line endings, but in that case it will generate files that are considered not formatter by a default buildifier (without the flag enabled), that can break developer workflows too.

[grossag]

My biggest concern with buildifier is that it is actively overwriting my line endings. I'm not asking for buildifier to change my line endings from \n to \r\n. Instead, I'm just asking that it not overwrite the existing line ending format of my file. As-is, I effectively need to wrap buildifier in a script where I check the line endings, run buildifier, then restore the line endings if they were changed. That seems unnecessary and an indication that buildifier is doing something destructive.

[davinci26]

Personally encountering this problem as well and I would love to get it fixed. Diff has a parameter --strip-trailing-cr

Is Bazel accepting patches for this feature?

I see there was an attempt at #50 but it never got merged

[crt-31]

Bump. This really should be addressed now that Bazel officially supports Windows. CRLF is what is expected by basically all Windows tools, and it is basically best practice to use Git's core.autocrlf=true.

As for a flag, I'd suggest maybe a buildifier option called "linendings", with options "os_default", "lf", and "preserve". I think default would be os_default, but up for debate I suppose.

I could whip up a PR if that will get the ball rolling on this again.

Sound good? Thoughts?

[vladmos]

I don't have a windows machine at the moment to check this out, but how does it work with go files and gofmt? E.g. if you check out this repository with core.autocrlf=true, what line endings will go files have? And will they stay the same after you run gofmt on them? And what about other languages (like python and pyformat)?

[crt-31]

Thanks, @vladmos for continuing this discussion. I tried the things you asked about on my Windows box:

• checkout this repo with core.autocrlf=true: go files have CRLF
• run gofmt: go files will have LF
• run pyformat (via autopep8): py files stay CRLF

Note that gofmt changing line endings has been heavily debated and they seem to have landed on "go is opinionated and this is gofmt's opinion", c'est la vie. Other language formatters seem to deal with line endings more flexible and proper ways, either by ignoring them, or with a config option to control the behavior.

In terms of skylark files, there is no need to be so opinionated. If we could add a line-ending config option to buildifier, it will be best of both worlds and allow whatever you want. What it should definately NOT do is change the line-endings without any way to config it. Other things in buildifier are configurable, so line-endings should be configurable too.

As for repos with files like *.go require endings to be LF, the proper thing is probably to use "*.go text eol=lf" in a .gitattrbutes file.