Skip to content

Commit

Permalink
Rollback of commit a396b07.
Browse files Browse the repository at this point in the history
*** Reason for rollback ***

Breaks tests on latest, as proto_lang_toolchain didn't make it into the 0.4.0 release

see http://ci.bazel.io/job/bazel-tests/306/BAZEL_VERSION=latest,PLATFORM_NAME=linux-x86_64/console

*** Original change description ***

Use proto_lang_toolchain() in java_proto_library.

--
MOS_MIGRATED_REVID=138372522
  • Loading branch information
aehlig committed Nov 7, 2016
1 parent f2d3cac commit b043faf
Show file tree
Hide file tree
Showing 9 changed files with 21 additions and 163 deletions.
1 change: 0 additions & 1 deletion WORKSPACE
Expand Up @@ -52,7 +52,6 @@ bind(name = "xcrunwrapper", actual = "@bazel_tools//tools/objc:xcrunwrapper")

bind(name = "protobuf/java_runtime", actual = "//third_party/protobuf:protobuf")
bind(name = "protobuf/javalite_runtime", actual = "//third_party/protobuf:protobuf-lite")
bind(name = "proto/toolchains/java", actual = "//third_party/protobuf:java_toolchain")

# For Skylark tests at //src/test/shell/bazel:maven_skylark_test
# Uncomment the following lines, and the test in src/test/shell/bazel/BUILD to run it.
Expand Down
1 change: 0 additions & 1 deletion src/main/java/com/google/devtools/build/lib/BUILD
Expand Up @@ -812,7 +812,6 @@ java_library(
srcs = ["rules/java/proto/RpcSupport.java"],
deps = [
":build-base",
":collect",
":java-compilation",
":packages-internal",
":proto-rules",
Expand Down
Expand Up @@ -14,21 +14,15 @@

package com.google.devtools.build.lib.bazel.rules.java.proto;

import static com.google.devtools.build.lib.collect.nestedset.Order.STABLE_ORDER;

import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.bazel.rules.java.BazelJavaSemantics;
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.AspectDefinition;
import com.google.devtools.build.lib.packages.AspectParameters;
import com.google.devtools.build.lib.rules.java.JavaLibraryHelper;
import com.google.devtools.build.lib.rules.java.proto.JavaProtoAspect;
import com.google.devtools.build.lib.rules.java.proto.RpcSupport;
import com.google.devtools.build.lib.rules.proto.ProtoCompileActionBuilder;
import java.util.List;

/** An Aspect which BazelJavaProtoLibrary injects to build Java SPEED protos. */
public class BazelJavaProtoAspect extends JavaProtoAspect {
Expand All @@ -43,24 +37,8 @@ public BazelJavaProtoAspect() {
private static class NoopRpcSupport
implements RpcSupport {
@Override
public List<ProtoCompileActionBuilder.ToolchainInvocation> getToolchainInvocation(
RuleContext ruleContext, Artifact sourceJar) {
return ImmutableList.of();
}

@Override
public boolean allowServices(RuleContext ruleContext) {
return true;
}

@Override
public NestedSet<Artifact> getBlacklist(RuleContext ruleContext) {
return NestedSetBuilder.emptySet(STABLE_ORDER);
}

@Override
public void mutateProtoCompileAction(RuleContext ruleContext, Artifact sourceJar,
ProtoCompileActionBuilder actionBuilder) {
public void mutateProtoCompileAction(
RuleContext ruleContext, Artifact sourceJar, ProtoCompileActionBuilder actionBuilder) {
// Intentionally left empty.
}

Expand Down
Expand Up @@ -33,7 +33,6 @@
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
import com.google.devtools.build.lib.analysis.TransitiveInfoProviderMap;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
Expand All @@ -55,9 +54,7 @@
import com.google.devtools.build.lib.rules.java.JavaSemantics;
import com.google.devtools.build.lib.rules.java.JavaSourceJarsProvider;
import com.google.devtools.build.lib.rules.proto.ProtoCompileActionBuilder;
import com.google.devtools.build.lib.rules.proto.ProtoCompileActionBuilder.ToolchainInvocation;
import com.google.devtools.build.lib.rules.proto.ProtoConfiguration;
import com.google.devtools.build.lib.rules.proto.ProtoLangToolchainProvider;
import com.google.devtools.build.lib.rules.proto.ProtoSourceFileBlacklist;
import com.google.devtools.build.lib.rules.proto.ProtoSourcesProvider;
import com.google.devtools.build.lib.rules.proto.ProtoSupportDataProvider;
Expand All @@ -68,8 +65,6 @@
/** An Aspect which JavaProtoLibrary injects to build Java SPEED protos. */
public class JavaProtoAspect extends NativeAspectClass implements ConfiguredAspectFactory {

private static final String SPEED_PROTO_TOOLCHAIN_ATTR = ":aspect_java_proto_toolchain";

private static final String SPEED_PROTO_RUNTIME_ATTR = "$aspect_java_lib";
private static final String SPEED_PROTO_RUNTIME_LABEL = "//external:protobuf/java_runtime";

Expand All @@ -96,15 +91,6 @@ public boolean useHostConfiguration() {
}
};

private static final Attribute.LateBoundLabel<BuildConfiguration> SPEED_PROTO_TOOLCHAIN_LABEL =
new Attribute.LateBoundLabel<BuildConfiguration>(
"//tools/proto/toolchains:java", ProtoConfiguration.class) {
@Override
public Label resolve(Rule rule, AttributeMap attributes, BuildConfiguration configuration) {
return configuration.getFragment(ProtoConfiguration.class).protoToolchainForJava();
}
};

private final JavaSemantics javaSemantics;

@Nullable private final String jacocoLabel;
Expand Down Expand Up @@ -142,8 +128,7 @@ public ConfiguredAspect create(
.getFragment(ProtoConfiguration.class, ConfigurationTransition.HOST)
.protoCompilerJavaFlags(),
javaSemantics,
rpcSupport,
ruleContext.getFragment(ProtoConfiguration.class).useToolchainForJavaProto())
rpcSupport)
.createProviders());

return aspect.build();
Expand All @@ -164,12 +149,6 @@ public AspectDefinition getDefinition(AspectParameters aspectParameters) {
attr(PROTO_SOURCE_FILE_BLACKLIST_ATTR, LABEL_LIST)
.cfg(HOST)
.value(BLACKLISTED_PROTOS))
.add(
attr(SPEED_PROTO_TOOLCHAIN_ATTR, LABEL)
.mandatoryNativeProviders(
ImmutableList.<Class<? extends TransitiveInfoProvider>>of(
ProtoLangToolchainProvider.class))
.value(SPEED_PROTO_TOOLCHAIN_LABEL))
.add(attr(":host_jdk", LABEL).cfg(HOST).value(JavaSemantics.HOST_JDK))
.add(
attr(":java_toolchain", LABEL)
Expand Down Expand Up @@ -200,21 +179,18 @@ private static class Impl {
*/
private final JavaCompilationArgsProvider dependencyCompilationArgs;
private final String protoCompilerPluginOptions;
private final boolean useToolchainForJavaProto;

Impl(
final RuleContext ruleContext,
final SupportData supportData,
String protoCompilerPluginOptions,
JavaSemantics javaSemantics,
RpcSupport rpcSupport,
boolean useToolchainForJavaProto) {
RpcSupport rpcSupport) {
this.ruleContext = ruleContext;
this.supportData = supportData;
this.protoCompilerPluginOptions = protoCompilerPluginOptions;
this.javaSemantics = javaSemantics;
this.rpcSupport = rpcSupport;
this.useToolchainForJavaProto = useToolchainForJavaProto;

dependencyCompilationArgs =
JavaCompilationArgsProvider.merge(
Expand Down Expand Up @@ -274,49 +250,24 @@ private boolean shouldGenerateCode() {
return false;
}

final ProtoSourceFileBlacklist protoBlackList;
if (useToolchainForJavaProto) {
NestedSetBuilder<Artifact> blacklistedProtos = NestedSetBuilder.stableOrder();
blacklistedProtos.addTransitive(getProtoToolchainProvider().blacklistedProtos());
blacklistedProtos.addTransitive(rpcSupport.getBlacklist(ruleContext));

protoBlackList = new ProtoSourceFileBlacklist(ruleContext, blacklistedProtos.build());
} else {
protoBlackList =
new ProtoSourceFileBlacklist(
ruleContext,
ruleContext
.getPrerequisiteArtifacts(PROTO_SOURCE_FILE_BLACKLIST_ATTR, Mode.HOST)
.list());
}

ProtoSourceFileBlacklist protoBlackList =
new ProtoSourceFileBlacklist(
ruleContext,
ruleContext
.getPrerequisiteArtifacts(PROTO_SOURCE_FILE_BLACKLIST_ATTR, Mode.HOST)
.list());
return protoBlackList.checkSrcs(supportData.getDirectProtoSources(), "java_proto_library");
}

private void createProtoCompileAction(Artifact sourceJar) {
if (useToolchainForJavaProto) {
ImmutableList.Builder<ToolchainInvocation> invocations = ImmutableList.builder();
invocations.add(
new ToolchainInvocation(
"java", checkNotNull(getProtoToolchainProvider()), sourceJar.getExecPathString()));
invocations.addAll(rpcSupport.getToolchainInvocation(ruleContext, sourceJar));
ProtoCompileActionBuilder.registerActions(
ruleContext,
invocations.build(),
supportData,
ImmutableList.of(sourceJar),
"Java (Immutable)",
rpcSupport.allowServices(ruleContext));
} else {
ProtoCompileActionBuilder actionBuilder =
new ProtoCompileActionBuilder(
ruleContext, supportData, "Java", "java", ImmutableList.of(sourceJar))
.allowServices(true)
.setLangParameter(
String.format(protoCompilerPluginOptions, sourceJar.getExecPathString()));
rpcSupport.mutateProtoCompileAction(ruleContext, sourceJar, actionBuilder);
ruleContext.registerAction(actionBuilder.build());
}
ProtoCompileActionBuilder actionBuilder =
new ProtoCompileActionBuilder(
ruleContext, supportData, "Java", "java", ImmutableList.of(sourceJar))
.allowServices(true)
.setLangParameter(
String.format(protoCompilerPluginOptions, sourceJar.getExecPathString()));
rpcSupport.mutateProtoCompileAction(ruleContext, sourceJar, actionBuilder);
ruleContext.registerAction(actionBuilder.build());
}

private JavaCompilationArgsProvider createJavaCompileAction(
Expand All @@ -328,28 +279,15 @@ private JavaCompilationArgsProvider createJavaCompileAction(
.setJavacOpts(ProtoJavacOpts.constructJavacOpts(ruleContext));
helper
.addDep(dependencyCompilationArgs)
.addDep(
ruleContext.getPrerequisite(
SPEED_PROTO_RUNTIME_ATTR, Mode.TARGET, JavaCompilationArgsProvider.class))
.setCompilationStrictDepsMode(StrictDepsMode.OFF);
if (useToolchainForJavaProto) {
TransitiveInfoCollection runtime = getProtoToolchainProvider().runtime();
if (runtime != null) {
helper.addDep(runtime.getProvider(JavaCompilationArgsProvider.class));
}
} else {
helper.addDep(
ruleContext.getPrerequisite(
SPEED_PROTO_RUNTIME_ATTR, Mode.TARGET, JavaCompilationArgsProvider.class));
}

rpcSupport.mutateJavaCompileAction(ruleContext, helper);
return helper.buildCompilationArgsProvider(
helper.build(javaSemantics), true /* isReportedAsStrict */);
}

private ProtoLangToolchainProvider getProtoToolchainProvider() {
return ruleContext.getPrerequisite(
SPEED_PROTO_TOOLCHAIN_ATTR, TARGET, ProtoLangToolchainProvider.class);
}

private Artifact getSourceJarArtifact() {
return ruleContext.getGenfilesArtifact(ruleContext.getLabel().getName() + "-speed-src.jar");
}
Expand Down
Expand Up @@ -16,24 +16,15 @@

import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.packages.AspectDefinition;
import com.google.devtools.build.lib.packages.AspectParameters;
import com.google.devtools.build.lib.rules.java.JavaLibraryHelper;
import com.google.devtools.build.lib.rules.proto.ProtoCompileActionBuilder;
import java.util.List;

/**
* Used by java_proto_library to support Google-specific features.
*/
public interface RpcSupport {
List<ProtoCompileActionBuilder.ToolchainInvocation> getToolchainInvocation(
RuleContext ruleContext, Artifact sourceJar);

boolean allowServices(RuleContext ruleContext);

NestedSet<Artifact> getBlacklist(RuleContext ruleContext);

void mutateProtoCompileAction(
RuleContext ruleContext, Artifact sourceJar, ProtoCompileActionBuilder actionBuilder);

Expand Down
Expand Up @@ -113,26 +113,6 @@ public static class Options extends FragmentOptions {
)
public Label protoToolchainForJavaLite;

@Option(
name = "proto_toolchain_for_java",
defaultValue = "//tools/proto/toolchains:java",
category = "flags",
converter = BuildConfiguration.EmptyToNullLabelConverter.class,
help = "Label of proto_lang_toolchain() which describes how to compile Java protos"
)
public Label protoToolchainForJava;

@Option(
name = "use_toolchain_for_java_proto",
defaultValue = "false",
category = "experimental",
help =
"If true, --proto_toolchain_for_java will be used for java_proto_library. "
+ "This flag is an escape-hatch and should be removed once toolchain-based builds "
+ "are tested."
)
public boolean useToolchainForJavaProto;

@Override
public FragmentOptions getHost(boolean fallback) {
Options host = (Options) super.getHost(fallback);
Expand All @@ -144,9 +124,7 @@ public FragmentOptions getHost(boolean fallback) {
host.protoCompilerJavaBlacklistedProtos = protoCompilerJavaBlacklistedProtos;
host.protoCompilerJavaLiteFlags = protoCompilerJavaLiteFlags;
host.protoCompilerJavaLitePlugin = protoCompilerJavaLitePlugin;
host.protoToolchainForJava = protoToolchainForJava;
host.protoToolchainForJavaLite = protoToolchainForJavaLite;
host.useToolchainForJavaProto = useToolchainForJavaProto;
return host;
}
}
Expand Down Expand Up @@ -179,9 +157,7 @@ public ImmutableSet<Class<? extends FragmentOptions>> requiredOptions() {
private final List<Label> protoCompilerJavaBlacklistedProtos;
private final String protoCompilerJavaLiteFlags;
private final Label protoCompilerJavaLitePlugin;
private final Label protoToolchainForJava;
private final Label protoToolchainForJavaLite;
private final boolean useToolchainForJavaProto;

public ProtoConfiguration(Options options) {
this.experimentalProtoExtraActions = options.experimentalProtoExtraActions;
Expand All @@ -191,9 +167,7 @@ public ProtoConfiguration(Options options) {
this.protoCompilerJavaLiteFlags = options.protoCompilerJavaLiteFlags;
this.protoCompilerJavaLitePlugin = options.protoCompilerJavaLitePlugin;
this.protoCompilerJavaBlacklistedProtos = options.protoCompilerJavaBlacklistedProtos;
this.protoToolchainForJava = options.protoToolchainForJava;
this.protoToolchainForJavaLite = options.protoToolchainForJavaLite;
this.useToolchainForJavaProto = options.useToolchainForJavaProto;
}

public ImmutableList<String> protocOpts() {
Expand Down Expand Up @@ -229,15 +203,7 @@ public List<Label> protoCompilerJavaBlacklistedProtos() {
return protoCompilerJavaBlacklistedProtos;
}

public Label protoToolchainForJava() {
return protoToolchainForJava;
}

public Label protoToolchainForJavaLite() {
return protoToolchainForJavaLite;
}

public boolean useToolchainForJavaProto() {
return useToolchainForJavaProto;
}
}
6 changes: 0 additions & 6 deletions third_party/protobuf/3.0.0/BUILD
Expand Up @@ -311,9 +311,3 @@ cc_library(
visibility = ["//visibility:public"],
deps = [":protobuf_clib"],
)

proto_lang_toolchain(
name = "java_toolchain",
command_line = "--java_out=shared,immutable:$(OUT)",
runtime = ":protobuf",
)
2 changes: 0 additions & 2 deletions third_party/protobuf/BUILD
Expand Up @@ -29,5 +29,3 @@ proto_alias("protobuf_lite", PROTOBUF_VERSION)
proto_alias("protobuf_clib", PROTOBUF_VERSION)

proto_alias("protoc_lib", PROTOBUF_VERSION)

proto_alias("java_toolchain", PROTOBUF_VERSION)
5 changes: 0 additions & 5 deletions tools/proto/toolchains/BUILD
Expand Up @@ -5,11 +5,6 @@ alias(
actual = "//external:proto/toolchains/javalite",
)

alias(
name = "java",
actual = "//external:proto/toolchains/java",
)

filegroup(
name = "srcs",
srcs = [
Expand Down

0 comments on commit b043faf

Please sign in to comment.