Page MenuHomeFreeBSD

Pre-commit CI with CIRRUS-CI
ClosedPublic

Authored by bofh on Aug 18 2022, 10:15 PM.
Tags
None
Referenced Files
F132207853: D36257.diff
Tue, Oct 14, 7:18 PM
Unknown Object (File)
Sun, Oct 12, 7:02 AM
Unknown Object (File)
Sun, Oct 12, 7:02 AM
Unknown Object (File)
Sun, Oct 12, 7:02 AM
Unknown Object (File)
Sun, Oct 12, 7:02 AM
Unknown Object (File)
Sun, Oct 12, 7:02 AM
Unknown Object (File)
Sun, Oct 12, 7:02 AM
Unknown Object (File)
Sat, Oct 11, 8:44 PM

Details

Summary

Currently we do not have pre-commit testing mechanism for our src tree. We have merged the CI test scripts into base(HEAD only). The plan is to replace the entire Jenkins scripts with the one from the base system.
Limitations of the scripts:

  • Full test does not work as intended like in a local environment. Our amd64/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 system with AWS.
  • Currently only works with main branch, stable/13 and stable/14 has not yet been processed. As the scripts are different.
Test Plan

With this patch if there is a new PR in Github this will automatically trigger new jobs in Cirrus CI if Cirrus-CI is configured for the repo.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

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

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
28

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)".Apr 7 2025, 12:29 PM
emaste requested changes to this revision.Apr 8 2025, 1:27 PM
emaste added inline comments.
.cirrus-ci/scripts/build/build-kernel-LINT.sh
17–24 ↗(On Diff #109539)

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 ↗(On Diff #109539)

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

.cirrus.yml
20–22

Why delete TARGET:?

23–54

We don't want to delete the existing arm64 case

This revision now requires changes to proceed.Apr 8 2025, 1:27 PM
bofh edited the summary of this revision. (Show Details)
bofh edited the test plan for this revision. (Show Details)

As the ci scripts are part of the base system now let's use the new scripts.

bofh marked 3 inline comments as done.May 2 2025, 9:20 PM

These comments are no longer relevant with the new codes.

I think this is fine. The only minor thing is if this will be in the commit message: "But in future we can try to do this using hosted system with AWS." I suggest we replace "with AWS" with something like "in the cloud" so it won't look like we're promoting certain cloud provider.

.cirrus.yml
195

The run fails on occasion when CIRRUS_CLONE_DEPTH: 1 is set - see
fcb4797c90f3f62a18b67542ae5b576a1271e6d4
80a840b8ba03bafe299112e2364959d937a87fe1
225605ec1681d5ac08b297aa4efbc64bcb950fc2

If we're going to have it though I see no reason it wouldn't apply globally in this file

Remove CIRRUS_CLONE_DEPTH.

This revision was not accepted when it landed; it landed in state Needs Review.May 27 2025, 4:23 PM
This revision was automatically updated to reflect the committed changes.