diff options
18 files changed, 946 insertions, 40 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD index c2848dbf3e..9dfb06ba1e 100644 --- a/src/main/java/com/google/devtools/build/lib/BUILD +++ b/src/main/java/com/google/devtools/build/lib/BUILD @@ -1007,10 +1007,12 @@ java_library( "//src/main/protobuf:action_cache_java_proto", "//src/main/protobuf:bazel_flags_java_proto", "//src/main/protobuf:build_java_proto", + "//src/main/protobuf:command_line_java_proto", "//src/main/protobuf:command_server_java_grpc", "//src/main/protobuf:command_server_java_proto", "//src/main/protobuf:extra_actions_base_java_proto", "//src/main/protobuf:invocation_policy_java_proto", + "//src/main/protobuf:option_filters_java_proto", "//src/main/protobuf:test_status_java_proto", "//third_party:guava", "//third_party:jsr305", diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventId.java b/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventId.java index 1e73efc3db..2479b59b3d 100644 --- a/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventId.java +++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventId.java @@ -88,11 +88,24 @@ public final class BuildEventId implements Serializable { BuildEventStreamProtos.BuildEventId.newBuilder().setStarted(startedId).build()); } - public static BuildEventId commandlineId() { - BuildEventStreamProtos.BuildEventId.CommandLineId commandLineId = - BuildEventStreamProtos.BuildEventId.CommandLineId.getDefaultInstance(); + public static BuildEventId unstructuredCommandlineId() { + BuildEventStreamProtos.BuildEventId.UnstructuredCommandLineId commandLineId = + BuildEventStreamProtos.BuildEventId.UnstructuredCommandLineId.getDefaultInstance(); return new BuildEventId( - BuildEventStreamProtos.BuildEventId.newBuilder().setCommandLine(commandLineId).build()); + BuildEventStreamProtos.BuildEventId.newBuilder() + .setUnstructuredCommandLine(commandLineId) + .build()); + } + + public static BuildEventId structuredCommandlineId(String commandLineLabel) { + BuildEventStreamProtos.BuildEventId.StructuredCommandLineId commandLineId = + BuildEventStreamProtos.BuildEventId.StructuredCommandLineId.newBuilder() + .setCommandLineLabel(commandLineLabel) + .build(); + return new BuildEventId( + BuildEventStreamProtos.BuildEventId.newBuilder() + .setStructuredCommandLine(commandLineId) + .build()); } public static BuildEventId optionsParsedId() { diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/BUILD b/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/BUILD index c1c65b2696..a4bd2ed85a 100644 --- a/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/BUILD +++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/BUILD @@ -22,6 +22,7 @@ proto_library( name = "build_event_stream_proto", srcs = ["build_event_stream.proto"], deps = [ + "//src/main/protobuf:command_line_proto", "//src/main/protobuf:invocation_policy_proto", ], ) diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto b/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto index 472f104ecc..bbe9963ab9 100644 --- a/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto +++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto @@ -20,6 +20,7 @@ option java_package = "com.google.devtools.build.lib.buildeventstream"; option java_outer_classname = "BuildEventStreamProtos"; import "src/main/protobuf/invocation_policy.proto"; +import "src/main/protobuf/command_line.proto"; // Identifier for a build event. It is deliberately structured to also provide // information about which build target etc the event is related to. @@ -53,7 +54,16 @@ message BuildEventId { // Identifier on an event indicating the original commandline received by // the bazel server. - message CommandLineId { + message UnstructuredCommandLineId { + } + + // Identifier on an event describing the commandline received by Bazel. + message StructuredCommandLineId { + // A title for this command line value, as there may be multiple. + // For example, a single invocation may wish to report both the literal and + // canonical command lines, and this label would be used to differentiate + // between both versions. + string command_line_label = 1; } // Identifier of an event indicating the workspace status. @@ -159,7 +169,8 @@ message BuildEventId { UnknownBuildEventId unknown = 1; ProgressId progress = 2; BuildStartedId started = 3; - CommandLineId command_line = 11; + UnstructuredCommandLineId unstructured_command_line = 11; + StructuredCommandLineId structured_command_line = 18; WorkspaceStatusId workspace_status = 14; OptionsParsedId options_parsed = 12; FetchId fetch = 17; @@ -260,7 +271,7 @@ message BuildStarted { // like name and relevant entries of rc-files and client environment variables. // However, it does contain enough information to reproduce the build // invocation. -message CommandLine { +message UnstructuredCommandLine { repeated string args = 1; } @@ -531,7 +542,8 @@ message BuildEvent { Progress progress = 3; Aborted aborted = 4; BuildStarted started = 5; - CommandLine command_line = 12; + UnstructuredCommandLine unstructured_command_line = 12; + command_line.CommandLine structured_command_line = 22; OptionsParsed options_parsed = 13; WorkspaceStatus workspace_status = 16; Fetch fetch = 21; diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/buildevent/BuildStartingEvent.java b/src/main/java/com/google/devtools/build/lib/buildtool/buildevent/BuildStartingEvent.java index e84df587de..df601bb1ca 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/buildevent/BuildStartingEvent.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/buildevent/BuildStartingEvent.java @@ -82,7 +82,7 @@ public final class BuildStartingEvent implements BuildEvent { public Collection<BuildEventId> getChildrenEvents() { return ImmutableList.of( ProgressEvent.INITIAL_PROGRESS_UPDATE, - BuildEventId.commandlineId(), + BuildEventId.unstructuredCommandlineId(), BuildEventId.optionsParsedId(), BuildEventId.workspaceStatusId(), BuildEventId.targetPatternExpanded(request.getTargets()), diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java index 5bd9ad9f54..12d87af3ce 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java @@ -231,9 +231,14 @@ public class BlazeCommandDispatcher { parseOptionsForCommand(rcfileNotes, commandAnnotation, optionsParser, optionsMap, null, null); if (commandAnnotation.builds()) { + // splits project files from targets in the traditional sense ProjectFileSupport.handleProjectFiles( - eventHandler, runtime.getProjectFileProvider(), workspaceDirectory, workingDirectory, - optionsParser, commandAnnotation.name()); + eventHandler, + runtime.getProjectFileProvider(), + workspaceDirectory, + workingDirectory, + optionsParser, + commandAnnotation.name()); } // Fix-point iteration until all configs are loaded. @@ -276,7 +281,8 @@ public class BlazeCommandDispatcher { long firstContactTime, Optional<List<Pair<String, String>>> startupOptionsTaggedWithBazelRc) throws ShutdownBlazeServerException, InterruptedException { - OriginalCommandLineEvent originalCommandLine = new OriginalCommandLineEvent(args); + OriginalUnstructuredCommandLineEvent originalCommandLine = + new OriginalUnstructuredCommandLineEvent(args); Preconditions.checkNotNull(clientDescription); if (args.isEmpty()) { // Default to help command if no arguments specified. args = HELP_COMMAND; @@ -370,7 +376,7 @@ public class BlazeCommandDispatcher { } private int execExclusively( - OriginalCommandLineEvent originalCommandLine, + OriginalUnstructuredCommandLineEvent unstructuredServerCommandLineEvent, InvocationPolicy invocationPolicy, List<String> args, OutErr outErr, @@ -395,7 +401,11 @@ public class BlazeCommandDispatcher { eventHandler, workspace, command, commandAnnotation, commandName, invocationPolicy, args, optionsResult, rcfileNotes); OptionsProvider options = optionsResult.get(); - + CommandLineEvent originalCommandLineEvent = + new CommandLineEvent.OriginalCommandLineEvent( + runtime, commandName, options, startupOptionsTaggedWithBazelRc); + CommandLineEvent canonicalCommandLineEvent = + new CommandLineEvent.CanonicalCommandLineEvent(runtime, commandName, options); // The initCommand call also records the start time for the timestamp granularity monitor. CommandEnvironment env = workspace.initCommand(commandAnnotation, options); // Record the command's starting time for use by the commands themselves. @@ -591,7 +601,11 @@ public class BlazeCommandDispatcher { return e.getExitCode().getNumericExitCode(); } - env.getEventBus().post(originalCommandLine); + // Log the command line now that the modules have all had a change to register their listeners + // to the event bus. + env.getEventBus().post(unstructuredServerCommandLineEvent); + env.getEventBus().post(originalCommandLineEvent); + env.getEventBus().post(canonicalCommandLineEvent); for (BlazeModule module : runtime.getBlazeModules()) { env.getSkyframeExecutor().injectExtraPrecomputedValues(module.getPrecomputedValues()); diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CommandLineEvent.java b/src/main/java/com/google/devtools/build/lib/runtime/CommandLineEvent.java new file mode 100644 index 0000000000..232aa45123 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/runtime/CommandLineEvent.java @@ -0,0 +1,369 @@ +// Copyright 2017 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.runtime; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.ImmutableList; +import com.google.devtools.build.lib.buildeventstream.BuildEventConverters; +import com.google.devtools.build.lib.buildeventstream.BuildEventId; +import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos; +import com.google.devtools.build.lib.buildeventstream.BuildEventWithOrderConstraint; +import com.google.devtools.build.lib.buildeventstream.GenericBuildEvent; +import com.google.devtools.build.lib.runtime.proto.CommandLineOuterClass.ChunkList; +import com.google.devtools.build.lib.runtime.proto.CommandLineOuterClass.CommandLine; +import com.google.devtools.build.lib.runtime.proto.CommandLineOuterClass.CommandLineSection; +import com.google.devtools.build.lib.runtime.proto.CommandLineOuterClass.Option; +import com.google.devtools.build.lib.runtime.proto.CommandLineOuterClass.OptionList; +import com.google.devtools.build.lib.util.Pair; +import com.google.devtools.common.options.OptionDefinition; +import com.google.devtools.common.options.OptionEffectTag; +import com.google.devtools.common.options.OptionMetadataTag; +import com.google.devtools.common.options.OptionPriority; +import com.google.devtools.common.options.OptionsParser; +import com.google.devtools.common.options.OptionsParsingException; +import com.google.devtools.common.options.OptionsProvider; +import com.google.devtools.common.options.ParsedOptionDescription; +import com.google.devtools.common.options.proto.OptionFilters; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.Optional; +import java.util.stream.Collectors; +import javax.annotation.Nullable; + +/** A build event reporting the command line by which Bazel was invoked. */ +public abstract class CommandLineEvent implements BuildEventWithOrderConstraint { + protected final String productName; + protected final OptionsProvider activeStartupOptions; + protected final String commandName; + protected final OptionsProvider commandOptions; + + CommandLineEvent( + String productName, + OptionsProvider activeStartupOptions, + String commandName, + OptionsProvider commandOptions) { + this.productName = productName; + this.activeStartupOptions = activeStartupOptions; + this.commandName = commandName; + this.commandOptions = commandOptions; + } + + @Override + public Collection<BuildEventId> getChildrenEvents() { + return ImmutableList.of(); + } + + @Override + public Collection<BuildEventId> postedAfter() { + return ImmutableList.of(BuildEventId.buildStartedId()); + } + + CommandLineSection getExecutableSection() { + return CommandLineSection.newBuilder() + .setSectionLabel("executable") + .setChunkList(ChunkList.newBuilder().addChunk(productName)) + .build(); + } + + CommandLineSection getCommandSection() { + return CommandLineSection.newBuilder() + .setSectionLabel("command") + .setChunkList(ChunkList.newBuilder().addChunk(commandName)) + .build(); + } + + /** + * Convert an array of tags to the equivalent proto-generated enum values. + * + * <p>The proto type is duplicate in order to not burden the OptionsParser with the proto + * dependency. A test guarantees that the two enum types are kept in sync with matching indices. + */ + static List<OptionFilters.OptionEffectTag> getProtoEffectTags(OptionEffectTag[] tagArray) { + ArrayList<OptionFilters.OptionEffectTag> effectTags = new ArrayList<>(tagArray.length); + for (OptionEffectTag tag : tagArray) { + effectTags.add(OptionFilters.OptionEffectTag.forNumber(tag.getValue())); + } + return effectTags; + } + + /** + * Convert an array of tags to the equivalent proto-generated enum values. + * + * <p>The proto type is duplicate in order to not burden the OptionsParser with the proto + * dependency. A test guarantees that the two enum types are kept in sync with matching indices. + */ + static List<OptionFilters.OptionMetadataTag> getProtoMetadataTags(OptionMetadataTag[] tagArray) { + ArrayList<OptionFilters.OptionMetadataTag> metadataTags = new ArrayList<>(tagArray.length); + for (OptionMetadataTag tag : tagArray) { + metadataTags.add(OptionFilters.OptionMetadataTag.forNumber(tag.getValue())); + } + return metadataTags; + } + + List<Option> getOptionListFromParsedOptionDescriptions( + List<ParsedOptionDescription> parsedOptionDescriptions) { + List<Option> options = new ArrayList<>(); + for (ParsedOptionDescription parsedOption : parsedOptionDescriptions) { + options.add( + createOption( + parsedOption.getOptionDefinition(), + parsedOption.getCommandLineForm(), + parsedOption.getUnconvertedValue())); + } + return options; + } + + private Option createOption( + OptionDefinition optionDefinition, String combinedForm, @Nullable String value) { + Option.Builder option = Option.newBuilder(); + option.setCombinedForm(combinedForm); + option.setOptionName(optionDefinition.getOptionName()); + if (value != null) { + option.setOptionValue(value); + } + option.addAllEffectTags(getProtoEffectTags(optionDefinition.getOptionEffectTags())); + option.addAllMetadataTags(getProtoMetadataTags(optionDefinition.getOptionMetadataTags())); + return option.build(); + } + + /** + * Returns the startup option section of the command line for the startup options as the server + * received them at its startup. Since not all client options get passed to the server as startup + * options, this might not represent the actual list of startup options as the user provided them. + */ + CommandLineSection getActiveStartupOptions() { + return CommandLineSection.newBuilder() + .setSectionLabel("startup options") + .setOptionList( + OptionList.newBuilder() + .addAllOption( + getOptionListFromParsedOptionDescriptions( + activeStartupOptions.asCompleteListOfParsedOptions()))) + .build(); + } + + /** + * Returns the final part of the command line, containing whatever was left after obtaining the + * command and its options. + */ + CommandLineSection getResidual() { + // Potential further split: how the residual, if any is accepted, gets interpreted depends on + // the command. For example, for build commands, we might want to consider separating out + // project files, as in runtime.commands.ProjectFileSupport. To properly report this, we would + // need to let the command customize how the residual is listed. This catch-all could serve + // as a default in this case. + return CommandLineSection.newBuilder() + .setSectionLabel("residual") + .setChunkList(ChunkList.newBuilder().addAllChunk(commandOptions.getResidue())) + .build(); + } + + /** This reports a reassembled version of the command line as Bazel received it. */ + public static class OriginalCommandLineEvent extends CommandLineEvent { + private static final String LABEL = "original"; + private final Optional<List<Pair<String, String>>> originalStartupOptions; + + public OriginalCommandLineEvent( + BlazeRuntime runtime, + String commandName, + OptionsProvider commandOptions, + Optional<List<Pair<String, String>>> originalStartupOptions) { + this( + runtime.getProductName(), + runtime.getStartupOptionsProvider(), + commandName, + commandOptions, + originalStartupOptions); + } + + @VisibleForTesting + OriginalCommandLineEvent( + String productName, + OptionsProvider activeStartupOptions, + String commandName, + OptionsProvider commandOptions, + Optional<List<Pair<String, String>>> originalStartupOptions) { + super(productName, activeStartupOptions, commandName, commandOptions); + this.originalStartupOptions = originalStartupOptions; + } + + @Override + public BuildEventId getEventId() { + return BuildEventId.structuredCommandlineId(LABEL); + } + + /** + * Returns the literal command line options as received. These are not the final parsed values, + * but are passed as is from the client, so we do not have the full OptionDefinition + * information. In this form, only set the "combinedForm" field. + */ + private CommandLineSection getStartupOptionSection() { + if (originalStartupOptions.isPresent()) { + List<Option> options = new ArrayList<>(); + for (Pair<String, String> sourceToOptionPair : originalStartupOptions.get()) { + // Only add the options that were added by the command line. + // TODO(b/19881919) decide the format that option source information should take and then + // add all options, tagged with the source, instead of filtering out the rc options. + if (sourceToOptionPair.first != null && sourceToOptionPair.first.isEmpty()) { + options.add( + Option.newBuilder().setCombinedForm(sourceToOptionPair.getSecond()).build()); + } + } + return CommandLineSection.newBuilder() + .setSectionLabel("startup options") + .setOptionList(OptionList.newBuilder().addAllOption(options)) + .build(); + } else { + // If we were not provided with the startup options, fallback to reporting the active ones + // stored by the Bazel Runtime. + return getActiveStartupOptions(); + } + } + + private CommandLineSection getExplicitCommandOptions() { + List<ParsedOptionDescription> explicitOptions = + commandOptions + .asListOfExplicitOptions() + .stream() + .filter( + parsedOptionDescription -> + parsedOptionDescription.getPriority() == OptionPriority.COMMAND_LINE) + .collect(Collectors.toList()); + return CommandLineSection.newBuilder() + .setSectionLabel("command options") + .setOptionList( + OptionList.newBuilder() + .addAllOption(getOptionListFromParsedOptionDescriptions(explicitOptions))) + .build(); + } + + @Override + public BuildEventStreamProtos.BuildEvent asStreamProto(BuildEventConverters converters) { + return GenericBuildEvent.protoChaining(this) + .setStructuredCommandLine( + CommandLine.newBuilder() + .setCommandLineLabel(LABEL) + .addSections(getExecutableSection()) + .addSections(getStartupOptionSection()) + .addSections(getCommandSection()) + .addSections(getExplicitCommandOptions()) + .addSections(getResidual()) + .build()) + .build(); + } + } + + /** This reports the canonical form of the command line. */ + public static class CanonicalCommandLineEvent extends CommandLineEvent { + private static final String LABEL = "canonical"; + + public CanonicalCommandLineEvent( + BlazeRuntime runtime, String commandName, OptionsProvider commandOptions) { + this( + runtime.getProductName(), + runtime.getStartupOptionsProvider(), + commandName, + commandOptions); + } + + @VisibleForTesting + CanonicalCommandLineEvent( + String productName, + OptionsProvider activeStartupOptions, + String commandName, + OptionsProvider commandOptions) { + super(productName, activeStartupOptions, commandName, commandOptions); + } + + @Override + public BuildEventId getEventId() { + return BuildEventId.structuredCommandlineId(LABEL); + } + + /** + * Returns the effective startup options. + * + * <p>Since in this command line the command options include invocation policy's and blazercs' + * contents expanded fully, the list of startup options should prevent reapplication of these + * contents. + * + * <p>The options parser does not understand the effect of these flags, since the relationship + * between these startup options and the command options is not held within the options parser, + * so instead, we add a small hack. Remove any explicit mentions of these flags, and explicitly + * add the options that prevent Blaze from looking for the default rc files. + */ + private CommandLineSection getCanonicalStartupOptions() { + List<Option> unfilteredOptions = getActiveStartupOptions().getOptionList().getOptionList(); + // Create the fake ones to prevent reapplication of the original rc file contents. + OptionsParser fakeOptions = OptionsParser.newOptionsParser(BlazeServerStartupOptions.class); + try { + fakeOptions.parse("--nomaster_blazerc", "--blazerc=/dev/null"); + } catch (OptionsParsingException e) { + // Unless someone changes the definition of these flags, this is impossible. + throw new IllegalStateException(e); + } + + // Remove any instances of the applied, and add the new blocking ones. + return CommandLineSection.newBuilder() + .setSectionLabel("startup options") + .setOptionList( + OptionList.newBuilder() + .addAllOption( + unfilteredOptions + .stream() + .filter( + option -> { + String optionName = option.getOptionName(); + return !optionName.equals("blazerc") + && !optionName.equals("master_blazerc") + && !optionName.equals("invocation_policy"); + }) + .collect(Collectors.toList())) + .addAllOption( + getOptionListFromParsedOptionDescriptions( + fakeOptions.asCompleteListOfParsedOptions()))) + .build(); + } + + /** Returns the canonical command options, overridden and default values are not listed. */ + // TODO(b/19881919) this should use OptionValueDescription's tracking of relevant option + // instances, but as this is not yet possible, list the full options list. + private CommandLineSection getCanonicalCommandOptions() { + return CommandLineSection.newBuilder() + .setSectionLabel("command options") + .setOptionList( + OptionList.newBuilder() + .addAllOption( + getOptionListFromParsedOptionDescriptions( + commandOptions.asCompleteListOfParsedOptions()))) + .build(); + } + + @Override + public BuildEventStreamProtos.BuildEvent asStreamProto(BuildEventConverters converters) { + return GenericBuildEvent.protoChaining(this) + .setStructuredCommandLine( + CommandLine.newBuilder() + .setCommandLineLabel(LABEL) + .addSections(getExecutableSection()) + .addSections(getCanonicalStartupOptions()) + .addSections(getCommandSection()) + .addSections(getCanonicalCommandOptions()) + .addSections(getResidual()) + .build()) + .build(); + } + } +} diff --git a/src/main/java/com/google/devtools/build/lib/runtime/GotOptionsEvent.java b/src/main/java/com/google/devtools/build/lib/runtime/GotOptionsEvent.java index cf7c363872..2193761000 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/GotOptionsEvent.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/GotOptionsEvent.java @@ -105,6 +105,7 @@ public class GotOptionsEvent implements BuildEventWithOrderConstraint { @Override public Collection<BuildEventId> postedAfter() { - return ImmutableList.of(BuildEventId.buildStartedId(), BuildEventId.commandlineId()); + return ImmutableList.of( + BuildEventId.buildStartedId(), BuildEventId.unstructuredCommandlineId()); } } diff --git a/src/main/java/com/google/devtools/build/lib/runtime/OriginalCommandLineEvent.java b/src/main/java/com/google/devtools/build/lib/runtime/OriginalUnstructuredCommandLineEvent.java index e6b02ba101..291f1e6f4a 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/OriginalCommandLineEvent.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/OriginalUnstructuredCommandLineEvent.java @@ -24,17 +24,17 @@ import java.util.Collection; import java.util.List; /** A build event reporting the original commandline by which bazel was invoked. */ -public class OriginalCommandLineEvent implements BuildEventWithOrderConstraint { +public class OriginalUnstructuredCommandLineEvent implements BuildEventWithOrderConstraint { private final ImmutableList<String> args; - public OriginalCommandLineEvent(List<String> args) { + public OriginalUnstructuredCommandLineEvent(List<String> args) { this.args = ImmutableList.copyOf(args); } @Override public BuildEventId getEventId() { - return BuildEventId.commandlineId(); + return BuildEventId.unstructuredCommandlineId(); } @Override @@ -50,7 +50,8 @@ public class OriginalCommandLineEvent implements BuildEventWithOrderConstraint { @Override public BuildEventStreamProtos.BuildEvent asStreamProto(BuildEventConverters converters) { return GenericBuildEvent.protoChaining(this) - .setCommandLine(BuildEventStreamProtos.CommandLine.newBuilder().addAllArgs(args).build()) + .setUnstructuredCommandLine( + BuildEventStreamProtos.UnstructuredCommandLine.newBuilder().addAllArgs(args).build()) .build(); } } diff --git a/src/main/java/com/google/devtools/common/options/OptionValueDescription.java b/src/main/java/com/google/devtools/common/options/OptionValueDescription.java index 0d81d49a84..aa808e2141 100644 --- a/src/main/java/com/google/devtools/common/options/OptionValueDescription.java +++ b/src/main/java/com/google/devtools/common/options/OptionValueDescription.java @@ -208,8 +208,11 @@ public abstract class OptionValueDescription { // The new value does not override the old value, as it has lower priority. warnings.add( String.format( - "The lower priority option '%s' does not override the previous value '%s'", - parsedOption.getCommandLineForm(), effectiveOptionInstance.getCommandLineForm())); + "The lower priority option '%s' (source %s) does not override the previous value " + + "'%s'", + parsedOption.getCommandLineForm(), + parsedOption.getSource(), + effectiveOptionInstance.getCommandLineForm())); } } diff --git a/src/main/java/com/google/devtools/common/options/OptionsProvider.java b/src/main/java/com/google/devtools/common/options/OptionsProvider.java index 1c7737fbda..5fd8ac00c1 100644 --- a/src/main/java/com/google/devtools/common/options/OptionsProvider.java +++ b/src/main/java/com/google/devtools/common/options/OptionsProvider.java @@ -39,8 +39,10 @@ public interface OptionsProvider extends OptionsClassProvider { * specified. If an option was specified multiple times, it is included in the result multiple * times. Does not include the residue. * - * <p>The returned list can be filtered if undocumented, hidden or implicit options should not be - * displayed. + * <p>The returned list includes undocumented, hidden or implicit options, and should be filtered + * as needed. Since it includes all options parsed, it will also include both an expansion option + * and the options it expanded to, and so blindly using this list for a new invocation will cause + * double-application of these options. */ List<ParsedOptionDescription> asCompleteListOfParsedOptions(); diff --git a/src/main/java/com/google/devtools/common/options/ParsedOptionDescription.java b/src/main/java/com/google/devtools/common/options/ParsedOptionDescription.java index 1f43172fb2..d5582635e3 100644 --- a/src/main/java/com/google/devtools/common/options/ParsedOptionDescription.java +++ b/src/main/java/com/google/devtools/common/options/ParsedOptionDescription.java @@ -115,7 +115,7 @@ public final class ParsedOptionDescription { return unconvertedValue; } - OptionPriority getPriority() { + public OptionPriority getPriority() { return origin.getPriority(); } diff --git a/src/main/protobuf/BUILD b/src/main/protobuf/BUILD index f700ffbe76..ca2ba0c1af 100644 --- a/src/main/protobuf/BUILD +++ b/src/main/protobuf/BUILD @@ -65,7 +65,7 @@ java_library_srcs( proto_library( name = "command_line_proto", srcs = ["command_line.proto"], - visibility = ["//visibility:private"], + visibility = ["//src/main/java/com/google/devtools/build/lib/buildeventstream/proto:__pkg__"], deps = [":option_filters_proto"], ) diff --git a/src/main/protobuf/command_line.proto b/src/main/protobuf/command_line.proto index e1ced1fb3f..d5fa6aceb8 100644 --- a/src/main/protobuf/command_line.proto +++ b/src/main/protobuf/command_line.proto @@ -26,7 +26,7 @@ message CommandLine { // In particular, a single invocation may wish to report both the literal and // canonical command lines, and this label would be used to differentiate // between both versions. This is a string for flexibility. - string label = 1; + string command_line_label = 1; // A Bazel command line is made of distinct parts. For example, // `bazel --nomaster_bazelrc test --nocache_test_results //foo:aTest` diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD index 61903f6e5e..8a143410c5 100644 --- a/src/test/java/com/google/devtools/build/lib/BUILD +++ b/src/test/java/com/google/devtools/build/lib/BUILD @@ -1085,8 +1085,10 @@ java_test( "//src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs", "//src/main/java/com/google/devtools/common/options", "//src/main/java/com/google/devtools/common/options:invocation_policy", + "//src/main/protobuf:command_line_java_proto", "//src/main/protobuf:invocation_policy_java_proto", "//src/main/protobuf:test_status_java_proto", + "//src/test/java/com/google/devtools/common/options:testutils", "//third_party:guava", "//third_party:junit4", "//third_party:mockito", diff --git a/src/test/java/com/google/devtools/build/lib/runtime/CommandLineEventTest.java b/src/test/java/com/google/devtools/build/lib/runtime/CommandLineEventTest.java new file mode 100644 index 0000000000..be02e6dcba --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/runtime/CommandLineEventTest.java @@ -0,0 +1,428 @@ +// Copyright 2017 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.runtime; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.common.collect.ImmutableList; +import com.google.devtools.build.lib.runtime.CommandLineEvent.CanonicalCommandLineEvent; +import com.google.devtools.build.lib.runtime.CommandLineEvent.OriginalCommandLineEvent; +import com.google.devtools.build.lib.runtime.proto.CommandLineOuterClass.CommandLine; +import com.google.devtools.build.lib.util.Pair; +import com.google.devtools.common.options.OptionPriority; +import com.google.devtools.common.options.OptionsParser; +import com.google.devtools.common.options.OptionsParsingException; +import com.google.devtools.common.options.TestOptions; +import java.util.Optional; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests for {@link CommandLineEvent}'s construction of the command lines. */ +@RunWith(JUnit4.class) +public class CommandLineEventTest { + + private void checkCommandLineSectionLabels(CommandLine line) { + assertThat(line.getSectionsCount()).isEqualTo(5); + + assertThat(line.getSections(0).getSectionLabel()).isEqualTo("executable"); + assertThat(line.getSections(1).getSectionLabel()).isEqualTo("startup options"); + assertThat(line.getSections(2).getSectionLabel()).isEqualTo("command"); + assertThat(line.getSections(3).getSectionLabel()).isEqualTo("command options"); + assertThat(line.getSections(4).getSectionLabel()).isEqualTo("residual"); + } + + @Test + public void testMostlyEmpty_OriginalCommandLine() { + OptionsParser fakeStartupOptions = + OptionsParser.newOptionsParser(BlazeServerStartupOptions.class); + OptionsParser fakeCommandOptions = OptionsParser.newOptionsParser(TestOptions.class); + + CommandLine line = + new OriginalCommandLineEvent( + "testblaze", + fakeStartupOptions, + "someCommandName", + fakeCommandOptions, + Optional.of(ImmutableList.of())) + .asStreamProto(null) + .getStructuredCommandLine(); + + assertThat(line).isNotNull(); + assertThat(line.getCommandLineLabel()).isEqualTo("original"); + checkCommandLineSectionLabels(line); + assertThat(line.getSections(0).getChunkList().getChunk(0)).isEqualTo("testblaze"); + assertThat(line.getSections(1).getOptionList().getOptionCount()).isEqualTo(0); + assertThat(line.getSections(2).getChunkList().getChunk(0)).isEqualTo("someCommandName"); + assertThat(line.getSections(3).getOptionList().getOptionCount()).isEqualTo(0); + assertThat(line.getSections(4).getChunkList().getChunkCount()).isEqualTo(0); + } + + @Test + public void testMostlyEmpty_CanonicalCommandLine() { + OptionsParser fakeStartupOptions = + OptionsParser.newOptionsParser(BlazeServerStartupOptions.class); + OptionsParser fakeCommandOptions = OptionsParser.newOptionsParser(TestOptions.class); + + CommandLine line = + new CanonicalCommandLineEvent( + "testblaze", fakeStartupOptions, "someCommandName", fakeCommandOptions) + .asStreamProto(null) + .getStructuredCommandLine(); + + assertThat(line).isNotNull(); + assertThat(line.getCommandLineLabel()).isEqualTo("canonical"); + checkCommandLineSectionLabels(line); + + assertThat(line.getSections(0).getChunkList().getChunk(0)).isEqualTo("testblaze"); + assertThat(line.getSections(1).getOptionList().getOptionCount()).isEqualTo(2); + assertThat(line.getSections(1).getOptionList().getOption(0).getCombinedForm()) + .isEqualTo("--nomaster_blazerc"); + assertThat(line.getSections(1).getOptionList().getOption(1).getCombinedForm()) + .isEqualTo("--blazerc=/dev/null"); + assertThat(line.getSections(2).getChunkList().getChunk(0)).isEqualTo("someCommandName"); + assertThat(line.getSections(3).getOptionList().getOptionCount()).isEqualTo(0); + assertThat(line.getSections(4).getChunkList().getChunkCount()).isEqualTo(0); + } + + @Test + public void testActiveBlazercs_OriginalCommandLine() throws OptionsParsingException { + OptionsParser fakeStartupOptions = + OptionsParser.newOptionsParser(BlazeServerStartupOptions.class); + fakeStartupOptions.parse( + "--blazerc=/some/path", "--master_blazerc", "--blazerc", "/some/other/path"); + OptionsParser fakeCommandOptions = OptionsParser.newOptionsParser(TestOptions.class); + + CommandLine line = + new OriginalCommandLineEvent( + "testblaze", + fakeStartupOptions, + "someCommandName", + fakeCommandOptions, + Optional.empty()) + .asStreamProto(null) + .getStructuredCommandLine(); + + assertThat(line).isNotNull(); + assertThat(line.getCommandLineLabel()).isEqualTo("original"); + checkCommandLineSectionLabels(line); + + // Expect the provided rc-related startup options are correctly listed + assertThat(line.getSections(0).getChunkList().getChunk(0)).isEqualTo("testblaze"); + assertThat(line.getSections(1).getOptionList().getOptionCount()).isEqualTo(3); + assertThat(line.getSections(1).getOptionList().getOption(0).getCombinedForm()) + .isEqualTo("--blazerc=/some/path"); + assertThat(line.getSections(1).getOptionList().getOption(1).getCombinedForm()) + .isEqualTo("--master_blazerc"); + assertThat(line.getSections(1).getOptionList().getOption(2).getCombinedForm()) + .isEqualTo("--blazerc /some/other/path"); + assertThat(line.getSections(2).getChunkList().getChunk(0)).isEqualTo("someCommandName"); + assertThat(line.getSections(3).getOptionList().getOptionCount()).isEqualTo(0); + assertThat(line.getSections(4).getChunkList().getChunkCount()).isEqualTo(0); + } + + @Test + public void testPassedInBlazercs_OriginalCommandLine() throws OptionsParsingException { + OptionsParser fakeStartupOptions = + OptionsParser.newOptionsParser(BlazeServerStartupOptions.class); + OptionsParser fakeCommandOptions = OptionsParser.newOptionsParser(TestOptions.class); + + CommandLine line = + new OriginalCommandLineEvent( + "testblaze", + fakeStartupOptions, + "someCommandName", + fakeCommandOptions, + Optional.of( + ImmutableList.of( + Pair.of("", "--blazerc=/some/path"), + Pair.of("", "--master_blazerc"), + Pair.of("", "--blazerc=/some/other/path"), + Pair.of("", "--invocation_policy=notARealPolicy")))) + .asStreamProto(null) + .getStructuredCommandLine(); + + assertThat(line).isNotNull(); + assertThat(line.getCommandLineLabel()).isEqualTo("original"); + checkCommandLineSectionLabels(line); + + // Expect the provided rc-related startup options are correctly listed + assertThat(line.getSections(0).getChunkList().getChunk(0)).isEqualTo("testblaze"); + assertThat(line.getSections(1).getOptionList().getOptionCount()).isEqualTo(4); + assertThat(line.getSections(1).getOptionList().getOption(0).getCombinedForm()) + .isEqualTo("--blazerc=/some/path"); + assertThat(line.getSections(1).getOptionList().getOption(1).getCombinedForm()) + .isEqualTo("--master_blazerc"); + assertThat(line.getSections(1).getOptionList().getOption(2).getCombinedForm()) + .isEqualTo("--blazerc=/some/other/path"); + assertThat(line.getSections(1).getOptionList().getOption(3).getCombinedForm()) + .isEqualTo("--invocation_policy=notARealPolicy"); + assertThat(line.getSections(2).getChunkList().getChunk(0)).isEqualTo("someCommandName"); + assertThat(line.getSections(3).getOptionList().getOptionCount()).isEqualTo(0); + assertThat(line.getSections(4).getChunkList().getChunkCount()).isEqualTo(0); + } + + @Test + public void testBlazercs_CanonicalCommandLine() throws OptionsParsingException { + OptionsParser fakeStartupOptions = + OptionsParser.newOptionsParser(BlazeServerStartupOptions.class); + fakeStartupOptions.parse( + "--blazerc=/some/path", "--master_blazerc", "--blazerc", "/some/other/path"); + OptionsParser fakeCommandOptions = OptionsParser.newOptionsParser(TestOptions.class); + + CommandLine line = + new CanonicalCommandLineEvent( + "testblaze", fakeStartupOptions, "someCommandName", fakeCommandOptions) + .asStreamProto(null) + .getStructuredCommandLine(); + + assertThat(line).isNotNull(); + assertThat(line.getCommandLineLabel()).isEqualTo("canonical"); + checkCommandLineSectionLabels(line); + + // Expect the provided rc-related startup options are removed and replaced with the + // rc-prevention options. + assertThat(line.getSections(0).getChunkList().getChunk(0)).isEqualTo("testblaze"); + assertThat(line.getSections(1).getOptionList().getOptionCount()).isEqualTo(2); + assertThat(line.getSections(1).getOptionList().getOption(0).getCombinedForm()) + .isEqualTo("--nomaster_blazerc"); + assertThat(line.getSections(1).getOptionList().getOption(1).getCombinedForm()) + .isEqualTo("--blazerc=/dev/null"); + assertThat(line.getSections(2).getChunkList().getChunk(0)).isEqualTo("someCommandName"); + assertThat(line.getSections(3).getOptionList().getOptionCount()).isEqualTo(0); + assertThat(line.getSections(4).getChunkList().getChunkCount()).isEqualTo(0); + } + + @Test + public void testOptionsAtVariousPriorities_OriginalCommandLine() throws OptionsParsingException { + OptionsParser fakeStartupOptions = + OptionsParser.newOptionsParser(BlazeServerStartupOptions.class); + OptionsParser fakeCommandOptions = OptionsParser.newOptionsParser(TestOptions.class); + fakeCommandOptions.parse( + OptionPriority.COMMAND_LINE, + "command line", + ImmutableList.of("--test_string=foo", "--test_multiple_string=bar")); + fakeCommandOptions.parse( + OptionPriority.INVOCATION_POLICY, + "fake invocation policy", + ImmutableList.of("--expanded_c=2")); + fakeCommandOptions.parse( + OptionPriority.RC_FILE, "fake rc file", ImmutableList.of("--test_multiple_string=baz")); + + CommandLine line = + new OriginalCommandLineEvent( + "testblaze", + fakeStartupOptions, + "someCommandName", + fakeCommandOptions, + Optional.of(ImmutableList.of())) + .asStreamProto(null) + .getStructuredCommandLine(); + + assertThat(line).isNotNull(); + assertThat(line.getCommandLineLabel()).isEqualTo("original"); + checkCommandLineSectionLabels(line); + + assertThat(line.getSections(0).getChunkList().getChunk(0)).isEqualTo("testblaze"); + assertThat(line.getSections(1).getOptionList().getOptionCount()).isEqualTo(0); + assertThat(line.getSections(2).getChunkList().getChunk(0)).isEqualTo("someCommandName"); + // Expect the rc file options and invocation policy options to not be listed with the explicit + // command line options. + assertThat(line.getSections(3).getOptionList().getOptionCount()).isEqualTo(2); + assertThat(line.getSections(3).getOptionList().getOption(0).getCombinedForm()) + .isEqualTo("--test_string=foo"); + assertThat(line.getSections(3).getOptionList().getOption(1).getCombinedForm()) + .isEqualTo("--test_multiple_string=bar"); + assertThat(line.getSections(4).getChunkList().getChunkCount()).isEqualTo(0); + } + + @Test + public void testOptionsAtVariousPriorities_CanonicalCommandLine() throws OptionsParsingException { + OptionsParser fakeStartupOptions = + OptionsParser.newOptionsParser(BlazeServerStartupOptions.class); + OptionsParser fakeCommandOptions = OptionsParser.newOptionsParser(TestOptions.class); + fakeCommandOptions.parse( + OptionPriority.COMMAND_LINE, + "command line", + ImmutableList.of("--test_string=foo", "--test_multiple_string=bar")); + fakeCommandOptions.parse( + OptionPriority.INVOCATION_POLICY, + "fake invocation policy", + ImmutableList.of("--expanded_c=2")); + fakeCommandOptions.parse( + OptionPriority.RC_FILE, "fake rc file", ImmutableList.of("--test_multiple_string=baz")); + + CommandLine line = + new CanonicalCommandLineEvent( + "testblaze", fakeStartupOptions, "someCommandName", fakeCommandOptions) + .asStreamProto(null) + .getStructuredCommandLine(); + + assertThat(line).isNotNull(); + assertThat(line.getCommandLineLabel()).isEqualTo("canonical"); + checkCommandLineSectionLabels(line); + + assertThat(line.getSections(0).getChunkList().getChunk(0)).isEqualTo("testblaze"); + assertThat(line.getSections(1).getOptionList().getOptionCount()).isEqualTo(2); + assertThat(line.getSections(2).getChunkList().getChunk(0)).isEqualTo("someCommandName"); + // In the canonical line, expect the rc option to show up before the higher priority options. + assertThat(line.getSections(3).getOptionList().getOptionCount()).isEqualTo(4); + assertThat(line.getSections(3).getOptionList().getOption(0).getCombinedForm()) + .isEqualTo("--test_multiple_string=baz"); + assertThat(line.getSections(3).getOptionList().getOption(1).getCombinedForm()) + .isEqualTo("--test_string=foo"); + assertThat(line.getSections(3).getOptionList().getOption(2).getCombinedForm()) + .isEqualTo("--test_multiple_string=bar"); + assertThat(line.getSections(3).getOptionList().getOption(3).getCombinedForm()) + .isEqualTo("--expanded_c=2"); + + assertThat(line.getSections(4).getChunkList().getChunkCount()).isEqualTo(0); + } + + @Test + public void testExpansionOption_OriginalCommandLine() throws OptionsParsingException { + OptionsParser fakeStartupOptions = + OptionsParser.newOptionsParser(BlazeServerStartupOptions.class); + OptionsParser fakeCommandOptions = OptionsParser.newOptionsParser(TestOptions.class); + fakeCommandOptions.parse( + OptionPriority.COMMAND_LINE, "command line", ImmutableList.of("--test_expansion")); + + CommandLine line = + new OriginalCommandLineEvent( + "testblaze", + fakeStartupOptions, + "someCommandName", + fakeCommandOptions, + Optional.of(ImmutableList.of())) + .asStreamProto(null) + .getStructuredCommandLine(); + + assertThat(line).isNotNull(); + assertThat(line.getCommandLineLabel()).isEqualTo("original"); + checkCommandLineSectionLabels(line); + + assertThat(line.getSections(0).getChunkList().getChunk(0)).isEqualTo("testblaze"); + assertThat(line.getSections(1).getOptionList().getOptionCount()).isEqualTo(0); + assertThat(line.getSections(2).getChunkList().getChunk(0)).isEqualTo("someCommandName"); + // Expect the rc file option to not be listed with the explicit command line options. + assertThat(line.getSections(3).getOptionList().getOptionCount()).isEqualTo(1); + assertThat(line.getSections(3).getOptionList().getOption(0).getCombinedForm()) + .isEqualTo("--test_expansion"); + assertThat(line.getSections(4).getChunkList().getChunkCount()).isEqualTo(0); + } + + @Test + public void testExpansionOption_CanonicalCommandLine() throws OptionsParsingException { + OptionsParser fakeStartupOptions = + OptionsParser.newOptionsParser(BlazeServerStartupOptions.class); + OptionsParser fakeCommandOptions = OptionsParser.newOptionsParser(TestOptions.class); + fakeCommandOptions.parse( + OptionPriority.COMMAND_LINE, "command line", ImmutableList.of("--test_expansion")); + + CommandLine line = + new CanonicalCommandLineEvent( + "testblaze", fakeStartupOptions, "someCommandName", fakeCommandOptions) + .asStreamProto(null) + .getStructuredCommandLine(); + + assertThat(line).isNotNull(); + assertThat(line.getCommandLineLabel()).isEqualTo("canonical"); + checkCommandLineSectionLabels(line); + + assertThat(line.getSections(0).getChunkList().getChunk(0)).isEqualTo("testblaze"); + assertThat(line.getSections(1).getOptionList().getOptionCount()).isEqualTo(2); + assertThat(line.getSections(2).getChunkList().getChunk(0)).isEqualTo("someCommandName"); + + // TODO(b/19881919) Expansion options shouldn't be listed along with their expansions, this + // could cause duplicate values for repeatable flags. There should be 4 flags here, without + // test_expansion listed. + assertThat(line.getSections(3).getOptionList().getOptionCount()).isEqualTo(5); + assertThat(line.getSections(3).getOptionList().getOption(0).getCombinedForm()) + .isEqualTo("--test_expansion"); + assertThat(line.getSections(3).getOptionList().getOption(1).getCombinedForm()) + .isEqualTo("--noexpanded_a"); + assertThat(line.getSections(3).getOptionList().getOption(2).getCombinedForm()) + .isEqualTo("--expanded_b=false"); + assertThat(line.getSections(3).getOptionList().getOption(3).getCombinedForm()) + .isEqualTo("--expanded_c 42"); + assertThat(line.getSections(3).getOptionList().getOption(4).getCombinedForm()) + .isEqualTo("--expanded_d bar"); + assertThat(line.getSections(4).getChunkList().getChunkCount()).isEqualTo(0); + } + + @Test + public void testOptionWithImplicitRequirement_OriginalCommandLine() + throws OptionsParsingException { + OptionsParser fakeStartupOptions = + OptionsParser.newOptionsParser(BlazeServerStartupOptions.class); + OptionsParser fakeCommandOptions = OptionsParser.newOptionsParser(TestOptions.class); + fakeCommandOptions.parse( + OptionPriority.COMMAND_LINE, + "command line", + ImmutableList.of("--test_implicit_requirement=foo")); + + CommandLine line = + new OriginalCommandLineEvent( + "testblaze", + fakeStartupOptions, + "someCommandName", + fakeCommandOptions, + Optional.of(ImmutableList.of())) + .asStreamProto(null) + .getStructuredCommandLine(); + + assertThat(line).isNotNull(); + assertThat(line.getCommandLineLabel()).isEqualTo("original"); + checkCommandLineSectionLabels(line); + + assertThat(line.getSections(0).getChunkList().getChunk(0)).isEqualTo("testblaze"); + assertThat(line.getSections(1).getOptionList().getOptionCount()).isEqualTo(0); + assertThat(line.getSections(2).getChunkList().getChunk(0)).isEqualTo("someCommandName"); + assertThat(line.getSections(3).getOptionList().getOptionCount()).isEqualTo(1); + assertThat(line.getSections(3).getOptionList().getOption(0).getCombinedForm()) + .isEqualTo("--test_implicit_requirement=foo"); + assertThat(line.getSections(4).getChunkList().getChunkCount()).isEqualTo(0); + } + + @Test + public void testOptionWithImplicitRequirement_CanonicalCommandLine() + throws OptionsParsingException { + OptionsParser fakeStartupOptions = + OptionsParser.newOptionsParser(BlazeServerStartupOptions.class); + OptionsParser fakeCommandOptions = OptionsParser.newOptionsParser(TestOptions.class); + fakeCommandOptions.parse( + OptionPriority.COMMAND_LINE, + "command line", + ImmutableList.of("--test_implicit_requirement=foo")); + + CommandLine line = + new CanonicalCommandLineEvent( + "testblaze", fakeStartupOptions, "someCommandName", fakeCommandOptions) + .asStreamProto(null) + .getStructuredCommandLine(); + + assertThat(line).isNotNull(); + assertThat(line.getCommandLineLabel()).isEqualTo("canonical"); + checkCommandLineSectionLabels(line); + + // Unlike expansion flags, implicit requirements are not listed separately. + assertThat(line.getSections(0).getChunkList().getChunk(0)).isEqualTo("testblaze"); + assertThat(line.getSections(1).getOptionList().getOptionCount()).isEqualTo(2); + assertThat(line.getSections(2).getChunkList().getChunk(0)).isEqualTo("someCommandName"); + assertThat(line.getSections(3).getOptionList().getOptionCount()).isEqualTo(1); + assertThat(line.getSections(3).getOptionList().getOption(0).getCombinedForm()) + .isEqualTo("--test_implicit_requirement=foo"); + assertThat(line.getSections(4).getChunkList().getChunkCount()).isEqualTo(0); + } +} diff --git a/src/test/java/com/google/devtools/common/options/BUILD b/src/test/java/com/google/devtools/common/options/BUILD index 10eda9581c..fbb837eae9 100644 --- a/src/test/java/com/google/devtools/common/options/BUILD +++ b/src/test/java/com/google/devtools/common/options/BUILD @@ -4,11 +4,38 @@ filegroup( visibility = ["//src:__pkg__"], ) +java_library( + name = "testutils", + testonly = 1, + srcs = [ + "InvocationPolicyEnforcerTestBase.java", + "TestOptions.java", + ], + visibility = [ + "//src/test/java:__subpackages__", + ], + deps = [ + "//src/main/java/com/google/devtools/build/lib:build-base", + "//src/main/java/com/google/devtools/common/options", + "//src/main/java/com/google/devtools/common/options:invocation_policy", + "//src/main/protobuf:invocation_policy_java_proto", + "//third_party:guava", + "//third_party:junit4", + ], +) + java_test( name = "options_test", - srcs = glob(["*.java"]), + srcs = glob( + ["*.java"], + exclude = [ + "TestOptions.java", + "InvocationPolicyEnforcerTestBase.java", + ], + ), test_class = "com.google.devtools.common.options.AllTests", deps = [ + ":testutils", "//src/main/java/com/google/devtools/build/lib:build-base", "//src/main/java/com/google/devtools/build/lib:util", "//src/main/java/com/google/devtools/common/options", diff --git a/src/test/shell/integration/build_event_stream_test.sh b/src/test/shell/integration/build_event_stream_test.sh index ec59bc3ed1..ad0864015c 100755 --- a/src/test/shell/integration/build_event_stream_test.sh +++ b/src/test/shell/integration/build_event_stream_test.sh @@ -192,30 +192,62 @@ function test_basic() { # Basic properties of the event stream # - a completed target explicity requested should be reported # - after success the stream should close naturally, without any - # reports about aborted events. - # - the command line is reported + # reports about aborted events + # - the command line is reported in structured and unstructured form # - the target_kind is reported # - for single-configuration builds, there is precisely one configuration # event reported; also make variables are shown - bazel test --build_event_text_file=$TEST_log --tool_tag=MyFancyTool pkg:true \ + bazel test -k --build_event_text_file=$TEST_log --tool_tag=MyFancyTool pkg:true \ || fail "bazel test failed" expect_log 'pkg:true' # Command line - expect_log 'args: "test"' - expect_log 'args: "--build_event_text_file=' - expect_log 'args: "pkg:true"' - # Options parsed - expect_log 'tool_tag: "MyFancyTool"' + expect_log_once 'args: "test"' + expect_log_once 'args: "--build_event_text_file=' + expect_log_once 'args: "-k"' + expect_log_once 'args: "--tool_tag=MyFancyTool"' + expect_log_once 'args: "pkg:true"' + + # Options parsed. Since cmd_line lines are a substring of the equivalent + # explicit_cmd_line lines, we expect 2 instances for these. + expect_log_n 'cmd_line: "--tool_tag=MyFancyTool"' 2 + expect_log_n 'cmd_line: "--keep_going"' 2 + expect_log_once 'explicit_cmd_line: "--keep_going"' + expect_log_once 'explicit_cmd_line: "--tool_tag=MyFancyTool"' + expect_log_once 'tool_tag: "MyFancyTool"' + + # Structured command line. Expect the explicit flags to appear twice, + # in the canonical and original command lines + expect_log 'command_line_label: "original"' + expect_log 'command_line_label: "canonical"' + + expect_log_n 'combined_form: "-k"' 2 + expect_log_n 'option_name: "keep_going"' 2 + expect_log 'option_value: "1"' # too vague to count. + + expect_log_n 'combined_form: "--tool_tag=MyFancyTool"' 2 + expect_log_n 'option_name: "tool_tag"' 2 + expect_log_n 'option_value: "MyFancyTool"' 2 + + expect_log_n "combined_form: \"--build_event_text_file=${TEST_log}\"" 2 + expect_log_n 'option_name: "build_event_text_file"' 2 + expect_log_n "option_value: \"${TEST_log}\"" 2 + + expect_log_n 'chunk: "test"' 2 + expect_log_n 'chunk: "pkg:true"' 2 + # Build Finished expect_log 'build_finished' expect_log 'SUCCESS' expect_log 'finish_time' expect_not_log 'aborted' - # target kind for the sh_test + + # Target kind for the sh_test expect_log 'target_kind:.*sh' - # test size should be reported + + # Test size should be reported expect_log 'test_size: SMALL' - # configuration reported with make variables + + # Configuration reported with make variables expect_log_once '^configuration ' expect_log 'key: "TARGET_CPU"' } @@ -563,7 +595,6 @@ function test_loading_failure() { # being expanded. (bazel build --build_event_text_file=$TEST_log \ //does/not/exist && fail "build failure expected") || true - expect_log_once '^progress ' expect_log_once 'aborted' expect_log_once 'reason: LOADING_FAILURE' expect_log 'description.*BUILD file not found on package path' |