Skip to content

Commit

Permalink
Add a flag to suppress hidden output groups from BEP
Browse files Browse the repository at this point in the history
We don't want to report the hidden output groups in BEP, especially not
the _hidden_top_level_INTERNAL_, which contains a lot of files that are
not intended for direct use, and that can cause a large amount of
network traffic uploading files as well as BEP events.

Unfortunately, it looks like there are already users who depend on the
hidden output groups, so we have to take this slowly.

This adds a flag --experimental_bep_report_only_important_artifacts to
only report important output groups. Note that output groups are
considered unimportant if the name starts with an underscore '_'
character.

PiperOrigin-RevId: 249783465
  • Loading branch information
ulfjack authored and copybara-github committed May 24, 2019
1 parent 3ae9f43 commit ef7290f
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import com.google.devtools.build.lib.actions.EventReportingArtifacts;
import com.google.devtools.build.lib.analysis.TopLevelArtifactHelper.ArtifactsInOutputGroup;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.config.CoreOptions;
import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
import com.google.devtools.build.lib.analysis.test.InstrumentedFilesInfo;
import com.google.devtools.build.lib.analysis.test.TestConfiguration;
Expand Down Expand Up @@ -114,6 +115,7 @@ public Artifact getExecutable() {
private final BuildEventId configEventId;
private final Iterable<String> tags;
private final ExecutableTargetData executableTargetData;
private final boolean bepReportOnlyImportantArtifacts;

private TargetCompleteEvent(
ConfiguredTargetAndData targetAndData,
Expand Down Expand Up @@ -152,6 +154,11 @@ private TargetCompleteEvent(
isTest
? targetAndData.getConfiguredTarget().getProvider(TestProvider.class).getTestParams()
: null;
// It should be safe to set this to true for targets that don't have a configuration - they
// should not have any output files either.
this.bepReportOnlyImportantArtifacts =
configuration == null
|| configuration.getOptions().get(CoreOptions.class).bepReportOnlyImportantArtifacts;
InstrumentedFilesInfo instrumentedFilesProvider =
targetAndData.getConfiguredTarget().get(InstrumentedFilesInfo.SKYLARK_CONSTRUCTOR);
if (instrumentedFilesProvider == null) {
Expand Down Expand Up @@ -361,7 +368,9 @@ public Collection<BuildEventId> postedAfter() {
public ReportedArtifacts reportedArtifacts() {
ImmutableSet.Builder<NestedSet<Artifact>> builder = ImmutableSet.builder();
for (ArtifactsInOutputGroup artifactsInGroup : outputs) {
builder.add(artifactsInGroup.getArtifacts());
if (!bepReportOnlyImportantArtifacts || artifactsInGroup.areImportant()) {
builder.add(artifactsInGroup.getArtifacts());
}
}
if (baselineCoverageArtifacts != null) {
builder.add(baselineCoverageArtifacts);
Expand All @@ -381,6 +390,9 @@ private Iterable<String> getTags() {
private Iterable<OutputGroup> getOutputFilesByGroup(ArtifactGroupNamer namer) {
ImmutableList.Builder<OutputGroup> groups = ImmutableList.builder();
for (ArtifactsInOutputGroup artifactsInOutputGroup : outputs) {
if (bepReportOnlyImportantArtifacts && !artifactsInOutputGroup.areImportant()) {
continue;
}
OutputGroup.Builder groupBuilder = OutputGroup.newBuilder();
groupBuilder.setName(artifactsInOutputGroup.getOutputGroup());
groupBuilder.addFileSets(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,15 @@ public class CoreOptions extends FragmentOptions implements Cloneable {
+ "target_environment values.")
public Label autoCpuEnvironmentGroup;

@Option(
name = "experimental_bep_report_only_important_artifacts",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.OUTPUT_PARAMETERS,
effectTags = {OptionEffectTag.AFFECTS_OUTPUTS, OptionEffectTag.LOADING_AND_ANALYSIS},
metadataTags = {OptionMetadataTag.EXPERIMENTAL},
help = "If false, the BEP no longer contains information about hidden output groups.")
public boolean bepReportOnlyImportantArtifacts;

/** Values for --experimental_dynamic_configs. */
public enum ConfigsMode {
/**
Expand Down
18 changes: 18 additions & 0 deletions src/test/shell/integration/build_event_stream_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -936,4 +936,22 @@ function test_server_pid() {
rm bep.txt
}

function test_bep_report_all_artifacts() {
bazel build --test_output=all --build_event_text_file=bep.txt \
--experimental_bep_report_only_important_artifacts=false //pkg:true \
|| fail "Build failed but should have succeeded"
cat bep.txt >> "$TEST_log"
expect_log "_hidden_top_level_INTERNAL_"
rm bep.txt
}

function test_bep_report_only_important_artifacts() {
bazel build --test_output=all --build_event_text_file=bep.txt \
--experimental_bep_report_only_important_artifacts=true //pkg:true \
|| fail "Build failed but should have succeeded"
cat bep.txt >> "$TEST_log"
expect_not_log "_hidden_top_level_INTERNAL_"
rm bep.txt
}

run_suite "Integration tests for the build event stream"

0 comments on commit ef7290f

Please sign in to comment.