Skip to content

Commit

Permalink
Prepare for Starlark java_toolchain
Browse files Browse the repository at this point in the history
Just a lot of error propagation, split off from the actual change for easier review. Also updated a few tests to work before and after.

PiperOrigin-RevId: 572550051
Change-Id: I8e65fc81f9485f9534afe1a5025259fc7e3f37f8
  • Loading branch information
hvadehra authored and copybara-github committed Oct 11, 2023
1 parent f799eb1 commit 379ee5f
Show file tree
Hide file tree
Showing 25 changed files with 91 additions and 51 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.packages.AspectParameters;
import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;

/**
* Creates the Skyframe node of an aspect.
Expand All @@ -38,7 +39,7 @@ ConfiguredAspect create(
RuleContext context,
AspectParameters parameters,
RepositoryName toolsRepository)
throws ActionConflictException, InterruptedException;
throws ActionConflictException, InterruptedException, RuleErrorException;

/** Adds any aspect implementation-specific requirements to the given builder. */
default void addAspectImplSpecificRequiredConfigFragments(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -552,7 +552,10 @@ public ConfiguredAspect createAspect(
BuildConfigurationValue aspectConfiguration,
@Nullable NestedSet<Package> transitivePackages,
AspectKeyCreator.AspectKey aspectKey)
throws InterruptedException, ActionConflictException, InvalidExecGroupException {
throws InterruptedException,
ActionConflictException,
InvalidExecGroupException,
RuleErrorException {
RuleContext ruleContext =
new RuleContext.Builder(env, associatedTarget, aspectPath, aspectConfiguration)
.setRuleClassProvider(ruleClassProvider)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,8 @@ public ImmutableList<String> getCompatibleJavacOptions(
}

@Override
public String addCoverageSupport(JavaCompilationHelper helper, Artifact executable) {
public String addCoverageSupport(JavaCompilationHelper helper, Artifact executable)
throws RuleErrorException {
// This method can be called only for *_binary/*_test targets.
Preconditions.checkNotNull(executable);
helper.addCoverageSupport();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,8 @@ private static NestedSet<Artifact> getCompileTimeJarsFromCollection(
* Collect Proguard Specs from transitives and proguard.txt if it exists in the AAR file. In the
* case the proguard.txt file does exists, we need to extract it from the AAR file
*/
private NestedSet<Artifact> extractProguardSpecs(RuleContext ruleContext, Artifact aar) {
private NestedSet<Artifact> extractProguardSpecs(RuleContext ruleContext, Artifact aar)
throws RuleErrorException {

NestedSet<Artifact> proguardSpecs =
new ProguardLibrary(ruleContext).collectProguardSpecs(ImmutableSet.of("deps", "exports"));
Expand Down Expand Up @@ -443,7 +444,8 @@ private static SpawnAction createAarEmbeddedJarsExtractorActions(
}

private static SpawnAction createAarJarsMergingActions(
RuleContext ruleContext, Artifact jarsTreeArtifact, Artifact mergedJar, Artifact paramFile) {
RuleContext ruleContext, Artifact jarsTreeArtifact, Artifact mergedJar, Artifact paramFile)
throws RuleErrorException {
SpawnAction.Builder builder = new SpawnAction.Builder().useDefaultShellEnvironment();
FilesToRunProvider singleJar = JavaToolchainProvider.from(ruleContext).getSingleJar();
return builder
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1266,7 +1266,7 @@ public static Artifact createDeployJar(
JavaTargetAttributes attributes,
boolean checkDesugarDeps,
Function<Artifact, Artifact> derivedJarFunction)
throws InterruptedException {
throws InterruptedException, RuleErrorException {

Artifact deployJar =
ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_BINARY_DEPLOY_JAR);
Expand Down Expand Up @@ -1311,7 +1311,7 @@ private static ProguardOutput applyProguard(
@Nullable Artifact startupProfile,
@Nullable Artifact baselineProfile,
String baselineProfileDir)
throws InterruptedException {
throws InterruptedException, RuleErrorException {
Artifact proguardOutputJar =
ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_BINARY_PROGUARD_JAR);

Expand Down Expand Up @@ -2029,7 +2029,7 @@ public static void createTemplatedMergerActions(
}

private static void createZipMergeAction(
RuleContext ruleContext, Artifact inputTree, Artifact outputZip) {
RuleContext ruleContext, Artifact inputTree, Artifact outputZip) throws RuleErrorException {
CustomCommandLine args =
CustomCommandLine.builder()
.add("--normalize")
Expand Down Expand Up @@ -2327,7 +2327,8 @@ private static SpawnAction.Builder createSpawnActionBuilder(RuleContext ruleCont
ruleContext.getRule(), ruleContext.isAllowTagsPropagation()));
}

private static SpawnAction.Builder singleJarSpawnActionBuilder(RuleContext ruleContext) {
private static SpawnAction.Builder singleJarSpawnActionBuilder(RuleContext ruleContext)
throws RuleErrorException {
FilesToRunProvider singleJar = JavaToolchainProvider.from(ruleContext).getSingleJar();
SpawnAction.Builder builder =
createSpawnActionBuilder(ruleContext).useDefaultShellEnvironment();
Expand All @@ -2340,7 +2341,7 @@ private static SpawnAction.Builder singleJarSpawnActionBuilder(RuleContext ruleC
* of the output.
*/
private static void createCleanDexZipAction(
RuleContext ruleContext, Artifact inputZip, Artifact outputZip) {
RuleContext ruleContext, Artifact inputZip, Artifact outputZip) throws RuleErrorException {
ruleContext.registerAction(
singleJarSpawnActionBuilder(ruleContext)
.setProgressMessage("Trimming %s", inputZip.getExecPath().getBaseName())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ private void compileResources(
}

private void packResourceSourceJar(JavaSemantics javaSemantics, Artifact resourcesJavaSrcJar)
throws InterruptedException {
throws InterruptedException, RuleErrorException {

resourceSourceJar =
ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_RESOURCES_SOURCE_JAR);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,8 @@ private static void setUpJavaCommon(
JavaCommon common,
JavaCompilationHelper helper,
JavaCompilationArtifacts javaCompilationArtifacts,
JavaTargetAttributes attributes) {
JavaTargetAttributes attributes)
throws RuleErrorException {
common.setJavaCompilationArtifacts(javaCompilationArtifacts);
common.setClassPathFragment(
new ClasspathConfiguredFragment(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ default Optional<Artifact> maybeDoLegacyManifestMerging(
* <p>These will come after the default options specified by the toolchain, and before the ones in
* the {@code javacopts} attribute.
*/
ImmutableList<String> getCompatibleJavacOptions(RuleContext ruleContext);
ImmutableList<String> getCompatibleJavacOptions(RuleContext ruleContext)
throws RuleErrorException;

/**
* Configures the builder for generating the output jar used to configure the main dex file.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
import com.google.devtools.build.lib.analysis.actions.SpawnAction;
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.Type;
import com.google.devtools.build.lib.rules.android.AndroidConfiguration.ApkSigningMethod;
Expand Down Expand Up @@ -182,7 +183,8 @@ public ApkActionsBuilder setDeterministicSigning(boolean deterministicSigning) {
}

/** Registers the actions needed to build the requested APKs in the rule context. */
public void registerActions(RuleContext ruleContext) throws InterruptedException {
public void registerActions(RuleContext ruleContext)
throws InterruptedException, RuleErrorException {
// If the caller did not request an unsigned APK, we still need to construct one so that
// we can sign it. So we make up an intermediate artifact.
Artifact intermediateUnsignedApk =
Expand Down Expand Up @@ -213,7 +215,8 @@ private void setSingleJarCreatedBy(RuleContext ruleContext, CustomCommandLine.Bu
}

/** Registers generating actions for {@code outApk} that build an unsigned APK using SingleJar. */
private void buildApk(RuleContext ruleContext, Artifact outApk) throws InterruptedException {
private void buildApk(RuleContext ruleContext, Artifact outApk)
throws InterruptedException, RuleErrorException {
Artifact compressedApk = getApkArtifact(ruleContext, "compressed_" + outApk.getFilename());

SpawnAction.Builder compressedApkActionBuilder =
Expand Down Expand Up @@ -417,8 +420,8 @@ private void signApk(
actionBuilder.addCommandLine(commandLine.build()).build(ruleContext));
}

private static void setSingleJarAsExecutable(
RuleContext ruleContext, SpawnAction.Builder builder) {
private static void setSingleJarAsExecutable(RuleContext ruleContext, SpawnAction.Builder builder)
throws RuleErrorException {
FilesToRunProvider singleJar = JavaToolchainProvider.from(ruleContext).getSingleJar();
builder.setExecutable(singleJar);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.packages.AttributeMap;
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.DeployArchiveBuilder;
import com.google.devtools.build.lib.rules.java.JavaConfiguration;
Expand Down Expand Up @@ -349,7 +350,7 @@ public static ProguardOutput createOptimizationActions(
@Nullable Artifact startupProfileIn,
@Nullable Artifact baselineProfileIn,
String baselineProfileDir)
throws InterruptedException {
throws InterruptedException, RuleErrorException {
Preconditions.checkArgument(!proguardSpecs.isEmpty());
Artifact libraryJar = null;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ private static NestedSet<Artifact> getArchiveInputs(
}

/** Builds the action as configured. */
public void build() throws InterruptedException {
public void build() throws InterruptedException, RuleErrorException {
ImmutableList<Artifact> classpathResources = attributes.getClassPathResources();
Set<String> classPathResourceNames = new HashSet<>();
for (Artifact artifact : classpathResources) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,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.collect.nestedset.Order;
import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
import com.google.devtools.build.lib.rules.java.JavaConfiguration.ImportDepsCheckingLevel;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -97,7 +98,7 @@ public ImportDepsCheckActionBuilder transitiveDeps(NestedSet<Artifact> transitiv
return this;
}

public void buildAndRegister(RuleContext ruleContext) {
public void buildAndRegister(RuleContext ruleContext) throws RuleErrorException {
checkNotNull(jarsToCheck);
checkNotNull(bootclasspath);
checkNotNull(declaredDeps);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,8 @@ public static NestedSet<Artifact> computePerPackageData(
return data.build();
}

public static PathFragment getHostJavaExecutable(RuleContext ruleContext) {
public static PathFragment getHostJavaExecutable(RuleContext ruleContext)
throws RuleErrorException {
return JavaRuntimeInfo.forHost(ruleContext).javaBinaryExecPathFragment();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,7 @@ private boolean separateResourceJar(
|| !attributes.getClassPathResources().isEmpty();
}

private ImmutableMap<String, String> getExecutionInfo() {
private ImmutableMap<String, String> getExecutionInfo() throws RuleErrorException {
ImmutableMap.Builder<String, String> executionInfo = ImmutableMap.builder();
ImmutableMap.Builder<String, String> workerInfo = ImmutableMap.builder();
if (javaToolchain.getJavacSupportsWorkers()) {
Expand All @@ -439,7 +439,7 @@ private ImmutableMap<String, String> getExecutionInfo() {
}

/** Returns the bootclasspath explicit set in attributes if present, or else the default. */
public BootClassPathInfo getBootclasspathOrDefault() {
public BootClassPathInfo getBootclasspathOrDefault() throws RuleErrorException {
JavaTargetAttributes attributes = getAttributes();
if (!attributes.getBootClassPath().isEmpty()) {
return attributes.getBootClassPath();
Expand All @@ -449,7 +449,7 @@ public BootClassPathInfo getBootclasspathOrDefault() {
}

/** Adds coverage support from java_toolchain. */
public void addCoverageSupport() {
public void addCoverageSupport() throws RuleErrorException {
FilesToRunProvider jacocoRunner = javaToolchain.getJacocoRunner();
if (jacocoRunner == null) {
ruleContext.ruleError(
Expand Down Expand Up @@ -492,7 +492,7 @@ private boolean shouldInstrumentJar() {
ruleContext.getConfiguration(), ruleContext.getLabel(), ruleContext.isTestTarget());
}

private boolean shouldUseHeaderCompilation() {
private boolean shouldUseHeaderCompilation() throws RuleErrorException {
if (!getJavaConfiguration().useHeaderCompilation()) {
return false;
}
Expand Down Expand Up @@ -538,7 +538,8 @@ private Artifact turbineOutput(Artifact classJar, String newExtension) {
* @param headerJar the jar output of this java compilation
* @param headerDeps the .jdeps output of this java compilation
*/
public void createHeaderCompilationAction(Artifact headerJar, Artifact headerDeps) {
public void createHeaderCompilationAction(Artifact headerJar, Artifact headerDeps)
throws RuleErrorException {

JavaTargetAttributes attributes = getAttributes();

Expand All @@ -560,7 +561,8 @@ public void createHeaderCompilationAction(Artifact headerJar, Artifact headerDep
builder.build(javaToolchain);
}

private JavaHeaderCompileAction.Builder getJavaHeaderCompileActionBuilder() {
private JavaHeaderCompileAction.Builder getJavaHeaderCompileActionBuilder()
throws RuleErrorException {
JavaTargetAttributes attributes = getAttributes();
JavaHeaderCompileAction.Builder builder = JavaHeaderCompileAction.newBuilder(ruleContext);
builder.setSourceFiles(attributes.getSourceFiles());
Expand Down Expand Up @@ -600,10 +602,8 @@ public boolean usesAnnotationProcessing() {
}

private void createGenJarAction(
Artifact classJar,
Artifact manifestProto,
Artifact genClassJar,
JavaRuntimeInfo hostJavabase) {
Artifact classJar, Artifact manifestProto, Artifact genClassJar, JavaRuntimeInfo hostJavabase)
throws RuleErrorException {
getRuleContext()
.registerAction(
new SpawnAction.Builder()
Expand All @@ -628,7 +628,7 @@ private void createGenJarAction(
}

/** Returns the GenClass deploy jar Artifact. */
private Artifact getGenClassJar(RuleContext ruleContext) {
private Artifact getGenClassJar(RuleContext ruleContext) throws RuleErrorException {
Artifact genClass = javaToolchain.getGenClass();
if (genClass != null) {
return genClass;
Expand All @@ -644,7 +644,8 @@ private boolean generatesOutputDeps() {
return getJavaConfiguration().getGenerateJavaDeps() && attributes.hasSources();
}

private void createResourceJarAction(Artifact resourceJar, ImmutableList<Artifact> extraJars) {
private void createResourceJarAction(Artifact resourceJar, ImmutableList<Artifact> extraJars)
throws RuleErrorException {
checkNotNull(resourceJar, "resource jar output must not be null");
JavaTargetAttributes attributes = getAttributes();
new ResourceJarActionBuilder()
Expand All @@ -659,7 +660,8 @@ private void createResourceJarAction(Artifact resourceJar, ImmutableList<Artifac
.build(semantics, ruleContext, execGroup);
}

public void createSourceJarAction(Artifact outputJar, @Nullable Artifact gensrcJar) {
public void createSourceJarAction(Artifact outputJar, @Nullable Artifact gensrcJar)
throws RuleErrorException {
JavaTargetAttributes attributes = getAttributes();
NestedSetBuilder<Artifact> resourceJars = NestedSetBuilder.stableOrder();
resourceJars.addAll(attributes.getSourceJars());
Expand All @@ -682,7 +684,7 @@ public void createSourceJarAction(Artifact outputJar, @Nullable Artifact gensrcJ
* @return the header jar (if requested), or ijar (if requested), or else the class jar
*/
public Artifact createCompileTimeJarAction(
Artifact runtimeJar, JavaCompilationArtifacts.Builder builder) {
Artifact runtimeJar, JavaCompilationArtifacts.Builder builder) throws RuleErrorException {
Artifact jar;
boolean isFullJar = false;
if (shouldUseHeaderCompilation()) {
Expand Down Expand Up @@ -839,7 +841,8 @@ static Artifact createIjarAction(
Artifact inputJar,
@Nullable Label targetLabel,
@Nullable String injectingRuleKind,
boolean addPrefix) {
boolean addPrefix)
throws RuleErrorException {
Artifact interfaceJar = getIjarArtifact(ruleContext, inputJar, addPrefix);
FilesToRunProvider ijarTarget = javaToolchain.getIjar();
if (!ruleContext.hasErrors()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import com.google.devtools.build.lib.collect.nestedset.Order;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible;
import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
import com.google.devtools.build.lib.rules.java.JavaCompileAction.ProgressMessage;
import com.google.devtools.build.lib.rules.java.JavaConfiguration.JavaClasspathMode;
import com.google.devtools.build.lib.rules.java.JavaPluginInfo.JavaPluginData;
Expand Down Expand Up @@ -171,7 +172,7 @@ public JavaCompileActionBuilder(
this.execGroup = execGroup;
}

public JavaCompileAction build() {
public JavaCompileAction build() throws RuleErrorException {
// TODO(bazel-team): all the params should be calculated before getting here, and the various
// aggregation code below should go away.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,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.collect.nestedset.Order;
import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
import com.google.devtools.build.lib.packages.TargetUtils;
import com.google.devtools.build.lib.rules.java.JavaCompileAction.ProgressMessage;
import com.google.devtools.build.lib.rules.java.JavaConfiguration.JavaClasspathMode;
Expand Down Expand Up @@ -338,7 +339,7 @@ public Builder enableDirectClasspath(boolean enableDirectClasspath) {
}

/** Builds and registers the action for a header compilation. */
public void build(JavaToolchainProvider javaToolchain) {
public void build(JavaToolchainProvider javaToolchain) throws RuleErrorException {
checkNotNull(outputDepsProto, "outputDepsProto must not be null");
checkNotNull(sourceFiles, "sourceFiles must not be null");
checkNotNull(sourceJars, "sourceJars must not be null");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.packages.BuiltinProvider;
import com.google.devtools.build.lib.packages.NativeInfo;
import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
import com.google.devtools.build.lib.rules.cpp.CcInfo;
import com.google.devtools.build.lib.rules.cpp.LibraryToLink;
import com.google.devtools.build.lib.starlarkbuildapi.java.JavaRuntimeInfoApi;
Expand Down Expand Up @@ -76,7 +77,7 @@ public boolean isImmutable() {

// Helper methods to access an instance of JavaRuntimeInfo.

public static JavaRuntimeInfo forHost(RuleContext ruleContext) {
public static JavaRuntimeInfo forHost(RuleContext ruleContext) throws RuleErrorException {
return JavaToolchainProvider.from(ruleContext).getJavaRuntime();
}

Expand Down
Loading

0 comments on commit 379ee5f

Please sign in to comment.