Skip to content

Commit

Permalink
Automated g4 rollback of commit da56606.
Browse files Browse the repository at this point in the history
NO_SQ=Only failing TAP targets in our project are from an unrelated bug.

*** Reason for rollback ***

Initially we thought this broke just a few builds that were fixed in the depot. The scope is wider than that, affecting at least these two targets:

[]

[]

There are hundreds of other failures in the TGP results that we haven't traced to a cause yet, but it's troubling that this CL was implicated twice in the small sampling I did.

[]

*** Original change description ***

Use JavaCompilationArtifacts instead of JavaCompilationArgs

to store Java compilation artifacts. Round-tripping through
JavaCompilationArgs loses dependency information that upstream
compilations use to optimize compile time classpaths.

PiperOrigin-RevId: 159154375
  • Loading branch information
brandjon authored and meteorcloudy committed Jun 16, 2017
1 parent 8effaa7 commit 68b9a7e
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration.StrictDepsMode;
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.rules.java.JavaConfiguration.JavaClasspathMode;
import com.google.devtools.build.lib.util.Preconditions;
import java.util.ArrayList;
Expand Down Expand Up @@ -142,8 +140,10 @@ public JavaLibraryHelper setCompilationStrictDepsMode(StrictDepsMode strictDepsM
return this;
}

/** Creates the compile actions. */
public JavaCompilationArtifacts build(
/**
* Creates the compile actions.
*/
public JavaCompilationArgs build(
JavaSemantics semantics,
JavaToolchainProvider javaToolchainProvider,
NestedSet<Artifact> hostJavabase,
Expand Down Expand Up @@ -184,7 +184,7 @@ public JavaCompilationArtifacts build(
helper.createCompileTimeJarAction(output, artifactsBuilder);
artifactsBuilder.addRuntimeJar(output);

return artifactsBuilder.build();
return JavaCompilationArgs.builder().merge(artifactsBuilder.build()).build();
}

/**
Expand All @@ -199,19 +199,15 @@ public JavaCompilationArtifacts build(
* compilation. Contrast this with {@link #setCompilationStrictDepsMode}.
*/
public JavaCompilationArgsProvider buildCompilationArgsProvider(
JavaCompilationArtifacts artifacts, boolean isReportedAsStrict) {
JavaCompilationArgs directArgs = JavaCompilationArgs.builder().merge(artifacts).build();
JavaCompilationArgs directArgs, boolean isReportedAsStrict) {
JavaCompilationArgs transitiveArgs =
JavaCompilationArgs.builder()
.addTransitiveArgs(directArgs, BOTH)
.addTransitiveDependencies(deps, true /* recursive */)
.build();

return JavaCompilationArgsProvider.create(
isReportedAsStrict ? directArgs : transitiveArgs,
transitiveArgs,
NestedSetBuilder.create(Order.STABLE_ORDER, artifacts.getCompileTimeDependencyArtifact()),
NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER));
isReportedAsStrict ? directArgs : transitiveArgs, transitiveArgs);
}

