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

Defining android_ndk_repository() will prevent non-Android targets from building if ANDROID_NDK_HOME is unset #2722

Closed
steven-johnson opened this issue Mar 21, 2017 · 11 comments
Assignees
Labels
P2 We'll consider working on this in future. (Assignee optional) type: bug

Comments

@steven-johnson
Copy link
Contributor

Using Bazel 0.4.5 on OSX.

Create trivial build environment like so:

WORKSPACE:

workspace(name = "ndktest")
android_sdk_repository(name = "androidsdk")
android_ndk_repository(name = "androidndk")

BUILD:

cc_binary(
    name = "ndktest",
    srcs = [ "main.cc" ],
)

main.cc:

#include <stdio.h>
int main() {
  printf("Hello, world\n");
}

Ensure that both ANDROID_HOME and ANDROID_NDK_HOME are unset, then bazel build //:ndktest

  • Expected:

since the target doesn't use the Android SDK or the Android NDK, should build fine even though the env vars aren't set.

  • Actual:

ERROR: in target '//external:android/crosstool': no such package '@androidndk//': Either the path attribute of android_ndk_repository or the ANDROID_NDK_HOME environment variable must be set.

@aj-michael
Copy link
Contributor

Hmm, this seems specific to android_ndk_repository. android_sdk_repository does not have this problem.

@aj-michael aj-michael added the P2 We'll consider working on this in future. (Assignee optional) label Mar 21, 2017
@aj-michael
Copy link
Contributor

Hmm, I think I've narrowed it down to something in the AndroidSplitTransition. Adding @ahumesky for expertise on that.

https://github.com/bazelbuild/bazel/blob/master/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java#L172

@aj-michael
Copy link
Contributor

+ @gregestren

Results of some digging:

So, the AndroidConfiguration is loaded for all builds (including your cc_binary). The android_crosstool_top option in the AndroidConfiguration defaults to //external:android/crosstool. Which by default is bound to @bazel_tools//tools/cpp:toolchain. However, the android_ndk_repository rule external bindings function also declares that it binds //external:android/crosstool. We solve this problem with the android_sdk option by introducing an intermediary alias @bazel_tools//tools/android:sdk which points to //external:android/sdk. However, doing that is not enough for android_crosstool_top because of the AndroidSplitTransition that is evaluated unconditionally at start up. Somehow, this is "looking through" the alias that android_crosstool_top defaults to. Dynamic configurations may be able to prevent the AndroidSplitTransition from being evaluated unconditionally.

@steven-johnson
Copy link
Contributor Author

Not sure how to interpret this response. Is there any workaround for 0.4.5?

@aj-michael
Copy link
Contributor

Sorry, that response was more for myself and other Bazel developers than for you.

Unfortunately I don't think there is a workaround for 0.4.5.

@ahumesky
Copy link
Contributor

Yeah bazel is trying too hard to get the ndk to work with the build, sorry for the inconvenience here. You can work around this by passing --android_crosstool_top= (that is, clearing the flag) when you build the non-android target.

@steven-johnson
Copy link
Contributor Author

Just got around to trying this out in 0.5rc6 -- still unfixed. Probably too late to get a fix for this for 0.5? :-(

@aj-michael
Copy link
Contributor

I don't have any updates on this. This is an unfortunate side-effect of Bazel's static configuration system, which has no clear fix other than the upcoming dynamic configurations being worked on by @gregestren.

As a recap (so that future me doesn't forget the problem), this issue is that static configurations create all possible configurations, including the AndroidSplitTransition even if your build will not actually use those configurations. The AndroidSplitTransition copies the android_crosstool_top into crosstool_top, then CppConfigurationLoader comes along and calls RedirectChaser.followRedirects on crosstool_top, because it reads the crosstool to set other parts of the configuration. This errors out because android_crosstool_top redirects to nothing.

The long term solution is the dynamic configurations system replacing the current static configuration system, which will allow us to stop creating this configuration for builds (like cc_binary) that don't need it.

I'm assigning this to @gregestren who is working on this, however I don't expect the solution to come in the short term.

@aj-michael aj-michael assigned gregestren and unassigned aj-michael and ahumesky May 22, 2017
@robertkarl
Copy link

Is this still a problem as of Bazel 0.7.0? I couldn't repro on Linux.

rk@RK-Linux ~/bazelbug $ echo $ANDROID_NDK_HOME

rk@RK-Linux ~/bazelbug $ echo $ANDROID_HOME

rk@RK-Linux ~/bazelbug $ bazel build //:ndktest
INFO: Analysed target //:ndktest (0 packages loaded).
INFO: Found 1 target...
Target //:ndktest up-to-date:
  bazel-bin/ndktest
INFO: Elapsed time: 0.211s, Critical Path: 0.00s
INFO: Build completed successfully, 1 total action
rk@RK-Linux ~/bazelbug $ 

@gregestren
Copy link
Contributor

gregestren commented Nov 29, 2017

The "dynamic configuration" upgrades Adam refers to - about not unconditionally calling the AndroidSplitTransition for non-Android builds - should be fully in effect as of 0.6.0 (i.e. once d63ee81 went in).

So yeah, I'd expect this to be okay now, and I'm happy you got a clean result, Robert.

Steven - do you still see an issue here? (assign back to me if so)

@jin
Copy link
Member

jin commented Jan 9, 2018

Closing this issue since dynamic configurations are in, and I cannot repro the original example in 0.9.0. Feel free to reopen if there's still an issue.

@jin jin closed this as completed Jan 9, 2018
jpsim added a commit to jpsim/envoy-mobile that referenced this issue Feb 2, 2022
Signed-off-by: JP Simard <jp@jpsim.com>
jpsim added a commit to jpsim/envoy-mobile that referenced this issue Feb 2, 2022
Signed-off-by: JP Simard <jp@jpsim.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 We'll consider working on this in future. (Assignee optional) type: bug
Projects
None yet
Development

No branches or pull requests

6 participants