The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #30193 (ci: move ASan job to GitHub Actions from Cirrus CI by m3dwards)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

cc @vasild

Concept ACK

IMPORTANT. The vmactions/* needs to be added to the "Actions permissions" section of the repository settings.

Not sure. What was the conclusion when GHA was added to the repo about enabling third party actions? IIRC it was unclear whether malicious or backdoored actions could compromise the repo, so the conclusion was to refrain from enabling them?

No objection to merging this, but I'd say unless it is clear that the repo can not be compromised, the action should not be enabled in this repo.

I am running nighly freeBSD CI runs for years, and I haven't yet seen an issue pop up, so the benefit seems unclear, albeit having the CI config could certainly be useful for manual testing sometimes.

Which FreeBSD version is this? The oldest maintained release is 13.2 I believe, though we'll have to install clang 15 on it.

@Sjors According to https://github.com/vmactions/freebsd-vm?tab=readme-ov-file#under-the-hood you can use with: release: to pick one.

I believe, though we'll have to install clang 15 on it.

Then we should pick something newer, or which won't be a blocker to using a newer Clang. Because soon our minimum Clang will be 16.

@Sjors According to https://github.com/vmactions/freebsd-vm?tab=readme-ov-file#under-the-hood you can use with: release: to pick one.

By default, the latest release is used, which is 14.0. See https://github.com/hebasto/bitcoin/actions/runs/9215593397/job/25354268473#step:3:7032:

Config file: freebsd-14.0.conf

On my FreeBSD 14.0 machine, the default compiler is

$ c++ --version
FreeBSD clang version 16.0.6 (https://github.com/llvm/llvm-project.git llvmorg-16.0.6-0-g7cbf1a259152)
Target: x86_64-unknown-freebsd14.0
Thread model: posix
InstalledDir: /usr/bin

IMPORTANT. The vmactions/* needs to be added to the "Actions permissions" section of the repository settings.

Not sure. What was the conclusion when GHA was added to the repo about enabling third party actions? IIRC it was unclear whether malicious or backdoored actions could compromise the repo, so the conclusion was to refrain from enabling them?

I think, this repository granted only Read permissions to the GH Actions workflows as their logs include:

GITHUB_TOKEN Permissions
  Contents: read
  Metadata: read
  Packages: read

We could pick 13 (13.3 if you have to specify) and then pkg install llvm16.

I'm able to compile on a VM running 13.2 and with either clang 15 or 16 manually installed:

 ./configure CC=/usr/local/bin/clang16 CXX=/usr/local/bin/clang++16 MAKE=gmake

It seems that the 13.* point releases are maintained for 3 months each, but 13 in general is supported until April 30, 2026.

https://www.freebsd.org/security/#sup

🐙 This pull request conflicts with the target branch and needs rebase.

Looks good to merge to allow devs to enable this for testing in their fork. However, it should not be enabled in this repo, because:

• CI resources and maintenance are limited, so only tasks that find meaningful errors regularly should be enabled.
• Albeit this seems fine (ci: Add FreeBSD GitHub Actions job #30164 (comment)), I think adding third-party actions should be limited as well.
• The third-party action has limited usage and exposure, so it may break from down under us, marking all CI runs red.

Rebased to resolve a conflict with #30193.

I am running nighly freeBSD CI runs for years, and I haven't yet seen an issue pop up, so the benefit seems unclear

Not encountering an issue in the past does not guarantee the same in the future :)

Maybe, if CI resources are really scarce

FWIW, the new job does not use the GHA cache storage, which is limited.

I am running nighly freeBSD CI runs for years, and I haven't yet seen an issue pop up, so the benefit seems unclear, albeit having the CI config could certainly be useful for manual testing sometimes.

We ran into a FreeBSD compilation error here: #30043 (comment)

Might be limited to low level networking code and other places where FreeBSD is materially different from macOS and Linux. It seems good to at least do a build.

Looks good to merge to allow devs to enable this for testing in their fork. However, it should not be enabled in this repo...

Merging without enabling won't work:

Error
vmactions/freebsd-vm@v1 is not allowed to be used in bitcoin/bitcoin. Actions in this workflow must be: within a repository owned by bitcoin or matching the following: actions/cache@, actions/cache/restore@, actions/cache/save@, actions/checkout@, ilammy/msvc-dev-cmd@*.

The third-party action has limited usage and exposure, so it may break from down under us, marking all CI runs red.

In that case this commit can be reverted back.

Merging without enabling won't work:

That seems like a GitHub bug. We shouldn't have to change permissions/configurations in this repository for people to do things in forks.

Not encountering an issue in the past does not guarantee the same in the future :)

Correct, but the cost of a trivial post-merge fixup, like adding a missing header or header-guard, in case it happens, should be trivial compared to the overhead to deal with redundant or unrelated intermittent issues, which are ignored by most devs, left to be dealt with by others (see https://github.com/bitcoin/bitcoin/issues?q=is%3Aissue+is%3Aopen+label%3A%22CI+failed%22). If it was free to add more CI tasks, a ton of more important stuff, like valgrind or nightly compiler checks could be added.

See also my previous reply: #30164 (comment)

We ran into a FreeBSD compilation error here: #30043 (comment)

Might be limited to low level networking code and other places where FreeBSD is materially different from macOS and Linux. It seems good to at least do a build.

I know, but the thread you linked to required compiling on different versions of FreeBSD, so manually doing that, or at least specifying the versions in the config would be required anyway.

Merging without enabling won't work:

Would have been nice to merge it this way, but unfortunate that it doesn't work.

Good to have the config sitting around in this pull request, anyway. This way it can be picked up easily by anyone in the future.

Implemented in hebasto/bitcoin-core-nightly#1.

Note that it's already implemented in https://github.com/maflcko/b-c-nightly.