CI: enhance FreeBSD staging tests by kinkie · Pull Request #2372 · squid-cache/squid

kinkie · 2026-02-09T22:40:55Z

Cover all FreeBSD and OpenBSD versions supported
by these projects.
Fully rely on packages, no ports.
Enable ccache
Restrict testing to the default and maximus test layers

kinkie · 2026-02-09T22:42:01Z

These workflows have been tested at
https://github.com/squid-platform-test/squid/actions/workflows/openbsd.yaml
and
https://github.com/squid-platform-test/squid/actions/workflows/freebsd.yaml

rousskov · 2026-02-09T22:51:20Z

.github/workflows/slow.yaml

-          - 14.3
+          - "15.0"
+          - "14.3"
+          - "13.5"


October commit 99fca3a noted that "FreeBSD 13[.4] packages are unreliably packaged, causing version mismatches that lead to CI test failures". If we do not know of any specific FreeBSD v13.5 policy/practices changes, then perhaps we should not go back to testing on that "unreliably packaged" FreeBSD flavor?

Suggested change

- "13.5"

In practice, it's showing to be working.
I have tweaked the build instructions, I think that's what's making the difference.
Unlike before, this version of the test does not attempt to update all software packages on the FreeBSD VM; this might be the deciding factor

Sounds good! Please record your justification for this reversal in the PR description, mentioning commit 99fca3a.

rousskov · 2026-02-09T22:55:41Z

.github/workflows/slow.yaml

+          - amd64
+          # - arm64 # too slow as of 2026-02
+          # - riscv64 # not working as of 2026-02
+    runs-on: ${{ matrix.platform != 'arm64' && 'ubuntu-24.04' || 'ubuntu-24.04-arm' }}


Since arm64 is commented out from matrix.platform, can this runs-on configuration be simplified?

Suggested change

runs-on: ${{ matrix.platform != 'arm64' && 'ubuntu-24.04' || 'ubuntu-24.04-arm' }}

runs-on: ${{ 'ubuntu-24.04' }}

I'd leave it in if possible at all.
Hopefully in a not too far future arm64 will not go through slow software emulation and we can use it; everything is ready to add it back with minimal work

I am not violently against adding matrix.platform code in this PR, even though there is only one platform for now, provided

this PR adds matrix.platform everywhere it will be needed (i.e. we do not add half-working code), and

we leave a comment explaining what is going on. For example:

platform: - amd64 # Our configuration is ready to support multiple platforms, # and we expect to enable at least one of these candidates, # but they are currently disabled for the reasons stated below. # - arm64 # takes more than XXX hours as of 2026-02 # - riscv64 # broken (XXX) as of 2026-02

... with those two XXX placeholders filled in, of course.

.github/workflows/slow.yaml

rousskov · 2026-02-09T23:04:35Z

.github/workflows/slow.yaml

+            export CC='ccache cc'
+            export CXX='ccache c++'
+            cd squid
+            exec ./test-builds.sh layer-00-default layer-02-maximus


This elimination of two(?) layers has not been disclosed in PR description. Please either disclose-and-justify this change in PR description or undo it.

added to the description

Alex: Please either disclose-and-justify this change in PR description or undo it.

Francesco: added to the description

Updated PR description: Restrict testing to the default and maximus test layers

The above is "disclose". Please justify this change (or undo it). This PR is introducing special/unusual layer selection logic and implicitly calls the new logic "enhancement". It is not clear (to me) why it does that. This PR should justify this change. It currently does not. I may or may not agree with the proposed justification or suggest further changes in line with that justification, of course, but the next step is on your side.

N.B. linux-distros test three layers, including layer-01-minimal. If you want to reduce the number of tested layers, FreeBSD could do that too, using "speed up tests and mimic what we do for linux-distros" justification.

.github/workflows/slow.yaml

rousskov · 2026-02-09T23:14:09Z

.github/workflows/slow.yaml


  freebsd:
    strategy:
+      fail-fast: false


We do not use that setting in other similar cases. Remove for consistency sake?

Suggested change

fail-fast: false

This change request was not acknowledged or addressed.

.github/workflows/slow.yaml

rousskov · 2026-02-09T23:21:22Z

.github/workflows/slow.yaml

+        with:
+          repository: squid-cache/squid
+          path: squid
+          ref: ${{ inputs.branch }}


Other "Checkout Sources" sections do not have these with details. Can we not add them (and undo the squid path changes elsewhere in this PR that probably were triggered by this change)?

artifact of the code coming from a different repository. reverting and trying out

kinkie · 2026-02-10T10:26:41Z

Test-run of the updated workflow: https://github.com/kinkie/squid/actions/runs/21858341109

rousskov

Thank you for advancing this PR.

rousskov · 2026-02-10T14:25:58Z

.github/workflows/slow.yaml

-          - 14.3
+          - "15.0"
+          - "14.3"
+          - "13.5"


Sounds good! Please record your justification for this reversal in the PR description, mentioning commit 99fca3a.

rousskov · 2026-02-10T14:35:11Z

.github/workflows/slow.yaml

+          - amd64
+          # - arm64 # too slow as of 2026-02
+          # - riscv64 # not working as of 2026-02
+    runs-on: ${{ matrix.platform != 'arm64' && 'ubuntu-24.04' || 'ubuntu-24.04-arm' }}


I am not violently against adding matrix.platform code in this PR, even though there is only one platform for now, provided

this PR adds matrix.platform everywhere it will be needed (i.e. we do not add half-working code), and

we leave a comment explaining what is going on. For example:

platform: - amd64 # Our configuration is ready to support multiple platforms, # and we expect to enable at least one of these candidates, # but they are currently disabled for the reasons stated below. # - arm64 # takes more than XXX hours as of 2026-02 # - riscv64 # broken (XXX) as of 2026-02

... with those two XXX placeholders filled in, of course.

rousskov · 2026-02-10T14:42:55Z

.github/workflows/slow.yaml

+            export CC='ccache cc'
+            export CXX='ccache c++'
+            cd squid
+            exec ./test-builds.sh layer-00-default layer-02-maximus


Alex: Please either disclose-and-justify this change in PR description or undo it.

Francesco: added to the description

Updated PR description: Restrict testing to the default and maximus test layers

The above is "disclose". Please justify this change (or undo it). This PR is introducing special/unusual layer selection logic and implicitly calls the new logic "enhancement". It is not clear (to me) why it does that. This PR should justify this change. It currently does not. I may or may not agree with the proposed justification or suggest further changes in line with that justification, of course, but the next step is on your side.

N.B. linux-distros test three layers, including layer-01-minimal. If you want to reduce the number of tested layers, FreeBSD could do that too, using "speed up tests and mimic what we do for linux-distros" justification.

rousskov · 2026-02-10T14:52:29Z

.github/workflows/slow.yaml

+                automake \
+                ccache \
+                cppunit \
+                git-tiny \


Does our test-build.sh require git, directly or indirectly? I failed to quickly find such a dependency, but it is easy to miss. Or does this PR needs to add git-tiny for some other reason?

rousskov · 2026-02-10T15:00:38Z

.github/workflows/slow.yaml

+      - name: prep ccache
+        uses: hendrikmuhs/ccache-action@v1
+        with:
+          key: ${{ github.job }}-${{ matrix.osversion }}-${{ matrix.platform }}


Please do not introduce a new/different way to name and configure ccache step OR justify introducing it.

Here are the two existing ccache steps:

- name: Setup ccache uses: hendrikmuhs/ccache-action@v1.2.19 with: verbose: 2 # default 0 key: ${{ matrix.os }}-${{ matrix.compiler.CC }}-${{ matrix.layer.nick }} max-size: "250MB" evict-old-files: "28d" -- - name: Setup ccache uses: hendrikmuhs/ccache-action@v1.2.19 with: verbose: 2 # default 0 key: ${{ matrix.os }}-${{ matrix.compiler.CC }}-${{ matrix.layer.nick }} max-size: "250MB" evict-old-files: "28d"

