Skip to content

Commit

Permalink
Automated rollback of commit 4162cc5.
Browse files Browse the repository at this point in the history 
*** Reason for rollback ***

Broken Bazel CI in Downstream pipeline
#10588

*** Original change description ***

proto_lang_toolchain: Add original sources of ProtoInfo to blacklistedProtos

Fixes #10484

Closes #10493.

PiperOrigin-RevId: 289828390
  • Loading branch information
@meteorcloudy
meteorcloudy authored and Copybara-Service committed Jan 15, 2020
1 parent a928a5f commit 9db93bc
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 115 deletions.
2 changes: 1 addition & 1 deletion src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java
View file
Expand Up @@ -276,7 +276,7 @@ private static class Impl {
private boolean areSrcsBlacklisted() {
return !new ProtoSourceFileBlacklist(
ruleContext, getProtoToolchainProvider().blacklistedProtos())
.checkSrcs(protoInfo.getOriginalTransitiveProtoSources().toList(), "cc_proto_library");
.checkSrcs(protoInfo.getOriginalDirectProtoSources(), "cc_proto_library");
}

private FeatureConfiguration getFeatureConfiguration() {
Expand Down
17 changes: 1 addition & 16 deletions src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCommon.java
View file
Expand Up @@ -127,19 +127,6 @@ private static NestedSet<Artifact> computeTransitiveProtoSources(
return result.build();
}

private static NestedSet<Artifact> computeTransitiveOriginalProtoSources(
RuleContext ruleContext, ImmutableList<Artifact> originalProtoSources) {
NestedSetBuilder<Artifact> result = NestedSetBuilder.naiveLinkOrder();

result.addAll(originalProtoSources);

for (ProtoInfo dep : ruleContext.getPrerequisites("deps", Mode.TARGET, ProtoInfo.PROVIDER)) {
result.addTransitive(dep.getOriginalTransitiveProtoSources());
}

return result.build();
}

static NestedSet<Artifact> computeDependenciesDescriptorSets(RuleContext ruleContext) {
NestedSetBuilder<Artifact> result = NestedSetBuilder.stableOrder();

Expand Down Expand Up @@ -475,8 +462,6 @@ public static ProtoInfo createProtoInfo(

NestedSet<Artifact> transitiveProtoSources =
computeTransitiveProtoSources(ruleContext, library.getSources());
NestedSet<Artifact> transitiveOriginalProtoSources =
computeTransitiveOriginalProtoSources(ruleContext, directProtoSources);
NestedSet<String> transitiveProtoSourceRoots =
computeTransitiveProtoSourceRoots(ruleContext, library.getSourceRoot());

Expand Down Expand Up @@ -506,8 +491,8 @@ public static ProtoInfo createProtoInfo(
ProtoInfo protoInfo =
new ProtoInfo(
library.getSources(),
directProtoSources,
library.getSourceRoot(),
transitiveOriginalProtoSources,
transitiveProtoSources,
transitiveProtoSourceRoots,
strictImportableProtosForDependents,
Expand Down
17 changes: 8 additions & 9 deletions src/main/java/com/google/devtools/build/lib/rules/proto/ProtoInfo.java
View file
Expand Up @@ -54,7 +54,7 @@ public Provider() {
public static final String LEGACY_SKYLARK_NAME = "proto";

private final ImmutableList<Artifact> directProtoSources;
private final NestedSet<Artifact> originalTransitiveProtoSources;
private final ImmutableList<Artifact> originalDirectProtoSources;
private final String directProtoSourceRoot;
private final NestedSet<Artifact> transitiveProtoSources;
private final NestedSet<String> transitiveProtoSourceRoots;
Expand All @@ -71,8 +71,8 @@ public Provider() {
@AutoCodec.Instantiator
public ProtoInfo(
ImmutableList<Artifact> directProtoSources,
ImmutableList<Artifact> originalDirectProtoSources,
String directProtoSourceRoot,
NestedSet<Artifact> originalTransitiveProtoSources,
NestedSet<Artifact> transitiveProtoSources,
NestedSet<String> transitiveProtoSourceRoots,
NestedSet<Artifact> strictImportableProtoSourcesForDependents,
Expand All @@ -86,7 +86,7 @@ public ProtoInfo(
Location location) {
super(PROVIDER, location);
this.directProtoSources = directProtoSources;
this.originalTransitiveProtoSources = originalTransitiveProtoSources;
this.originalDirectProtoSources = originalDirectProtoSources;
this.directProtoSourceRoot = directProtoSourceRoot;
this.transitiveProtoSources = transitiveProtoSources;
this.transitiveProtoSourceRoots = transitiveProtoSourceRoots;
Expand All @@ -103,18 +103,17 @@ public ProtoInfo(

/**
* The proto source files that are used in compiling this {@code proto_library}.
*
* <p>Different from {@link #getOriginalDirectProtoSources()} when a virtual import root is used.
*/
@Override
public ImmutableList<Artifact> getDirectProtoSources() {
return directProtoSources;
}

/**
* The non-virtual transitive proto source files. Different from {@link
* #getTransitiveProtoSources()} if a transitive dependency has {@code import_prefix} or the like.
*/
public NestedSet<Artifact> getOriginalTransitiveProtoSources() {
return originalTransitiveProtoSources;
/** The proto sources of the {@code proto_library} declaring this provider. */
public ImmutableList<Artifact> getOriginalDirectProtoSources() {
return originalDirectProtoSources;
}

/** The source root of the current library. */
Expand Down
4 changes: 1 addition & 3 deletions src/main/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchain.java
View file
Expand Up @@ -44,10 +44,8 @@ public ConfiguredTarget create(RuleContext ruleContext)
ProtoInfo protoInfo = protos.get(ProtoInfo.PROVIDER);
// TODO(cushon): it would be nice to make this mandatory and stop adding files to build too
if (protoInfo != null) {
blacklistedProtos.addTransitive(protoInfo.getOriginalTransitiveProtoSources());
blacklistedProtos.addTransitive(protoInfo.getTransitiveProtoSources());
} else {
// Only add files from FileProvider if |protos| is not a proto_library to avoid adding
// the descriptor_set of proto_library to the list of blacklisted files.
blacklistedProtos.addTransitive(protos.getProvider(FileProvider.class).getFilesToBuild());
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilderTest.java
View file
Expand Up @@ -65,8 +65,8 @@ private ProtoInfo protoInfo(
NestedSet<Pair<Artifact, String>> exportedProtos) {
return new ProtoInfo(
directProtos,
/* directProtoSourceRoot= */ "",
transitiveProtos,
directProtos,
"",
transitiveProtos,
transitiveProtoSourceRoots,
/* strictImportableProtosForDependents */ NestedSetBuilder.emptySet(Order.STABLE_ORDER),
Expand Down
98 changes: 14 additions & 84 deletions src/test/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchainTest.java
View file
Expand Up @@ -39,27 +39,10 @@ public void setUp() throws Exception {
invalidatePackages();
}

private void validateProtoLangToolchain(ProtoLangToolchainProvider toolchain) throws Exception {
assertThat(toolchain.commandLine()).isEqualTo("cmd-line");
assertThat(toolchain.pluginExecutable().getExecutable().getRootRelativePathString())
.isEqualTo("third_party/x/plugin");

TransitiveInfoCollection runtimes = toolchain.runtime();
assertThat(runtimes.getLabel())
.isEqualTo(Label.parseAbsolute("//third_party/x:runtime", ImmutableMap.of()));

assertThat(prettyArtifactNames(toolchain.blacklistedProtos()))
.containsExactly(
"third_party/x/metadata.proto",
"third_party/x/descriptor.proto",
"third_party/x/any.proto");
}

@Test
public void protoToolchain() throws Exception {
scratch.file(
"third_party/x/BUILD",
"licenses(['unencumbered'])",
"x/BUILD",
"cc_binary(name = 'plugin', srcs = ['plugin.cc'])",
"cc_library(name = 'runtime', srcs = ['runtime.cc'])",
"filegroup(name = 'descriptors', srcs = ['metadata.proto', 'descriptor.proto'])",
Expand All @@ -69,82 +52,29 @@ public void protoToolchain() throws Exception {
scratch.file(
"foo/BUILD",
TestConstants.LOAD_PROTO_LANG_TOOLCHAIN,
"licenses(['unencumbered'])",
"proto_lang_toolchain(",
" name = 'toolchain',",
" command_line = 'cmd-line',",
" plugin = '//third_party/x:plugin',",
" runtime = '//third_party/x:runtime',",
" blacklisted_protos = ['//third_party/x:blacklist']",
" plugin = '//x:plugin',",
" runtime = '//x:runtime',",
" blacklisted_protos = ['//x:blacklist']",
")");

update(ImmutableList.of("//foo:toolchain"), false, 1, true, new EventBus());

validateProtoLangToolchain(
getConfiguredTarget("//foo:toolchain").getProvider(ProtoLangToolchainProvider.class));
}

@Test
public void protoToolchainBlacklistProtoLibraries() throws Exception {
scratch.file(
"third_party/x/BUILD",
TestConstants.LOAD_PROTO_LIBRARY,
"licenses(['unencumbered'])",
"cc_binary(name = 'plugin', srcs = ['plugin.cc'])",
"cc_library(name = 'runtime', srcs = ['runtime.cc'])",
"proto_library(name = 'descriptors', srcs = ['metadata.proto', 'descriptor.proto'])",
"proto_library(name = 'any', srcs = ['any.proto'], strip_import_prefix = '/third_party')");

scratch.file(
"foo/BUILD",
TestConstants.LOAD_PROTO_LANG_TOOLCHAIN,
"proto_lang_toolchain(",
" name = 'toolchain',",
" command_line = 'cmd-line',",
" plugin = '//third_party/x:plugin',",
" runtime = '//third_party/x:runtime',",
" blacklisted_protos = ['//third_party/x:descriptors', '//third_party/x:any']",
")");

update(ImmutableList.of("//foo:toolchain"), false, 1, true, new EventBus());

validateProtoLangToolchain(
getConfiguredTarget("//foo:toolchain").getProvider(ProtoLangToolchainProvider.class));
}

@Test
public void protoToolchainMixedBlacklist() throws Exception {
scratch.file(
"third_party/x/BUILD",
TestConstants.LOAD_PROTO_LIBRARY,
"licenses(['unencumbered'])",
"cc_binary(name = 'plugin', srcs = ['plugin.cc'])",
"cc_library(name = 'runtime', srcs = ['runtime.cc'])",
"proto_library(name = 'metadata', srcs = ['metadata.proto'])",
"proto_library(",
" name = 'descriptor',",
" srcs = ['descriptor.proto'],",
" strip_import_prefix = '/third_party')",
"filegroup(name = 'any', srcs = ['any.proto'])");
ProtoLangToolchainProvider toolchain =
getConfiguredTarget("//foo:toolchain").getProvider(ProtoLangToolchainProvider.class);

scratch.file(
"foo/BUILD",
TestConstants.LOAD_PROTO_LANG_TOOLCHAIN,
"proto_lang_toolchain(",
" name = 'toolchain',",
" command_line = 'cmd-line',",
" plugin = '//third_party/x:plugin',",
" runtime = '//third_party/x:runtime',",
" blacklisted_protos = [",
" '//third_party/x:metadata',",
" '//third_party/x:descriptor',",
" '//third_party/x:any']",
")");
assertThat(toolchain.commandLine()).isEqualTo("cmd-line");
assertThat(toolchain.pluginExecutable().getExecutable().getRootRelativePathString())
.isEqualTo("x/plugin");

update(ImmutableList.of("//foo:toolchain"), false, 1, true, new EventBus());
TransitiveInfoCollection runtimes = toolchain.runtime();
assertThat(runtimes.getLabel())
.isEqualTo(Label.parseAbsolute("//x:runtime", ImmutableMap.of()));

validateProtoLangToolchain(
getConfiguredTarget("//foo:toolchain").getProvider(ProtoLangToolchainProvider.class));
assertThat(prettyArtifactNames(toolchain.blacklistedProtos()))
.containsExactly("x/metadata.proto", "x/descriptor.proto", "x/any.proto");
}

@Test
Expand Down

0 comments on commit 9db93bc

Please sign in to comment.