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

Add support for M1 Macbook desktop testing #4665

Closed
MatthewTighe opened this issue Nov 18, 2021 · 17 comments · Fixed by #4792
Closed

Add support for M1 Macbook desktop testing #4665

MatthewTighe opened this issue Nov 18, 2021 · 17 comments · Fixed by #4792
Assignees

Comments

@MatthewTighe
Copy link
Contributor

MatthewTighe commented Nov 18, 2021

Building A-S for Android needs some updates to be out-of-the-box compatible with M1 Macbooks. I'm trying to work through the issues locally, but am new to the project so it's a bit slow going. I'll keep this issue updated with blockers and their solutions as I encounter them:

  1. Android sdkmanager command line tools required by build not compatible with M1. The error I saw was Could not find or load main class com.android.sdklib.tool.sdkmanager.SdkManagerCli, but there may be others. See this SO post.
  • Use Android Studio Canary version to download the required SDK tools. If done correctly, the executable will be found in: Users/<user>/Library/Android/sdk/cmdline-tools/latest/bin/sdkmanager.
  1. Running ./libs/verify-android-environment.sh fails with Cannot get property 'nativeRustTarget' on extra properties extension as it does not exist.
  • Add the following case to the nativeRustTarget switch:
switch (DefaultPlatform.RESOURCE_PREFIX) {
     case 'darwin':
     case 'darwin-x86-64':
+    case 'darwin-aarch64':
         ext.nativeRustTarget = 'darwin'
         break
     case 'linux-x86-64':
  1. Re-running ./libs/verify-android-environment.sh appears to have another error:
xcode-select: error: tool 'xcodebuild' requires Xcode, but active developer directory '/Library/Developer/CommandLineTools' is a command line tools instance

xcode-select: error: tool 'xcodebuild' requires Xcode, but active developer directory '/Library/Developer/CommandLineTools' is a command line tools instance

Traceback (most recent call last):
  File "/usr/local/bin/gyp", line 11, in <module>
    load_entry_point('gyp==0.1', 'console_scripts', 'gyp')()
  File "/Library/Python/2.7/site-packages/gyp-0.1-py2.7.egg/gyp/__init__.py", line 552, in script_main
    return main(sys.argv[1:])
  File "/Library/Python/2.7/site-packages/gyp-0.1-py2.7.egg/gyp/__init__.py", line 545, in main
    return gyp_main(args)
  File "/Library/Python/2.7/site-packages/gyp-0.1-py2.7.egg/gyp/__init__.py", line 530, in gyp_main
    generator.GenerateOutput(flat_list, targets, data, params)
  File "/Library/Python/2.7/site-packages/gyp-0.1-py2.7.egg/gyp/generator/ninja.py", line 2493, in GenerateOutput
    pool.map(CallGenerateOutputForConfig, arglists)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/multiprocessing/pool.py", line 253, in map
    return self.map_async(func, iterable, chunksize).get()
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/multiprocessing/pool.py", line 572, in get
    raise self._value
AttributeError: 'NoneType' object has no attribute 'groups'
  1. Running cargo test causes the following error. This happens with different crates on subsequent runs:

    error: failed to run custom build command for sql-support v0.1.0 (/Users/matthewtighe/dev/application-services/components/support/sql)

    Caused by:
    process didn't exit successfully: /Users/matthewtighe/dev/application-services/target/debug/build/sql-support-22f2ec6dbcbc1fcd/build-script-build (exit status: 101)
    --- stdout
    cargo:rerun-if-changed=build.rs
    cargo:rerun-if-env-changed=NSS_DIR

    --- stderr
    thread 'main' panicked at 'It looks like NSS is not built. Please run libs/verify-[platform]-environment.sh first!', components/support/rc_crypto/nss/nss_build_common/src/lib.rs:55:9
    note: run with RUST_BACKTRACE=1 environment variable to display a backtrace
    warning: build failed, waiting for other jobs to finish...
    error: build failedMy guess now is that nss is being compiled for x86_64 instead of aarch64, but I'm still trying to figure out how to check/change that.

┆Issue is synchronized with this Jira Task
┆Epic: local development, ci and release (backlog)
┆Sprint End Date: 2022-02-03

@skhamis
Copy link
Contributor

skhamis commented Nov 18, 2021

Thanks for the excellent write-up on issues onboarding onto application-services via M1. A couple of things initially:

  1. Were you able to successfully run cargo test for the desktop step? Specifically Step 4 of this https://github.com/mozilla/application-services/blob/main/docs/building.md#building-the-rust-components
  2. Does the Xcode error still show even though you ran these two commands (as shown in the issue Remove extraneous newline in build_ffi.sh #569) xcode-select: error: tool 'xcodebuild' requires Xcode, but active developer directory '/Library/Developer/CommandLineTools' is a command line tools instance nodejs/node-gyp#569
    xcode-select --install # Install Command Line Tools if you haven't already.
    sudo xcode-select --switch /Library/Developer/CommandLineTools # Enable command line tools
    

Do those commands above show that they successfully installed? I think the warning can only be ignored if those actually get installed

  1. Are you using homebrew for your Ninja and gyp install? Specifically step 4 of these steps https://github.com/mozilla/application-services/blob/main/docs/building.md#macos

@MatthewTighe
Copy link
Contributor Author

  1. cargo test fails with the error listed in step 4 of the OP when run from project root. It also fails with that error when run from the components directory.
  2. I did run those two commands. The xcode error still shows.
  3. I used homebrew for my Ninja install, but gyp was installed using the steps listed in the gyp repo linked from step 4

@MatthewTighe
Copy link
Contributor Author

Progress update from today:

Turns out I should follow directions (who knew) and was missing the Xcode install.

I can now successfully run cargo test from project root, but ./gradlew assembleDebug fails with the following error:

> Task :full-megazord:cargoBuildDarwin
   Compiling sql-support v0.1.0 (/Users/matthewtighe/dev/application-services/components/support/sql)
   Compiling nss_sys v0.1.0 (/Users/matthewtighe/dev/application-services/components/support/rc_crypto/nss/nss_sys)
   Compiling jexl-eval v0.1.7
   Compiling sync-guid v0.1.0 (/Users/matthewtighe/dev/application-services/components/support/guid)
   Compiling types v0.1.0 (/Users/matthewtighe/dev/application-services/components/support/types)
   Compiling rkv v0.17.0
   Compiling uniffi v0.14.0
   Compiling places v0.1.0 (/Users/matthewtighe/dev/application-services/components/places)
   Compiling autofill v0.1.0 (/Users/matthewtighe/dev/application-services/components/autofill)
   Compiling logins v0.1.0 (/Users/matthewtighe/dev/application-services/components/logins)
   Compiling tabs v0.1.0 (/Users/matthewtighe/dev/application-services/components/tabs)
   Compiling crashtest v0.1.0 (/Users/matthewtighe/dev/application-services/components/crashtest)
   Compiling fxa-client v0.1.0 (/Users/matthewtighe/dev/application-services/components/fxa-client)
   Compiling push v0.1.0 (/Users/matthewtighe/dev/application-services/components/push)
   Compiling nimbus-sdk v0.10.0 (/Users/matthewtighe/dev/application-services/components/nimbus)
error: could not find native static library `gcm-aes-x86_c_lib`, perhaps an -L flag is missing?

error: could not compile `nss_sys` due to previous error
warning: build failed, waiting for other jobs to finish...
error: build failed

As a reminder, I've added darwin-aarch64 cases to the relevant switch statements in the top-level build.gradle.

gcm-aes-x86_c_lib seems to have been built and exists at libs/android/x86/nss/lib/libgcm-aes-x86_c_lib.a, so I'm wondering if somehow the target architecture isn't propagating through the toolchain correctly, so that the cargo build is maybe looking within libs/arm64-v8a.

@skhamis
Copy link
Contributor

skhamis commented Nov 20, 2021

Glad you're able to move forward! I think you might want to remove the relevant switches as I think the current iterations build for M1s without that (but we can look into that to see if there is something we're missing). It looks like you need to run the steps to build NSS + other deps (I noticed our steps to make this clear).

  1. Navigate to the libs directory in a-s
  2. Run ./build-all.sh desktop for good measure
  3. Run ./build-all.sh android
    There should be a long build while it downloads and compiles NSS required

Then try run the gradle command again and report back!

@MatthewTighe
Copy link
Contributor Author

Got a successful build! Needed to update this switch with the following case:

     case 'darwin-x86-64':
         ext.nativeRustTarget = 'darwin'
         break
+    case 'darwin-aarch64':
+        ext.nativeRustTarget = 'arm64'
+        break
     case 'linux-x86-64':
         ext.nativeRustTarget = 'linux-x86-64'
         break

It looks like the darwin rust target gets mapped to an x86 architecture. If you're using an M1 as well, is there a chance you'd overwritten the rust target in your local.properties?

Unfortunately, running tests fails with an unsatisfied linker error:

java.lang.UnsatisfiedLinkError: Unable to load library 'megazord':
dlopen(libmegazord.dylib, 9): image not found
dlopen(libmegazord.dylib, 9): image not found
dlopen(/Users/matthewtighe/Library/Frameworks/megazord.framework/megazord, 9): image not found
dlopen(/Library/Frameworks/megazord.framework/megazord, 9): image not found
dlopen(/System/Library/Frameworks/megazord.framework/megazord, 9): image not found
Native library (darwin-aarch64/libmegazord.dylib) not found in resource path (...)
	at com.sun.jna.NativeLibrary.loadLibrary(NativeLibrary.java:301)

@skhamis
Copy link
Contributor

skhamis commented Nov 23, 2021

It looks like the darwin rust target gets mapped to an x86 architecture. If you're using an M1 as well, is there a chance you'd overwritten the rust target in your local.properties?

No my local.properties doesn't have any specific hack for this. Hmmmm I would be a little surprised that the above snippet allowed your build to run sucessfully as we already add arm64 to the rustTargets before any platform-specific switches:
https://github.com/mozilla/application-services/blob/main/build.gradle#L224

However I do see Glean (a project at mozilla we work with heavily) adds aarch64 to the existing darwin cases so there definitely might be something to update.

Regarding the linker error. Do you know if the megazord lib actually exists in ../Library/Frameworks/megazord.framework/megazord ? This will help us know if it's a library generating issue or a JNA issue

Thanks for continuing to push through this! This has been really helpful to identify areas we need to improve on for onboarding.

@MatthewTighe
Copy link
Contributor Author

Switching the case to also set ext.nativeRustTarget = 'darwin' causes the build to fail with that nss error mentioned in this comment. It looks like the ext.nativeRustTarget is used in some other places, in addition to the to ext.rustTargets.

What is ../Library/Frameworks/megazord.framework/megazord relative to? /Library/Frameworks does not contain it, nor does $HOME/Library. Should that be somewhere within the project?

@skhamis
Copy link
Contributor

skhamis commented Nov 25, 2021

After chatting offline with Matt, turns out I can repro this. I had a x86_64 version of the JVM -- switching to an arm-based produces the same errors as above -- Will be investigating further!

@MatthewTighe
Copy link
Contributor Author

Looks like this issue is going to block me from a subset of A-C contributions as well. Trying to run these tests fails with the error at the bottom of this post. Looks like it's potentially the same runtime linking issue we ran into in this post above.

For context, the test suite in that class is instantiating a PlacesApi from A-S.

Caused by: java.lang.UnsatisfiedLinkError: /Users/matthewtighe/Library/Caches/JNA/temp/jna11129591045367077596.tmp: dlopen(/Users/matthewtighe/Library/Caches/JNA/temp/jna11129591045367077596.tmp, 1): no suitable image found.  Did find:
	/Users/matthewtighe/Library/Caches/JNA/temp/jna11129591045367077596.tmp: no matching architecture in universal wrapper
	/Users/matthewtighe/Library/Caches/JNA/temp/jna11129591045367077596.tmp: no matching architecture in universal wrapper
	at java.base/java.lang.ClassLoader$NativeLibrary.load0(Native Method)
	at java.base/java.lang.ClassLoader$NativeLibrary.load(ClassLoader.java:2442)
	at java.base/java.lang.ClassLoader$NativeLibrary.loadLibrary(ClassLoader.java:2498)
	at java.base/java.lang.ClassLoader.loadLibrary0(ClassLoader.java:2694)
	at java.base/java.lang.ClassLoader.loadLibrary(ClassLoader.java:2627)
	at java.base/java.lang.Runtime.load0(Runtime.java:768)
	at java.base/java.lang.System.load(System.java:1837)
	at com.sun.jna.Native.loadNativeDispatchLibraryFromClasspath(Native.java:1018)
	at com.sun.jna.Native.loadNativeDispatchLibrary(Native.java:988)
	at com.sun.jna.Native.<clinit>(Native.java:195)
	at com.sun.jna.Structure.setAlignType(Structure.java:280)
	at com.sun.jna.Structure.<init>(Structure.java:197)
	at com.sun.jna.Structure.<init>(Structure.java:193)
	at com.sun.jna.Structure.<init>(Structure.java:180)
	at com.sun.jna.Structure.<init>(Structure.java:172)
	at mozilla.appservices.places.RustError.<init>(RustError.kt:15)
	at mozilla.appservices.places.RustError$ByReference.<init>(RustError.kt:17)
	at mozilla.appservices.places.PlacesApi.<init>(PlacesConnection.kt:1543)
	at mozilla.components.browser.storage.sync.RustPlacesConnection.init(Connection.kt:79)
	at mozilla.components.browser.storage.sync.PlacesStorage$places$2.invoke(PlacesStorage.kt:48)
	at mozilla.components.browser.storage.sync.PlacesStorage$places$2.invoke(PlacesStorage.kt:47)
	at kotlin.SynchronizedLazyImpl.getValue(LazyJVM.kt:74)
	at mozilla.components.browser.storage.sync.PlacesStorage.getPlaces$browser_storage_sync_debug(PlacesStorage.kt:47)
	at mozilla.components.browser.storage.sync.PlacesHistoryStorage$deleteEverything$2.invokeSuspend(PlacesHistoryStorage.kt:159)
	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
	at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:106)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)

