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

Support of macOS arm64 #11

Merged
merged 2 commits into from
Nov 23, 2020
Merged

Support of macOS arm64 #11

merged 2 commits into from
Nov 23, 2020

Conversation

catap
Copy link
Contributor

@catap catap commented Nov 23, 2020

This PR introduced a support of macOS arm64, and open a door to add support another non x86_64 platforms.

It contains two commits:

  1. Switched from hardcoded arch to dynamic one,
  2. Added precompiled interface for macOS 64.

A .dylib looks like:

catap@Kirills-mini-m1 ipcsocket % file src/main/resources/darwin/*/*
src/main/resources/darwin/arm64/libsbtipcsocket.dylib:  Mach-O 64-bit dynamically linked shared library arm64
src/main/resources/darwin/x86_64/libsbtipcsocket.dylib: Mach-O 64-bit dynamically linked shared library x86_64
catap@Kirills-mini-m1 ipcsocket % 

Also it fixes #10 and covers sbt/sbt#6162

@eatkins
Copy link
Contributor

eatkins commented Nov 23, 2020

This is great. I knew that the apple silicon was likely to cause problems for us. Thanks for building the binary.

I don't think the logic for selecting the arch works on all platforms, which is why CI failed. That should be straightforward to fix.

Were you actually able to get sbt to startup after this change on your computer? Naively I wouldn't expect this to be enough to get sbt working on apple silicon because the server uses the JNA implementation of ipcsocket by default. We could change the default to be JNI though. I'd love to get this released soon so that we could get a nightly version of sbt out that uses jni by default that you could test on your machine.

@catap
Copy link
Contributor Author

catap commented Nov 23, 2020

@eatkins I made one more attempt. Honestly the first push was to see how CI is failed.

Now it should be better. Much better.

And yes, feel free to ping me as soon as you need my assisten to test night build on real machine.

@eatkins
Copy link
Contributor

eatkins commented Nov 23, 2020

Would you mind pasting the command that you ran to build the binary?

@catap
Copy link
Contributor Author

catap commented Nov 23, 2020

@eatkins

gcc -arch arm64 -o /Users/catap/src/ipcsocket/src/main/resources/darwin/arm64/libsbtipcsocket.dylib -I/Library/Java/JavaVirtualMachines/openjdk8/Contents/Home/include -I/Library/Java/JavaVirtualMachines/openjdk8/Contents/Home/include/darwin -shared -O2 -Wall -Wextra /Users/catap/src/ipcsocket/jni/org_scalasbt_ipcsocket_JNIUnixDomainSocketLibraryProvider.c

I run sbt -debug buildDarwin on mac on intel, get a command and adjust it.

@eatkins
Copy link
Contributor

eatkins commented Nov 23, 2020

Perfect. Thanks.

It is funny but `System.getProperty("os.arch")` returns `aarch64`.
@catap
Copy link
Contributor Author

catap commented Nov 23, 2020

Renamed arm64 to aarch64 because it is the right name for the system.

@eatkins
Copy link
Contributor

eatkins commented Nov 23, 2020

see sbt/sbt#6165

@eed3si9n eed3si9n merged commit e92bc13 into sbt:master Nov 23, 2020
@catap catap deleted the macos-arm64 branch November 23, 2020 21:22
@eatkins
Copy link
Contributor

eatkins commented Nov 24, 2020

We may want to revert this in favor of creating a universal binary: https://developer.apple.com/documentation/xcode/building_a_universal_macos_binary.

@catap
Copy link
Contributor Author

catap commented Nov 24, 2020

@eatkins but universal binary exists only at macos :)

@eatkins
Copy link
Contributor

eatkins commented Nov 24, 2020

Right, but at the moment we don't care about other platforms for JNI. We only use JNI for platforms that can't support JNA for whatever reason.

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.

Doesn't work at Apple's arm64
3 participants