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

targets: Use OS=linux when GOOS=android #762

Merged
merged 1 commit into from
Oct 10, 2018

Conversation

zachriggle
Copy link
Contributor

@zachriggle zachriggle commented Oct 9, 2018

This avoids the issue of "android" not having any registered configurations
or syscalls / ioctls / etc, when built with GOOS=android.

This occurs when building in Google3, since --config=android_arm64 selects
the Android toolchain.

This supercedes #758 which was much more complex than it needed to be.

Per the feedback there, I've used GetTarget as the main hook point, and removed many of the changes which were only needed due to unnecessary changes in targets.go.

This avoids the issue of "android" not having any registered configurations
or syscalls / ioctls / etc, when built with GOOS=android.

This occurs when building in Google3, since --config=android_arm64 selects
the Android toolchain.
@zachriggle
Copy link
Contributor Author

zachriggle commented Oct 9, 2018

As to some of the concerns on #758 which were not addressed directly there:

  • The _linux suffix is used for GOOS=android builds, so those files are fine
  • There are many places that use runtime.GOOS directly, but most of it is fine (e.g. error messages) and all of the other cases go through GetTarget which should behave correctly
    • The specifically enumerated casese of ipc_test.go and host_linux.go do this, for example. Same for execprog.go.
    • Other cases that are host-side-only (e.g. syz-manager) will have GOOS=linux already, which is fine

@dvyukov
Copy link
Collaborator

dvyukov commented Oct 10, 2018

Very nice!
Seems you found the right spot for the change.

The _linux suffix is used for GOOS=android builds, so those files are fine

Good, so it's not a problem

@dvyukov dvyukov merged commit f37861d into google:master Oct 10, 2018
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