FreeBSD configuration may need a different key template because we are not testing multiple compilers here, and we test multiple layers inside one step. Please remove or reduce other differences as much as possible.

rousskov · 2026-02-10T15:19:15Z

.github/workflows/slow.yaml


-    runs-on: ubuntu-24.04
-    name: freebsd(${{ matrix.osversion }})
+    name: freebsd(${{ matrix.osversion }}, ${{ matrix.platform }})


We do not use a space when naming linux-distros jobs:

Suggested change

name: freebsd(${{ matrix.osversion }}, ${{ matrix.platform }})

name: freebsd(${{ matrix.osversion }},${{ matrix.platform }})

rousskov · 2026-02-10T15:37:32Z

.github/workflows/slow.yaml

+          - amd64
+          # - arm64 # too slow as of 2026-02
+          # - riscv64 # not working as of 2026-02
+    runs-on: ${{ matrix.platform != 'arm64' && 'ubuntu-24.04' || 'ubuntu-24.04-arm' }}


GitHub documentation says that Quotation marks are not required around simple strings like self-hosted, but they are required for expressions like "${{ inputs.chosen-os }}". We already go against that recommendation in quick.yaml, but I suggest following it here by adding quotes.

I do not insist on this change.

rousskov · 2026-02-10T15:38:54Z

.github/workflows/slow.yaml


  freebsd:
    strategy:
+      fail-fast: false


This change request was not acknowledged or addressed.

rousskov · 2026-02-10T15:40:17Z

.github/workflows/slow.yaml

-            ./test-builds.sh
+            export CC='ccache cc'
+            export CXX='ccache c++'
+            exec ./test-builds.sh layer-00-default layer-02-maximus


If possible, please remove exec because we do not use that in similar workflow contexts.

rousskov · 2026-02-10T15:53:18Z

.github/workflows/slow.yaml

+            ccache --set-config=compression=true

          run: |
+            set -e


If possible, please remove set -e because it is probably the default for Linux runners, we do not use that setting in similar workflow contexts, and none of the prep "export" commands can fail AFAICT.

Suggested change

set -e

CI: enhance freebsd and openbsd staging tests

b2c5a80

rousskov requested changes Feb 9, 2026

View reviewed changes

rousskov added the S-waiting-for-author author action is expected (and usually required) label Feb 9, 2026

kinkie added 2 commits February 10, 2026 08:49

sort freebsd packages by name

3916ca7

include all logs, fix artifact name

13167ea

kinkie changed the title ~~CI: enhance FreeBSD and OpenBSD staging tests~~ CI: enhance FreeBSD staging tests Feb 10, 2026

kinkie added 3 commits February 10, 2026 08:55

revert openbsd path change

88db4a7

do not use subtree for checkout

336afdd

quote paths

4e690ca

kinkie added S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box and removed S-waiting-for-author author action is expected (and usually required) labels Feb 10, 2026

kinkie requested a review from rousskov February 10, 2026 10:26

rousskov requested changes Feb 10, 2026

View reviewed changes

rousskov added S-waiting-for-author author action is expected (and usually required) and removed S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box labels Feb 10, 2026

	runs-on: ${{ matrix.platform != 'arm64' && 'ubuntu-24.04' \|\| 'ubuntu-24.04-arm' }}
	runs-on: ${{ 'ubuntu-24.04' }}

	name: freebsd(${{ matrix.osversion }}, ${{ matrix.platform }})
	name: freebsd(${{ matrix.osversion }},${{ matrix.platform }})

Conversation

kinkie commented Feb 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kinkie commented Feb 9, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kinkie commented Feb 10, 2026

Uh oh!

rousskov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kinkie commented Feb 9, 2026 •

edited

Loading