Skip to content

[TableGen] Handle Windows line endings in x86-fold-tables.td test #112997

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 21, 2024

Conversation

dpaoliello
Copy link
Contributor

@dpaoliello dpaoliello commented Oct 18, 2024

The x86-fold-tables.td has been failing for me and in CI if Git happens to decide to check out the baseline file with Windows line endings.

This fix for this is to add the --strip-trailing-cr option to diff to normalize the line endings before comparing them.

@dpaoliello dpaoliello requested a review from nico October 18, 2024 23:15
@dpaoliello dpaoliello changed the title Handle Windows line endings in x86-fold-tables.td test [TableGen] Handle Windows line endings in x86-fold-tables.td test Oct 18, 2024
@dpaoliello dpaoliello requested a review from jurahul October 18, 2024 23:27
@dpaoliello dpaoliello merged commit 7eb8238 into llvm:main Oct 21, 2024
7 of 9 checks passed
@dpaoliello dpaoliello deleted the foldtables branch October 21, 2024 16:59
@abhina-sree
Copy link
Contributor

Hi, this is causing a failure on the aix bot https://lab.llvm.org/buildbot/#/builders/64/builds/1269 would you be able to take a look? The cause is because diff does not support --strip-trailing-cr option. There was a similar case that was resolved here #108871. Thanks in advance!

@mstorsjo
Copy link
Member

It's possible that the original issue (CRLF checkouts on the Windows bots) is resolved now, thanks to the revert and restart of the buildbots (see https://discourse.llvm.org/t/windows-premerge-buildbot-broken-for-5-days/82571/8). So in that case we could revert this, and possibly add a .gitattributes file to mark the problematic file as always requiring checking out as LF.

It's a bit tricky to test it though - you'd need to run it through some other CI mechanism (as the regular buildbot doesn't apply .gitattributes on untouched files when updating), where you'd reapply dccebdd and check to see that the test still passes as expected, or at least that this file gets checked out like you'd expect.

@dpaoliello
Copy link
Contributor Author

Hi, this is causing a failure on the aix bot https://lab.llvm.org/buildbot/#/builders/64/builds/1269 would you be able to take a look? The cause is because diff does not support --strip-trailing-cr option. There was a similar case that was resolved here #108871. Thanks in advance!

Ah, perfect, I was going to ask if there was a way to do that: should have a PR out shortly...

@dpaoliello
Copy link
Contributor Author

Given that we've seen this issue on AIX before, and there wasn't a good place to only use the internal shell for a single test, I decided to modify lit to always use the internal shell on AIX: #113355

dpaoliello added a commit that referenced this pull request Oct 23, 2024
Diff on AIX doesn't have all the required features used in tests (see
<#108871> and
<#112997 (comment)>),
so always use the internal shell.
NoumanAmir657 pushed a commit to NoumanAmir657/llvm-project that referenced this pull request Nov 4, 2024
Diff on AIX doesn't have all the required features used in tests (see
<llvm#108871> and
<llvm#112997 (comment)>),
so always use the internal shell.
schwitanski pushed a commit to RWTH-HPC/llvm-lit-mirror that referenced this pull request Feb 27, 2025
Diff on AIX doesn't have all the required features used in tests (see
<llvm/llvm-project#108871> and
<llvm/llvm-project#112997 (comment)>),
so always use the internal shell.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants