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
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to