Skip to content

Conversation

@Meinersbur
Copy link
Member

@Meinersbur Meinersbur commented Nov 6, 2025

Allow the main llvm-project repository to contain the buildbot builder instructions, instead of storing them in llvm-zorg. The corresponding llvm-zorg PR is llvm/llvm-zorg#648.

Using polly-x86_64-linux-test-suite as a proof-of-convept because that builder is currently offline, I am its maintainer, and is easier to build than an configuration supporting offload. Once the disign has been decided, more builders can follow.

Advantages are:

  • Are easier to make on the llvm-project repository. There are more reviewers than for the llvm-zorg repository
  • Buildbot changes can be made in the same PR with changes that require updating the buildbot, e.g. changing the name of a CMake option.
  • Some builders store a CMake cache file in the llvm-project repository for the reasons above. However, the number of changes that can be made with a CMake cache file alone are limited

Compared to AnnotatedBuilder, advantages are:

  • Reproducing a buildbot configuration locally made easy: just execute the script in-place. No llvm-zorg, local buildbot worker, or buildbot master needed.
  • Same for testing a change of a builder before landing it in llvm-zorg. Doing so wuth an AnnotatedBuilder requires two llvm-zorg checkouts: One for making the change of the builder script itself, which then is pushed to a private llvm-zorg branch on GitHub, and a second that is modified to fetch that branch instead of https://github.com/llvm/llvm-zorg/tree/main.
  • The AnnotatedBuilder scripts are located in the llvm-zorg repository and the buildbot-workers always checkout the top-of-trunk. This means that a buildbot configuration is split over three checkouts:
    • The checkout of llvm-zorg that the buildbot-master is running, which is updated only when the master is manually restarted.
    • The checkout of llvm-zorg by the buildbot-worker fetches; always the top-of-trunk and connot be selected using the buildbot master's "Force build" feature, .i.e may not match the revision of llvm-project that is executed such as the CMake cache files located there.
    • The checkout of llvm-project to be tested
  • The "Force Build" feature also allows for test-building any llvm-project PR. This is correctly handled by zorg's addGetSourcecodeSteps, but does not work with AnnotatedBuilders that checkout the llvm-project source on their own.

The goal is to move as much as possible into the llvm-project repository such that there cannot be a mismatch between checkouts of the different repository. Ideally, the buildbot-master only needs to be updated+restarted for adding/removing workers, not for build configuration changes.

This has been discussed in the Bi-Weekly LLVM Offload Meeting (Agenda item 13). There were no concerns.

Add annotated builder Test of Polly builder run fix Don't clobber twice
@github-actions
Copy link

github-actions bot commented Nov 6, 2025

✅ With the latest revision this PR passed the Python code formatter.

@Meinersbur Meinersbur requested a review from jplehr November 6, 2025 17:21
@Meinersbur
Copy link
Member Author

Meinersbur commented Nov 6, 2025

@jplehr First draft about the proposed move of the build instruction next to the CMake cache. I know you had an RFC where to locate the script, but I forgot where the location was. Will update after first feedback.

A proof-of-concept buildbot master with worker using this change is running here: http://meinersbur.de:8011/#/builders/143. That particular builder has not run for a long time, it is expected to be failing. It is also useful to test a failing step.

@jplehr
Copy link
Contributor

jplehr commented Nov 7, 2025

@Meinersbur thank you for putting time/work into this!

I agree with the advantages you listed. We have found ourselves more than once in the situation that we needed to coordinate changes across repos, or where a configuration in llvm-zorg was not updated properly. Moving the test scripts in-tree solves this issue, as you mentioned, because the changes can be included into the original PR and reviewed together.
IMHO this is similar to how pre-merge scripts are handled, they also live in-tree.

I think I mostly discussed where to put files within the scope of the offload project. There we thought about putting the project-specific scripts into offload/ci. Which is probably a project-specific decision anyway. I also do not have a very strong opinion here.

IIRC that discussion did not consider common infra, so I appreciate that you include it here. I don't know if we should aim to add it to the existing .ci folder? While the things inside that directory are currently pre-merge only FWICT, it might be a good place as opposed to create another hidden top-level folder.

@jplehr jplehr requested a review from Kewen12 November 7, 2025 08:28
@Meinersbur Meinersbur marked this pull request as ready for review November 19, 2025 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants