From 841cf455e280c2e4cbe6778e55388cd59588f1d2 Mon Sep 17 00:00:00 2001 From: Lukacs Berki Date: Mon, 20 Jul 2015 09:24:12 +0000 Subject: Make --android_crosstool_top default to the android_ndk_repository specified in the WORKSPACE file. The error reporting if an android_ndk_repository rule is present is not very user-friendly (it just uses the non-Android toolchain, resulting in compile errors) but given that --android_crosstool_top is an interim solution until we get reasonable multi-platform support, I suppose it's fine. As a side effect, instead of prefixing fat APK output directories with "fat-apk-", we prefix Android output directories with "android-". This makes it possible to build Android apps with zero command line options. Rejoice! -- MOS_MIGRATED_REVID=98624120 --- .../com/google/devtools/build/lib/Constants.java | 3 ++ .../rules/android/AndroidNdkRepositoryRule.java | 20 +++++++ .../rules/android/AndroidSdkRepositoryRule.java | 2 +- .../lib/bazel/rules/android/android.WORKSPACE | 1 + .../lib/rules/android/AndroidConfiguration.java | 63 ++++++++++++++++++++-- .../lib/rules/android/AndroidRuleClasses.java | 19 +++++-- .../lib/rules/objc/ObjcCommandLineOptions.java | 4 +- 7 files changed, 102 insertions(+), 10 deletions(-) (limited to 'src/main/java/com/google/devtools/build/lib') diff --git a/src/main/java/com/google/devtools/build/lib/Constants.java b/src/main/java/com/google/devtools/build/lib/Constants.java index edcdea3aec..b178b826a7 100644 --- a/src/main/java/com/google/devtools/build/lib/Constants.java +++ b/src/main/java/com/google/devtools/build/lib/Constants.java @@ -19,6 +19,8 @@ import com.google.common.collect.ImmutableSet; /** * Various constants required by Bazel. + *

The extra {@code .toString()} calls are there so that javac doesn't inline these constants + * so that we can replace this class file within the Bazel binary. */ public class Constants { private Constants() { @@ -81,5 +83,6 @@ public class Constants { public static final ImmutableSet IOS_DEVICE_RULE_CLASSES = ImmutableSet.of("ios_device"); public static final String ANDROID_DEFAULT_SDK = "//external:android/sdk".toString(); + public static final String ANDROID_DEFAULT_CROSSTOOL = "//external:android/crosstool".toString(); public static final String ANDROID_DEP_PREFIX = "//external:android/".toString(); } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryRule.java index c939241a2a..73b1091a84 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryRule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryRule.java @@ -17,13 +17,21 @@ import static com.google.devtools.build.lib.packages.Attribute.attr; import static com.google.devtools.build.lib.packages.Type.INTEGER; import static com.google.devtools.build.lib.packages.Type.STRING; +import com.google.common.base.Function; +import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.analysis.RuleDefinition; import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment; import com.google.devtools.build.lib.bazel.rules.workspace.WorkspaceBaseRule; import com.google.devtools.build.lib.bazel.rules.workspace.WorkspaceConfiguredTargetFactory; +import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.packages.RuleClass.Builder; import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType; +import com.google.devtools.build.lib.syntax.Label; + +import java.util.Map; + +import javax.annotation.Nullable; /** * Definition of the {@code android_ndk_repository} rule. @@ -31,11 +39,23 @@ import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType; public class AndroidNdkRepositoryRule implements RuleDefinition { public static final String NAME = "android_ndk_repository"; + private static final Function> BINDINGS_FUNCTION = + new Function< Rule, Map>() { + @Nullable + @Override + public Map apply(Rule rule) { + return ImmutableMap.of( + "android/crosstool", + Label.parseAbsoluteUnchecked("@" + rule.getName() + "//:toolchain")); + } + }; + @Override public RuleClass build(Builder builder, RuleDefinitionEnvironment environment) { return builder .setUndocumented() .setWorkspaceOnly() + .setExternalBindingsFunction(BINDINGS_FUNCTION) .add(attr("path", STRING).mandatory().nonconfigurable("WORKSPACE rule")) .add(attr("api_level", INTEGER).mandatory().nonconfigurable("WORKSPACE rule")) .build(); diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryRule.java index a061728520..5e97f303a0 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryRule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryRule.java @@ -39,7 +39,7 @@ import javax.annotation.Nullable; public class AndroidSdkRepositoryRule implements RuleDefinition { public static final String NAME = "android_sdk_repository"; - public static final Function> BINDINGS_FUNCTION = + private static final Function> BINDINGS_FUNCTION = new Function< Rule, Map>() { @Nullable @Override diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/android.WORKSPACE b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/android.WORKSPACE index ccc6a56e1e..e59df8ea64 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/android.WORKSPACE +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/android.WORKSPACE @@ -12,3 +12,4 @@ bind(name = "android/aar_generator", actual = "//tools/android:aar_generator") bind(name = "android/shuffle_jars", actual = "//tools/android:shuffle_jars") bind(name = "android/merge_dexzips", actual = "//tools/android:merge_dexzips") bind(name = "android/sdk") +bind(name = "android/crosstool", actual = "//tools/cpp:toolchain") diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java index ee55622edb..0383f03191 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java @@ -31,6 +31,7 @@ import com.google.devtools.build.lib.analysis.config.InvalidConfigurationExcepti import com.google.devtools.build.lib.packages.Attribute.SplitTransition; import com.google.devtools.build.lib.syntax.Label; import com.google.devtools.common.options.Converters; +import com.google.devtools.common.options.EnumConverter; import com.google.devtools.common.options.Option; import java.util.List; @@ -39,10 +40,49 @@ import java.util.List; * Configuration fragment for Android rules. */ public class AndroidConfiguration extends BuildConfiguration.Fragment { + /** + * Value used to avoid multiple configurations from conflicting. + * + *

