[libcxx-commits] [clang] [clang-tools-extra] [compiler-rt] [flang] [lld] [lldb] [llvm] [mlir] [openmp] [pstl] Finally formalise our defacto line-ending policy (PR #86318)

Saleem Abdulrasool via libcxx-commits libcxx-commits at lists.llvm.org
Thu Apr 25 09:57:30 PDT 2024


compnerd wrote:

> @compnerd I just realised I didn't respond to your concern. Apologies.
> 
> > I think that the concern that I have is that do we have sufficient testing for supporting line-ending dependent behaviour in the compiler?
> 
> For the first part: I don't know that it matters, since this patch changes how the LLVM source code is stored, but not how its parsers operate. In any case, we run parser testing on LF and CRLF systems since the precommit testing runs on Linux, and Win32 at least. In addition, and there are several tests dedicated to line ending, so I think we should be good there. Happy to tend to this and fix anything I've missed if someone lets me know something is broken after we merge.

I don't know if the pre-commit testing guarantees that. Configuration settings will permit the files to be checked out in either Unix (`\n`) or Windows (`\r\n`) line-endings. If the builders are all configured to run in Unix line endings we lose that coverage. While I understand that this change only changes how the LLVM sources are stored, the issue is that the LLVM sources include the _tests_ directory. These need to be tested in various manners to ensure that we test how we handle the different inputs (in clang). One option might be to exclude the tests directory from the line ending conversion if we cannot verify that we have tests in both formats.

> As for the second part
> 
> > Additionally, do we have sufficient documentation for others to figure out how to ensure that git does not munge the line endings if they are introducing a test which is dependent on it? In such a case, how do we ensure that they are aware that the SCM will do so without actually checking the post-commit state with a hex editor?
> 
> I don't think we do, but also this patch doesn't really change the problem since right now basically _anything_ can happen due to local configuration overrides (`~/.gitconfig`). I'll add a comment to the testing infrastructure page to be mindful of how line endings are storedand link to the `.gitattributes` documentation. Does that sound enough?

I think that the documentation should be good. While, yes, it is possible to get the wrong behaviour currently, if the user configures git explicitly for a line-ending, I would expect them to be aware of that. The use of `.gitattributes` means that their defaults are not necessarily honoured and thus this can catch them by surprise.

https://github.com/llvm/llvm-project/pull/86318


More information about the libcxx-commits mailing list