Skip to content

Commit

Permalink
Fix NPE in RClassGenerator when dev has code in unnamed package
Browse files Browse the repository at this point in the history
Just put the R class in the unnamed package.

If we disable the RClassGenerator and use the
old code path, we would have constructed a cmdline
flag set to "null" for aapt, and aapt would write
"package null;" which also wouldn't work...

This may allow you to build an android_library
but it may be hard to build an android_binary
because ultimately AAPT will reject an empty
"package" attribute in the AndroidManifest.xml
when you try to build the APK, so you'd need
to stamp that differently.

--
PiperOrigin-RevId: 140945125
MOS_MIGRATED_REVID=140945125
  • Loading branch information
Googler authored and damienmg committed Dec 5, 2016
1 parent d3e2d04 commit 758f81f
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,31 @@ public void checkIntArrays(boolean finalFields) throws Exception {
);
}

@Test
public void emptyPackage() throws Exception {
boolean finalFields = true;
// Make sure we handle an empty package string.
SymbolLoader symbolValues = createSymbolFile("R.txt", "int string some_string 0x7f200000");
SymbolLoader symbolsInLibrary = symbolValues;
Path out = temp.resolve("classes");
Files.createDirectories(out);
RClassGenerator writer =
RClassGenerator.fromSymbols(
out, "", symbolValues, ImmutableList.of(symbolsInLibrary), finalFields);
writer.write();

Path packageDir = out.resolve("");
checkFilesInPackage(packageDir, "R.class", "R$string.class");
Class<?> outerClass = checkTopLevelClass(out, "R", "R$string");
checkInnerClass(
out,
"R$string",
outerClass,
ImmutableMap.of("some_string", 0x7f200000),
ImmutableMap.<String, List<Integer>>of(),
finalFields);
}

// Test utilities

private Path createFile(String name, String... contents) throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.android.utils.StdLogger;
import com.google.common.base.Preconditions;
import com.google.common.base.Stopwatch;
import com.google.common.base.Strings;
import com.google.common.io.Files;
import com.google.devtools.build.android.AndroidResourceProcessor.AaptConfigOptions;
import com.google.devtools.build.android.Converters.ExistingPathConverter;
Expand Down Expand Up @@ -162,12 +163,11 @@ public static void main(String[] args) throws Exception {
logger.fine(String.format("Setup finished at %sms", timer.elapsed(TimeUnit.MILLISECONDS)));

VariantType packageType = VariantType.LIBRARY;
String packageName = options.packageForR;
AndroidResourceClassWriter resourceClassWriter =
new AndroidResourceClassWriter(
new AndroidFrameworkAttrIdJar(aaptConfigOptions.androidJar),
generatedSources,
packageName);
Strings.nullToEmpty(options.packageForR));
resourceClassWriter.setIncludeClassFile(true);
resourceClassWriter.setIncludeJavaFile(false);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,12 @@ public static RClassGenerator fromSymbols(
String packageName,
SymbolLoader values,
Collection<SymbolLoader> packageSymbols,
boolean finalFields) throws IOException {
boolean finalFields)
throws IOException {
Table<String, String, SymbolEntry> symbolsTable = getAllSymbols(packageSymbols);
Table<String, String, SymbolEntry> valuesTable = getSymbols(values);
Map<ResourceType, List<FieldInitializer>> initializers = getInitializers(symbolsTable,
valuesTable);
Map<ResourceType, List<FieldInitializer>> initializers =
getInitializers(symbolsTable, valuesTable);
return new RClassGenerator(outFolder, packageName, initializers, finalFields);
}

Expand All @@ -99,37 +100,32 @@ public RClassGenerator(
}

private static Table<String, String, SymbolEntry> getAllSymbols(
Collection<SymbolLoader> symbolLoaders)
throws IOException {
Collection<SymbolLoader> symbolLoaders) throws IOException {
Table<String, String, SymbolEntry> symbols = HashBasedTable.create();
for (SymbolLoader symbolLoader : symbolLoaders) {
symbols.putAll(getSymbols(symbolLoader));
}
return symbols;
}


private static Table<String, String, SymbolEntry> getSymbols(SymbolLoader symbolLoader)
throws IOException {
// TODO(bazel-team): remove when we update android_ide_common to a version w/ public visibility
try {
Method getSymbols = SymbolLoader.class.getDeclaredMethod("getSymbols");
getSymbols.setAccessible(true);
@SuppressWarnings("unchecked")
Table<String, String, SymbolEntry> result = (Table<String, String, SymbolEntry>)
getSymbols.invoke(symbolLoader);
Table<String, String, SymbolEntry> result =
(Table<String, String, SymbolEntry>) getSymbols.invoke(symbolLoader);
return result;
} catch (ReflectiveOperationException e) {
throw new IOException(e);
}
}