This is set to {@code ANDROID} in Android configurations and to {@code MAIN} otherwise. This + * influences the output directory name: if it didn't, an Android and a non-Android configuration + * would conflict if they had the same toolchain identifier. + * + *

Note that this is not just a theoretical concern: even if {@code --crosstool_top} and + * {@code --android_crosstool_top} point to different labels, they may end up being redirected to + * the same thing, and this is exactly what happens on OSX X. + */ + public enum ConfigurationDistinguisher { + MAIN(null), + ANDROID("android"); + + private final String suffix; + + private ConfigurationDistinguisher(String suffix) { + this.suffix = suffix; + }; + } + + /** + * Converter for {@link com.google.devtools.build.lib.rules.android.AndroidConfiguration.ConfigurationDistinguisher} + */ + public static final class ConfigurationDistinguisherConverter + extends EnumConverter { + public ConfigurationDistinguisherConverter() { + super(ConfigurationDistinguisher.class, "Android configuration distinguisher"); + } + } + /** * Android configuration options. */ public static class Options extends FragmentOptions { + // Spaces make it impossible to specify this on the command line + @Option(name = "Android configuration distinguisher", + defaultValue = "MAIN", + converter = ConfigurationDistinguisherConverter.class, + category = "undocumented") + public ConfigurationDistinguisher configurationDistinguisher; + @Option(name = "android_crosstool_top", defaultValue = "null", category = "semantics", @@ -120,8 +160,8 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { labelMap.put("android_proguard", proguard); } - if (androidCrosstoolTop != null) { - labelMap.put("android_crosstool_top", androidCrosstoolTop); + if (realAndroidCrosstoolTop() != null) { + labelMap.put("android_crosstool_top", realAndroidCrosstoolTop()); } labelMap.put("android_sdk", realSdk()); @@ -136,6 +176,21 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { : sdk; } + // This method is here because Constants.ANDROID_DEFAULT_CROSSTOOL cannot be a constant, because + // we replace the class file in the .jar after compilation. However, that means that we cannot + // use it as an attribute value in an annotation. + public Label realAndroidCrosstoolTop() { + if (androidCrosstoolTop != null) { + return androidCrosstoolTop; + } + + if (Constants.ANDROID_DEFAULT_CROSSTOOL.equals("null")) { + return null; + } + + return Label.parseAbsoluteUnchecked(Constants.ANDROID_DEFAULT_CROSSTOOL); + } + @Override public ImmutableList getDefaultsRules() { return ImmutableList.of("android_tools_defaults_jar(name = 'android_jar')"); @@ -173,6 +228,7 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { private final boolean legacyNativeSupport; private final String cpu; private final boolean fatApk; + private final ConfigurationDistinguisher configurationDistinguisher; private final Label proguard; private final boolean useJackForDexing; private final boolean jackSanityChecks; @@ -183,6 +239,7 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { this.legacyNativeSupport = options.legacyNativeSupport; this.cpu = options.cpu; this.fatApk = !options.fatApkCpus.isEmpty(); + this.configurationDistinguisher = options.configurationDistinguisher; this.proguard = options.proguard; this.useJackForDexing = options.useJackForDexing; this.jackSanityChecks = options.jackSanityChecks; @@ -230,7 +287,7 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { @Override public String getOutputDirectoryName() { - return fatApk ? "fat-apk" : null; + return configurationDistinguisher.suffix; } public Label getProguardLabel() { diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java index 269a46cf4b..0f576bab21 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java @@ -45,6 +45,7 @@ import com.google.devtools.build.lib.packages.RuleClass.Builder; import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType; import com.google.devtools.build.lib.packages.TriState; import com.google.devtools.build.lib.packages.Type; +import com.google.devtools.build.lib.rules.android.AndroidConfiguration.ConfigurationDistinguisher; import com.google.devtools.build.lib.rules.cpp.CppOptions; import com.google.devtools.build.lib.rules.java.JavaCompilationArgsProvider; import com.google.devtools.build.lib.rules.java.JavaSemantics; @@ -178,21 +179,31 @@ public final class AndroidRuleClasses { } private void setCrosstoolToAndroid(BuildOptions output, BuildOptions input) { - AndroidConfiguration.Options androidOptions = input.get(AndroidConfiguration.Options.class); + AndroidConfiguration.Options inputAndroidOptions = + input.get(AndroidConfiguration.Options.class); + AndroidConfiguration.Options outputAndroidOptions = + output.get(AndroidConfiguration.Options.class); + CppOptions cppOptions = output.get(CppOptions.class); - if (androidOptions.androidCrosstoolTop != null) { + if (inputAndroidOptions.realAndroidCrosstoolTop() != null + && !cppOptions.crosstoolTop.equals(inputAndroidOptions.realAndroidCrosstoolTop())) { if (cppOptions.hostCrosstoolTop == null) { cppOptions.hostCrosstoolTop = cppOptions.crosstoolTop; } - cppOptions.crosstoolTop = androidOptions.androidCrosstoolTop; + cppOptions.crosstoolTop = inputAndroidOptions.realAndroidCrosstoolTop(); } + + outputAndroidOptions.configurationDistinguisher = ConfigurationDistinguisher.ANDROID; } @Override public List split(BuildOptions buildOptions) { AndroidConfiguration.Options androidOptions = buildOptions.get(AndroidConfiguration.Options.class); - if (androidOptions.fatApkCpus.isEmpty() && androidOptions.androidCrosstoolTop == null) { + CppOptions cppOptions = buildOptions.get(CppOptions.class); + Label androidCrosstoolTop = androidOptions.realAndroidCrosstoolTop(); + if (androidOptions.fatApkCpus.isEmpty() + && (androidCrosstoolTop == null || androidCrosstoolTop.equals(cppOptions.crosstoolTop))) { return ImmutableList.of(); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommandLineOptions.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommandLineOptions.java index 0a927afb15..9f405e7b53 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommandLineOptions.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommandLineOptions.java @@ -151,7 +151,7 @@ public class ObjcCommandLineOptions extends FragmentOptions { // transitions for this purpose. // TODO(bazel-team): Remove this once we have dynamic configurations but make sure that different // configurations (e.g. by min os version) always use different output paths. - @Option(name = "DO_NOT_USE_configuration_distinguisher", + @Option(name = "iOS configuration distinguisher", defaultValue = "UNKNOWN", converter = ConfigurationDistinguisherConverter.class, category = "undocumented") @@ -189,7 +189,7 @@ public class ObjcCommandLineOptions extends FragmentOptions { public static final class ConfigurationDistinguisherConverter extends EnumConverter { public ConfigurationDistinguisherConverter() { - super(ConfigurationDistinguisher.class, "configuration distinguisher"); + super(ConfigurationDistinguisher.class, "Objective C configuration distinguisher"); } } } -- cgit v1.2.3