@@ -0,0 +1 @@
+dos-style-eol.txt
MaskRay wrote:
missing text=auto eol=crlf ?
https://github.com/llvm/llvm-project/pull/86318
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
ldrumm wrote:
> There are a bunch of tests that fail if you don't.
Yes. Even better than that: this patch just uncovered a [legitimate bug in the
C++ AST parser tests for clang
> Our instructions tell people they need to disable autocrlf
I didn't see that. Shall I remove that instruction
llvm-beanz wrote:
> 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.
Today on Windows you basically have to check out LLVM as unix line endings.
There are a
https://github.com/ldrumm edited https://github.com/llvm/llvm-project/pull/86318
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
ldrumm wrote:
Perhaps things have got lost during the discussion, but my this part of my
original commit message is perhaps worth re-reading:
> In simple terms this means "unless otherwise specified, convert all files
> considered "text" files to LF in the project history, but checkout them
ldrumm wrote:
> Note, the changes to clang-format-vs should likely all be reverted. .sln is a
> Microsoft Visual Studio solution file, as are many of the others
Right. .sln is a text file:
ldrumm wrote:
> Also, it's a bit funny to have .bat files without CRLF endings given that
> they run on Windows
They do have CRLF line endings:
compnerd wrote:
> Also, it's a bit funny to have .bat files without CRLF endings given that
> they run on Windows.
I'm not sure about the funny bit - but certainly dangerous. I've had cmd
misinterpret batch files with LF vs CRLF.
https://github.com/llvm/llvm-project/pull/86318
https://github.com/AaronBallman commented:
I don't want this to be considered a comment which blocks the changes if others
feel strongly in favor of them, but I'm not really convinced we should do this.
The number of problems this patch will solve is approximately zero but the
number of
compnerd wrote:
> changes to `clang/test/CXX/lex/lex.literal/lex.string/p4.cpp` should be
> reverted (it's a CRLF related test)
This is the type of problems that I am concerned about. We certainly have some
tests which are line-ending sensitive, and each test should be audited before
we make
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,
cor3ntin wrote:
changes to `clang/test/CXX/lex/lex.literal/lex.string/p4.cpp` should be
reverted (it's a CRLF related test)
https://github.com/llvm/llvm-project/pull/86318
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
ldrumm wrote:
> 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?
Added
ldrumm 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
https://github.com/llvm-beanz approved this pull request.
I'm happy with this approach. We can adjust and iteratively address issues if
any appear.
https://github.com/llvm/llvm-project/pull/86318
___
cfe-commits mailing list
ldrumm wrote:
I've added the natvis changes and added a couple of clangd test exceptions.
For the sanity of pending reviewers, and to make rebasing this trivial I've
split this into two commits: the gitattributes changes, followed by the
`--renormalize` fixup.
For downstreams, I'm
compnerd wrote:
Philosophically, I agree with this change. Enshrining the information about the
line endings into the SCM tool makes sense.
I think that the concern that I have is that do we have sufficient testing for
supporting line-ending dependent behaviour in the compiler? Additionally,
llvm-beanz wrote:
> I'm a bit confused. Are you saying that as I've expressed it, `autocrlf=true`
> won't work correctly? I _think_ you're saying you're in favour of this patch
> _in principal_, but I need to fix the mixed line endings case?
Sorry for being unclear. I meant to express that
llvm-beanz wrote:
> This ^ seems a bit problematic to me, though (where the contents of the repo
> isn't representative of the bits we want - but perhaps it's schrodinger's
> line endings & it doesn't matter if it's always checked out as crlf - though
> if we one day converted to another
llvm-beanz wrote:
> I am not saying this isn't a problem at all, but how often has anyone done a
> one line change and caused a 50k diff, and submitted it without noticing?
Way more often than you would hope. I don't have numbers but if you search
through the git history for `crlf` or
fmayer wrote:
> . The point of this patch is not to lambast developers or interfere with
> their local setups; it's to get the line-ending issues out of the way for
> good so they can focus on what they do best.
Fair enough. I don't think it will fully make them go away for good, as you
ldrumm wrote:
> > That wish is fine until you start working with others.
>
> Do we actually have that little faith in developers that we think they will
> check in a 50k line diff?
depending on their diff settings, or the web interface they use, it may not
show until they actually look at a
fmayer wrote:
> That wish is fine until you start working with others.
Do we actually have that little faith in developers that we think they will
check in a 50k line diff?
https://github.com/llvm/llvm-project/pull/86318
___
cfe-commits mailing list
ldrumm wrote:
> I don't have a strong opinion, but fundamentally I would prefer if the source
> control system stored exactly the files I have in my checkout, not mess with
> them in any way. I understand there are practical concerns, but a linter for
> unexpected CRLF would maybe be an
fmayer wrote:
I don't have a strong opinion, but fundamentally I would prefer if the source
control system stored exactly the files I have in my checkout, not mess with
them in any way. I understand there are practical concerns, but a linter for
unexpected CRLF would maybe be an option?
dwblaikie wrote:
> For those files in the repository that do need CRLF endings, I've adopted a
> policy of eol=crlf which means that git will store them in history with LF,
> but regardless of user config, they'll be checked out in tree with CRLF.
This ^ seems a bit problematic to me, though
ldrumm wrote:
> Should we come up with an expected filename format that we can use with
> gitattributes and rename the existing files rather than just listing each
> file that has special needs by itself?
Either way, the author has to do or know *something*. I'm not a fan of "magic"
like
llvm-beanz wrote:
Philosophically I agree with this change, but I think some subprojects may need
to adopt specific policies about how they handle files that are expected to be
specific line endings. Specifically, Clang needs testing that verifies correct
behavior on both CRLF and LF files in
ldrumm wrote:
On Mon Mar 25, 2024 at 1:48 PM GMT, Tobias Hieta wrote:
> > I think .natvis files should be CRLF (those are used with Visual Studio for
> > debug visualizers).
>
> Agreed, Visual Studio should handle this correctly, but who knows
> 路
>
Thanks. I'll add a rule and update
tru wrote:
> I think .natvis files should be CRLF (those are used with Visual Studio for
> debug visualizers).
Agreed, Visual Studio should handle this correctly, but who knows 路
https://github.com/llvm/llvm-project/pull/86318
___
cfe-commits
https://github.com/AaronBallman commented:
The changes seem reasonable to me, but there are test failures in
`llvm/utils/lit/tests` that could perhaps be related to your changes.
I think .natvis files should be CRLF (those are used with Visual Studio for
debug visualizers).
ldrumm wrote:
> LGTM overall, but the CI failure looks real.
```
Failed Tests (3):
Clang :: CXX/drs/dr23xx.cpp
Clang :: CXX/drs/dr6xx.cpp
Clang :: Lexer/raw-string-dlim-invalid.cpp
```
None of these files are in the diffstat, and none of them include any headers.
I'm a bit stumped. Is
joker-eph wrote:
LGTM overall, but the CI failure looks real.
https://github.com/llvm/llvm-project/pull/86318
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rnk wrote:
> As a data point, I've been setting core.autocrlf=true on Windows for years.
If that's the case, my information is stale, which I accept.
> To be clear, this may do nothing on checkout if a user has set a config
> option.
That addresses my concerns.
ldrumm wrote:
> So, is there a way to do CRLF -> LF on checkin, but do nothing on checkout?
To be clear, this *may* do nothing on checkout if a user has set a config
option. The point of the present policy is not to control local preferences as
far as the filetype isn't *reqired* to have a
pogo59 wrote:
As a data point, I've been setting `core.autocrlf=true` on Windows for years.
I've been trained to make sure _new_ files have LF endings (usually I copy an
existing file instead of creating a new file) but both Notepad and the Visual
Studio editor are willing to preserve the
rnk wrote:
I think it makes sense to remove carriage returns on checkin, but I'm not sure
it makes sense to add them back on checkout on Windows. Historically, it's been
really easy to write LLVM tests that fail when the source is checked out with
CRLF. Many Windows developers have noted that
adrian-prantl wrote:
While I'm sure this commit will manage to break _something_ unexpectedly, I
think this is reasonable to do! Thanks for taking this on.
https://github.com/llvm/llvm-project/pull/86318
___
cfe-commits mailing list
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff cd8286a667d568c4319b09baa63ba899e3101a19
2dfda2e4b11c7ea0a81dd4dcd43aa426039d1e0a --
ldrumm wrote:
I'm sure there are lots of people that will want to look at this, so please
bring in reviewers at will
https://github.com/llvm/llvm-project/pull/86318
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
llvmbot wrote:
@llvm/pr-subscribers-backend-x86
Author: None (ldrumm)
Changes
Historically, we've not automatically enforced how git tracks line endings, but
there are many, many commits that "undo" unintended CRLFs getting into history.
`git log --pretty=oneline --grep=CRLF` shows
llvmbot wrote:
@llvm/pr-subscribers-clangd
@llvm/pr-subscribers-lldb
Author: None (ldrumm)
Changes
Historically, we've not automatically enforced how git tracks line endings, but
there are many, many commits that "undo" unintended CRLFs getting into history.
`git log --pretty=oneline
42 matches
Mail list logo
