diff options
author | Michael Staib <mstaib@google.com> | 2016-08-24 22:34:37 +0000 |
---|---|---|
committer | John Cater <jcater@google.com> | 2016-08-25 20:16:34 +0000 |
commit | b1ae312f4eb220bc2729f1310dba7cd9b75f8d7f (patch) | |
tree | 4b4f7e375dd18a93d7091afeb6cd243902ac632e /src/main/java/com/google/devtools/build/lib/rules/android | |
parent | 0ace43c7a458dd35d2c32a322c49d8ab0eb4b8d3 (diff) |
Rollback of commit b5ac354867c4d8deb543285e87e636504078fb69.
*** Reason for rollback ***
Roll forward with fixes.
--
MOS_MIGRATED_REVID=131224077
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib/rules/android')
4 files changed, 41 insertions, 76 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java index 42e911842b..daf7d5b086 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java @@ -165,10 +165,13 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { toolchainMap.put(cpu, toolchain); } - NativeLibs nativeLibs = shouldLinkNativeDeps(ruleContext) - ? NativeLibs.fromLinkedNativeDeps(ruleContext, androidSemantics.getNativeDepsFileName(), - depsByArchitecture, toolchainMap, configurationMap) - : NativeLibs.fromPrecompiledObjects(ruleContext, depsByArchitecture); + NativeLibs nativeLibs = + NativeLibs.fromLinkedNativeDeps( + ruleContext, + androidSemantics.getNativeDepsFileName(), + depsByArchitecture, + toolchainMap, + configurationMap); // TODO(bazel-team): Resolve all the different cases of resource handling so this conditional // can go away: recompile from android_resources, and recompile from android_binary attributes. @@ -1667,19 +1670,6 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { } /** - * Returns whether to use NativeDepsHelper to link native dependencies. - */ - public static boolean shouldLinkNativeDeps(RuleContext ruleContext) { - TriState attributeValue = ruleContext.attributes().get( - "legacy_native_support", BuildType.TRISTATE); - if (attributeValue == TriState.AUTO) { - return !ruleContext.getFragment(AndroidConfiguration.class).getLegacyNativeSupport(); - } else { - return attributeValue == TriState.NO; - } - } - - /** * Returns the multidex mode to apply to this target. */ public static MultidexMode getMultidexMode(RuleContext ruleContext) { 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 4b9c95003d..be0360514e 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 @@ -230,13 +230,6 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { help = "Specifies Android SDK/platform that is used to build Android applications.") public Label sdk; - @Option(name = "legacy_android_native_support", - defaultValue = "true", - category = "semantics", - help = "Switches back to old native support for android_binaries. Disable to link together " - + "native deps of android_binaries into a single .so by default.") - public boolean legacyNativeSupport; - // TODO(bazel-team): Maybe merge this with --android_cpu above. @Option(name = "fat_apk_cpu", converter = Converters.CommaSeparatedOptionListConverter.class, @@ -397,7 +390,6 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { private final Label sdk; private final StrictDepsMode strictDeps; - private final boolean legacyNativeSupport; private final String cpu; private final boolean incrementalNativeLibs; private final ConfigurationDistinguisher configurationDistinguisher; @@ -415,7 +407,6 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { this.sdk = androidSdk; this.incrementalNativeLibs = options.incrementalNativeLibs; this.strictDeps = options.strictDeps; - this.legacyNativeSupport = options.legacyNativeSupport; this.cpu = options.cpu; this.configurationDistinguisher = options.configurationDistinguisher; this.useJackForDexing = options.useJackForDexing; @@ -443,10 +434,6 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment { return sdk; } - public boolean getLegacyNativeSupport() { - return legacyNativeSupport; - } - public StrictDepsMode getStrictDeps() { return strictDeps; } 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 ba70ab5092..dc1312662e 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 @@ -694,28 +694,10 @@ com/google/common/base/Objects.class re-used to apply the same mapping to a new build. <!-- #END_BLAZE_RULE.ATTRIBUTE --> */ .add(attr("proguard_apply_mapping", LABEL).legacyAllowAnyFileType()) - /* <!-- #BLAZE_RULE($android_binary_base).ATTRIBUTE(legacy_native_support) --> - Enables legacy native support, where pre-compiled native libraries are copied - directly into the APK. - Possible values: - <ul> - <li><code>legacy_native_support = 1</code>: Pre-built .so files found in the - dependencies of cc_libraries in the transitive closure will be copied into - the APK without being modified in any way. All cc_libraries in the transitive - closure of this rule must wrap .so files. (<em class="harmful">deprecated</em> - - legacy_native_support = 0 will become the default and this attribute will be - removed in a future Blaze release.)</li> - <li><code>legacy_native_support = 0</code>: Native dependencies in the transitive - closure will be linked together into a single lib[ruleName].so - before being placed in the APK. This ensures that, e.g., only one copy of - //base will be loaded into memory. This lib[ruleName].so can be loaded - via System.loadLibrary as normal.</li> - <li><code>legacy_native_support = -1</code>: Linking is controlled by the - <a href="../blaze-user-manual.html#flag--legacy_android_native_support"> - --[no]legacy_android_native_support</a> Blaze flag.</li> - </ul> - <!-- #END_BLAZE_RULE.ATTRIBUTE --> */ - .add(attr("legacy_native_support", TRISTATE).value(TriState.AUTO)) + // TODO(mstaib): Remove this attribute and the matching flag after some cleanup of users + .add(attr("legacy_native_support", TRISTATE) + .value(TriState.AUTO) + .undocumented("No-op, soon to be removed")) .add(attr(":extra_proguard_specs", LABEL_LIST).value(JavaSemantics.EXTRA_PROGUARD_SPECS)) .advertiseProvider(JavaCompilationArgsProvider.class) .build(); diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/NativeLibs.java b/src/main/java/com/google/devtools/build/lib/rules/android/NativeLibs.java index cbffcd8f0e..24d7c22d7e 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/NativeLibs.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/NativeLibs.java @@ -48,18 +48,6 @@ public final class NativeLibs { public static final NativeLibs EMPTY = new NativeLibs(ImmutableMap.<String, Iterable<Artifact>>of(), null); - public static NativeLibs fromPrecompiledObjects( - RuleContext ruleContext, Multimap<String, TransitiveInfoCollection> depsByArchitecture) { - ImmutableMap.Builder<String, Iterable<Artifact>> builder = ImmutableMap.builder(); - for (Map.Entry<String, Collection<TransitiveInfoCollection>> entry : - depsByArchitecture.asMap().entrySet()) { - NestedSet<LinkerInput> nativeLibraries = - AndroidCommon.collectTransitiveNativeLibraries(entry.getValue()); - builder.put(entry.getKey(), filterUniqueSharedLibraries(ruleContext, nativeLibraries)); - } - return new NativeLibs(builder.build(), null); - } - public static NativeLibs fromLinkedNativeDeps( RuleContext ruleContext, String nativeDepsFileName, @@ -68,6 +56,7 @@ public final class NativeLibs { Map<String, BuildConfiguration> configurationMap) throws InterruptedException { Map<String, Iterable<Artifact>> result = new LinkedHashMap<>(); + String nativeDepsLibraryBasename = null; for (Map.Entry<String, Collection<TransitiveInfoCollection>> entry : depsByArchitecture.asMap().entrySet()) { CcLinkParams linkParams = @@ -76,15 +65,20 @@ public final class NativeLibs { ImmutableList.of("-Wl,-soname=lib" + ruleContext.getLabel().getName())) .get(/* linkingStatically */ true, /* linkShared */ true); - Artifact nativeDepsLibrary = NativeDepsHelper.maybeCreateAndroidNativeDepsAction( - ruleContext, linkParams, configurationMap.get(entry.getKey()), - toolchainMap.get(entry.getKey())); + Artifact nativeDepsLibrary = + NativeDepsHelper.linkAndroidNativeDepsIfPresent( + ruleContext, + linkParams, + configurationMap.get(entry.getKey()), + toolchainMap.get(entry.getKey())); ImmutableList.Builder<Artifact> librariesBuilder = ImmutableList.builder(); - librariesBuilder.addAll(filterUniqueSharedLibraries(ruleContext, linkParams.getLibraries())); if (nativeDepsLibrary != null) { librariesBuilder.add(nativeDepsLibrary); + nativeDepsLibraryBasename = nativeDepsLibrary.getExecPath().getBaseName(); } + librariesBuilder.addAll( + filterUniqueSharedLibraries(ruleContext, nativeDepsLibrary, linkParams.getLibraries())); ImmutableList<Artifact> libraries = librariesBuilder.build(); if (!libraries.isEmpty()) { @@ -93,17 +87,18 @@ public final class NativeLibs { } if (result.isEmpty()) { return NativeLibs.EMPTY; + } else if (nativeDepsLibraryBasename == null) { + return new NativeLibs(ImmutableMap.copyOf(result), null); } else { - Artifact anyNativeLibrary = - result.entrySet().iterator().next().getValue().iterator().next(); // The native deps name file must be the only file in its directory because ApkBuilder does // not have an option to add a particular file to the .apk, only one to add every file in a // particular directory. Artifact nativeDepsName = ruleContext.getUniqueDirectoryArtifact( "nativedeps_filename", nativeDepsFileName, ruleContext.getBinOrGenfilesDirectory()); - ruleContext.registerAction(new FileWriteAction(ruleContext.getActionOwner(), nativeDepsName, - anyNativeLibrary.getExecPath().getBaseName(), false)); + ruleContext.registerAction( + new FileWriteAction( + ruleContext.getActionOwner(), nativeDepsName, nativeDepsLibraryBasename, false)); return new NativeLibs(ImmutableMap.copyOf(result), nativeDepsName); } @@ -188,9 +183,12 @@ public final class NativeLibs { } private static Iterable<Artifact> filterUniqueSharedLibraries( - RuleContext ruleContext, NestedSet<? extends LinkerInput> libraries) { + RuleContext ruleContext, Artifact linkedLibrary, NestedSet<? extends LinkerInput> libraries) { Map<String, Artifact> basenames = new HashMap<>(); Set<Artifact> artifacts = new HashSet<>(); + if (linkedLibrary != null) { + basenames.put(linkedLibrary.getExecPath().getBaseName(), linkedLibrary); + } for (LinkerInput linkerInput : libraries) { String name = linkerInput.getArtifact().getFilename(); if (!(CppFileTypes.SHARED_LIBRARY.matches(name) @@ -208,9 +206,17 @@ public final class NativeLibs { if (oldArtifact != null) { // There may be name collisions in the libraries which were provided, so // check for this at this step. - ruleContext.ruleError("Each library in the transitive closure must have a unique " - + "basename, but two libraries had the basename '" + basename + "': " - + artifact.prettyPrint() + " and " + oldArtifact.prettyPrint()); + ruleContext.ruleError( + "Each library in the transitive closure must have a unique basename to avoid " + + "name collisions when packaged into an apk, but two libraries have the basename '" + + basename + + "': " + + artifact.prettyPrint() + + " and " + + oldArtifact.prettyPrint() + + ((oldArtifact == linkedLibrary) + ? " (the library compiled for this target)" + : "")); } } return artifacts; |