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

Pack arm64 libs into linux package + x86_64/arm64 linux libs into OSX package #3

Open
wants to merge 5 commits into
base: weka-1.30
Choose a base branch
from

Conversation

yanok
Copy link

@yanok yanok commented Jan 17, 2025

No description provided.

@yanok yanok force-pushed the yanok/linux-libs-on-mac branch from 1393c53 to 5024127 Compare January 17, 2025 22:04
@yanok yanok marked this pull request as ready for review January 17, 2025 22:06
@yanok yanok force-pushed the yanok/linux-libs-on-mac branch 2 times, most recently from 97ae6d7 to 15a5f1c Compare January 19, 2025 14:21
@yanok
Copy link
Author

yanok commented Jan 19, 2025

@JohanEngelen PTAL, it seems to be working, CI is failing because there is no osx-arm64-withAsserts version of LLVM pre-packaged.

@yanok yanok force-pushed the yanok/linux-libs-on-mac branch from 15a5f1c to bb66181 Compare January 19, 2025 15:35
yanok added 2 commits January 20, 2025 11:29
And add corresponding config sections.

This enables prebuilt OS X compiler to be useful for local agent build.
@yanok yanok force-pushed the yanok/linux-libs-on-mac branch from bb66181 to 1a4f0b0 Compare January 20, 2025 10:29
@JohanEngelen
Copy link
Collaborator

Only tagged commits with v1.30... work in CI, fixing it in #4

{
switches = [
\"-defaultlib=phobos2-ldc,druntime-ldc\",
\"--mcpu=neoverse-n1\",
Copy link
Collaborator

Choose a reason for hiding this comment

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

is setting the CPU really needed? (seems better to keep the runtime build more general as it probably won't really matter whether it uses newer CPU instructions or not)

Copy link
Author

@yanok yanok Jan 27, 2025

Choose a reason for hiding this comment

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

But that's not for the runtime build, that's the default flags to use then building for ARM64. Since Weka requires these settings and this is a Weka-specific compiler package, I thought it makes sense to add them to the config.

But of course one could argue that we should keep these settings in the Wekapp repo internally.

I'm fine both ways. I think it only matters if we want to try merging these changes upstream. If it's going to stay Weka-specific, seems to better keep the required flags in the config.

@JohanEngelen
Copy link
Collaborator

LGTM. Think about the CPU thing, and hit squash+merge!

You can download the build package here: https://github.com/weka/ldc/actions/runs/12965886228?pr=3, so you can test locally if it's working correctly.

@yanok
Copy link
Author

yanok commented Jan 27, 2025

LGTM. Think about the CPU thing, and hit squash+merge!

You can download the build package here: https://github.com/weka/ldc/actions/runs/12965886228?pr=3, so you can test locally if it's working correctly.

Thanks! I did test cross compiling from arm64 Mac to arm64 linux. That works great. I don't see the merge button, I guess I don't have that permission :)

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.

2 participants