private void addDepsToAttributes(JavaTargetAttributes.Builder attributes) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ public JavaProvider createJavaCompileAction(
: hostJavabaseProvider.getMiddlemanArtifact();
JavaToolchainProvider javaToolchainProvider =
checkNotNull(javaToolchain.getProvider(JavaToolchainProvider.class));
JavaCompilationArtifacts artifacts =
JavaCompilationArgs artifacts =
helper.build(
javaSemantics,
javaToolchainProvider,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ public static ProtoJavaApiInfoProvider create(
JavaCompilationArgs transitiveJavaCompilationArgss1,
JavaCompilationArgs transitiveJavaCompilationArgssMutable,
JavaCompilationArgs transitiveJavaCompilationArgssImmutable,
JavaCompilationArtifacts javaCompilationArgs1,
JavaCompilationArtifacts javaCompilationArgsMutable,
JavaCompilationArtifacts javaCompilationArgsImmutable,
JavaCompilationArgs javaCompilationArgs1,
JavaCompilationArgs javaCompilationArgsMutable,
JavaCompilationArgs javaCompilationArgsImmutable,
Artifact sourceJar1,
Artifact sourceJarMutable,
Artifact sourceJarImmutable,
Expand Down Expand Up @@ -111,20 +111,22 @@ public static ProtoJavaApiInfoProvider create(
*/
public abstract JavaCompilationArgs getTransitiveJavaCompilationArgsImmutable();

/** Returns the artifacts for java compilation (API version 1) for only this target. */
public abstract JavaCompilationArtifacts getJavaCompilationArtifacts1();
/**
* Returns the artifacts for java compilation (API version 1) for only this target.
*/
public abstract JavaCompilationArgs getJavaCompilationArgs1();

/**
* Returns the artifacts for java compilation (API version 2, code for mutable API) for only this
* target.
* Returns the artifacts for java compilation (API version 2, code for mutable API)
* for only this target.
*/
public abstract JavaCompilationArtifacts getJavaCompilationArtifactsMutable();
public abstract JavaCompilationArgs getJavaCompilationArgsMutable();

/**
* Returns the artifacts for java compilation (API version 2, code for immutable API) for only
* this target.
* Returns the artifacts for java compilation (API version 2, code for immutable API)
* for only this target.
*/
public abstract JavaCompilationArtifacts getJavaCompilationArtifactsImmutable();
public abstract JavaCompilationArgs getJavaCompilationArgsImmutable();

// The following 3 fields are the -src.jar artifact created by proto_library. If a certain
// proto_library does not produce some artifact, it'll be null. This can happen for example when
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import com.google.devtools.build.lib.collect.nestedset.Order;
import com.google.devtools.build.lib.rules.java.JavaCompilationArgs;
import com.google.devtools.build.lib.rules.java.JavaCompilationArgsProvider;
import com.google.devtools.build.lib.rules.java.JavaCompilationArtifacts;
import com.google.devtools.build.lib.rules.java.JavaRuleOutputJarsProvider;
import com.google.devtools.build.lib.rules.java.JavaSkylarkApiProvider;
import com.google.devtools.build.lib.rules.java.JavaSourceJarsProvider;
Expand All @@ -53,7 +52,7 @@ public static boolean reuseExistingActions(
return false;
}

JavaCompilationArtifacts directJars = javaApi.getJavaCompilationArtifactsImmutable();
JavaCompilationArgs directJars = javaApi.getJavaCompilationArgsImmutable();
if (isEmpty(directJars.getCompileTimeJars()) || javaApi.sourceJarImmutable() == null) {
return false;
}
Expand All @@ -62,7 +61,7 @@ public static boolean reuseExistingActions(
JavaCompilationArgs.builder()
.addTransitiveArgs(javaApi.getTransitiveJavaCompilationArgsImmutable(), BOTH)
.addTransitiveDependencies(javaApi.getProtoRuntimeImmutable(), true /* recursive */)
.merge(directJars)
.addTransitiveArgs(directJars, BOTH)
.build();

Artifact outputJar = getOnlyElement(directJars.getRuntimeJars());
Expand All @@ -71,10 +70,9 @@ public static boolean reuseExistingActions(

JavaCompilationArgsProvider compilationArgsProvider =
JavaCompilationArgsProvider.create(
JavaCompilationArgs.builder().merge(directJars).build(),
directJars,
transitiveJars,
NestedSetBuilder.create(
Order.STABLE_ORDER, directJars.getCompileTimeDependencyArtifact()),
NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER),
NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER));

JavaSkylarkApiProvider.Builder skylarkApiProvider =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@
import com.google.devtools.build.lib.packages.AttributeMap;
import com.google.devtools.build.lib.packages.NativeAspectClass;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.rules.java.JavaCompilationArgs;
import com.google.devtools.build.lib.rules.java.JavaCompilationArgsProvider;
import com.google.devtools.build.lib.rules.java.JavaCompilationArtifacts;
import com.google.devtools.build.lib.rules.java.JavaCompilationHelper;
import com.google.devtools.build.lib.rules.java.JavaConfiguration;
import com.google.devtools.build.lib.rules.java.JavaHelper;
Expand Down Expand Up @@ -255,7 +255,7 @@ private JavaCompilationArgsProvider createJavaCompileAction(
helper.addDep(runtime.getProvider(JavaCompilationArgsProvider.class));
}
helper.setCompilationStrictDepsMode(StrictDepsMode.OFF);
JavaCompilationArtifacts artifacts =
JavaCompilationArgs artifacts =
helper.build(
javaSemantics,
JavaCompilationHelper.getJavaToolchainProvider(ruleContext),
Expand Down

0 comments on commit 68b9a7e

Please sign in to comment.