diff options
Diffstat (limited to 'src/main/java/com/google/devtools')
5 files changed, 83 insertions, 81 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 0048c1565f..c1f129d029 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 @@ -174,10 +174,13 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { toolchainMap.put(config.getCpu(), CppHelper.getToolchain(ruleContext)); } - 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. @@ -1698,19 +1701,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 d8f7cbb724..c33a27365b 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 @@ -231,13 +231,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, @@ -395,7 +388,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 boolean usesAndroidCrosstool; @@ -414,7 +406,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.usesAndroidCrosstool = (options.androidCrosstoolTop != null); this.configurationDistinguisher = options.configurationDistinguisher; @@ -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 6d116f1970..2a8d499f40 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 @@ -689,28 +689,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 fc736ae5e1..a4b55a469c 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 @@ -50,18 +50,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, @@ -69,20 +57,26 @@ public final class NativeLibs { Map<String, CcToolchainProvider> toolchainMap, Map<String, BuildConfiguration> configurationMap) { Map<String, Iterable<Artifact>> result = new LinkedHashMap<>(); + String nativeDepsLibraryBasename = null; for (Map.Entry<String, Collection<TransitiveInfoCollection>> entry : depsByArchitecture.asMap().entrySet()) { CcLinkParams linkParams = AndroidCommon.getCcLinkParamsStore(entry.getValue()) .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()) { @@ -91,17 +85,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); } @@ -186,9 +181,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) @@ -206,9 +204,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; diff --git a/src/main/java/com/google/devtools/build/lib/rules/nativedeps/NativeDepsHelper.java b/src/main/java/com/google/devtools/build/lib/rules/nativedeps/NativeDepsHelper.java index 30f4d58962..ed1bdbbfb8 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/nativedeps/NativeDepsHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/nativedeps/NativeDepsHelper.java @@ -27,6 +27,7 @@ import com.google.devtools.build.lib.rules.cpp.CppBuildInfo; import com.google.devtools.build.lib.rules.cpp.CppConfiguration; import com.google.devtools.build.lib.rules.cpp.CppHelper; import com.google.devtools.build.lib.rules.cpp.CppLinkAction; +import com.google.devtools.build.lib.rules.cpp.Link; import com.google.devtools.build.lib.rules.cpp.Link.LinkStaticness; import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType; import com.google.devtools.build.lib.rules.cpp.LinkerInputs; @@ -82,16 +83,22 @@ public abstract class NativeDepsHelper { * <p>linkshared=1 means we prefer the ".pic.a" files to the ".a" files, and the LinkTargetType is * set to DYNAMIC_LIBRARY which causes Link.java to include "-shared" in the linker options. * + * <p>It is possible that this function may have no work to do if there are no native libraries + * in the transitive closure, or if the only native libraries in the transitive closure are + * already shared libraries. In this case, this function returns {@code null}. + * * @param ruleContext the rule context to determine the native deps library * @param linkParams the {@link CcLinkParams} for the rule, collected with linkstatic = 1 and * linkshared = 1 - * @return the native deps library runfiles. If the transitive deps closure of the rule contains - * no native code libraries, its fields are null. + * @return the native deps library, or null if there was no code which needed to be linked in the + * transitive closure. */ - public static Artifact maybeCreateAndroidNativeDepsAction(final RuleContext ruleContext, - CcLinkParams linkParams, final BuildConfiguration configuration, + public static Artifact linkAndroidNativeDepsIfPresent( + final RuleContext ruleContext, + CcLinkParams linkParams, + final BuildConfiguration configuration, CcToolchainProvider toolchain) { - if (linkParams.getLibraries().isEmpty()) { + if (!containsCodeToLink(linkParams.getLibraries())) { return null; } @@ -112,6 +119,36 @@ public abstract class NativeDepsHelper { .getLibrary(); } + /** Determines if there is any code to be linked in the input iterable. */ + private static boolean containsCodeToLink(Iterable<LibraryToLink> libraries) { + for (LibraryToLink library : libraries) { + if (containsCodeToLink(library)) { + return true; + } + } + return false; + } + + /** Determines if the input library is or contains an archive which must be linked. */ + private static boolean containsCodeToLink(LibraryToLink library) { + if (Link.SHARED_LIBRARY_FILETYPES.matches(library.getArtifact().getFilename())) { + // this is a shared library so we're going to have to copy it + return false; + } + if (!library.containsObjectFiles()) { + // this is an opaque library so we're going to have to link it + return true; + } + for (Artifact object : library.getObjectFiles()) { + if (!Link.SHARED_LIBRARY_FILETYPES.matches(object.getFilename())) { + // this library was built with a non-shared-library object so we should link it + return true; + } + } + // there weren't any artifacts besides shared libraries compiled in the library + return false; + } + public static NativeDepsRunfiles createNativeDepsAction( final RuleContext ruleContext, CcLinkParams linkParams, |