System-default line separators are implicitly enforced despite lax configurations

[ParkerM]

Context: I'm a Windows user that often builds Spring projects from source. My global .gitconfig sets core.autocrlf=input because I want to avoid CRLF line endings by any means necessary. This clashes specifically with spring-javaformat, and makes it a huge pain to build any Spring project.

Example output with spring-projects/spring-boot

$ git config --get core.autocrlf
input

$ ./gradlew build

> Task :spring-boot-project:spring-boot-tools:spring-boot-antlib:integrationTest  
Trying to override old definition of task fail
Trying to override old definition of datatype resources
Trying to override old definition of task buildnumber

> Task :spring-boot-project:spring-boot-tools:spring-boot-maven-plugin:checkFormatMain FAILED

> Task :spring-boot-project:spring-boot:compileJava
Note: Processing Log4j annotations
...

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':spring-boot-project:spring-boot-tools:spring-boot-maven-plugin:checkFormatMain'.
> Formatting violations found in the following files:
   * src\main\java\org\springframework\boot\maven\AbstractDependencyFilterMojo.java
   * src\main\java\org\springframework\boot\maven\AbstractPackagerMojo.java
   * src\main\java\org\springframework\boot\maven\AbstractRunMojo.java
   * src\main\java\org\springframework\boot\maven\ArtifactsLibraries.java
   * src\main\java\org\springframework\boot\maven\BuildImageMojo.java
   * src\main\java\org\springframework\boot\maven\BuildInfoMojo.java
   * src\main\java\org\springframework\boot\maven\CustomLayersProvider.java
...

  Run `format` to fix.

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. Run with --scan to get full insights.

* Get more help at https://help.gradle.org

BUILD FAILED in 40s
846 actionable tasks: 552 executed, 287 from cache, 7 up-to-date

A build scan was not published as you have not authenticated with server 'ge.spring.io'.

# let's run format
$ ./gradlew format

BUILD SUCCESSFUL in 10s
224 actionable tasks: 224 executed

A build scan was not published as you have not authenticated with server 'ge.spring.io'.

# it just converts all my line-endings to CRLF
$ git st | wc -l
5986

$ git diff
warning: CRLF will be replaced by LF in spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/OnEndpointElementCondition.java.
The file will have its original line endings in your working directory
warning: CRLF will be replaced by LF in spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/amqp/RabbitHealthContributorAutoConfiguration.java.
...

I first assumed this had something to do with Checkstyle, but I do not believe this is the case, especially since the default NewlineAtEndOfFileCheck was relaxed to accept any trailing newline in 0b36c1c.

Another contributor touched on the crux of the problem here:

Anyways what I have found is, when sources are checked out with LF and formatter runs on Windows reformatted code has line endings that comes from org.eclipse.jdt.internal.compiler.util.Util#LINE_SEPARATOR constant.
I think that this glitch should be documented somehow (maybe here or at spring-cloud docs). What do you think?
Thank you

It seems the discussion fell short once the issue was closed as fixed, but the fix only supports the "Windows user with CRLF line endings" case.

From what I've gathered, whenever a line separator is not explicitly specified, it is passed down as null to the internals of the Eclipse TextEdit stuff that occurs during the validate/format phases, wherein the default line separator null turns into System.lineSeparator() (which does not abide my git config).

---

My proposal is to attempt to detect and pass along the line endings of each file before it is submitted to the whims of the Eclipse formatter. Essentially:

• If io.spring.javaformat.formatter.Formatter#format is called with lineSeparator = null
  • Try to naively determine the configured line separator for the given source by checking each line separator.
  • Iff every line separator is the same, then pass it as an arg to this.delegate.format.
  • Otherwise, fallback to the existing behavior of passing a null line separator to this.delegate.format by default.

Here's a clumsy and un-tested whack at the impl I'm describing: ParkerM@92579ca

---

I'd be happy to create some repros via various CI runners and git configs, but will hold off until it's deemed necessary.

[philwebb]

We had a recent pull-request to Spring Boot to add a .gitattributes file. I'm wondering if this would force LF as the default line ending?

[ParkerM]

Yeah. If my understanding is correct, that .gitattributes file would enforce CRLF on .bat text files, and enforce LF on all other text files (i.e. all the rest of the source code).

The .gitattributes file for posterity:

* text=auto eol=lf

*.bat text eol=crlf
*.jar binary 

If this .gitattributes was added to Spring Boot, then it's likely that spring-javaformat would make it impossible to build the project on any Windows machine despite the user's git config, once again due to the Eclipse formatter falling back to system-default line separators.

---

Edit: Here's an example of the formatting-related failures on a Windows runner after adding the .gitattributes described above: https://github.com/ParkerM/spring-boot/runs/3367396813#step:5:2108

[ParkerM]

@philwebb Could we add a bug label to this?

[ParkerM]

I tinkered with my PoC a bit and reached a seemingly-working state, where a fresh clone builds without fuss (tested on Windows and Ubuntu): https://github.com/ParkerM/spring-javaformat/tree/preserve-eol

9f0173b adds some test fixtures for CR/LF/CRLF line endings by specifying them in .gitattributes files, borrowing from Checkstyle's approach.
The impl is in ef0d637, and the followup commits are small test changes to fix system-dependent build failures.

If there's any interest let me know and I'll open a PR.

[ParkerM]

@philwebb I opened #340 for visibility if you or others would like to try it out. I'm curious whether or not my approach aligns with the intended behavior wrt handling line separators.

[philwebb]

Closing in favor of PR #340. Thanks @ParkerM!