Skip to content
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

[CI] clang-format action broken #97060

Closed
nikic opened this issue Jun 28, 2024 · 13 comments
Closed

[CI] clang-format action broken #97060

nikic opened this issue Jun 28, 2024 · 13 comments
Assignees

Comments

@nikic
Copy link
Contributor

nikic commented Jun 28, 2024

The code formatting action appears to be broken, and no longer reports incorrect formatting. See e.g. #97029, but I've noticed this in multiple PRs recently. Not sure when exactly it broke.

@nikic nikic added the infrastructure Bugs about LLVM infrastructure label Jun 28, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jun 28, 2024

@llvm/issue-subscribers-infrastructure

Author: Nikita Popov (nikic)

The code formatting action appears to be broken, and no longer reports incorrect formatting. See e.g. https://github.com//pull/97029, but I've noticed this in multiple PRs recently. Not sure when exactly it broke.

@EugeneZelenko EugeneZelenko added github:workflow and removed infrastructure Bugs about LLVM infrastructure labels Jun 28, 2024
@tstellar
Copy link
Collaborator

Maybe this one: 5914a56 ?

It's not just that the comment is missing, the job is passing even if there is incorrect formatting, right?

@nikic
Copy link
Contributor Author

nikic commented Jun 28, 2024

Yeah, the job is passing.

7620fe0 is another possible culprit.

@boomanaiden154 boomanaiden154 self-assigned this Jun 28, 2024
@boomanaiden154
Copy link
Contributor

I'll take a look.

@boomanaiden154
Copy link
Contributor

boomanaiden154 commented Jun 28, 2024

It doesn't even seem like the job is running currently? There are quite a few other jobs missing in new PRs as well. The last code formatting job currently ran 25+ minutes ago and PRs have definitely been updated more recently.

I also can't get the job to run on my fork currently.

It seems like maybe just pull_request jobs? All the pull_request_target jobs, like the PR labelers, seem to be working fine.

@boomanaiden154
Copy link
Contributor

https://www.githubstatus.com/incidents/9vwllhs2w1kj

I guess I'll wait for that to be fixed and then get to investigating.

@tstellar
Copy link
Collaborator

Here is a job from a few hours ago that correctly failed: https://github.com/llvm/llvm-project/actions/runs/9714796529

@boomanaiden154
Copy link
Contributor

boomanaiden154 commented Jun 28, 2024

It correctly failed on my fork too: https://github.com/boomanaiden154/llvm-project/actions/runs/9716540595/job/26820327869?pr=48

I wonder if it's the one cpp file case?

@boomanaiden154
Copy link
Contributor

It's 7620fe0.

It breaks at least the one cpp file case.

I'll revert to get things back to the way they were and ping on the PR.

boomanaiden154 added a commit that referenced this issue Jun 28, 2024
…95794)"

This reverts commit 7620fe0.

This patch was causing some significant portion of code formatting jobs
to succeed when they should have failed. More investigation is needed.

Tracked in #97060.
lravenclaw pushed a commit to lravenclaw/llvm-project that referenced this issue Jul 3, 2024
…lvm#95794)"

This reverts commit 7620fe0.

This patch was causing some significant portion of code formatting jobs
to succeed when they should have failed. More investigation is needed.

Tracked in llvm#97060.
@ldionne
Copy link
Member

ldionne commented Jul 4, 2024

It's 7620fe0.

It breaks at least the one cpp file case.

Do you mean the case where a single libc++ header is modified and that header has no extension?

I'll revert to get things back to the way they were and ping on the PR.

Thanks, I'll take a look. I was away at a WG21 meeting last week.

@boomanaiden154
Copy link
Contributor

Do you mean the case where a single libc++ header is modified and that header has no extension?

A single c++ file in llvm/. I haven't done a lot of investigation, but something like the following diff would pass even though clang format should fail it:

diff --git a/llvm/lib/CodeGen/MLRegAllocEvictAdvisor.cpp b/llvm/lib/CodeGen/MLRegAllocEvictAdvisor.cpp
index 5698f6d6fea00..2c3e0f782c567 100644
--- a/llvm/lib/CodeGen/MLRegAllocEvictAdvisor.cpp
+++ b/llvm/lib/CodeGen/MLRegAllocEvictAdvisor.cpp
@@ -10,6 +10,8 @@
 //
 //===----------------------------------------------------------------------===//
 
+// This line should never get through the formatting job clang-format please format this into something that is actually reasonable.
+
 #include "AllocationOrder.h"
 #include "RegAllocEvictionAdvisor.h"
 #include "RegAllocGreedy.h"

@ldionne
Copy link
Member

ldionne commented Jul 9, 2024

@boomanaiden154 Thanks, investigating in #98227

@ldionne
Copy link
Member

ldionne commented Aug 29, 2024

This should have been closed when I merged #98227.

@ldionne ldionne closed this as completed Aug 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants