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

build: use native clang as darwin compiler #618

Merged
merged 57 commits into from
Aug 19, 2024
Merged

Conversation

DeckerSU
Copy link

@DeckerSU DeckerSU commented Mar 8, 2024

This pull request switches the compiler for darwin from gcc to native clang. However, this branch is based on the patch-remove-libsnark branch. Therefore, if we decide to accept it, we should first accept #613 and then merge this pull request.

@DeckerSU
Copy link
Author

DeckerSU commented Mar 8, 2024

Seems we have similar issue boostorg/lexical_cast#49 here.

2024-03-08T22:22:53.7077450Z ./boost/math/special_functions/sign.hpp:30:22: error: reference to unresolved using declaration
2024-03-08T22:22:53.7081790Z         return (std::signbit)(x) ? 1 : 0;
...
2024-03-08T22:22:53.7120050Z ./boost/math/special_functions/fpclassify.hpp:155:17: error: reference to unresolved using declaration
2024-03-08T22:22:53.7122730Z    return (std::fpclassify)(t);
...
2024-03-08T22:22:53.7226390Z ./boost/math/special_functions/fpclassify.hpp:456:22: error: reference to unresolved using declaration
2024-03-08T22:22:53.7229940Z         return (std::isinf)(x);

Additionally, this issue does not occur with Xcode 13.2.1. Still investigating the issue ...

p.s. Possible solutions:

  • Consider install Xcode 13.2.1 in the current build environment. Alternatively, switch the runner back to macOS 11, where 13.2.1 is the default version.
  • Keep using gcc@9 as the default compiler for Darwin.
  • Consider upgrading the Boost version.

Before proceeding, we need to decide on the specific approach we will take.

@TheComputerGenie
Copy link

Additionally, this issue does not occur with Xcode 13.2.1.

Would the logical path forward not be to update the min version of Xcode to something newer than 4 years old and the SDK to something newer than 2019? (Xcode 11.3.1 release date: 13 Jan 2020 -- Catalina [10.15.1] End of Life: 12 Sep 2022)

Why is time and resources being allocated to making the code older than even the OS companies support? Is there some logical reason that we observers are missing that justifies not making things for the future rather than so old that no one supports them?

@DeckerSU
Copy link
Author

DeckerSU commented Mar 9, 2024

Would the logical path forward not be to update the min version of Xcode to something newer than 4 years old and the SDK to something newer than 2019?

Thank you for your concern, but we are already using fresh version of XCode in our Mac CI.

runs-on: macos-latest

As you can see, we are using the macos-latest image for the runner, which includes XCode 14.2 by default.

However, this behavior may change because we are creating static binaries that are intended to work on most versions of macOS, including macOS Monterey (version 12).

@TheComputerGenie
Copy link

TheComputerGenie commented Mar 10, 2024

this might help to see what packages are being dealt with on the runners:
macos-11 installed-software (again, I note that 11 is depreciated)
macos-13 installed-software

@DeckerSU
Copy link
Author

And ... this is indeed funny, in last commit I added the following to the build script:

# logging debug info
sw_vers
xcodebuild -version
ld -v
xxd -l 16 -g 1 ./src/komodod || true
xxd -l 16 -g 1 ./src/test-linkage || true

To view the executable file headers right after build. And you know what i saw in logs:

