Page MenuHomeFreeBSD

Pre-commit CI with CIRRUS-CI
Needs RevisionPublic

Authored by bofh on Aug 18 2022, 10:15 PM.

Details

Summary

Currently we do not have pre-commit testing mechanism for our src tree. These scripts which are primarily based on the scripts that run on our Jenkins system are modified to handle the same tests in CIRRUS-CI gcp cloud through Github.
Limitations of the scripts:

  • Currently only works on amd64 and i386. Our aarch64/arm64 test requires somewhere near 4+ hours which is not supported by CIRRUS-CI hosted systems yet as the hard limitation is 120m. But in future we can try to do this using hosted arm system with AWS.
  • Currently only works with main branch, stable/12 and stable/13 has not yet been processed. As the scripts are different.
  • Github pull requests are not handled by these scripts yet. In the future if we plan on migrating to Github we will reconsider this.
  • Junit artifacts are currently not handled by this due to a bug in the CIRRUS-CI.
  • Currently one big limitation is a fixed name of working tree which is jarvis. As we have our own smoke tests on CIRRUS and we have lots of branches it's a bit difficult to isolate the other branches being built. Hence I had to hard code a branch name and the pre-commit tests will run only if committed with that branch.
Test Plan
  1. Download this patch
  2. Create a fork in Github from our our repo
  3. Make sure CIRRUS-CI is installed/enabled for this repo
  4. In your host run the following commands:
git clone git@github.com:<GITHUB USERNAME/freebsd-src.git
cd freebsd-src
git checkout -b jarvis
patch -s -p1 < <PATH TO DOWNLOADED PATCH>
git add .
git commit -m "CIRRUS-CI tests"
git push -u origin jarvis

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

bofh created this object with visibility "All Users".
bofh requested review of this revision.Aug 18 2022, 10:15 PM
.cirrus.yml
34

Does this do what you expect?

The values of variables are ignored regardless of their setting; even if
they would be set to “FALSE” or “NO”.  The presence of an option causes
it to be honored by make(1).

One quick suggestion is like what we discussed previously, how about putting the scripts from freebsd-ci repository to tools/ or somewhere like it? The main reason of doing it is for code sharing in different CI pipelines.

.cirrus.yml
34

No, it doesn't. But if you consider yaml language with other languages it's hard to be human readable. For this specific purpose if you consider WITH_LIB32; this value is toggled only once. But I want to mention all the DEFAULT values here so that while reading the rest of the file it is easily understandable by everyone that what is the DEFAULT value.

One quick suggestion is like what we discussed previously, how about putting the scripts from freebsd-ci repository to tools/ or somewhere like it? The main reason of doing it is for code sharing in different CI pipelines.

Yes these will be done at a later stage. Because actually we have to handle three different scenario: jenkins, make ci(please propose something), and CIRRUS-CI. So we have to split the scripts into different parts as for make ci and CIRRUS-CI the concept of artifact is a bit different and there is no ARTIFACT server. And making a script compatible with three different use case scenarios is time convincing. Specially I have to also deploy a local jenkins setup to test these scripts. Beside that I cannot also reproduce uploading the images to the artifact server from my side.

And this is too early to put these scripts into the main src tree. Till now there has been no pre-commit CI. If we at least give the developers a chance to use it for a while we will receive some responses and feedback about their expectations and requirements. Based on that we can make it better.

One more thing is finalizing this before finalizing our workflow might also be counter productive. We might chose a system where we have to redesign the entire thing.

bofh changed the visibility from "All Users" to "Public (No Login Required)".Mon, Apr 7, 12:29 PM
emaste requested changes to this revision.Tue, Apr 8, 1:27 PM
emaste added inline comments.
.cirrus-ci/scripts/build/build-kernel-LINT.sh
17–24

Let's move these to a variable to avoid repetition, or a e.g. do_make() function

.cirrus-ci/scripts/build/build-world-kernel.sh
22–35

Let's put these into a variable to avoid the repetition, or move to a e.g. do_make() function

.cirrus.yml
124–125

Why delete TARGET:?

127–132

We don't want to delete the existing arm64 case

This revision now requires changes to proceed.Tue, Apr 8, 1:27 PM