/**
* Convert the {@link SymbolLoader} data, to a map of {@link FieldInitializer}.
*/
/** Convert the {@link SymbolLoader} data, to a map of {@link FieldInitializer}. */
private static Map<ResourceType, List<FieldInitializer>> getInitializers(
Table<String, String, SymbolEntry> symbols,
Table<String, String, SymbolEntry> values) {
Table<String, String, SymbolEntry> symbols, Table<String, String, SymbolEntry> values) {
Map<ResourceType, List<FieldInitializer>> initializers = new EnumMap<>(ResourceType.class);
for (String typeName : symbols.rowKeySet()) {
ResourceType resourceType = ResourceType.getEnum(typeName);
Expand All @@ -156,22 +152,20 @@ private static List<FieldInitializer> getInitializers(
initializers.add(IntFieldInitializer.of(value.getName(), value.getValue()));
} else {
Preconditions.checkArgument(value.getType().equals("int[]"));
initializers
.add(IntArrayFieldInitializer.of(value.getName(), value.getValue()));
initializers.add(IntArrayFieldInitializer.of(value.getName(), value.getValue()));
}
} else {
// Value may be missing if resource overriding eliminates resources at the binary
// level, which were originally present at the library level.
logger.fine(String.format("Skipping R.%s.%s -- value not known in binary's R.txt",
typeName, symbolName));
logger.fine(
String.format(
"Skipping R.%s.%s -- value not known in binary's R.txt", typeName, symbolName));
}
}
return initializers;
}

/**
* Builds the bytecode and writes out the R.class file, and R$inner.class files.
*/
/** Builds the bytecode and writes out the R.class file, and R$inner.class files. */
public void write() throws IOException {
Iterable<String> folders = PACKAGE_SPLITTER.split(packageName);
Path packageDir = outFolder;
Expand All @@ -187,7 +181,7 @@ public void write() throws IOException {
Path rClassFile = packageDir.resolve(SdkConstants.FN_COMPILED_RESOURCE_CLASS);

String packageWithSlashes = packageName.replaceAll("\\.", "/");
String rClassName = packageWithSlashes + "/R";
String rClassName = packageWithSlashes.isEmpty() ? "R" : (packageWithSlashes + "/R");
ClassWriter classWriter = new ClassWriter(ClassWriter.COMPUTE_MAXS);
classWriter.visit(
JAVA_VERSION,
Expand Down Expand Up @@ -221,7 +215,8 @@ private void writeInnerClass(
List<FieldInitializer> initializers,
Path packageDir,
String fullyQualifiedOuterClass,
String innerClass) throws IOException {
String innerClass)
throws IOException {
ClassWriter innerClassWriter = new ClassWriter(ClassWriter.COMPUTE_MAXS);
String fullyQualifiedInnerClass =
writeInnerClassHeader(fullyQualifiedOuterClass, innerClass, innerClassWriter);
Expand All @@ -245,8 +240,8 @@ private void writeInnerClass(
Files.write(innerFile, innerClassWriter.toByteArray());
}

private String writeInnerClassHeader(String fullyQualifiedOuterClass, String innerClass,
ClassWriter innerClassWriter) {
private String writeInnerClassHeader(
String fullyQualifiedOuterClass, String innerClass, ClassWriter innerClassWriter) {
String fullyQualifiedInnerClass = fullyQualifiedOuterClass + "$" + innerClass;
innerClassWriter.visit(
JAVA_VERSION,
Expand All @@ -266,12 +261,9 @@ private String writeInnerClassHeader(String fullyQualifiedOuterClass, String inn
}

private static void writeConstructor(ClassWriter classWriter) {
MethodVisitor constructor = classWriter.visitMethod(
Opcodes.ACC_PUBLIC,
"<init>",
"()V",
null, /* signature */
null /* exceptions */);
MethodVisitor constructor =
classWriter.visitMethod(
Opcodes.ACC_PUBLIC, "<init>", "()V", null, /* signature */ null /* exceptions */);
constructor.visitCode();
constructor.visitVarInsn(Opcodes.ALOAD, 0);
constructor.visitMethodInsn(Opcodes.INVOKESPECIAL, SUPER_CLASS, "<init>", "()V", false);
Expand All @@ -281,26 +273,18 @@ private static void writeConstructor(ClassWriter classWriter) {
}

private static void writeStaticClassInit(
ClassWriter classWriter,
String className,
List<FieldInitializer> initializers) {
MethodVisitor visitor = classWriter.visitMethod(
Opcodes.ACC_STATIC,
"<clinit>",
"()V",
null, /* signature */
null /* exceptions */);
ClassWriter classWriter, String className, List<FieldInitializer> initializers) {
MethodVisitor visitor =
classWriter.visitMethod(
Opcodes.ACC_STATIC, "<clinit>", "()V", null, /* signature */ null /* exceptions */);
visitor.visitCode();
int stackSlotsNeeded = 0;
InstructionAdapter insts = new InstructionAdapter(visitor);
for (FieldInitializer fieldInit : initializers) {
stackSlotsNeeded = Math.max(
stackSlotsNeeded,
fieldInit.writeCLInit(insts, className));
stackSlotsNeeded = Math.max(stackSlotsNeeded, fieldInit.writeCLInit(insts, className));
}
insts.areturn(Type.VOID_TYPE);
visitor.visitMaxs(stackSlotsNeeded, 0);
visitor.visitEnd();
}

}

0 comments on commit 758f81f

Please sign in to comment.