Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Draft
wants to merge 11 commits into
base: develop
Choose a base branch
from

Conversation

kwvg
Copy link
Collaborator

@kwvg kwvg commented Nov 9, 2024

Motivation

This PR is a spiritual continuation of dash#5375. Unlike upstream, Dash uses a development image for CI and therefore, is limited by the packages provided by the base distro (in our case, Ubuntu 22.04 jammy) unless additional efforts are placed to make those packages available, those efforts were expended for Clang and in this PR, it is being expanded to GCC to be able to catch issues like the ones resolved in dash#5064 within CI itself.

Additional Information

  • Depends on ci: merge bitcoin#27314, #28954, #29765, introduce dependency options in GitHub Actions builds, fix multiprocess builds #6400

  • Dependency for backport: merge bitcoin#23060, #24164, #23565, #24337, #27682, #29208, #29091, #29165, #29934, #30261, partial bitcoin#27662, #28210, #28348, #30263 (bump minimum compiler) #6389

  • While building GCC ourselves increases the build time of the container, mitigations have been made to reduce the build time.

    The following alternatives were explored before settling for building:

    • Using the Ubuntu Toolchain PPA (which provides up to GCC 12 for jammy using the ppa branch, source, and GCC 13 using test, source)
    • Using a container base that comes with GCC 14 (requiring significant retooling for CI to support using multiple images)
    • Moving the development image base to noble as GCC 14 is available for it (source) (but means we aren't testing against the lowest supported LTS).
    • Downloading pre-built binaries from GNU (they don't provide them and their binaries page lists other sources and none of them are Linux, here)
    • Using a GCC Docker container as a base (they're not based on the same LTS we're using, source)

    The following mitigations were made to reduce build time:

    • Dropping support for Obj(C++) and Fortran as we aren't expecting macOS cross compilation and we don't have Fortran code to compile
    • Dropping libsanitizer (as tsan uses Clang and ubsan uses distro-supplied GCC), libstdcxx-debug, Native Language Support (we expect CI diagnostic information to remain in English) and ISL (used for loop optimizations, no significant performance regressions found)
    • Using distro's package manager for dependencies (GMP, MPC, MPFR) instead of fetching them and building them in-tree.
    • Reducing the packages installed in first RUN layer to the minimum needed for GCC and LLVM installations and placing GCC build and install to the second RUN layer.
  • Using LD_LIBRARY_PATH to point to our compiled GCC's library files (and extending that to LLVM's library files) are acceptable and will not interfere with executing binaries built using the distro's packaged compiler as it will eventually search default paths and find the libraries shipped with the distro (source).

    • depends built using GCC 11.4 (distro's GCC) can be used to build Dash Core using GCC 14.2 (compiled GCC) but the reverse doesn't hold true.

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)

@kwvg kwvg added this to the 22 milestone Nov 9, 2024
@kwvg kwvg force-pushed the gcc_and_friends branch 2 times, most recently from 29101ef to 62d4636 Compare November 10, 2024 10:11
@kwvg
Copy link
Collaborator Author

kwvg commented Nov 10, 2024

GitHub Actions on 62d4636 works as expected on my fork, see https://github.com/kwvg/dash/actions/runs/11764281671/job/32769805675#step:6:1

Copy link

This pull request has conflicts, please rebase.

@kwvg kwvg changed the title ci: build more recent GCC and compile with it, bump Clang to 18, stage built packages in /opt, fix LLDB personality issue ci: build GCC 14, build nowallet depends and source with it, stage built packages in /opt, allowlist LLVM libc++ Nov 21, 2024
@kwvg
Copy link
Collaborator Author

kwvg commented Nov 21, 2024

GitHub Actions run for 3263879, https://github.com/kwvg/dash/actions/runs/11949630923.

gnuArch="$(dpkg-architecture --query DEB_BUILD_GNU_TYPE)"; \
ln -s "/usr/include/${gnuArch}/asm" "/usr/include/asm"; \
majorVersion="${GCC_VERSION%%.*}"; \
./configure \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am against of building gcc everytime. It's heavy process, it make things that supposed to be close to instant to be "1 hour long".

If you really need custom gcc build - let's build it, put somewhere as a binary, and only download it and verify hashes. It's not necessary should be a build that is done by gcc's team, but it can be 3rd party repo

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gcc is default compiler in my ubuntu 24.10, I don't see a reason to build it from scratch. It's not a bleeding-edge-night-build which we need.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator Author

@kwvg kwvg Nov 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's heavy process, it make things that supposed to be close to instant to be "1 hour long".

It doesn't take an hour, it takes ~25 minutes (build) and efforts have been made to make it cacheable as possible (as mentioned in the PR description, see below, along with all the alternatives explored, see above). Builds without the GCC compile take ~5 minutes (build), so the slowdown impact relative to the current deployment in a worst-case scenario (cache-miss) is 20 minutes and the subsequent caching should should give us pull times comparable to regular cache hits.

  • Reducing the packages installed in first RUN layer to the minimum needed for GCC and LLVM installations and placing GCC build and install to the second RUN layer.

I'm not against building them separately and downloading binaries for them. What about baking in the first two steps into their own Docker container and FROM'ing it? It should bypass cache misses and the rebuilds that it would attract.


gcc is default compiler in my ubuntu 24.10, I don't see a reason to build it from scratch. It's not a bleeding-edge-night-build which we need.

This has been addressed in the PR description (see below)

  • Moving the development image base to noble as GCC 14 is available for it (source) (but means we aren't testing against the lowest supported LTS).

maybe their's? https://github.com/xpack-dev-tools/gcc-xpack

I had suggested this to Pasta and he seemed lukewarm towards the idea (@PastaPastaPasta, thoughts?)

Copy link

This pull request has conflicts, please rebase.

@kwvg kwvg mentioned this pull request Dec 15, 2024
5 tasks
PastaPastaPasta added a commit that referenced this pull request Dec 17, 2024
04ce1fe ci: deduplicate macOS SDK setup logic (Kittywhiskers Van Gogh)
8dd0db7 ci: fix "LC_ALL: cannot change locale (en_US.UTF-8)" in Guix container (Kittywhiskers Van Gogh)
187fe17 ci: don't stage packages in `/tmp`, reduce layers for `cppcheck` build (Kittywhiskers Van Gogh)
eef8635 ci: install `i386` packages only if host is `amd64`, merge layers (Kittywhiskers Van Gogh)
e770229 ci: purge package manager cache after each interaction (Kittywhiskers Van Gogh)
b7099ee ci: remove redundant `version` attribute, avoid `lldb` personality error (Kittywhiskers Van Gogh)
64cdc42 ci: add LLVM library path to `LD_LIBRARY_PATH` and GDB allowlist (Kittywhiskers Van Gogh)
440fd3f ci: drop distro LLVM packages, move Clang install up, set defaults (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * This pull request pulls container-specific changes from [dash#6387](#6387), [dash#6400](#6400) and [dash#6421](#6421)

  * The `HOST` check before running `setup_sdk.sh` isn't a part of the script itself as the script is written to be independent of external variables set. The caller is expected to know the conditions needed to run `setup_sdk.sh` as the script is _relatively_ agnostic to its environment.

  * The `version` attribute in the [`develop`](https://github.com/dashpay/dash/blob/a8e2316d6f9c6726a498bcae2c5c5d7354769511/contrib/containers/develop/docker-compose.yml) and [`guix`](https://github.com/dashpay/dash/blob/a8e2316d6f9c6726a498bcae2c5c5d7354769511/contrib/containers/guix/docker-compose.yml) container's `docker-compose.yml` has been dropped as the attribute has been deprecated in the compose spec ([source](https://github.com/compose-spec/compose-spec/blob/65ef9f4a5d713b405a77c45c64263f2543e65267/spec.md#version-top-level-element-obsolete)).

  * Using `LD_LIBRARY_PATH` to point to LLVM's libraries are acceptable and will not interfere with executing binaries built using the distro's packaged compiler as it will eventually search default paths and find the libraries shipped with the distro ([source](https://man7.org/linux/man-pages/man8/ld.so.8.html)).

  * Currently, running LLDB will result in a "personality set failed: Operation not permitted" error ([source](https://discourse.llvm.org/t/running-lldb-in-a-container/76801)). This is caused by its attempt at disabling ASLR for debugging.

    To work around this error, the container will now operate under relaxed restrictions (`seccomp=unconfined`). As disabling ASLR is valuable when debugging and the container is meant for developers (i.e. it isn't used for CI), we have opted to relax restrictions instead of skipping ASLR disablement.

  * As of `develop` (a8e2316), packages built by the container are stored in `/tmp`, which is inadvisable as it is the same directory used to store functional test runs and it's not too difficult to delete `/tmp`'s contents to save space in a long running [`develop`](https://github.com/dashpay/dash/blob/a8e2316d6f9c6726a498bcae2c5c5d7354769511/contrib/containers/develop/docker-compose.yml) container and then realize that both `shellcheck` and `cppcheck` are stored there and now you have to ditch the container you're working in and restart it.
    * To remedy this, packages are now built and stored in `/opt` in accordance with the FHS ([source](https://refspecs.linuxfoundation.org/FHS_3.0/fhs/ch03s13.html)). `/usr/local` was a contender but it's pre-populated, meanwhile `ls /opt` would give you a very quick picture of what's built for the container.

    * `/tmp` will not be entirely empty because [pypa/pip#10753](pypa/pip#10753) results in residual `.pem` files leaking into `/tmp` and `pyenv` stores its build log there and keeping it around has some debug value.

  ## Breaking Changes

  None expected.

  ## Checklist:

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

ACKs for top commit:
  UdjinM6:
    ACK 04ce1fe
  PastaPastaPasta:
    utACK 04ce1fe

Tree-SHA512: 5442ae06cb73b9bc4eec908803548195ae8fd9150422789e5f98578ad01a303b5361f9ba42fe8faee27ac91e38328b7771e4ba42b296dfa70ecbbfc7d10436b6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants