Skip to content

Commit

Permalink
Specify non-final fields for R-class generation if resource shrinking…
Browse files Browse the repository at this point in the history 
… is on.

RELNOTES: None.
PiperOrigin-RevId: 221124051
  • Loading branch information
Googler authored and Copybara-Service committed Nov 12, 2018
1 parent c397936 commit 64a111e
Show file tree
Hide file tree
Showing 5 changed files with 203 additions and 32 deletions.
60 changes: 33 additions & 27 deletions src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,35 +199,41 @@ private static RuleConfiguredTargetBuilder init(
? ruleContext.attributes().get("manifest_merger", STRING)
: null);

boolean shrinkResourceCycles =
shouldShrinkResourceCycles(dataContext.getAndroidConfig(), ruleContext, shrinkResources);
AndroidAaptVersion aaptVersion = AndroidAaptVersion.chooseTargetAaptVersion(ruleContext);
final ResourceApk resourceApk =
ProcessedAndroidData processedAndroidData =
ProcessedAndroidData.processBinaryDataFrom(
dataContext,
ruleContext,
manifest,
/* conditionalKeepRules = */ shouldShrinkResourceCycles(
dataContext.getAndroidConfig(), ruleContext, shrinkResources),
manifestValues,
aaptVersion,
AndroidResources.from(ruleContext, "resource_files"),
AndroidAssets.from(ruleContext),
resourceDeps,
AssetDependencies.fromRuleDeps(ruleContext, /* neverlink = */ false),
ResourceFilterFactory.fromRuleContextAndAttrs(ruleContext),
ruleContext.getExpander().withDataLocations().tokenized("nocompress_extensions"),
ruleContext.attributes().get("crunch_png", Type.BOOLEAN),
ruleContext.attributes().isAttributeValueExplicitlySpecified("feature_of")
? ruleContext
.getPrerequisite("feature_of", Mode.TARGET, ApkInfo.PROVIDER)
.getApk()
: null,
ruleContext.attributes().isAttributeValueExplicitlySpecified("feature_after")
? ruleContext
.getPrerequisite("feature_after", Mode.TARGET, ApkInfo.PROVIDER)
.getApk()
: null,
DataBinding.contextFrom(ruleContext, dataContext.getAndroidConfig()))
.generateRClass(dataContext, aaptVersion);
dataContext,
ruleContext,
manifest,
/* conditionalKeepRules= */ shrinkResourceCycles,
manifestValues,
aaptVersion,
AndroidResources.from(ruleContext, "resource_files"),
AndroidAssets.from(ruleContext),
resourceDeps,
AssetDependencies.fromRuleDeps(ruleContext, /* neverlink = */ false),
ResourceFilterFactory.fromRuleContextAndAttrs(ruleContext),
ruleContext.getExpander().withDataLocations().tokenized("nocompress_extensions"),
ruleContext.attributes().get("crunch_png", Type.BOOLEAN),
ruleContext.attributes().isAttributeValueExplicitlySpecified("feature_of")
? ruleContext.getPrerequisite("feature_of", Mode.TARGET, ApkInfo.PROVIDER).getApk()
: null,
ruleContext.attributes().isAttributeValueExplicitlySpecified("feature_after")
? ruleContext
.getPrerequisite("feature_after", Mode.TARGET, ApkInfo.PROVIDER)
.getApk()
: null,
DataBinding.contextFrom(ruleContext, dataContext.getAndroidConfig()));
final ResourceApk resourceApk =
new RClassGeneratorActionBuilder()
.targetAaptVersion(aaptVersion)
.withDependencies(resourceDeps)
.finalFields(!shrinkResourceCycles)
.setClassJarOut(
dataContext.createOutputArtifact(AndroidRuleClasses.ANDROID_RESOURCES_CLASS_JAR))
.build(dataContext, processedAndroidData);

ruleContext.assertNoErrors();

Expand Down
10 changes: 9 additions & 1 deletion src/main/java/com/google/devtools/build/lib/rules/android/RClassGeneratorActionBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ public class RClassGeneratorActionBuilder {

private AndroidAaptVersion version;

private boolean finalFields = true;

public RClassGeneratorActionBuilder withDependencies(ResourceDependencies resourceDeps) {
this.dependencies = resourceDeps;
return this;
Expand All @@ -51,6 +53,11 @@ public RClassGeneratorActionBuilder targetAaptVersion(AndroidAaptVersion version
return this;
}

public RClassGeneratorActionBuilder finalFields(boolean finalFields) {
this.finalFields = finalFields;
return this;
}

public RClassGeneratorActionBuilder setClassJarOut(Artifact classJarOut) {
this.classJarOut = classJarOut;
return this;
Expand All @@ -68,7 +75,8 @@ private void build(
BusyBoxActionBuilder.create(dataContext, "GENERATE_BINARY_R")
.addInput("--primaryRTxt", rTxt)
.addInput("--primaryManifest", manifest.getManifest())
.maybeAddFlag("--packageForR", manifest.getPackage());
.maybeAddFlag("--packageForR", manifest.getPackage())
.addFlag(finalFields ? "--finalFields" : "--nofinalFields");

if (dependencies != null && !dependencies.getResourceContainers().isEmpty()) {
builder
Expand Down
137 changes: 137 additions & 0 deletions src/test/java/com/google/devtools/build/android/RClassGeneratorActionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import java.io.IOException;
import java.lang.reflect.Field;
import java.lang.reflect.Modifier;
import java.net.URL;
import java.net.URLClassLoader;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
Expand Down Expand Up @@ -199,6 +203,64 @@ public void withNoBinaryAndLibraries() throws Exception {
"com/google/bar/R.class",
"META-INF/",
"META-INF/MANIFEST.MF");
assertFieldsFinal(jarPath, "com.google.foo.R$attr", true);
assertFieldsFinal(jarPath, "com.google.foo.R$id", true);
assertFieldsFinal(jarPath, "com.google.foo.R$string", true);
assertFieldsFinal(jarPath, "com.google.bar.R$attr", true);
assertFieldsFinal(jarPath, "com.google.bar.R$drawable", true);
ZipMtimeAsserter.assertEntries(zipEntries);
}
}

@Test
public void withNoBinaryAndLibraries_noFinalFields() throws Exception {
Path libFooManifest =
ManifestBuilder.of(tempDir.resolve("libFoo"))
.createManifest("AndroidManifest.xml", "com.google.foo", "");
Path libBarManifest =
ManifestBuilder.of(tempDir.resolve("libBar"))
.createManifest("AndroidManifest.xml", "com.google.bar", "");

Path libFooSymbols =
createFile(
"libFoo.R.txt", "int attr agility 0x1", "int id someTextView 0x1", "int string ok 0x1");
Path libBarSymbols =
createFile("libBar.R.txt", "int attr dexterity 0x1", "int drawable heart 0x1");

Path jarPath = tempDir.resolve("app_resources.jar");

RClassGeneratorAction.main(
ImmutableList.<String>of(
"--library",
libFooSymbols + "," + libFooManifest,
"--library",
libBarSymbols + "," + libBarManifest,
"--nofinalFields",
"--classJarOutput",
jarPath.toString())
.toArray(new String[0]));

assertThat(Files.exists(jarPath)).isTrue();

try (ZipFile zip = new ZipFile(jarPath.toFile())) {
List<? extends ZipEntry> zipEntries = Collections.list(zip.entries());
Iterable<String> entries = getZipFilenames(zipEntries);
assertThat(entries)
.containsExactly(
"com/google/foo/R$attr.class",
"com/google/foo/R$id.class",
"com/google/foo/R$string.class",
"com/google/foo/R.class",
"com/google/bar/R$attr.class",
"com/google/bar/R$drawable.class",
"com/google/bar/R.class",
"META-INF/",
"META-INF/MANIFEST.MF");
assertFieldsFinal(jarPath, "com.google.foo.R$attr", false);
assertFieldsFinal(jarPath, "com.google.foo.R$id", false);
assertFieldsFinal(jarPath, "com.google.foo.R$string", false);
assertFieldsFinal(jarPath, "com.google.bar.R$attr", false);
assertFieldsFinal(jarPath, "com.google.bar.R$drawable", false);
ZipMtimeAsserter.assertEntries(zipEntries);
}
}
Expand Down Expand Up @@ -243,6 +305,70 @@ public void withBinaryNoLibraries() throws Exception {
"com/google/app/R.class",
"META-INF/",
"META-INF/MANIFEST.MF");
assertFieldsFinal(jarPath, "com.google.app.R$attr", true);
assertFieldsFinal(jarPath, "com.google.app.R$drawable", true);
assertFieldsFinal(jarPath, "com.google.app.R$id", true);
assertFieldsFinal(jarPath, "com.google.app.R$integer", true);
assertFieldsFinal(jarPath, "com.google.app.R$string", true);
ZipMtimeAsserter.assertEntries(zipEntries);
}
}

@Test
public void withBinaryNoLibraries_noFinalFields() throws Exception {
Path binaryManifest =
ManifestBuilder.of(tempDir.resolve("binary"))
.createManifest(
"AndroidManifest.xml",
"com.google.app",
"<application android:name=\"com.google.app\">",
"<activity android:name=\"com.google.bar.activityFoo\" />",
"</application>");

Path binarySymbols =
createFile(
"R.txt",
"int attr agility 0x7f010000",
"int attr dexterity 0x7f010001",
"int drawable heart 0x7f020000",
"int id someTextView 0x7f080000",
"int integer maxNotifications 0x7f090000",
"int string alphabet 0x7f100000",
"int string ok 0x7f100001");

Path jarPath = tempDir.resolve("app_resources.jar");

RClassGeneratorAction.main(
ImmutableList.<String>of(
"--primaryRTxt",
binarySymbols.toString(),
"--primaryManifest",
binaryManifest.toString(),
"--nofinalFields",
"--classJarOutput",
jarPath.toString())
.toArray(new String[0]));

assertThat(Files.exists(jarPath)).isTrue();

try (ZipFile zip = new ZipFile(jarPath.toFile())) {
List<? extends ZipEntry> zipEntries = Collections.list(zip.entries());
Iterable<String> entries = getZipFilenames(zipEntries);
assertThat(entries)
.containsExactly(
"com/google/app/R$attr.class",
"com/google/app/R$drawable.class",
"com/google/app/R$id.class",
"com/google/app/R$integer.class",
"com/google/app/R$string.class",
"com/google/app/R.class",
"META-INF/",
"META-INF/MANIFEST.MF");
assertFieldsFinal(jarPath, "com.google.app.R$attr", false);
assertFieldsFinal(jarPath, "com.google.app.R$drawable", false);
assertFieldsFinal(jarPath, "com.google.app.R$id", false);
assertFieldsFinal(jarPath, "com.google.app.R$integer", false);
assertFieldsFinal(jarPath, "com.google.app.R$string", false);
ZipMtimeAsserter.assertEntries(zipEntries);
}
}
Expand Down Expand Up @@ -362,6 +488,17 @@ public String apply(ZipEntry input) {
});
}

