llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.23k stars 11.66k forks source link

[CI] clang-format action broken #97060

Closed nikic closed 1 month ago

nikic commented 3 months ago

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

llvmbot commented 3 months ago

@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/llvm/llvm-project/pull/97029, but I've noticed this in multiple PRs recently. Not sure when exactly it broke.
tstellar commented 3 months ago

Maybe this one: 5914a5671a1596f68189c8408f8d877e6b6373bf ?

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

nikic commented 3 months ago

Yeah, the job is passing.

https://github.com/llvm/llvm-project/commit/7620fe0d2d1e0257611c0ab0d96f3bf1bf7a1079 is another possible culprit.

boomanaiden154 commented 3 months ago

I'll take a look.

boomanaiden154 commented 3 months ago

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 commented 3 months ago

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

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

tstellar commented 3 months ago

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

boomanaiden154 commented 3 months ago

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 commented 3 months ago

It's https://github.com/llvm/llvm-project/commit/7620fe0d2d1e0257611c0ab0d96f3bf1bf7a1079.

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.

ldionne commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

@boomanaiden154 Thanks, investigating in https://github.com/llvm/llvm-project/pull/98227

ldionne commented 1 month ago

This should have been closed when I merged https://github.com/llvm/llvm-project/pull/98227.