Bazel bots should not make tests red

Project Infrastructure
1

This PR has its tests “red” because of a Bazel failure.

As we all know, Bazel is a peripheral tier of support and should not impede general progress.

I know we don’t block merge on red tests, but the general user doesn’t know what’s a test that needs to pass and those that don’t. It’s confusing for reviewers and authors what to ignore and may cause people ignoring genuine problems. This has been brought up numerous times in buildbot quality conversations (@gkistanova @akorobeynikov @tstellar) and is not a good tihng.

Similar bots (clang-tidy) are being proposed but we have not resolved how to represent that in Github to actively help authors and reviewers how to interpret the result.

Until it’s clear what’s peripheral and what’s not, I’d like to ask to not run the Bazel bots on every pre-commit CI.

@pranavk @petrhosek @boomanaiden154-1 @AaronBallman

1 Like
[RFC] Add non-blocking precommit `clang-tidy` analysis
2

Bazel checks are already configured to only run if the path being modified includes utils/bazel/: llvm-project/.github/workflows/bazel-checks.yml at main · llvm/llvm-project · GitHub

on:
  push:
    paths:
      - '.github/workflows/bazel-checks.yml'
      - 'utils/bazel/**'
    branches:
      - main
  pull_request:
    paths:
      - '.github/workflows/bazel-checks.yml'
      - 'utils/bazel/**'
1 Like
3

Ah! It was right at the bottom of the file list. Ok, in this case, all is good. Sorry about the noise.

Now, I looked at the error and could not find anything actionable. It looks like a system error?

1 Like
4

The output says the following:

./utils/bazel/llvm-project-overlay/llvm/BUILD.bazel # reformat

Which means it needs to be reformatted with buildifier in order to fix the CI. It’s non-blocking though and not an issue to land with red bazel CI.

1 Like
5

We don’t actually have a Bazel infra. Do you prefer to land that way and you clean it up, or revert the bazel changes and let you do the delta with the right format?

@asiemien, for the future, I recommend not to touch Bazel files since we have no way of testing locally.

1 Like
6

Bazel can be run locally without any special infrastructure similar to CMake/ninja. buildifier requires nothing as it’s just a (mostly) static go binary.

In general, it’s probably preferable that bazel changes are just omitted entirely if they are not in a position to debug any failures locally/ensure that things work. In this case given it’s just a formatting failure, any option is probably fine.

2 Likes
7

Notes taken, I should’ve skipped touching anything Bazel.

I’ll opt to revert any changes to avoid landing partial solution. Even if it’s just formatting.

1 Like