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

releases: use LLVM 18 for macOS #30022

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion ci/test/00_setup_env_mac_cross.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export SDK_URL=${SDK_URL:-https://bitcoincore.org/depends-sources/sdks}
export CONTAINER_NAME=ci_macos_cross
export CI_IMAGE_NAME_TAG="docker.io/ubuntu:22.04"
export HOST=x86_64-apple-darwin
export PACKAGES="zip"
export PACKAGES="libtinfo5 zip"
export XCODE_VERSION=15.0
export XCODE_BUILD_ID=15A240d
export RUN_UNIT_TESTS=false
Expand Down
2 changes: 1 addition & 1 deletion contrib/devtools/symbol-check.py
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ def check_MACHO_sdk(binary) -> bool:
return False

def check_MACHO_ld64(binary) -> bool:
if binary.build_version.tools[0].version == [17, 0, 6]:
if binary.build_version.tools[0].version == [18, 1, 4]:
return True
return False

Expand Down
2 changes: 1 addition & 1 deletion contrib/guix/libexec/prelude.bash
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ fi
time-machine() {
# shellcheck disable=SC2086
guix time-machine --url=https://git.savannah.gnu.org/git/guix.git \
--commit=dc4842797bfdc5f9f3f5f725bf189c2b68bd6b5a \
--commit=1fa1325c0b4bb5c28e564526f387d82083733104 \
--cores="$JOBS" \
--keep-failed \
--fallback \
Expand Down
6 changes: 3 additions & 3 deletions contrib/guix/manifest.scm
Original file line number Diff line number Diff line change
Expand Up @@ -532,9 +532,9 @@ inspecting signatures in Mach-O binaries.")
((string-contains target "darwin")
(list ;; Native GCC 11 toolchain
gcc-toolchain-11
clang-toolchain-17
lld-17
(make-lld-wrapper lld-17 #:lld-as-ld? #t)
clang-toolchain-18
lld-18
(make-lld-wrapper lld-18 #:lld-as-ld? #t)
python-signapple
zip))
(else '())))))
8 changes: 4 additions & 4 deletions depends/packages/native_llvm.mk
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
package=native_llvm
$(package)_version=17.0.6
$(package)_version=18.1.4
$(package)_major_version=$(firstword $(subst ., ,$($(package)_version)))
$(package)_download_path=https://github.com/llvm/llvm-project/releases/download/llvmorg-$($(package)_version)
ifneq (,$(findstring aarch64,$(BUILD)))
$(package)_file_name=clang+llvm-$($(package)_version)-aarch64-linux-gnu.tar.xz
$(package)_sha256_hash=6dd62762285326f223f40b8e4f2864b5c372de3f7de0731cb7cd55ca5287b75a
$(package)_sha256_hash=8c2f4d1606d24dc197a590acce39453abe7a302b9b92e762108f9b5a9701b1df
else
$(package)_file_name=clang+llvm-$($(package)_version)-x86_64-linux-gnu-ubuntu-22.04.tar.xz
$(package)_sha256_hash=884ee67d647d77e58740c1e645649e29ae9e8a6fe87c1376be0f3a30f3cc9ab3
$(package)_file_name=clang+llvm-$($(package)_version)-x86_64-linux-gnu-ubuntu-18.04.tar.xz
Copy link
Member

Choose a reason for hiding this comment

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

this download link works but ubuntu-18?

Copy link
Member

Choose a reason for hiding this comment

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

this download link works but ubuntu-18?

This should be fixed in LLVM 19: https://discourse.llvm.org/t/rfc-improve-binary-security/78121/56

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea. While using the pre-built bins, we have to take what is available. For some reason the person building the x86_64 bins here has reverted to that. It also means we'll need to revert to installing libtinfo5 in the macOS CI, which is also annoying, because you also can't actually install that on Ubuntu 24.04 (although we currently still use 22.04 here, so it'll work for now).

Copy link
Member

Choose a reason for hiding this comment

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

Or waiting after #21778, which should be fine as well, as there is no rush to use clang 18?

Copy link
Member

@maflcko maflcko May 29, 2024

Choose a reason for hiding this comment

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

It should be possible now to nuke this and require the user (if they want to cross-compile to macOS) to type apt install clang-18 instead?

I presume leaving this to the user will allow to run the cross-compile on more distros and arches, as opposed to being possibly confined to x86_64-linux-gnu-ubuntu when using the llvm artefacts?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea, the plan is to switch away from vendoring Clang entirely, which is more straightforward now that #21778 is merged. I'll be followingup.

Copy link
Member Author

Choose a reason for hiding this comment

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

First step in #30198.

$(package)_sha256_hash=1607375b4aa2aec490b6db51846a04b265675a87e925bcf5825966401ff9b0b1
endif

define $(package)_stage_cmds
Expand Down
2 changes: 2 additions & 0 deletions depends/packages/qt.mk
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ $(package)_patches += duplicate_lcqpafonts.patch
$(package)_patches += guix_cross_lib_path.patch
$(package)_patches += fix-macos-linker.patch
$(package)_patches += memory_resource.patch
$(package)_patches += clang_18_libpng.patch
$(package)_patches += utc_from_string_no_optimize.patch
$(package)_patches += windows_lto.patch
$(package)_patches += zlib-timebits64.patch
Expand Down Expand Up @@ -249,6 +250,7 @@ define $(package)_preprocess_cmds
patch -p1 -i $($(package)_patch_dir)/qtbase-moc-ignore-gcc-macro.patch && \
patch -p1 -i $($(package)_patch_dir)/memory_resource.patch && \
patch -p1 -i $($(package)_patch_dir)/no_warnings_for_symbols.patch && \
patch -p1 -i $($(package)_patch_dir)/clang_18_libpng.patch && \
patch -p1 -i $($(package)_patch_dir)/rcc_hardcode_timestamp.patch && \
patch -p1 -i $($(package)_patch_dir)/duplicate_lcqpafonts.patch && \
patch -p1 -i $($(package)_patch_dir)/utc_from_string_no_optimize.patch && \
Expand Down
40 changes: 40 additions & 0 deletions depends/patches/qt/clang_18_libpng.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
fix Qt macOS build with Clang 18

See:
https://github.com/pnggroup/libpng/commit/893b8113f04d408cc6177c6de19c9889a48faa24.

In a similar manner as zlib (madler/zlib#895),
libpng contains a header configuration that's no longer valid and
hasn't been exercised for the macOS target.

- The target OS conditional macros are misused. Specifically
`TARGET_OS_MAC` covers all Apple targets, including iOS, and it
should not be checked with `#if defined` as they would always be
defined (to either 1 or 0) on Apple platforms.
- `#include <fp.h>` no longer works for the macOS target and results
in a compilation failure. macOS ships all required functions in
`math.h`, and clients should use `math.h` instead.

--- a/qtbase/src/3rdparty/libpng/pngpriv.h
+++ b/qtbase/src/3rdparty/libpng/pngpriv.h
@@ -514,18 +514,8 @@
*/
# include <float.h>

-# if (defined(__MWERKS__) && defined(macintosh)) || defined(applec) || \
- defined(THINK_C) || defined(__SC__) || defined(TARGET_OS_MAC)
- /* We need to check that <math.h> hasn't already been included earlier
- * as it seems it doesn't agree with <fp.h>, yet we should really use
- * <fp.h> if possible.
- */
-# if !defined(__MATH_H__) && !defined(__MATH_H) && !defined(__cmath__)
-# include <fp.h>
-# endif
-# else
-# include <math.h>
-# endif
+# include <math.h>
+
# if defined(_AMIGA) && defined(__SASC) && defined(_M68881)
/* Amiga SAS/C: We must include builtin FPU functions when compiling using
* MATH=68881