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

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

Merged
merged 16 commits into from
Feb 7, 2025

Conversation

kwvg
Copy link
Collaborator

@kwvg kwvg commented Nov 10, 2024

Additional Information

Breaking Changes

GCC 11.1 or later, or Clang 16.0 or later, are now required to compile Dash Core.

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
  • I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

Copy link

This pull request has conflicts, please rebase.

PastaPastaPasta added a commit that referenced this pull request Nov 17, 2024
, bitcoin#28622, bitcoin#28880, bitcoin#29185, bitcoin#29170, bitcoin#29233, bitcoin#29298, bitcoin#29598, bitcoin#29732, bitcoin#29890, bitcoin#29739, bitcoin#30074, bitcoin#30198, bitcoin#29072 (toolchain backports: part 2)

1506d9d merge bitcoin#29072: use `-no_exported_symbols` on macOS (Kittywhiskers Van Gogh)
9247960 merge bitcoin#30198: qt 5.15.14 and fix macOS build with Clang 18 (Kittywhiskers Van Gogh)
5585e7a merge bitcoin#30074: use ENV flags in get_arch (Kittywhiskers Van Gogh)
decd420 merge bitcoin#29739: swap cctools otool for llvm-objdump (Kittywhiskers Van Gogh)
0f8c420 merge bitcoin#29890: remove some tools when cross-compiling for macOS (Kittywhiskers Van Gogh)
936da1a merge bitcoin#29732: qt 5.15.13 (Kittywhiskers Van Gogh)
c294b47 revert: patch qt to make placeholders differ from actual text (Kittywhiskers Van Gogh)
af7090c merge bitcoin#29598: don't use -h with touch on OpenBSD (Kittywhiskers Van Gogh)
ebf8ff2 merge bitcoin#29298: patch libtool out of libnatpmp/miniupnpc (Kittywhiskers Van Gogh)
070b876 merge bitcoin#29233: depends move macOS C(XX) FLAGS out of C & CXX (Kittywhiskers Van Gogh)
d838481 revert dash#2398: Force fvisibility=hidden when compiling on macos (Kittywhiskers Van Gogh)
59a18f9 merge bitcoin#29170: add macho branch protection check (Kittywhiskers Van Gogh)
cb024d9 merge bitcoin#29185: remove `--enable-lto` (Kittywhiskers Van Gogh)
6d75a81 merge bitcoin#28880: switch to using LLVM 17.x for macOS builds (Kittywhiskers Van Gogh)
7b0a1f2 merge bitcoin#28622: use macOS 14 SDK (Xcode 15.0) (Kittywhiskers Van Gogh)
02eb735 merge bitcoin#24948: fix typo in permissions (Kittywhiskers Van Gogh)
2739107 merge bitcoin#24534: make gen-sdk deterministic (Kittywhiskers Van Gogh)
ab10bf9 merge bitcoin#24241: cleanup doc on need of Developer Account to obtain macOS SDK (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * Dependent on #6384
  * Dependency for #6389
  * The Qt patch introduced in [dash#5596](#5596), `fix_qt_placeholders.patch`, was a portion of a suggested workaround for QTBUG-92199 ([source](https://bugreports.qt.io/browse/QTBUG-92199?focusedId=669719&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-669719)) but since then, a fix ([here](https://codereview.qt-project.org/c/qt/qtbase/+/434310)) has made its way to 5.15.12 and we are upgrading to 5.15.14 from 5.15.11.

    So we can safely remove this patch.

  ## 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 **(note: N/A)**
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [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:
    utACK 1506d9d
  PastaPastaPasta:
    utACK 1506d9d

Tree-SHA512: df8e4ea0ce9e7b269d248518698f0566b5eca1a54cdfb53f5b213b90fb5177e5a5df44eaeb6f3fc014cd93351c9245736bb2fd52bc2af4ae274d8fa93e601b07
@kwvg kwvg added this to the 23 milestone Jan 14, 2025
@kwvg kwvg marked this pull request as ready for review February 5, 2025 10:29
Copy link

coderabbitai bot commented Feb 5, 2025

Walkthrough

The changes span CI configuration, build scripts, documentation, and source code. In CI files, the container image was updated from CentOS 8 to CentOS 9 along with a corresponding task name change, and a retry wrapper was removed from installation commands. Build macros for checking multiplication overflow and std::filesystem support were removed from auxiliary m4 files and the configure script was adjusted accordingly. The dependency documentation was restructured: detailed lists were replaced with references to an external dependencies file, and instructions for macOS were updated to emphasize LLVM installation. In the source code, the return type in a network function was simplified, built-in overflow tests were removed from fuzz tests, and a MinGW64-specific workaround for file renaming was eliminated. Additionally, linking flags for fuzz testing were simplified and release notes were updated with new minimum compiler version requirements.

Tip

🌐 Web search-backed reviews and chat
  • We have enabled web search-based reviews and chat for all users. This feature allows CodeRabbit to access the latest documentation and information on the web.
  • You can disable this feature by setting web_search: false in the knowledge_base settings.
  • Please share any feedback in the Discord discussion.
✨ Finishing Touches
  • 📝 Generate Docstrings (Beta)

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (4)
src/net.cpp (1)

1610-1648: LGTM! Consider adding debug logging for invalid cases.

The message type decoding implementation is correct and handles both short and long encodings properly. The input validation and error handling are thorough.

Consider adding debug logging when returning nullopt to help troubleshoot invalid message types:

 if (contents.size() == 0) {
+    LogPrint(BCLog::NET, "V2Transport::GetMessageType: empty contents\n");
     return std::nullopt; // Empty contents
 }
 // ...
 if (contents.size() < CMessageHeader::COMMAND_SIZE) {
+    LogPrint(BCLog::NET, "V2Transport::GetMessageType: insufficient bytes for long encoding (%d)\n", contents.size());
     return std::nullopt; // Long encoding needs 12 message type bytes.
 }
doc/dependencies.md (1)

1-6: Refine Introductory Text for Clarity
The opening lines now explain the dependency overview. Consider shortening the phrase “in reference to the release binaries” (e.g. “for release binaries”) to reduce wordiness as hinted by the static analysis.

🧰 Tools
🪛 LanguageTool

[style] ~5-~5: ‘in reference to’ might be wordy. Consider a shorter alternative.
Context: .... "Runtime" and "Version Used" are both in reference to the release binaries. | Compiler | Min...

(EN_WORDINESS_PREMIUM_IN_REFERENCE_TO)

.cirrus.yml (1)

81-88: Update Task for CentOS 9 Environment
The task name update to '32-bit + dash [gui] [CentOS 9]' and the container image change to centos:9 ensure consistency with the new CI requirements. Additionally, setting PACKAGE_MANAGER_INSTALL: "yum install -y" is appropriate for the CentOS environment. Please double-check that YAML formatting (e.g. spacing before colons) complies with linting guidelines.

🧰 Tools
🪛 YAMLlint (1.35.1)

[warning] 82-82: too many spaces before colon

(colons)

doc/build-osx.md (1)

55-68: Clarify LLVM Upgrade Instructions
The added instructions for installing a more recent version of LLVM and setting the appropriate environment variables are clear and useful. Given that this update supports the new minimum compiler requirements (GCC 11.1 / Clang 16.0+), consider adding a brief note explaining the motivation behind using LLVM (e.g., improved language feature support, compatibility with new C++ standards, etc.). This extra context could help users understand the necessity of the change.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 21fb709 and 0861692.

📒 Files selected for processing (17)
  • .cirrus.yml (1 hunks)
  • build-aux/m4/bitcoin_runtime_lib.m4 (0 hunks)
  • build-aux/m4/l_filesystem.m4 (0 hunks)
  • ci/test/04_install.sh (1 hunks)
  • ci/test/05_before_script.sh (1 hunks)
  • configure.ac (2 hunks)
  • doc/build-freebsd.md (2 hunks)
  • doc/build-osx.md (1 hunks)
  • doc/build-unix.md (1 hunks)
  • doc/dependencies.md (1 hunks)
  • doc/release-notes-6389.md (1 hunks)
  • src/Makefile.test.include (1 hunks)
  • src/net.cpp (1 hunks)
  • src/test/fuzz/addition_overflow.cpp (0 hunks)
  • src/test/fuzz/multiplication_overflow.cpp (0 hunks)
  • src/util/hasher.h (1 hunks)
  • src/util/system.cpp (0 hunks)
💤 Files with no reviewable changes (5)
  • src/test/fuzz/multiplication_overflow.cpp
  • src/test/fuzz/addition_overflow.cpp
  • build-aux/m4/bitcoin_runtime_lib.m4
  • build-aux/m4/l_filesystem.m4
  • src/util/system.cpp
✅ Files skipped from review due to trivial changes (1)
  • doc/release-notes-6389.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
doc/build-freebsd.md

67-67: Heading levels should only increment by one level at a time
Expected: h5; Actual: h6

(MD001, heading-increment)

🪛 YAMLlint (1.35.1)
.cirrus.yml

[warning] 82-82: too many spaces before colon

(colons)

🪛 LanguageTool
doc/dependencies.md

[style] ~5-~5: ‘in reference to’ might be wordy. Consider a shorter alternative.
Context: .... "Runtime" and "Version Used" are both in reference to the release binaries. | Compiler | Min...

(EN_WORDINESS_PREMIUM_IN_REFERENCE_TO)

🔇 Additional comments (11)
src/util/hasher.h (1)

42-42: LGTM! Documentation URL updated to latest GCC version.

The URL reference has been correctly updated to point to the latest GCC 13.2.0 documentation.

src/Makefile.test.include (1)

246-246: LGTM! Simplified linking flags for fuzz testing binary.

Removed $(RUNTIME_LDFLAGS) as it's no longer needed with the updated compiler requirements.

configure.ac (2)

481-482: LGTM! Added -Wsuggest-override warning flag.

Added compiler warning flag to encourage use of override specifiers in C++ code, which improves code clarity and helps catch inheritance-related bugs.


1373-1390: LGTM! Improved fuzz binary configuration logic.

Restructured the fuzz binary configuration to:

  • Only check for main function when fuzz binary is enabled
  • Improved conditional logic and error handling
  • Added proper messaging
ci/test/05_before_script.sh (1)

28-33: Simplify SHELL_OPTS Assignment for CentOS
The removal of the LC_ALL=en_US.UTF-8 part and setting SHELL_OPTS="CONFIG_SHELL=/bin/bash" now simplifies the environment configuration for CentOS. Please verify that none of the downstream commands need the explicit UTF-8 locale setting in the new CentOS 9 environment.

doc/dependencies.md (2)

7-54: Enhanced Structured Documentation
The reorganization into clear sections – "Required", "Optional", "GUI", "Networking", "Notifications", and "Wallet" – with corresponding version and runtime details greatly improves readability and maintainability. All the version references appear consistent with the updated build requirements.


55-56: Note Formatting Review
The note regarding libgmp’s minimum supported version on macOS is clear. Make sure any cross-references elsewhere (e.g. in platform-specific build docs) are updated to reflect this change.

doc/build-freebsd.md (1)

17-18: Centralize Dependency Information
Replacing the embedded dependency details with a reference to dependencies.md is a good move toward centralizing and maintaining documentation. Ensure that platform-specific nuances are still properly addressed in any separate guides if needed.

ci/test/04_install.sh (1)

61-64: Remove Retry Mechanism for Yum Install Commands
The removal of ${CI_RETRY_EXE} in front of the yum install commands aligns the script with the CentOS 9 setup. Please verify that the removal of the retry logic won’t negatively affect the robustness of package installations, especially during transient network issues in CI.

doc/build-osx.md (1)

49-50: Centralizing Dependency Information
Using a reference to dependencies.md streamlines the documentation by avoiding redundancy. This helps ensure that all dependency details are maintained in a single, authoritative source.

doc/build-unix.md (1)

31-32: Reference to Centralized Dependencies
Adding the line to refer users to dependencies.md for a complete overview is consistent with the updated documentation in other build guides. This change reduces duplication and keeps dependency details in one place for easier maintenance.

Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

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

utACK 0861692

Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

utACK 0861692

@PastaPastaPasta PastaPastaPasta merged commit 09ffdea into dashpay:develop Feb 7, 2025
23 checks passed
PastaPastaPasta added a commit that referenced this pull request Feb 12, 2025
…oble`), merge bitcoin#29985

1599cc6 fix: suppress `float-cast-overflow` UBSan error from `qRound(double)` (Kittywhiskers Van Gogh)
a57d128 ci: drop i386 conditional altogether, we don't need `wine32` anymore (Kittywhiskers Van Gogh)
b862411 ci: drop conflicting `g++-multilib` package and related workaround (Kittywhiskers Van Gogh)
7b1caa9 fix: remove now-invalid package entry (Kittywhiskers Van Gogh)
9f9e965 fix: use default non-root user `ubuntu` introduced in Ubuntu 22.10 (Kittywhiskers Van Gogh)
9b4c95e ci: update containers and CI to use Ubuntu 24.04 LTS (`noble`) (Kittywhiskers Van Gogh)
8aec25b merge bitcoin#29985: Fix build of Qt for 32-bit platforms with recent glibc (Kittywhiskers Van Gogh)

Pull request description:

  ## Motivation

  Since [dash#6389](#6389), the minimum supported version of GCC has been 11.1 or later. Our current base, Ubuntu 22.04 LTS (`jammy`) ships with GCC 11.2 ([source](https://packages.ubuntu.com/jammy/gcc)). The cross-compilation package for `arm-linux-gnueabihf` is GCC 11.2 as well ([source](https://packages.ubuntu.com/jammy/gcc-arm-linux-gnueabihf)). Unfortunately, this isn't the case for `mingw-w64-x86-64`, which ships with GCC 10.3 ([source](https://packages.ubuntu.com/jammy/gcc-mingw-w64-x86-64-posix)).

  So far, an workaround was utilized to allow GCC 10.3 to pass muster upstream through bitcoin#28379 ([source](bitcoin@fa67f09#diff-0baeabda402ee522682c25cef25a248ff6d9e2904f6d66f93f6babf55b576675L986)). Dash Core's enablement of C++20 experimental support in [dash#4600](#4600) was relatively even more permissive ([source](https://github.com/dashpay/dash/pull/4600/files#diff-0baeabda402ee522682c25cef25a248ff6d9e2904f6d66f93f6babf55b576675R168)).

  Though since then, enforcement of those newer minimum requirements through backports like [bitcoin#30228](bitcoin#30228) are frustrated by the version of GCC shipped for Windows cross-compilation.

  This can be resolved by bumping the image to the next available LTS release, Ubuntu 24.04 (`noble`), which ships with GCC 13.2 natively ([source](https://packages.ubuntu.com/noble/gcc)), for ARM Linux ([source](https://packages.ubuntu.com/noble/gcc-arm-linux-gnueabihf)) and AMD64 Windows ([source](https://packages.ubuntu.com/noble/gcc-mingw-w64-x86-64-posix)), moreover, it is acknowledged as a _de facto_ lowest supported distro needed for Windows cross compilation by [bitcoin#30580](bitcoin#30580 (comment)).

  Skipping the enforcement of newer minimum requirements is inadvisable, primarily because it will eventually conflict with upcoming code changes ([comment](bitcoin#30228 (comment))) like [dash#6378](#6378).

  ## Additional Information

  * Dependency for #6380

  * A default unprivileged user named `ubuntu` was introduced in Ubuntu 22.10 ([source](https://bugs.launchpad.net/cloud-images/+bug/2005129)) with UID `1000` and GID `1000`.

    We allow specifying UID and GID to ensure they match with the ownership of directories expected to be mounted to avoid permissions issues (especially on platforms like macOS where the user can have a UID of `501` and GID of `20`).
    * To retain this behavior, the `ubuntu` user and the `ubuntu` group will have its UID and GID updated. This is a no-op if defaults are selected.
    * In some cases where it may be undesirable to have the username or home directory deviate from the present name  (`dash`), it will be additionally renamed from `ubuntu`.
      * GitLab is unhappy if the container doesn't have a user named `dash` ([build](https://gitlab.com/dashpay/dash/-/jobs/9082688485#L24)) and it results in a runner failure.

  * Due to a conflict between `gcc-multilib` and `gcc-arm-linux-gnueabihf`, installation of the latter will result in the uninstallation of the former. This has been documented behavior for a while now ([source](https://bugs.launchpad.net/ubuntu/+source/gcc-defaults/+bug/1300211)) and continues till today (see below).

    <details>

    <summary>Error message:</summary>

    ```
    ubuntu@ec46b76eeb2c:/src/dash$ sudo apt install gcc-multilib gcc-arm-linux-gnueabihf
    Reading package lists... Done
    Building dependency tree... Done
    Reading state information... Done
    gcc-arm-linux-gnueabihf is already the newest version (4:13.2.0-7ubuntu1).
    gcc-arm-linux-gnueabihf set to manually installed.
    Some packages could not be installed. This may mean that you have
    requested an impossible situation or if you are using the unstable
    distribution that some required packages have not yet been created
    or been moved out of Incoming.
    The following information may help to resolve the situation:

    The following packages have unmet dependencies:
     gcc-arm-linux-gnueabihf : Depends: gcc-13-arm-linux-gnueabihf (>= 13.2.0-11~) but it is not installable
    E: Unable to correct problems, you have held broken packages.
    ```

    </details>

    Since [dash#5372](#5372), we have stopped supporting i686 and dropped support for 32-bit Windows even earlier. There doesn't seem to be much reason to keep `gcc-multilib` around, especially since it _cannot_ be around and has been silently uninstalled for a while now, so the package has now been dropped.

    * Since Wine 7.0, WoW64 support has been included ([source](https://www.winehq.org/announce/7.0)), which allows for some applications to run without a corresponding `wine32` setup ([source](https://wiki.debian.org/Wine#Step_1:_Enable_multiarch)). Support has improved furthermore in Wine 9.0 ([source](https://gitlab.winehq.org/wine/wine/-/releases/wine-9.0)), which is available in `noble` ([source](https://packages.ubuntu.com/noble/wine64)). This allows us to drop the i386 conditional altogether.

  * `libncurses5`, while a valid package on `jammy` ([source](https://packages.ubuntu.com/jammy/libncurses5)), is not in any future version, including `noble` ([source](https://packages.ubuntu.com/noble/libncurses5), error page) though `libncurses5-dev` continues to remain a valid package ([source](https://packages.ubuntu.com/noble/libncurses5-dev)) and remains used as a Python dependency ([source](https://github.com/dashpay/dash/blob/1930572b05c53d2d8335bcfc22270871d5b0bf2f/contrib/containers/ci/Dockerfile#L91)). As a result, `libncurses5` has been dropped.

  * A bump in distro base also causes a bump in glibc version, which causes zlib, a Qt dependency to fail a compile-time check ([build](https://github.com/dashpay/dash/actions/runs/13220255380/job/36904408129?pr=6564#step:6:8050)), this is avoided by backporting [bitcoin#29985](bitcoin#29985).

  * Due to [QTBUG-133261](https://bugreports.qt.io/browse/QTBUG-133261), UBSan will raise a `float-cast-overflow` originating from `qRound(double)` ([build](https://gitlab.com/dashpay/dash/-/jobs/9083558880#L3132)), as of this writing, a fix is currently underway ([source](https://codereview.qt-project.org/c/qt/qtbase/+/621931)) but it is unclear when it would be available in the next Qt 5.15 revision.

    As this doesn't seem to be a regression ([source](https://bugreports.qt.io/browse/QTBUG-133261?focusedId=860311&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-860311)) despite being undesirable behavior, we can suppress the alarm.

  ## Checklist

  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [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:
  PastaPastaPasta:
    utACK 1599cc6
  UdjinM6:
    utACK 1599cc6

Tree-SHA512: 92f786e9da1440830057729a5d9cdf6448ef9da8c66e0d4c3b80da2e22dd599353f44a31dcbe0ed103febdf627d2a1a332d72de1c3ec312b6ceb665f4bbcb5d1
PastaPastaPasta added a commit that referenced this pull request Feb 20, 2025
…s, GCC 11 for building `sqlite` source

7089045 ci: install GCC 11 and use it on sqlite build (Kittywhiskers Van Gogh)
95e24f5 ci: install GCC 14 and use it on nowallet depends and build (Kittywhiskers Van Gogh)

Pull request description:

  ## Motivation

  This PR is a spiritual continuation of [dash#5375](#5375) to help catch issues like the ones resolved in [dash#5064](#5064) during CI.

  ## Additional Information

  * Earlier iterations of this PR built GCC 14 as the toolchain PPA wouldn't go higher than GCC 12 for `jammy` ([source](https://launchpad.net/~ubuntu-toolchain-r/+archive/ubuntu/ppa?field.series_filter=jammy)) (GCC 13 if we were willing to use the `test` branch, [source](https://launchpad.net/~ubuntu-toolchain-r/+archive/ubuntu/test?field.series_filter=jammy)) and we ruled out upgrading our base image to `noble` (which does offer GCC 14 even without a PPA, [source](https://packages.ubuntu.com/noble/g++-14)) so we could test against the lowest supported LTS.

    * Unfortunately, staying on `jammy` was no longer tenable as we require at least GCC 11.1 (since [dash#6389](#6389)) but `jammy` shipped GCC 10.3 for the Windows cross-compiler ([source](https://packages.ubuntu.com/jammy/gcc-mingw-w64-x86-64-posix)) and this proved to be a blocker for backports. See [dash#6564](#6564) for more information.

  * Since we've upgraded to `noble` now, we not only can use GCC 14 (the latest major version of GCC as of this writing), we can also use older versions and GCC 11.4 is readily available on `noble` ([source](https://packages.ubuntu.com/noble/g++-11)), which can help us test for unexpected regressions on the lowest supported version and versions likely to be used by rolling release distros.

  ## 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 **(note: N/A)**
  - [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:
  PastaPastaPasta:
    utACK 7089045; will run my own CI and confirm functionality
  UdjinM6:
    utACK 7089045

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

Successfully merging this pull request may close these issues.

3 participants