Skip to content

Commit

Permalink
Update AndroidSdkProvider to throw the rule error when the dummy SDK …
Browse files Browse the repository at this point in the history
…is used.

Previously the warning could be shadowed by later errors.

Part of #16285.

PiperOrigin-RevId: 572899622
Change-Id: Ief6ae24b5c4d77e41b167f5d92878eec7d7a6181
  • Loading branch information
katre authored and copybara-github committed Oct 12, 2023
1 parent 62ceeaf commit 33ad6c0
Show file tree
Hide file tree
Showing 15 changed files with 52 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ private static SpawnAction createAarEmbeddedProguardExtractorActions(
.build(ruleContext);
}

private NestedSet<Artifact> getBootclasspath(RuleContext ruleContext) {
private NestedSet<Artifact> getBootclasspath(RuleContext ruleContext) throws RuleErrorException {
if (AndroidCommon.getAndroidConfig(ruleContext).desugarJava8()) {
return NestedSetBuilder.<Artifact>stableOrder()
.addTransitive(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1202,7 +1202,8 @@ private static void symlinkOptimizationOutputs(
}
}

static Java8LegacyDexOutput buildJava8LegacyDex(RuleContext ruleContext, Artifact jarToDex) {
static Java8LegacyDexOutput buildJava8LegacyDex(RuleContext ruleContext, Artifact jarToDex)
throws RuleErrorException {
Artifact java8LegacyDexRules = getDxArtifact(ruleContext, "_java8_legacy.dex.pgcfg");
Artifact java8LegacyDex = getDxArtifact(ruleContext, "_java8_legacy.dex.zip");
Artifact java8LegacyDexMap = getDxArtifact(ruleContext, "_java8_legacy.dex.map");
Expand Down Expand Up @@ -2560,7 +2561,8 @@ private static int getMinSdkVersion(RuleContext ruleContext) {
* Returns true if the runtime contained in the Android SDK used to build this rule supports the
* given version of multidex mode specified, false otherwise.
*/
private static boolean supportsMultidexMode(RuleContext ruleContext, MultidexMode mode) {
private static boolean supportsMultidexMode(RuleContext ruleContext, MultidexMode mode)
throws RuleErrorException {
if (mode == MultidexMode.NATIVE) {
// Native mode is not supported by Android devices running Android before v21.
String runtime =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,8 @@ private static void createInstallAction(
Artifact resourceApk,
Artifact apk,
NativeLibs nativeLibs,
Artifact stubDataFile) {
Artifact stubDataFile)
throws RuleErrorException {

FilesToRunProvider adb = AndroidSdkProvider.fromRuleContext(ruleContext).getAdb();
SpawnAction.Builder builder =
Expand Down Expand Up @@ -437,7 +438,8 @@ private static void createSplitInstallAction(
Artifact argsArtifact,
Artifact splitMainApk,
NestedSet<Artifact> splitApks,
Artifact stubDataFile) {
Artifact stubDataFile)
throws RuleErrorException {
FilesToRunProvider adb = AndroidSdkProvider.fromRuleContext(ruleContext).getAdb();
SpawnAction.Builder builder =
new InstallActionBuilder()
Expand Down Expand Up @@ -471,7 +473,7 @@ private static Artifact createSplitApkResources(
ProcessedAndroidManifest mainManifest,
String splitName,
boolean hasCode)
throws InterruptedException {
throws InterruptedException, RuleErrorException {
Artifact splitManifest =
mainManifest.createSplitManifest(ruleContext, splitName, hasCode).getManifest();
Artifact splitResources = getMobileInstallArtifact(ruleContext, "split_" + splitName + ".ap_");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,8 @@ public static void createDexAction(
Artifact classesDex,
List<String> dexOptions,
int minSdkVersion,
Artifact mainDexList) {
Artifact mainDexList)
throws RuleErrorException {
CustomCommandLine.Builder commandLine = CustomCommandLine.builder();
commandLine.add("--dex");

Expand Down Expand Up @@ -900,7 +901,8 @@ static JavaCommon createJavaCommonWithAndroidDataBinding(
JavaSemantics semantics,
DataBindingContext dataBindingContext,
boolean isLibrary,
boolean shouldCompileJavaSrcs) {
boolean shouldCompileJavaSrcs)
throws RuleErrorException {

ImmutableList<Artifact> ruleSources = ruleContext.getPrerequisiteArtifacts("srcs").list();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.BuildType;
import com.google.devtools.build.lib.packages.ImplicitOutputsFunction.SafeImplicitOutputsFunction;
import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
import com.google.devtools.build.lib.packages.TargetUtils;
import com.google.devtools.build.lib.packages.TriState;
import com.google.devtools.build.lib.starlarkbuildapi.android.AndroidDataContextApi;
Expand Down Expand Up @@ -73,11 +74,11 @@ public class AndroidDataContext implements AndroidDataContextApi {
private final boolean throwOnResourceConflict;
private final ImmutableMap<String, String> executionInfo;

public static AndroidDataContext forNative(RuleContext ruleContext) {
public static AndroidDataContext forNative(RuleContext ruleContext) throws RuleErrorException {
return makeContext(ruleContext);
}

public static AndroidDataContext makeContext(RuleContext ruleContext) {
public static AndroidDataContext makeContext(RuleContext ruleContext) throws RuleErrorException {
AndroidConfiguration androidConfig =
ruleContext.getConfiguration().getFragment(AndroidConfiguration.class);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.packages.BuildType;
import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
import com.google.devtools.build.lib.packages.Type;
import com.google.devtools.build.lib.rules.java.JavaUtil;
import com.google.devtools.build.lib.rules.java.ProguardSpecProvider;
Expand Down Expand Up @@ -97,7 +98,8 @@ public AndroidIdlHelper(RuleContext ruleContext, Artifact baseArtifact) {
* class files in the class jar.
*/
public void addTransitiveInfoProviders(
RuleConfiguredTargetBuilder builder, Artifact classJar, Artifact manifestProtoOutput) {
RuleConfiguredTargetBuilder builder, Artifact classJar, Artifact manifestProtoOutput)
throws RuleErrorException {
if (!translatedIdlSources.isEmpty()) {
generateAndroidIdlCompilationActions(ruleContext, androidIdlProvider, translatedIdlSources);
createIdlClassJarAction(
Expand Down Expand Up @@ -194,7 +196,8 @@ public static boolean hasIdlSrcs(RuleContext ruleContext) {
* Returns a new list with the idl libs added to the given list if necessary, or the same list.
*/
public static ImmutableList<TransitiveInfoCollection> maybeAddSupportLibs(
RuleContext ruleContext, ImmutableList<TransitiveInfoCollection> deps) {
RuleContext ruleContext, ImmutableList<TransitiveInfoCollection> deps)
throws RuleErrorException {
if (!hasIdlSrcs(ruleContext)) {
return deps;
}
Expand All @@ -206,7 +209,8 @@ public static ImmutableList<TransitiveInfoCollection> maybeAddSupportLibs(
}

public static void maybeAddSupportLibProguardConfigs(
RuleContext ruleContext, NestedSetBuilder<Artifact> proguardConfigsBuilder) {
RuleContext ruleContext, NestedSetBuilder<Artifact> proguardConfigsBuilder)
throws RuleErrorException {
if (!hasIdlSrcs(ruleContext)) {
return;
}
Expand Down Expand Up @@ -301,7 +305,8 @@ private static ImmutableMap<Artifact, Artifact> generateTranslatedIdlArtifacts(
private static void generateAndroidIdlCompilationActions(
RuleContext ruleContext,
AndroidIdlProvider transitiveIdlImportData,
Map<Artifact, Artifact> translatedIdlSources) {
Map<Artifact, Artifact> translatedIdlSources)
throws RuleErrorException {
AndroidSdkProvider sdk = AndroidSdkProvider.fromRuleContext(ruleContext);
List<String> preprocessedArgs = new ArrayList<>();

Expand Down Expand Up @@ -390,7 +395,8 @@ private static void createAndroidIdlAction(
Artifact idl,
NestedSet<Artifact> idlImports,
Artifact output,
List<String> importArgs) {
List<String> importArgs)
throws RuleErrorException {
AndroidSdkProvider sdk = AndroidSdkProvider.fromRuleContext(ruleContext);
ruleContext.registerAction(
new SpawnAction.Builder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -260,12 +260,12 @@ private static FilesToRunProvider getTargetDevice(RuleContext ruleContext) {
}

/** ADB binary from the Android SDK. */
private static FilesToRunProvider getAdb(RuleContext ruleContext) {
private static FilesToRunProvider getAdb(RuleContext ruleContext) throws RuleErrorException {
return AndroidSdkProvider.fromRuleContext(ruleContext).getAdb();
}

/** AAPT binary from the Android SDK. */
private static FilesToRunProvider getAapt(RuleContext ruleContext) {
private static FilesToRunProvider getAapt(RuleContext ruleContext) throws RuleErrorException {
return AndroidSdkProvider.fromRuleContext(ruleContext).getAapt();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,8 @@ public Provider getProvider() {
* --incompatible_enable_android_toolchain_resolution=true, else, uses the legacy attribute..
*/
@Nullable
public static AndroidSdkProvider fromRuleContext(RuleContext ruleContext) {
public static AndroidSdkProvider fromRuleContext(RuleContext ruleContext)
throws RuleErrorException {
// Determine the toolchain type.
Label toolchainType = getToolchainTypeFromAttribute(ruleContext);
return fromRuleContext(ruleContext, ":android_sdk", toolchainType);
Expand All @@ -135,7 +136,8 @@ public static AndroidSdkProvider fromRuleContext(RuleContext ruleContext) {
*/
@Nullable
public static AndroidSdkProvider fromRuleContext(
RuleContext ruleContext, String sdkAttribute, @Nullable Label toolchainType) {
RuleContext ruleContext, String sdkAttribute, @Nullable Label toolchainType)
throws RuleErrorException {
BuildConfigurationValue configuration = ruleContext.getConfiguration();
if (configuration == null
|| !configuration.hasFragment(AndroidConfiguration.class)
Expand Down Expand Up @@ -200,17 +202,16 @@ public static AndroidSdkProvider fromRuleContext(
}

private static boolean usingDummyToolchain(
RuleContext ruleContext, AndroidSdkProvider androidSdkProvider) {
RuleContext ruleContext, AndroidSdkProvider androidSdkProvider) throws RuleErrorException {

if (androidSdkProvider.getAndroidJar().getFilename().matches("dummy\\.jar$")) {
// This is an invalid SDK, and probably due to a default configuration.
ruleContext.ruleError(
throw ruleContext.throwWithRuleError(
String.format(
"'%s' rule '%s' requested an android sdk via toolchain resolution but hasn't set an"
+ " appropriate --android_platforms value: Either set"
+ " --noincompatible_enable_android_toolchain_resolution or --android_platforms.",
ruleContext.getRuleClassNameForLogging(), ruleContext.getLabel()));
return true;
}

return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,8 @@ AndroidBinary.DexPostprocessingOutput postprocessClassesDexZip(
Artifact mainDexList)
throws InterruptedException;

default AndroidDataContext makeContextForNative(RuleContext ruleContext) {
default AndroidDataContext makeContextForNative(RuleContext ruleContext)
throws RuleErrorException {
return AndroidDataContext.forNative(ruleContext);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,8 @@ private void buildApk(RuleContext ruleContext, Artifact outApk)
}

/** Uses the zipalign tool to align the zip boundaries for uncompressed resources by 4 bytes. */
private void zipalignApk(RuleContext ruleContext, Artifact inputApk, Artifact zipAlignedApk) {
private void zipalignApk(RuleContext ruleContext, Artifact inputApk, Artifact zipAlignedApk)
throws RuleErrorException {
ruleContext.registerAction(
createSpawnActionBuilder(ruleContext)
.addInput(inputApk)
Expand All @@ -367,7 +368,8 @@ private void zipalignApk(RuleContext ruleContext, Artifact inputApk, Artifact zi
* alignment cannot be performed after v2 signing without invalidating the signature.
*/
private void signApk(
RuleContext ruleContext, Artifact unsignedApk, Artifact signedAndZipalignedApk) {
RuleContext ruleContext, Artifact unsignedApk, Artifact signedAndZipalignedApk)
throws RuleErrorException {
ApkSigningMethod signingMethod =
ruleContext.getFragment(AndroidConfiguration.class).getApkSigningMethod();
SpawnAction.Builder actionBuilder =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,7 @@ private NestedSet<Artifact> getBootclasspath(ConfiguredTarget base, RuleContext
}

@Nullable
private Artifact getAndroidJar(RuleContext ruleContext) {
private Artifact getAndroidJar(RuleContext ruleContext) throws RuleErrorException {
Label toolchainType = Label.parseCanonicalUnchecked(toolsRepository + sdkToolchainLabel);
AndroidSdkProvider androidSdk =
AndroidSdkProvider.fromRuleContext(ruleContext, ":dex_archive_android_sdk", toolchainType);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ void supplyAnnotationProcessor(
* <p>This triggers the annotation processor. Annotation processor settings are configured
* separately in {@link #supplyJavaCoptsUsing(RuleContext, boolean, Consumer)}.
*/
ImmutableList<Artifact> getAnnotationSourceFiles(RuleContext ruleContext);
ImmutableList<Artifact> getAnnotationSourceFiles(RuleContext ruleContext)
throws RuleErrorException;

/**
* Adds the appropriate {@link UsesDataBindingProvider} for a rule if it should expose one.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,8 @@ private static ImmutableMultimap<String, String> getJavaPackagesWithDatabindingT
}

@Override
public ImmutableList<Artifact> getAnnotationSourceFiles(RuleContext ruleContext) {
public ImmutableList<Artifact> getAnnotationSourceFiles(RuleContext ruleContext)
throws RuleErrorException {
ImmutableList.Builder<Artifact> srcs = ImmutableList.builder();

srcs.addAll(DataBinding.getAnnotationFile(ruleContext));
Expand All @@ -274,7 +275,8 @@ public ImmutableList<Artifact> getAnnotationSourceFiles(RuleContext ruleContext)
return srcs.build();
}

private ImmutableList<Artifact> createBaseClasses(RuleContext ruleContext) {
private ImmutableList<Artifact> createBaseClasses(RuleContext ruleContext)
throws RuleErrorException {
if (!AndroidResources.definesAndroidResources(ruleContext.attributes())) {
return ImmutableList.of(); // no resource, no base classes or class info
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.collect.nestedset.Order;
import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
import com.google.devtools.build.lib.rules.android.AndroidAssetsTest.WithPlatforms;
import com.google.devtools.build.lib.rules.android.AndroidAssetsTest.WithoutPlatforms;
import com.google.devtools.build.lib.vfs.PathFragment;
Expand Down Expand Up @@ -181,7 +182,7 @@ private AssetDependencies makeDeps(RuleContext ruleContext, boolean neverlink) {

private MergedAndroidAssets assertMerge(
RuleContext ruleContext, ParsedAndroidAssets parsed, AssetDependencies deps)
throws InterruptedException {
throws InterruptedException, RuleErrorException {
MergedAndroidAssets merged =
MergedAndroidAssets.mergeFrom(AndroidDataContext.forNative(ruleContext), parsed, deps);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ java_test(
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
"//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
"//src/main/java/com/google/devtools/build/lib/collect/nestedset",
"//src/main/java/com/google/devtools/build/lib/packages",
"//src/main/java/com/google/devtools/build/lib/rules/android",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//third_party:guava",
Expand Down

0 comments on commit 33ad6c0

Please sign in to comment.