@skhamis
Copy link
Contributor

skhamis commented Nov 30, 2021

@badboy have you run into this issue at all trying to build application-services? I didn't run into it because I had a x86_64 jdk but I think I recall you having an arm64 homebrew jdk

@MatthewTighe
Copy link
Contributor Author

@skhamis helped worked me through this issue offline. This can be circumvented by ensuring that Android Studio (or however you're running your gradle tasks) is using an x86_64 version of Java

@badboy
Copy link
Member

badboy commented Dec 1, 2021

@badboy have you run into this issue at all trying to build application-services? I didn't run into it because I had a x86_64 jdk but I think I recall you having an arm64 homebrew jdk

I've never built the android part of a-s on this machine. I do indeed have a x64 java around.
Glean Kotlin testing I have to run under Rosetta2 because of other issues (some bundled sqlite isn't arm64).

@MatthewTighe That makes sense. How would one verify that?

@MatthewTighe
Copy link
Contributor Author

@badboy You can find and change the JDK version being used by Android Studio by searching "gradle" in the preferences menu, as seen here:

image

The java executable should be under that directory at <jdk path>/bin/java and the architecture can be inspected using the file command.

@MatthewTighe
Copy link
Contributor Author

MatthewTighe commented Dec 1, 2021

Just as a point of clarity: I hit this issue in Android Studio running Android Component tests that use real appservices objects. This means that that any JVM unit test for downstream consumers that is reliant on appservices will fail on M1 macs unless they use the workaround we discovered, and the workaround noticeably slows other Gradle tasks (like building) since it requires the use of Rosetta.

@skhamis
Copy link
Contributor

skhamis commented Dec 1, 2021

I'll need to be switching gears for a little bit but wanted to do a bit of a brain dump:

It seems the issue is relating to the wrong target_arch being computed during the cargoBuildDarwin megazord tasking that is generated here:

def nativeCargoBuildTask = "cargoBuild${rootProject.ext.nativeRustTarget.capitalize()}"

Which gets the nativeRustTarget when we add the aarch64 here:

ext.nativeRustTarget = 'darwin'

This means that there will be a megazord generated for dawin under the task cargoBuildDarwin which is the failing task Matt mentioned above. However it's failing due to not finding the x86 gcm-aes, which means it's hitting this part of the nss build system:

if target_arch == "x86_64" || target_arch == "x86" {
static_libs.push("gcm-aes-x86_c_lib");
}

This means the rust line let target_arch = env::var("CARGO_CFG_TARGET_ARCH").unwrap(); is =x86_64 for Darwin EVEN with an M1 build (thus why an x86 JDK works).

Removing those lines to try and get it to compile fails (expectantly) on the linker part with something like:

error: linking with `cc` failed: exit status: 1
  |
  = note: "cc" "-Wl,-exported_symbols_list,/var/folders/lx/1w8044gs3vg0r42tr6xh6lfc0000gn/T/rustcCQZ7lo/list" "-m64" "-arch" "x86_64" "/{...}/application-services/target/x86_64-apple-darwin/release/deps/megazord.2x5ms041zhxf7m58.rcgu.o" 

Which confirms that the bug is basically when rust is being compiled from gradle, there is not proper support for detecting M1 (Mac/Darwin arm). Haven't gone further than that for now but will investigate further when time permits.

Things I've tried:

  • Upgrading gradle to 7.0
  • Upgrade Rust android gradle version to 0.9
  • Different JDKs 11 that's not the one from homerew (Azul, oracle)

@skhamis
Copy link
Contributor

skhamis commented Dec 2, 2021

Update: It's most likely the rust gradle plugin (by Mozilla!) that might be causing the x86_64 to be built. I think we'll need some kind of patch on their end to support darwin arm64 specifically here: https://github.com/mozilla/rust-android-gradle/blob/master/plugin/src/main/kotlin/com/nishtahir/RustAndroidPlugin.kt#L28-L33

When I get time I'll try to locally test that plugin with some potential changes and see if I'm able to get it building nss correctly

@data-sync-user
Copy link
Collaborator

➤ Janet Dragojevic commented:

This is ready for testing by people with macs (m1 or intel) and should check all the way through to Fenix.

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 a pull request may close this issue.

4 participants