Skip to content

Commit

Permalink
Avoid generating an ap_ file for android_library
Browse files Browse the repository at this point in the history
Saves roughly 200ms per aapt invocation, when aapt is only used
for validation of resources.

--
MOS_MIGRATED_REVID=121863470
  • Loading branch information
Googler authored and aehlig committed May 10, 2016
1 parent 92a3a81 commit b64150d
Show file tree
Hide file tree
Showing 7 changed files with 24 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public ConfiguredTarget create(RuleContext ruleContext) throws InterruptedExcept
return null;
}
resourceApk = applicationManifest.packWithDataAndResources(
ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_RESOURCES_APK),
null, /* resourceApk -- not needed for library */
ruleContext,
true, /* isLibrary */
ResourceDependencies.fromRuleDeps(ruleContext, JavaCommon.isNeverLink(ruleContext)),
Expand Down Expand Up @@ -148,14 +148,12 @@ public ConfiguredTarget create(RuleContext ruleContext) throws InterruptedExcept
aar = null;
ApplicationManifest applicationManifest = ApplicationManifest.generatedManifest(ruleContext);

Artifact apk = ruleContext.getImplicitOutputArtifact(
AndroidRuleClasses.ANDROID_RESOURCES_APK);

String javaPackage = AndroidCommon.getJavaPackage(ruleContext);

ResourceContainer resourceContainer = new ResourceContainer(ruleContext.getLabel(),
javaPackage, null /* renameManifestPackage */, false /* inlinedConstants */,
apk, applicationManifest.getManifest(),
null /* resourceApk -- not needed for library */,
applicationManifest.getManifest(),
ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_JAVA_SOURCE_JAR),
ImmutableList.<Artifact>of(), ImmutableList.<Artifact>of(),
ImmutableList.<PathFragment>of(), ImmutableList.<PathFragment>of(),
Expand All @@ -164,7 +162,6 @@ public ConfiguredTarget create(RuleContext ruleContext) throws InterruptedExcept

primaryResources = new AndroidResourcesProcessorBuilder(ruleContext)
.setLibrary(true)
.setApkOut(apk)
.setRTxtOut(resourceContainer.getRTxt())
.setManifestOut(ruleContext.getImplicitOutputArtifact(
AndroidRuleClasses.ANDROID_PROCESSED_MANIFEST))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,19 +55,20 @@ public AndroidResourceContainerBuilder withInlinedConstants(boolean inlineContan

/** Creates a {@link ResourceContainer} from a {@link RuleContext}.
* @throws InterruptedException */
public ResourceContainer buildFromRule(RuleContext ruleContext, Artifact apk)
public ResourceContainer buildFromRule(RuleContext ruleContext, @Nullable Artifact apk)
throws InterruptedException {
Preconditions.checkNotNull(this.manifest);
Preconditions.checkNotNull(this.data);
Artifact rJavaSrcJar = ruleContext.getImplicitOutputArtifact(
AndroidRuleClasses.ANDROID_JAVA_SOURCE_JAR);
return new AndroidResourcesProvider.ResourceContainer(
ruleContext.getLabel(),
getJavaPackage(ruleContext, apk),
getJavaPackage(ruleContext, rJavaSrcJar),
getRenameManifestPackage(ruleContext),
inlineConstants,
apk,
manifest,
ruleContext.getImplicitOutputArtifact(
AndroidRuleClasses.ANDROID_JAVA_SOURCE_JAR),
rJavaSrcJar,
data.getAssets(),
data.getResources(),
data.getAssetRoots(),
Expand All @@ -77,18 +78,18 @@ public ResourceContainer buildFromRule(RuleContext ruleContext, Artifact apk)
symbolsFile);
}

private String getJavaPackage(RuleContext ruleContext, Artifact apk) {
private String getJavaPackage(RuleContext ruleContext, Artifact rJavaSrcJar) {
if (hasCustomPackage(ruleContext)) {
return ruleContext.attributes().get("custom_package", Type.STRING);
}
// TODO(bazel-team): JavaUtil.getJavaPackageName does not check to see if the path is valid.
// So we need to check for the JavaRoot.
if (JavaUtil.getJavaRoot(apk.getExecPath()) == null) {
if (JavaUtil.getJavaRoot(rJavaSrcJar.getExecPath()) == null) {
ruleContext.ruleError("You must place your code under a directory named 'java' or "
+ "'javatests' for blaze to work. That directory (java,javatests) will be treated as "
+ "your java source root. Alternatively, you can set the 'custom_package' attribute.");
}
return JavaUtil.getJavaPackageName(apk.getExecPath());
return JavaUtil.getJavaPackageName(rJavaSrcJar.getExecPath());
}

private boolean hasCustomPackage(RuleContext ruleContext) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -389,9 +389,9 @@ public ResourceContainer build(ActionConstructionContext context) {
primary.getRenameManifestPackage(),
primary.getConstantsInlined(),
// If there is no apk to be generated, use the apk from the primary resources.
// All ResourceContainers have to have an apk, but if a new one is not requested to be built
// for this resource processing action (in case of just creating an R.txt or
// proguard merging), reuse the primary resource from the dependencies.
// All android_binary ResourceContainers have to have an apk, but if a new one is not
// requested to be built for this resource processing action (in case of just creating an
// R.txt or proguard merging), reuse the primary resource from the dependencies.
apkOut != null ? apkOut : primary.getApk(),
manifestOut != null ? manifestOut : primary.getManifest(),
sourceJarOut,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,10 @@ public static final class ResourceContainer {
public ResourceContainer(Label label,
String javaPackage,
@Nullable String renameManifestPackage,
boolean constantsInlined, Artifact apk,
Artifact manifest, Artifact javaSourceJar,
boolean constantsInlined,
@Nullable Artifact apk,
Artifact manifest,
Artifact javaSourceJar,
ImmutableList<Artifact> assets,
ImmutableList<Artifact> resources,
ImmutableList<PathFragment> assetsRoots,
Expand All @@ -123,7 +125,7 @@ public ResourceContainer(Label label,
this.javaPackage = Preconditions.checkNotNull(javaPackage);
this.renameManifestPackage = renameManifestPackage;
this.constantsInlined = constantsInlined;
this.apk = Preconditions.checkNotNull(apk);
this.apk = apk;
this.manifest = Preconditions.checkNotNull(manifest);
this.assets = Preconditions.checkNotNull(assets);
this.resources = Preconditions.checkNotNull(resources);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,6 @@ public Iterable<String> getImplicitOutputs(AttributeMap attributes) {
if (LocalResourceContainer.definesAndroidResources(attributes)) {
implicitOutputs.add(
AndroidRuleClasses.ANDROID_JAVA_SOURCE_JAR,
AndroidRuleClasses.ANDROID_RESOURCES_APK,
AndroidRuleClasses.ANDROID_R_TXT);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,11 +237,9 @@ public ResourceApk packWithAssets(
null /* Artifact mergedResources */);
}

/** Packages up the manifest with resource and assets from the rule and dependent resources.
* @param manifestOut TODO(corysmith):
* @throws InterruptedException */
/** Packages up the manifest with resource and assets from the rule and dependent resources. */
public ResourceApk packWithDataAndResources(
Artifact resourceApk,
@Nullable Artifact resourceApk,
RuleContext ruleContext,
boolean isLibrary,
ResourceDependencies resourceDeps,
Expand Down Expand Up @@ -296,7 +294,8 @@ public ResourceApk packWithDataAndResources(
mergedResources);
}

private ResourceApk createApk(Artifact resourceApk,
private ResourceApk createApk(
@Nullable Artifact resourceApk,
RuleContext ruleContext,
boolean isLibrary,
ResourceDependencies resourceDeps,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ public void processResources(
.maybeAdd("-m", sourceOut != null)
.maybeAdd("-J", prepareOutputPath(sourceOut), sourceOut != null)
.maybeAdd("--output-text-symbols", prepareOutputPath(sourceOut), sourceOut != null)
.add("-F", packageOut)
.maybeAdd("-F", packageOut, packageOut != null)
.add("-G", proguardOut)
.maybeAdd("-D", mainDexProguardOut, new FullRevision(24))
.add("-P", publicResourcesOut)
Expand Down

0 comments on commit b64150d

Please sign in to comment.