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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants