ci: merge bitcoin#27314, #28954, introduce dependency options in GitHub Actions builds, fix multiprocess builds, bump to Clang 18

[kwvg]

Additional Information

• Depends on ci: use actions/cache to manage depends cache #6406

• Dependency for ci: build GCC 14, build nowallet depends and source with it, stage built packages in /opt, allowlist LLVM libc++ #6387

• bitcoin#27314 has been backported in this PR as bitcoin#25838 (backported in dash#6384) broke Clang depends builds and this PR introduces Clang depends builds in CI.

• Support for multiprocess builds were enabled proper in dash#6143 but unfortunately, the configuration params for multiprocess builds were not processed by CI as the build variant was not added to matrix.sh (source). This is evident by comparing two variants with Boost::Process enablement (--with-boost-process), linux64_fuzz (source) and linux64_multiprocess (source).

  Looking at a develop (6a51ab2) build, the fuzz build has it enabled (source) while the multiprocess build doesn't (source) despite both scripts having the enablement argument.

• bitcoin#28954 has been backported to fix a problem associated with multiprocess runs (build)

• The fuzz build will continue to use GCC-built depends instead of Clang-built depends as originally planned, as Clang-built depends are multiprocess-enabled and currently multiprocess and fuzz don't play nice together (see upstream build failure for example, source, corresponding with local failure, source).

• Clang has been bumped to 18 in anticipation of bitcoin#30201, which will drop (native_llvm, formerly known as native_clang) and mandate the presence of Clang 18 or higher for cross-compilation.

• The version attribute in the develop container's docker-compose.yml has been dropped as the attribute has been deprecated in the compose spec (source).

• To avoid LLDB's "personality set failed: Operation not permitted" error caused by its attempt at disabling ASLR for debugging (source), the container will now operate under relaxed restrictions (seccomp=unconfined). As disabling ASLR is valuable when debugging and the container isn't used by CI but is meant for individual developers, we have opted to relax restrictions instead of skipping ASLR disablement.

  This does not affect CI as these relaxations are defined in the docker-compose.yml of the develop container, which inherits from the CI container but is a container of its own.

• As DEP_OPTS is a new parameter that could require rebuilding depends, it needs to be incorporated into the cache key. The simplest way to do it is to append the hash of the file that defines DEP_OPTS (build.yml) to the cache key. This comes with the drawback that any change to build.yml could result in a cache miss due to a changed hash.

  This has been mitigated by appending the hash of DEP_OPT's value instead. As GitHub doesn't make this easy, it has to be done in a prior "Determine params" step.

  • This was taken as an opportunity to separate the matrix configuration out to matrix.json and use jq to interpret it. We can use jq to query values (dep_opts) meant for one matrix (deps) using a value (depends_name) from the other matrix (src).
  • The above setup also removes having to mention host in both matrices as the value of host is taken by the second matrix (src) by fetching it from the array of the first matrix (deps) with matching depends_name.
  • head trims the output of sha256sum to 64 characters, the hash itself isn't being trimmed but everything after it is (trailing hyphen and newline). This is also why echo -n is used in some places, to avoid newline addition resulting in a different hash value.

Breaking Changes

None expected

Checklist

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas (note: N/A)
• I have added or updated relevant unit/integration/functional/e2e tests (note: N/A)
• I have made corresponding changes to the documentation (note: N/A)
• I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

[github-actions]

This pull request has conflicts, please rebase.

[github-actions]

This pull request has conflicts, please rebase.

[kwvg]

GitHub Actions run for 3cc8c70, https://github.com/kwvg/dash/actions/runs/11935641601.

• No Qt regression (source).
• DEP_OPTS propagating correctly to depends build step (source).
• Hashing working correctly (source).
• DEP_OPTS and HOST retrieved correctly (source).

[PastaPastaPasta]

light utACK 3cc8c70

I have concerns about this PR; but I'm not sure how to communicate them, and don't have good actionable feedback.

I'm concerned the patch-set as is will make the GitHub CI / build.yml harder to understand and harder to improve; but I also cannot at this moment figure out an alternative way to accomplish what is needed to be accomplished. Some of the commits just "feel" wrong, but ultimately it's just for CI, so it's not that big of a deal, and I don't have ways imagined for how to improve the situation; so probably best path is to merge, and maybe I'll figure out something I like more later.

[PastaPastaPasta]

I kinda hate using jq in the build.yml like this; but I guess it's ok

[kwvg]

The alternative would be having to maintain two JSON files (they would have to be JSON if we want to share params between them without resorting to duplication), which wouldn't be the worst thing in the world but imo since src will be reading from dep for values and they are configuring the same actions run, just different jobs, consolidation would be better.

Plus we'd be using jq later on regardless to read through the matrix definition later on to find dep_opts and host from depends_name.

[knst]

these 2 JSON seems doens't have any dependency on each other, maybe better to have 2 files?

Is there any case, when you need both of them at once?

[kwvg]

Because they the second section (src) refers to the first section (dep) later on and they ultimately represent two halves of one workflow.

dash/.github/workflows/build.yml

Lines 126 to 136 in 3cc8c70

	- name: Determine params
	id: det-params
	run: |
	dep_name="${{ matrix.depends_name }}"
	dep_opts="$(jq -r ".dep.include[] | select(.depends_name == \"${{ matrix.depends_name }}\") | .dep_opts" -c .github/workflows/matrix.json)"
	dep_hash="$(echo -n ${dep_opts} | sha256sum | head -c 64)"
	echo "\"${dep_name}\" has DEP_OPTS \"${dep_opts}\" with hash \"${dep_hash}\""
	echo "dep_hash=${dep_hash}" >> $GITHUB_OUTPUT
	dep_host="$(jq -r ".dep.include[] | select(.depends_name == \"${{ matrix.depends_name }}\") | .host" -c .github/workflows/matrix.json)"
	echo "\"${dep_name}\" has HOST \"${dep_host}\""
	echo "dep_host=${dep_host}" >> $GITHUB_OUTPUT

Examples of these are explained in the PR description.

[PastaPastaPasta]

I fear this overcomplicates build.yml and will make it harder to improve / debug / logic on

[kwvg]

Outputs have to be generated in the previous job in order to be used in the current job and creating a new job just to read the matrix seems a bit wasteful, so the build image step is being used to also read, generate and export as output, the matrices needed for the next two jobs.

We can probably try reusable workflows (source) in the future, it'll be more GitLab-like but would invite duplication and a different kind of convoluted syntax. Depends on how reliable this setup proves to be.

[UdjinM6]

I think I found a way to do this with no jq/json and no duplication either (and bring two CI-s a bit closer to each other while at it), pls check UdjinM6@c40a595 (CI: Github + Gitlab)

[kwvg]

@UdjinM6, I like the idea of going back to honoring the DEP_OPTS in the matrix definitions but the proposed approach requires ensuring that every build target has the same DEP_OPTS expected by the depends build to ensure a cache hit (i.e. DEP_OPTS don't trickle down, we have to make sure that when it's re-evaluated, the same value is returned).

One of the arrangements are to use the multiprocess depends for multiprocess and tsan (and DEP_OPTS were adjusted in multiprocess with the proposed changes to make sure they build correctly) but tsan uses different DEP_OPTS (here), which is reflected in the "Determine params" step (here) (it also uses the debug depends but that changing that isn't an issue).

Another example is the SQLite build, which has a different DEP_OPTS (source) but falls back to the correct debug depends' key (source). This works so long as there aren't competing DEP_OPTS (otherwise fallback may not behave deterministically) but the reason we are introducing a DEP_OPTS key is due to the anticipation of there being competing variants (say, in between CI runs where the compiler is bumped).

This can be trivially fixed by ensuring all the DEP_OPTS match but also means that if there is a misalignment of expected DEP_OPTS, unless they're incompatible (in which case we'll get linker errors), it will continue to happen silently.

So the proposed approach does deduplicate between GitHub and GitLab configurations but requires duplication in every matrix definition script. If this is an acceptable tradeoff, I'll cherry-pick the changes. Thoughts?

[UdjinM6]

Interesting... So we were using wrong depends for these build targets in CI all this time. Smth like 94e37ef (CI: https://github.com/UdjinM6/dash/actions/runs/11999294004 + https://gitlab.com/UdjinM6/dash/-/pipelines/1558281977) on top of my previous commit should fix it (NOTE: -stdlib=libc++ was skipped for BITCOIN_CONFIG in 09fe21b, shouldn't be in DEP_OPTS either to make it work).

[UdjinM6]

Also, I wonder why we bump clang to 18 manually via 77795a5 instead of doing it via corresponding backports. Can you clarify this pls?

[knst]

these 2 JSON seems doens't have any dependency on each other, maybe better to have 2 files?

Is there any case, when you need both of them at once?

[knst]

why clang-16 is enforced here?

[kwvg]

To reduce ambiguity. Currently, both Clang 16 and distro-supplied Clang are installed and you have to specify Clang 16 using the suffix -16 while the variables above would be using distro-supplied clang.

This ambiguity has been sorted by removing distro-supplied Clang and LLVM but I'd prefer explicitly stating the compiler version we're expecting, esp. when we're outwardly deciding the version.

[knst]

why tsan became multiprocess? it should not be changed IMO

[kwvg]

In trying to strike a balance between testing different compilers and limiting the number of builds (both depends and source) to save resources, having multiprocess build with Clang 18 and then using it to also build TSan so we have access to a more recent thread sanitizer is the goal.

Upstream eventually wants to enable multiprocess in its depends anyways and their primary obstacle is multiprocess not playing nice with fuzz (as expressed in PR description).

[knst]

it's not good to depends on bash, sh should be enough:

$ sh -c 'for i in a b c ; do echo $i ; done'
a
b
c

[kwvg]

bash is part of the base Ubuntu distribution and will be required in dash#6387 regardless.

[knst]

looks a bit too complex, maybe better to make sh script for it instead line for better readability?

[kwvg]

Making a script that won't be reused/user-triggerable doesn't seem necessary, but I can add echo comments to serve as documentation for steps. Would that be alright?

[kwvg]

Also, I wonder why we bump clang to 18 manually via 77795a5 instead of doing it via corresponding backports. Can you clarify this pls?

@UdjinM6, we can do it through backports like bitcoin#29765. Will switch change to backport on next push!

Smth like 94e37ef (CI: https://github.com/UdjinM6/dash/actions/runs/11999294004 + https://gitlab.com/UdjinM6/dash/-/pipelines/1558281977) on top of my previous commit should fix it.

I had done similar changes in dash#6387 by adding a separate variant for GCC 14 depends but @PastaPastaPasta has suggested to reuse an existing build variant to reduce the number of overall variants to conserve resources (now in that PR nowallet is used instead). The whole exercise of not using ./ci/dash/build_depends.sh (and hardcoding DEP_OPTS into CI, as done in dash#3377, instead of relying on the matrix) is resource conservation.

I think we can go ahead with your solution if we're okay with explicitly specifying that DEP_OPTS have to be synced between the depends' BUILD_TARGET script and sources' BUILD_TARGET script (since I really like getting rid of the hardcoded DEP_OPTS) but if that isn't the case, then the JSON-manipulation based solution currently proposed seems to be workable.