Skip to content

Commit

Permalink
Make --experimental_skymeld_ui no-op.
Browse files Browse the repository at this point in the history
This is unnecessary. We don't have a use case now where this flag is enabled and
the skymeld flag isn't.

#14057

PiperOrigin-RevId: 539598912
Change-Id: I5e2bda47085c606728b3a4a19d38ee0afa214812
  • Loading branch information
joeleba authored and copybara-github committed Jun 12, 2023
1 parent 0254c61 commit dd7bdf2
Show file tree
Hide file tree
Showing 7 changed files with 37 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,14 @@ public static class BuildGraveyardOptions extends OptionsBase {
converter = Converters.CommaSeparatedOptionListConverter.class,
help = "Deprecated no-op.")
public List<String> availabilityInfoExempt;

@Option(
name = "experimental_skymeld_ui",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
effectTags = {OptionEffectTag.NO_OP},
help = "No-op. To be removed.")
public boolean skymeldUi;
}

/** This is where deprecated Bazel-specific options only used by the build command go to die. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,9 @@ private BlazeCommandResult execExclusively(

DebugLoggerConfigurator.setupLogging(commonOptions.verbosity);

EventHandler handler = createEventHandler(outErr, eventHandlerOptions);
EventHandler handler =
createEventHandler(
outErr, eventHandlerOptions, env.withMergedAnalysisAndExecutionSourceOfTruth());
reporter.addHandler(handler);
env.getEventBus().register(handler);

Expand All @@ -458,7 +460,10 @@ private BlazeCommandResult execExclusively(
// modified.
if (!eventHandlerOptions.useColor()) {
UiEventHandler ansiAllowingHandler =
createEventHandler(colorfulOutErr, eventHandlerOptions);
createEventHandler(
colorfulOutErr,
eventHandlerOptions,
env.withMergedAnalysisAndExecutionSourceOfTruth());
reporter.registerAnsiAllowingHandler(handler, ansiAllowingHandler);
env.getEventBus().register(new PassiveExperimentalEventHandler(ansiAllowingHandler));
}
Expand Down Expand Up @@ -794,10 +799,12 @@ private OptionsParser createOptionsParser(BlazeCommand command)
}

/** Returns the event handler to use for this Blaze command. */
private UiEventHandler createEventHandler(OutErr outErr, UiOptions eventOptions) {
private UiEventHandler createEventHandler(
OutErr outErr, UiOptions eventOptions, boolean skymeldMode) {
Path workspacePath = runtime.getWorkspace().getDirectories().getWorkspace();
PathFragment workspacePathFragment = workspacePath == null ? null : workspacePath.asFragment();
return new UiEventHandler(outErr, eventOptions, runtime.getClock(), workspacePathFragment);
return new UiEventHandler(
outErr, eventOptions, runtime.getClock(), workspacePathFragment, skymeldMode);
}

/** Returns the runtime instance shared by the commands that this dispatcher dispatches to. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,11 @@ public void flush() throws IOException {
}

public UiEventHandler(
OutErr outErr, UiOptions options, Clock clock, @Nullable PathFragment workspacePathFragment) {
OutErr outErr,
UiOptions options,
Clock clock,
@Nullable PathFragment workspacePathFragment,
boolean skymeldMode) {
this.terminalWidth = (options.terminalColumns > 0 ? options.terminalColumns : 80);
this.maxStdoutErrBytes = options.maxStdoutErrBytes;
this.outErr =
Expand All @@ -178,7 +182,7 @@ public UiEventHandler(
// additional character is written. Another column is lost for the continuation character
// in the wrapping process.

if (options.skymeldUi) {
if (skymeldMode) {
this.stateTracker =
this.cursorControl
? new SkymeldUiStateTracker(clock, /*targetWidth=*/ this.terminalWidth - 2)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -272,15 +272,6 @@ public UseCursesConverter() {
+ "-1 implies no limit.")
public int maxStdoutErrBytes;

@Option(
name = "experimental_skymeld_ui",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
effectTags = {OptionEffectTag.TERMINAL_OUTPUT},
help =
"Displays both analysis and execution phase progress when both are running concurrently.")
public boolean skymeldUi;

public boolean useColor() {
return useColorEnum == UseColor.YES || (useColorEnum == UseColor.AUTO && isATty);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ public final class UiEventHandlerStdOutAndStdErrTest {
"\033[31m\033[1mERROR: \033[0mBuild did NOT complete successfully" + System.lineSeparator();

@TestParameter private TestedOutput testedOutput;
@TestParameter private boolean skymeldMode;

private UiEventHandler uiEventHandler;
private FlushCollectingOutputStream output;
Expand Down Expand Up @@ -84,7 +85,12 @@ private void createUiEventHandler(UiOptions uiOptions) {
}

uiEventHandler =
new UiEventHandler(outErr, uiOptions, new ManualClock(), /* workspacePathFragment= */ null);
new UiEventHandler(
outErr,
uiOptions,
new ManualClock(),
/* workspacePathFragment= */ null,
/* skymeldMode= */ skymeldMode);
uiEventHandler.mainRepoMappingComputationStarted(new MainRepoMappingComputationStartingEvent());
uiEventHandler.buildStarted(
BuildStartingEvent.create(
Expand Down
2 changes: 1 addition & 1 deletion src/test/shell/bazel/external_integration_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2792,7 +2792,7 @@ genrule(
EOF
execroot="$(bazel info execution_root)"

bazel build --experimental_merged_skyframe_analysis_execution --experimental_skymeld_ui //:foo \
bazel build --experimental_merged_skyframe_analysis_execution //:foo \
|| fail 'Expected build to succeed with Skymeld'

test -h "$execroot/external/ext" || fail "Expected symlink to external repo."
Expand Down
8 changes: 4 additions & 4 deletions src/test/shell/integration/ui_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ function test_timestamp() {
}

function test_skymeld_ui() {
bazel build --experimental_skymeld_ui pkg:true &> "$TEST_log" \
bazel build --experimental_merged_skyframe_analysis_execution pkg:true &> "$TEST_log" \
|| fail "${PRODUCT_NAME} test failed."
expect_log 'Build completed successfully'
}
Expand Down Expand Up @@ -339,15 +339,15 @@ string_flag(
)
EOF

bazel build --experimental_skymeld_ui \
bazel build --experimental_merged_skyframe_analysis_execution \
--//$pkg/flags:flag=a \
$pkg:true &> "$TEST_log" || fail "${PRODUCT_NAME} test failed."
expect_log 'Build completed successfully'
}

# Regression test for b/244163231.
function test_skymeld_ui_works_with_timestamps() {
bazel build --experimental_skymeld_ui --show_timestamps \
bazel build --experimental_merged_skyframe_analysis_execution --show_timestamps \
pkg:true &> "$TEST_log" \
|| fail "${PRODUCT_NAME} test failed."
expect_log 'Build completed successfully'
Expand Down Expand Up @@ -493,7 +493,7 @@ function test_experimental_ui_attempt_to_print_relative_paths_failing_action() {
# unconditionally uses an uppercase drive letter (see
# WindowsOsPathPolicy#normalize). I want these tests to check for exact
# string contents (that's the entire goal of the flag being tested), but I
# don't want them to be brittle across different Windows enviromments, so
# don't want them to be brittle across different Windows environments, so
# I've disabled them for now.
# TODO(nharmata): Fix this.
[[ "$is_windows" == "true" ]] && return 0
Expand Down

0 comments on commit dd7bdf2

Please sign in to comment.