private static void assertFieldsFinal(Path jarPath, String className, boolean expectedFinal)
throws Exception {
try (URLClassLoader urlClassLoader = new URLClassLoader(new URL[] {jarPath.toUri().toURL()})) {
Class<?> clazz = urlClassLoader.loadClass(className);
assertThat(clazz.getFields()).isNotEmpty();
for (Field field : clazz.getFields()) {
assertThat(Modifier.isFinal(field.getModifiers())).isEqualTo(expectedFinal);
}
}
}

private static final class ZipMtimeAsserter {
private static final long ZIP_EPOCH = Instant.parse("1980-01-01T00:00:00Z").getEpochSecond();
private static final long ZIP_EPOCH_PLUS_ONE_DAY =
Expand Down
16 changes: 14 additions & 2 deletions src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3590,7 +3590,13 @@ public void testAapt2ResourceShrinkingAction() throws Exception {
" proguard_specs = ['proguard-spec.pro'],)");

useConfiguration("--android_sdk=//sdk:sdk");
ConfiguredTarget binary = getConfiguredTarget("//java/com/google/android/hello:hello");
ConfiguredTargetAndData targetAndData =
getConfiguredTargetAndData("//java/com/google/android/hello:hello");
ConfiguredTarget binary = targetAndData.getConfiguredTarget();

Artifact jar = getResourceClassJar(targetAndData);
assertThat(getGeneratingAction(jar).getMnemonic()).isEqualTo("RClassGenerator");
assertThat(getGeneratingSpawnActionArgs(jar)).contains("--finalFields");

Set<Artifact> artifacts = actionsTestUtil().artifactClosureOf(getFilesToBuild(binary));

Expand Down Expand Up @@ -3649,7 +3655,13 @@ public void testAapt2ResourceCycleShrinking() throws Exception {
" shrink_resources = 1,",
" proguard_specs = ['proguard-spec.pro'],)");

ConfiguredTarget binary = getConfiguredTarget("//java/com/google/android/hello:hello");
ConfiguredTargetAndData targetAndData =
getConfiguredTargetAndData("//java/com/google/android/hello:hello");
ConfiguredTarget binary = targetAndData.getConfiguredTarget();

Artifact jar = getResourceClassJar(targetAndData);
assertThat(getGeneratingAction(jar).getMnemonic()).isEqualTo("RClassGenerator");
assertThat(getGeneratingSpawnActionArgs(jar)).contains("--nofinalFields");

Set<Artifact> artifacts = actionsTestUtil().artifactClosureOf(getFilesToBuild(binary));

Expand Down
12 changes: 10 additions & 2 deletions src/tools/android/java/com/google/devtools/build/android/RClassGeneratorAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,14 @@ public static final class Options extends OptionsBase {
)
public Path classJarOutput;

@Option(
name = "finalFields",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
effectTags = {OptionEffectTag.UNKNOWN},
help = "A boolean to control whether fields get declared as final, defaults to true.")
public boolean finalFields;

@Option(
name = "targetLabel",
defaultValue = "null",
Expand Down Expand Up @@ -169,7 +177,7 @@ public static void main(String[] args) throws Exception {
String.format("Load symbols finished at %sms", timer.elapsed(TimeUnit.MILLISECONDS)));
// For now, assuming not used for libraries and setting final access for fields.
fullSymbolValues.writeClassesTo(
libSymbolMap, appPackageName, classOutPath, /* finalFields= */ true);
libSymbolMap, appPackageName, classOutPath, options.finalFields);
logger.fine(
String.format("Finished R.class at %sms", timer.elapsed(TimeUnit.MILLISECONDS)));
} else if (!options.libraries.isEmpty()) {
Expand All @@ -179,7 +187,7 @@ public static void main(String[] args) throws Exception {
logger.fine(
String.format("Load symbols finished at %sms", timer.elapsed(TimeUnit.MILLISECONDS)));
// For now, assuming not used for libraries and setting final access for fields.
fullSymbolValues.writeClassesTo(libSymbolMap, null, classOutPath, /* finalFields= */ true);
fullSymbolValues.writeClassesTo(libSymbolMap, null, classOutPath, options.finalFields);
logger.fine(
String.format("Finished R.class at %sms", timer.elapsed(TimeUnit.MILLISECONDS)));
} else {
Expand Down

0 comments on commit 64a111e

Please sign in to comment.