ProductName:	macOS
ProductVersion:	12.7.3
BuildVersion:	21H1015
Xcode 14.2
Build version 14C18
@(#)PROGRAM:ld  PROJECT:ld64-820.1
BUILD 20:07:01 Nov  7 2022
configured to support archs: armv6 armv7 armv7s arm64 arm64e arm64_32 i386 x86_64 x86_64h armv6m armv7k armv7m armv7em
LTO support using: LLVM version 14.0.0, (clang-1400.0.29.202) (static support for 29, runtime is 29)
TAPI support using: Apple TAPI version 14.0.0 (tapi-1400.0.11)
00000000: cf fa ed fe 07 00 00 01 03 00 00 00 02 00 00 00  ................
00000000: cf fa ed fe 07 00 00 01 03 00 00 00 02 00 00 00  ...............

It means that right after build komodod is valid (!) Mach-O 64-bit executable x86_64. So, it damages somewhere later ...

@DeckerSU
Copy link
Author

this might help to see what packages are being dealt with on the runners

I appreciate your assistance, but the problem does not lie in the software versions. It appears that the binaries are building fine on the runner. The issue with the zeroized komodod binary was actually hidden elsewhere, possibly in the archiving or uploading of artifacts. However, on the runner host, the binaries were correct. I am still investigating, but I am very close to finding the solution.

@DeckerSU
Copy link
Author

DeckerSU commented Mar 10, 2024

Solved! I spent a significant amount of time on this.

⚠️ The root cause of the issue was quite simple - actions/runner-images#2619. It appears that the tar on the MacOS runner was corrupting files during the creation of archive. Please be cautious and avoid using the tar in your workflows on Mac.

Image have 2 versions of tar:

  • bsdtar 3.5.1 - available by 'tar' alias
  • GNU Tar 1.35 - available by 'gtar' alias

The first one breaking the files.

@DeckerSU
Copy link
Author

After utilizing GNU Tar 1.35 (gtar) for creating resulting .tar.gz, all the binaries are Ok. Will clean-up the PR soon from debug/test things.

@TheComputerGenie
Copy link

TheComputerGenie commented Mar 10, 2024

given that upload-artifact has its own compression mechanism, what's the reasoning for doing any of that rather than utilizing the functions of the action already in use? (ie, why tar it at all?)

The runner will assemble the zip archive in memory, streaming in files as part of the upload specification and chunked uploads to blob storage. This allows us to better calculate the file size, as well as compute a checksum of the content. And since this is a singular file upload, it saves precious time on all the network round trips that previously had to be made for every single file. We also have additional inputs like compression-level that can be tweaked to further increase upload speed (or artifact size) depending on how well the content can be compressed.

@DeckerSU
Copy link
Author

ie, why tar it at all?

Ask the person who initially wrote this workflow 😄 I will not modify this behavior since other parts of the workflow rely on actions/download-artifact@v1, which requires the komodo-macos.tar.gz archive.

@TheComputerGenie
Copy link

TheComputerGenie commented Mar 10, 2024

ie, why tar it at all?

Ask the person who initially wrote this workflow 😄 I will not modify this behavior since other parts of the workflow rely on actions/download-artifact@v1, which requires the komodo-macos.tar.gz archive.

I guess that would beg the question: Why are we still using actions/download-artifact@v1, which is a 2019 version and not even needed?
I understand the notion of not wanting to make too many broad-sweeping changes under one heading because it makes what got changed when, where, and why too difficult to follow in many cases; however, the reality remains that when touching a file there's a logic to updating that file while it's in one's hands, no?

I guess the tl;dr of my questions is:
Why make things "just work good enough" instead of making them work the best they can?

It's kind of like seeing someone bash their head against a wall trying to break through it, asking them "why aren't you using the door", and their reply being "Well, I already started this, I might as well continue doing it".

@smk762
Copy link

smk762 commented Aug 16, 2024

Following the README.md in this branch, I am unable to build the deamon locally.

Device/OS: MacMini / MacOS Sonoma 14.6.1

Continuing with the README, build fails with error below:

...failed gcc.compile.c++ bin.v2/libs/thread/build/gcc-15.0.0/release/link-static/threadapi-pthread/threading-multi/visibility-hidden/pthread/thread.o...
...skipped <pbin.v2/libs/thread/build/gcc-15.0.0/release/link-static/threadapi-pthread/threading-multi/visibility-hidden>libboost_thread.a(clean) for lack of <pbin.v2/libs/thread/build/gcc-15.0.0/release/link-static/threadapi-pthread/threading-multi/visibility-hidden>pthread/thread.o...
...skipped <pbin.v2/libs/thread/build/gcc-15.0.0/release/link-static/threadapi-pthread/threading-multi/visibility-hidden>libboost_thread.a for lack of <pbin.v2/libs/thread/build/gcc-15.0.0/release/link-static/threadapi-pthread/threading-multi/visibility-hidden>pthread/thread.o...
...skipped <p/Users/smk762/komodo/depends/work/build/x86_64-apple-darwin23.4.0/boost/1_72_0-85f55273efd/stage/lib>libboost_thread.a for lack of <pbin.v2/libs/thread/build/gcc-15.0.0/release/link-static/threadapi-pthread/threading-multi/visibility-hidden>libboost_thread.a...
...skipped <pbin.v2/libs/thread/build/gcc-15.0.0/release/link-static/threadapi-pthread/threading-multi/visibility-hidden>libboost_thread-variant-static.cmake for lack of <pbin.v2/libs/thread/build/gcc-15.0.0/release/link-static/threadapi-pthread/threading-multi/visibility-hidden>libboost_thread.a...
...skipped <p/Users/smk762/komodo/depends/work/build/x86_64-apple-darwin23.4.0/boost/1_72_0-85f55273efd/stage/lib/cmake/boost_thread-1.72.0>libboost_thread-variant-static.cmake for lack of <pbin.v2/libs/thread/build/gcc-15.0.0/release/link-static/threadapi-pthread/threading-multi/visibility-hidden>libboost_thread-variant-static.cmake...
...failed updating 1 target...
...skipped 5 targets...
...updated 188 targets...
make: *** [/Users/smk762/komodo/depends/work/build/x86_64-apple-darwin23.4.0/boost/1_72_0-85f55273efd/./.stamp_built] Error 1

@DeckerSU
Copy link
Author

Following the README.md in this branch, I am unable to build the deamon locally.

Although your report doesn't contain the exact error that prevents Boost from building on your system, it's clear that you can't build this branch on Sonoma. This branch was created before Sonoma's release on September 26, 2023, so it's expected that it might not compile with the latest macOS. However, the successor of this branch, patch-s8-prepare, includes several fixes that should help with building the daemon on Sonoma. Essentially, patch-s8-prepare is patch-macos-build-clang plus macOS build fixes and other updates. Therefore, I suggest trying to build patch-s8-prepare on your Mac instead. Additionally, patch-s8-prepare now successfully passes the CI build. We should consider patch-macos-build-clang as part of patch-s8-prepare rather than as a separate branch to build.

@ca333 ca333 changed the base branch from patch-remove-libsnark to dev August 19, 2024 07:59
@ca333 ca333 merged commit 6697cef into dev Aug 19, 2024
18 checks passed
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.

5 participants