In this build:
https://buildkite.com/llvm-project/github-pull-requests/builds/126961
The builds actually failed, probably because prerequisite of a test
suite failed to build.
However they still ran other tests and all those passed. This meant that
the test reports were green even though the build was red. On some level
this is technically correct, but it is very misleading in practice.
So I've also passed the build script's return code, as it was when we
entered the on exit handler, to the generator, so that when this happens
again, the report will draw the viewer's attention to the overall
failure. There will be a link in the report to the build's log file, so
the next step to investigate is clear.
It would be nice to say "tests failed and there was some other build
error", but we cannot tell what the non-zero return code was caused by.
Could be either.
The script handles the following situations now:
| Have Result Files? | Tests reported failed? | Return code | Report |
|--------------------|------------------------|-------------|-----------------------------------------------------------------------------|
| Yes | No | 0 | Success style report. |
| Yes | Yes | 0 | Shouldn't happen, but if it did, failure style report
showing the failures. |
| Yes | No | 1 | Failure style report, showing no failures but noting
that the build failed. |
| Yes | Yes | 1 | Failure style report, showing the test failures. |
| No | ? | 0 | No test report, success shown in the normal build
display. |
| No | ? | 1 | No test report, failure shown in the normal build
display. |
This patch makes the metrics job also detect failures in individual
steps. This is necessary now that we are setting continue-on-error in
the premerge jobs to prevent sending out unnecessary email to detect
what jobs actually fail.
LLVM Premerge Checks is running on the new GCP cluster. Tracking its
metrics will allow us to determine the stability of the presubmit and
make sure the new infra is working as intended.
---------
Signed-off-by: Nathan Gauër <brioche@google.com>
This patch modifies the monolithic shell scrips to only run if the
buildkite-agent application is present. This allows for running the
scripts to completion outside of buildkite (eg inside of a GHA
pipeline).
This patch refactors some common functionality present in the CI scripts
to a separate shell script. This is mainly intended to make it easier to
reuse this functionality inside of a Github Actions pipeline as we make
the switch.
This patch includes the script that pulls information from Github and
pushes it to Grafana. This is currently running in the cluster and
pushes information to
https://llvm.grafana.net/public-dashboards/6a1c1969b6794e0a8ee5d494c72ce2cd.
This script is designed to accept other jobs relatively easily and can
be easily modified to look at other metrics.
This resulted in the style being None and despite the report being
empty as well, we tried to send it to the agent and Python can't
send None as an argument.
To fix this return "success" style and also check whether the
report has any content before calling the agent.
This reverts commit 8a1ca6cad9cd0e972c322910cdfbbe9552c6c7ca.
I have fixed 2 things:
* The report is now sent by stdin so we do not hit the limit on the size
of command line arguments.
* The report is limited to 1MB in size and if we exceed that we fall back
to listing only the totals with a note telling you to check the full log.
This reverts commit e74a002433b4cf7f891ceedb61bd862867218a8b.
As it is failing on Linux with "OSError: [Errno 7] Argument list too long: 'buildkite-agent'".
The CI builds now send the results of every lit run to a unique file.
This means we can read them all to make a combined report for all
tests.
This report will be shown as an "annotation" in the build results:
https://buildkite.com/docs/agent/v3/cli-annotate#creating-an-annotation
Here is an example:
https://buildkite.com/llvm-project/github-pull-requests/builds/112660
(make sure it is showing "All" instead of "Failures")
This is an alternative to using the existing Buildkite plugin:
https://github.com/buildkite-plugins/junit-annotate-buildkite-plugin
As the plugin is:
* Specific to Buildkite, and we may move away from Buildkite.
* Requires docker, unless we were to fork it ourselves.
* Does not let you customise the report format unless again,
we make our own fork.
Annotations use GitHub's flavour of Markdown so the main code in the
script generates that text. There is an extra "style" argument generated
to make the formatting nicer in Buildkite.
"context" is the name of the annotation that will be created. By using
different context names for Linux and Windows results we get 2 separate
annotations.
The script also handles calling the buildkite-agent. This makes passing
extra arguments to the agent easier, rather than piping the output of
this script into the agent.
In the future we can remove the agent part of it and simply use
the report content. Either printed to stdout or as a comment on
the GitHub PR.
In this patch I'm using a new lit option so that the pipeline writes
many results files, one for each time lit is run:
```
--use-unique-output-file-name
When enabled, lit will add a unique element to the output file name, before the extension. For example "results.xml" will become "results.<something>.xml". The
"<something>" is not ordered in any way and is chosen so that existing files are not overwritten. [Default: Off]
```
(I added this to lit recently)
Alternatives were considered:
* mkfifo - does not work on bash for Windows.
* tail -f - does not print full content on file truncation
* lit wrapper script - more complication than using an option to lit
itself
* ninja/mv file/ninja/mv file etc - lots of changes needed to make the
scripts build each target separately
And after feedback I decided that using an option to lit itself is the
cleanest way to go. It can be removed when we no longer need it.
If I run the Linux build after this change:
```
$ bash ./.ci/monolithic-linux.sh "clang;lldb;lld" "check-lldb-shell check-lld" "libcxx;libcxxabi" "check-libcxx check-libcxxabi"
```
I get multiple test result files. In my case some tests fail so runtimes
aren't checked, but all projects are so there is 1 file for lldb and one
for lld:
```
$ ls build/*.xml
build/test-results.klc82utf.xml build/test-results.majylh73.xml
```
This change just collects the XML files as artifacts. Once I know that's
working, I can set up test reporting to make a summary of them.
Fixes#110265
Adding check-all causes us to run some tests twice if a project specific
target like check-clang is also added.
check-pstl is an alternative but as far as I can tell, check-all does
not include this so we have not been running the tests in CI anyway.
When I tried to run check-pstl locally I got a lot of compiler errors
but have not found any instructions on how to setup a correct build
environment. Even if such instructions exist, it's probably more than we
want to do in CI.
According to Louis Dionne, the project is probably not active. So if
it's ever revived it'll be up to the new contributors to enable testing.
Instead of "check-all" which leads to us running some tests twice if
there are other "check-..." targets. For example on one of my PRs this
script produced:
```
commands:
- './.ci/monolithic-linux.sh "clang;clang;lld;clang-tools-extra;compiler-rt;llvm" "check-all check-clang check-clang-tools" "libcxx;libcxxabi;libunwind" "check-cxx check-cxxabi check-unwind"'
commands:
- 'C:\BuildTools\Common7\Tools\VsDevCmd.bat -arch=amd64 -host_arch=amd64'
- 'bash .ci/monolithic-windows.sh "clang;clang-tools-extra;llvm" "check-clang check-clang-tools"'
```
Which meant that Linux ran the clang and clang-tools tests twice. These
extra tests were about 24% of the test run and increased testing time
(on my local machine) by 45%.
This problem can also happen with other projects but there isn't a
simple fix like this one at the moment.
* pstl has a check-pstl target but it is not part of check-all and when
I tried it locally I couldn't build it.
* libclc has no check- target.
I will deal with those projects later.
This is a temporary measure to alleviate Linux pre-commit CI waiting
times that started snowballing
[recently](https://discourse.llvm.org/t/long-wait-for-linux-presubmit-testing/79547/5).
My [initial
estimate](https://github.com/llvm/llvm-project/pull/94208#issuecomment-2155972973)
of 4 additional minutes spent per built seems to be in the right
ballpark, but looks like that was the last straw to break camel's back.
It seems that CI load got past the tipping point, and now it's not able
to burn through the queue over the night on workdays.
I don't intend to overthrow the consensus we reached in #94208, but it
shouldn't come at the expense of the whole LLVM community. I'll enable
this back as soon as we have news that we got more capacity for Linux
pre-commit CI.
This patch removes LLDB from a list of projects that are excluded from
building and testing on pre-merge CI on Linux.
Windows environment needs to be prepared in order to test LLDB
(https://github.com/llvm/llvm-project/pull/94208#issuecomment-2146256857),
but we don't have enough maintenance resources to do that at the moment.
Because LLDB has been in the list of projects that need to be tested on
Clang changes, this PR make this happen on Linux. This seems to be the
consensus in the discussion of this PR.
Flang triggers some OOM on Windows CI right now. This is disruptive to
MLIR and LLVM changes that don't touch Flang, as such we disable
building Flang on Windows only for these PR that don't touch flang. The
testing on Linux is unchanged, and the post-merge Windows testing is
still fully covering here.
Following the discussion in
https://github.com/llvm/llvm-project/pull/93233#issuecomment-2127920882,
this patch merges `clang-ci` pipeline into main `GitHub Pull Requests`
pipeline. `clang-ci` enables additional test coverage for Clang by
compiling it, and then using it to compile and test libc++, libc++abi,
and libunwind in C++03, C++26, and Clang Modules modes.
Additional work we skip and total time savings we should see:
1. Checking out the repo to generate the clang-ci pipeline (2 minutes)
2. Building Clang (3.5 minutes)
3. Uploading the artifacts once, then downloading them 3 times and
unpacking 3 times (0.5 minutes)
Note that because previously-split jobs for each mode are now under a
single Linux job, it now takes around 8 minutes more see the Linux CI
results despite total time savings.
The primary goal of this patch is to reduce the load of CI by removing
duplicated work. I consider this goal achieved. I could keep the job
parallelism we had (3 libc++ jobs depending on a main Linux job), but I
don't consider it worth the effort and opportunity cost, because
parallelism is not helping once the pool of builders is fully
subscribed.
This fixes all shellcheck warnings we have in `monolithic-linux.sh` and
`monolithic-windows.sh`.
All of them have to do with
[SC2086](https://www.shellcheck.net/wiki/SC2086) - Double quote to
prevent globbing and word splitting.
Clang has a custom separate pipeline integrated with libc++ that only
runs in release mode. It means that changes which touches only clang
won't run the clang tests in the configuration used by LLVM premerge and
will break it unknowingly.
#84828 added `-k 0` to pre-merge CI so that if one job fails the others
would continue building. This pull request fixes the location of `-k 0`
in the ninja command line.
Resolves#84842 and #83371
The -k option allows to continue the build after failures as much as
possible. This is useful here because when we run
> ninja check-llvm check-clang
we would like the clang tests to run even if there is a failure in a
llvm tests.
The downside is that a build failure in one file that would prevent from
running any test does not prevent from building more targets, wasting
build resources potentially.
Fixes#83371
We should diff against the base branch, not always against `main`. This
allows the BuildKite pre-commit CI to work properly when we target other
branches, such as `release/18.x`.
The "scheduled build" pipeline on BuildKite had been disabled for months
and doesn't exist anymore, so this script is effectively dead code. When
we set up a cron-activated build again, we should do it using Github
actions (which could trigger a BK pipeline if needed).
Keeping this script around just creates additional confusion about
what's used and what's not used for doing CI.
PR for: https://discourse.llvm.org/t/add-check-mlir-python-to-the-mlir-pre-commit-tests/74041
It’s easy to forget about the Python bindings and not build/test them
locally. It’s also easy to change something that’ll break the python
binding tests and not find out till after you’ve committed your change.
These tests seem to run quickly and don’t require much extra setup, so
let's add them to the general MLIR pre-merge tests.
we tried to generate a full diff against main in
ec9d80e but it resulted in wrong diffs.
It seems that the issue was that 'main' was not
updated after agent restart and diff main...HEAD kept growing.
Not enabling diff main...HEAD just yet and will check logs for new PRs
first.
Since we moved to Github PRs, the workflow has changed a bit and folks
often merge `main` back into their PR branch. This is fine, except the
previous way of determining modified files for pre-commit CI would use
the content modified just in the latest commit, whatever it is. This
means that in case someone merged main back into their PR branch, we'd
think that the files in the merge commit were modified by the PR, and
we'd spuriously trigger a CI run. This should fix this issue.
The downside is that the merge target is hardcoded to `main`, which
might not always be what we want. I still think this is an improvement
over the status quo.
- fixed build for linux (clang was missing)
- removed /monolithic-.. from build directory - it does not add anything
and makes path longer for windows which is not great;
- added env-based configuration to control cache and agent targeting;
- print (s)ccache stats to file not to pullute normal log.
