Commit e0403933 authored by Josiah Gaskin's avatar Josiah Gaskin
Browse files

Fix non-predexed builds by only augmenting R.java patterns for predex splitter

1 merge request!1980[android] Fix non-predexed builds
Showing with 57 additions and 35 deletions
+57 -35
......@@ -197,32 +197,18 @@ public class AndroidBinaryDescription
args.getMinimizePrimaryDexSize()
? DexSplitStrategy.MINIMIZE_PRIMARY_DEX_SIZE
: DexSplitStrategy.MAXIMIZE_PRIMARY_DEX_SIZE;
ImmutableList<String> primaryDexPatterns;
if (args.isAllowRDotJavaInSecondaryDex()) {
primaryDexPatterns = args.getPrimaryDexPatterns();
} else {
primaryDexPatterns =
ImmutableList.<String>builder()
.addAll(args.getPrimaryDexPatterns())
.add(
"/R^",
"/R$",
// Pin this to the primary for test apps with no primary dex classes.
// The exact match makes it fairly efficient.
"^com/facebook/buck_generated/AppWithoutResourcesStub^")
.build();
}
return new DexSplitMode(
args.getUseSplitDex(),
dexSplitStrategy,
args.getDexCompression().orElse(defaultDexStore),
args.getLinearAllocHardLimit(),
primaryDexPatterns,
args.getPrimaryDexPatterns(),
args.getPrimaryDexClassesFile(),
args.getPrimaryDexScenarioFile(),
args.isPrimaryDexScenarioOverflowAllowed(),
args.getSecondaryDexHeadClassesFile(),
args.getSecondaryDexTailClassesFile());
args.getSecondaryDexTailClassesFile(),
args.isAllowRDotJavaInSecondaryDex());
}
@Override
......
......@@ -213,7 +213,8 @@ public class AndroidBundleDescription
args.getPrimaryDexScenarioFile(),
args.isPrimaryDexScenarioOverflowAllowed(),
args.getSecondaryDexHeadClassesFile(),
args.getSecondaryDexTailClassesFile());
args.getSecondaryDexTailClassesFile(),
args.isAllowRDotJavaInSecondaryDex());
}
@Override
......
......@@ -17,6 +17,7 @@
package com.facebook.buck.android;
import com.facebook.buck.android.dalvik.ZipSplitter;
import com.facebook.buck.android.dalvik.ZipSplitter.DexSplitStrategy;
import com.facebook.buck.core.rulekey.AddToRuleKey;
import com.facebook.buck.core.rulekey.AddsToRuleKey;
import com.facebook.buck.core.sourcepath.SourcePath;
......@@ -39,7 +40,8 @@ class DexSplitMode implements AddsToRuleKey {
/* primaryDexScenarioFile */ Optional.empty(),
/* isPrimaryDexScenarioOverflowAllowed */ false,
/* secondaryDexHeadClassesFile */ Optional.empty(),
/* secondaryDexTailClassesFile */ Optional.empty());
/* secondaryDexTailClassesFile */ Optional.empty(),
/* allowRDotJavaInSecondaryDex */ false);
/**
* By default, assume we have 5MB of linear alloc, 1MB of which is taken up by the framework, so
......@@ -114,6 +116,12 @@ class DexSplitMode implements AddsToRuleKey {
*/
@AddToRuleKey private final Optional<SourcePath> secondaryDexTailClassesFile;
/**
* Boolean identifying whether we should allow the dex splitting to move R classes into secondary
* dex files.
*/
@AddToRuleKey private boolean allowRDotJavaInSecondaryDex;
/**
* @param primaryDexPatterns Set of substrings that, when matched, will cause individual input
* class or resource files to be placed into the primary jar (and thus the primary dex
......@@ -121,20 +129,20 @@ class DexSplitMode implements AddsToRuleKey {
* @param primaryDexClassesFile Path to a file containing a list of classes that must be included
* in the primary dex. These classes are required for correctness.
* @param primaryDexScenarioFile Path to a file containing a list of classes used in a scenario
* that should be included in the primary dex along with all dependency classes required for
* preverification. These dependencies will be calculated by buck. This list is used for
* performance, not correctness.
* that should be included in the primary dex along with all dependency classes required for
* preverification. These dependencies will be calculated by buck. This list is used for
* performance, not correctness.
* @param isPrimaryDexScenarioOverflowAllowed A boolean indicating whether to fail the build if
* any classes required by primaryDexScenarioFile cannot fit (false) or to allow the build to
* to proceed on a best-effort basis (true).
* any classes required by primaryDexScenarioFile cannot fit (false) or to allow the build to
* to proceed on a best-effort basis (true).
* @param secondaryDexHeadClassesFile Path to a file containing a list of classes that are put in
* the first secondary dexes.
* the first secondary dexes.
* @param secondaryDexTailClassesFile Path to a file containing a list of classes that are put in
* the last secondary dexes.
* @param allowRDotJavaInSecondaryDex whether to allow R.java classes in the secondary dex files
*/
public DexSplitMode(
boolean shouldSplitDex,
ZipSplitter.DexSplitStrategy dexSplitStrategy,
DexSplitStrategy dexSplitStrategy,
DexStore dexStore,
long linearAllocHardLimit,
Collection<String> primaryDexPatterns,
......@@ -142,7 +150,8 @@ class DexSplitMode implements AddsToRuleKey {
Optional<SourcePath> primaryDexScenarioFile,
boolean isPrimaryDexScenarioOverflowAllowed,
Optional<SourcePath> secondaryDexHeadClassesFile,
Optional<SourcePath> secondaryDexTailClassesFile) {
Optional<SourcePath> secondaryDexTailClassesFile,
boolean allowRDotJavaInSecondaryDex) {
this.shouldSplitDex = shouldSplitDex;
this.dexSplitStrategy = dexSplitStrategy;
this.dexStore = dexStore;
......@@ -153,6 +162,7 @@ class DexSplitMode implements AddsToRuleKey {
this.isPrimaryDexScenarioOverflowAllowed = isPrimaryDexScenarioOverflowAllowed;
this.secondaryDexHeadClassesFile = secondaryDexHeadClassesFile;
this.secondaryDexTailClassesFile = secondaryDexTailClassesFile;
this.allowRDotJavaInSecondaryDex = allowRDotJavaInSecondaryDex;
}
public DexStore getDexStore() {
......@@ -195,4 +205,8 @@ class DexSplitMode implements AddsToRuleKey {
public Optional<SourcePath> getSecondaryDexTailClassesFile() {
return secondaryDexTailClassesFile;
}
public boolean isAllowRDotJavaInSecondaryDex() {
return allowRDotJavaInSecondaryDex;
}
}
......@@ -272,10 +272,25 @@ public class PreDexMerge extends AbstractBuildRuleWithDeclaredAndExtraDeps {
buildableContext.recordArtifact(paths.successDir);
buildableContext.recordArtifact(paths.additionalJarfilesSubdir);
final ImmutableSet<String> primaryDexPatterns;
if (dexSplitMode.isAllowRDotJavaInSecondaryDex()) {
primaryDexPatterns = dexSplitMode.getPrimaryDexPatterns();
} else {
primaryDexPatterns =
ImmutableSet.<String>builder()
.addAll(dexSplitMode.getPrimaryDexPatterns())
.add(
"/R^",
"/R$",
// Pin this to the primary for test apps with no primary dex classes.
// The exact match makes it fairly efficient.
"^com/facebook/buck_generated/AppWithoutResourcesStub^")
.build();
}
PreDexedFilesSorter preDexedFilesSorter =
new PreDexedFilesSorter(
dexFilesToMergeBuilder.build(),
dexSplitMode.getPrimaryDexPatterns(),
primaryDexPatterns,
apkModuleGraph,
paths.scratchDir,
// We kind of overload the "getLinearAllocHardLimit" parameter
......
......@@ -117,7 +117,8 @@ public class AndroidBinaryFilesInfoTest {
/* primaryDexScenarioFile */ Optional.empty(),
/* isPrimaryDexScenarioOverflowAllowed */ false,
/* secondaryDexHeadClassesFile */ Optional.empty(),
/* secondaryDexTailClassesFile */ Optional.empty()),
/* secondaryDexTailClassesFile */ Optional.empty(),
/* allowRDotJavaInSecondaryDex */ false),
apkModuleGraph,
null,
MoreExecutors.newDirectExecutorService(),
......
......@@ -173,7 +173,8 @@ public class SplitZipStepTest {
/* primaryDexScenarioFile */ Optional.empty(),
/* isPrimaryDexScenarioOverflowAllowed */ false,
/* secondaryDexHeadClassesFile */ Optional.empty(),
/* secondaryDexTailClassesFile */ Optional.empty()),
/* secondaryDexTailClassesFile */ Optional.empty(),
/* allowRDotJavaInSecondaryDex */ false),
Optional.empty(),
Optional.of(Paths.get("the/manifest.txt")),
Optional.empty(),
......@@ -263,7 +264,8 @@ public class SplitZipStepTest {
/* primaryDexScenarioFile */ Optional.empty(),
/* isPrimaryDexScenarioOverflowAllowed */ false,
/* secondaryDexHeadClassesFile */ Optional.empty(),
/* secondaryDexTailClassesFile */ Optional.empty()),
/* secondaryDexTailClassesFile */ Optional.empty(),
/* allowRDotJavaInSecondaryDex */ false),
Optional.empty(),
Optional.of(Paths.get("the/manifest.txt")),
Optional.empty(),
......@@ -338,7 +340,8 @@ public class SplitZipStepTest {
/* primaryDexScenarioFile */ Optional.empty(),
/* isPrimaryDexScenarioOverflowAllowed */ false,
/* secondaryDexHeadClassesFile */ Optional.empty(),
/* secondaryDexTailClassesFile */ Optional.empty()),
/* secondaryDexTailClassesFile */ Optional.empty(),
/* allowRDotJavaInSecondaryDex */ false),
Optional.empty(),
Optional.empty(),
Optional.empty(),
......@@ -412,7 +415,8 @@ public class SplitZipStepTest {
Optional.of(FakeSourcePath.of("the/primary_dex_scenario.txt")),
/* isPrimaryDexScenarioOverflowAllowed */ false,
/* secondaryDexHeadClassesFile */ Optional.empty(),
/* secondaryDexTailClassesFile */ Optional.empty()),
/* secondaryDexTailClassesFile */ Optional.empty(),
/* allowRDotJavaInSecondaryDex */ false),
Optional.of(Paths.get("the/primary_dex_scenario.txt")),
Optional.empty(),
Optional.empty(),
......@@ -495,7 +499,8 @@ public class SplitZipStepTest {
Optional.of(FakeSourcePath.of("the/primary_dex_scenario.txt")),
/* isPrimaryDexScenarioOverflowAllowed */ false,
/* secondaryDexHeadClassesFile */ Optional.empty(),
/* secondaryDexTailClassesFile */ Optional.empty()),
/* secondaryDexTailClassesFile */ Optional.empty(),
/* allowRDotJavaInSecondaryDex */ false),
Optional.of(Paths.get("the/primary_dex_scenario.txt")),
Optional.empty(),
Optional.empty(),
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment