[clang-format] Solution to #205036 may cause problems on Windows machines with diff in PATH
- Dominant language
- LLVM
- Stars
- 40.5k
- Forks
- 18.7k
- PR merge metrics
- PR metrics pending
Description
Reference to PR: https://github.com/llvm/llvm-project/pull/205036
This PR may cause issues if a windows machine has diff (and generally some flavor of unix tools) available in the PATH as it will then run that diff.
This will fail under certain circumstances due to the following:
- differences in line endings (when git pulls files from a repo it may or may not add CR characters).
- difference in the operation of clang-format with respect to the environment.
I'm not a clang-format developer so I have not dived into the details here but only run trials on a Jenkins windows pipeline.
I've seen the following:
1. Changing the diff command like below in clang/lib/Format/CMakeLists.txt to use `--strip-trailing-cr` reduces the problem but I still get a few weirdly formatted files in the test and thus failures on Windows (this might be a bug in itself).
```
add_custom_command(OUTPUT check_format_depend_${i}
COMMAND clang-format ${file} | diff --strip-trailing-cr -u ${file} - &&
touch check_format_depend_${i}
VERBATIM
```
2. If, in addition, I add LineEnding in clang/lib/Format/.clang-format and set it like this:
```
BasedOnStyle: clang-format
LineEnding: DeriveCRLF
```
Everything works fine. This seems to hide or get rid of the bug I experience when the LineEnding: LF is in play (from .clang-format in the root llvm directory).
The `--strip-trailing-cr` is necessary on Windows because `stdout` is opened by diff in text mode and will replace the CRLF produced by clang-format with LF.
Someone with more knowledge and involvement in clang-format should probably look into this. My suggestion would be to go for modification 2.
Contributor guide
Research direction
Start with clang/lib/Format/CMakeLists.txt and compare the root .clang-format with clang/lib/Format/.clang-format, then review PR 205036's change. Reproduce the Jenkins Windows scenario with diff in PATH and verify the clang-format check handles line endings and produces stable formatting without failures.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cmake
- Domain
- build-system, tooling
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100