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

[SNAP FORK][UPSTREAMED] remove linux constraint #59

Open
wants to merge 1 commit into
base: pre-alpha
Choose a base branch
from

Conversation

mauriciogg
Copy link
Contributor

Remove the linux constrains in order to make sdk compatible with macos

Remove the linux constrains in order to make sdk compatible with macos
@@ -18,7 +18,6 @@ toolchain(
name = "android_sdk_tools",
exec_compatible_with = [
"@platforms//cpu:x86_64",
"@platforms//os:linux",
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi Mauricio,

I'm assuming the answer if yes, but have you verified this works on x86 + Mac on your end? We don't have non-Linux x86 CI jobs set up yet for rules_android, so we have no way of guarding against regressions for the time being.

Copy link
Contributor

@nkoroste nkoroste Apr 19, 2023

Choose a reason for hiding this comment

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

Yea, we're using this on Mac x86, Mac M1 and Mac M2. I will let Mauricio to confirm though

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah this works for us

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool! TBH I am a little surprised that this actually just works, since the various SDK binaries differ between the Linux and Mac versions, but maybe the toolchain selection for the individual tools just occurs somewhere else.

Copy link
Contributor

@ted-xie ted-xie left a comment

Choose a reason for hiding this comment

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

I'm getting a merge conflict when attempting to import this internally. Would you mind rebasing?

@ted-xie
Copy link
Contributor

ted-xie commented Apr 19, 2023

Actuallty, it looks like this was already superceded #60. @mauriciogg can you confirm that #60 accomplishes the same thing that this PR set to do? If yes, we can close this PR as obsolete.

ted-xie pushed a commit to ted-xie/rules_android that referenced this pull request Jul 16, 2024
Bazel 7.0 still contains the native version of android_ndk_repository
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.

3 participants