Skip to content

Commit

Permalink
Remove some dead code.
Browse files Browse the repository at this point in the history
    RELNOTES: None
    PiperOrigin-RevId: 256346218
  • Loading branch information
Luca Di Grazia committed Sep 4, 2022
1 parent 921947d commit 75b435b
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 59 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,6 @@
import java.nio.file.Files;
import java.nio.file.Paths;
import java.time.Duration;
import java.util.HashMap;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Executors;
Expand Down Expand Up @@ -424,17 +422,15 @@ public void blazeShutdown() {
}
}

private void waitForBuildEventTransportsToClose(
Map<BuildEventTransport, ListenableFuture<Void>> transportFutures)
throws AbruptExitException {
private void waitForBuildEventTransportsToClose() throws AbruptExitException {
final ScheduledExecutorService executor =
Executors.newSingleThreadScheduledExecutor(
new ThreadFactoryBuilder().setNameFormat("bes-notify-ui-%d").build());
ScheduledFuture<?> waitMessageFuture = null;

try {
// Notify the UI handler when a transport finished closing.
transportFutures.forEach(
closeFuturesWithTimeoutsMap.forEach(
(bepTransport, closeFuture) ->
closeFuture.addListener(
() -> {
Expand All @@ -443,7 +439,8 @@ private void waitForBuildEventTransportsToClose(
executor));

try (AutoProfiler p = AutoProfiler.logged("waiting for BES close", logger)) {
Uninterruptibles.getUninterruptibly(Futures.allAsList(transportFutures.values()));
Uninterruptibles.getUninterruptibly(
Futures.allAsList(closeFuturesWithTimeoutsMap.values()));
}
} catch (ExecutionException e) {
// Futures.withTimeout wraps the TimeoutException in an ExecutionException when the future
Expand Down Expand Up @@ -514,24 +511,21 @@ private void closeBepTransports() throws AbruptExitException {
constructCloseFuturesMapWithTimeouts(streamer.getCloseFuturesMap());
halfCloseFuturesWithTimeoutsMap =
constructCloseFuturesMapWithTimeouts(streamer.getHalfClosedMap());
switch (besOptions.besUploadMode) {
case WAIT_FOR_UPLOAD_COMPLETE:
waitForBuildEventTransportsToClose();
return;

Map<BuildEventTransport, ListenableFuture<Void>> blockingTransportFutures = new HashMap<>();
for (Map.Entry<BuildEventTransport, ListenableFuture<Void>> entry :
closeFuturesWithTimeoutsMap.entrySet()) {
BuildEventTransport bepTransport = entry.getKey();
if (!bepTransport.mayBeSlow()
|| besOptions.besUploadMode
== BuildEventServiceOptions.BesUploadMode.WAIT_FOR_UPLOAD_COMPLETE) {
blockingTransportFutures.put(bepTransport, entry.getValue());
} else {
case NOWAIT_FOR_UPLOAD_COMPLETE:
case FULLY_ASYNC:
// When running asynchronously notify the UI immediately since we won't wait for the
// uploads to close.
reporter.post(new BuildEventTransportClosedEvent(bepTransport));
}
}
if (!blockingTransportFutures.isEmpty()) {
waitForBuildEventTransportsToClose(blockingTransportFutures);
for (BuildEventTransport bepTransport : bepTransports) {
reporter.post(new BuildEventTransportClosedEvent(bepTransport));
}
return;
}
throw new IllegalStateException("Unknown BesUploadMode found: " + besOptions.besUploadMode);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,8 +290,7 @@ private CompilationInfo compile(
// TODO(b/70777494): Find out how deps get used and remove if not needed.
Iterable<? extends TransitiveInfoCollection> deps,
ObjcCppSemantics semantics,
String purpose,
boolean generateModuleMap)
String purpose)
throws RuleErrorException {
CcCompilationHelper result =
new CcCompilationHelper(
Expand Down Expand Up @@ -337,7 +336,7 @@ private CompilationInfo compile(
result.addNonModuleMapHeader(pchHdr);
}

if (getCustomModuleMap(ruleContext).isPresent() || !generateModuleMap) {
if (getCustomModuleMap(ruleContext).isPresent()) {
result.doNotGenerateModuleMap();
}

Expand Down Expand Up @@ -387,8 +386,7 @@ private Pair<CcCompilationOutputs, ImmutableMap<String, NestedSet<Artifact>>> cc
pchHdr,
deps,
semantics,
purpose,
/* generateModuleMap= */ true);
purpose);

purpose = String.format("%s_non_objc_arc", semantics.getPurpose());
extensionBuilder.setArcEnabled(false);
Expand All @@ -406,9 +404,7 @@ private Pair<CcCompilationOutputs, ImmutableMap<String, NestedSet<Artifact>>> cc
pchHdr,
deps,
semantics,
purpose,
// Only generate the module map once (see above) and re-use it here.
/* generateModuleMap= */ false);
purpose);

FeatureConfiguration featureConfiguration =
getFeatureConfiguration(ruleContext, ccToolchain, buildConfiguration, objcProvider);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,8 @@ private BlazeCommandResult execExclusively(
}

try (SilentCloseable closeable = Profiler.instance().profile("setup event handler")) {
UiOptions eventHandlerOptions = options.getOptions(UiOptions.class);
BlazeCommandEventHandler.Options eventHandlerOptions =
options.getOptions(BlazeCommandEventHandler.Options.class);
OutErr colorfulOutErr = outErr;

if (!eventHandlerOptions.useColor()) {
Expand Down Expand Up @@ -641,7 +642,8 @@ private OptionsParser createOptionsParser(BlazeCommand command)
}

/** Returns the event handler to use for this Blaze command. */
private EventHandler createEventHandler(OutErr outErr, UiOptions eventOptions) {
private EventHandler createEventHandler(
OutErr outErr, BlazeCommandEventHandler.Options eventOptions) {
Path workspacePath = runtime.getWorkspace().getDirectories().getWorkspace();
PathFragment workspacePathFragment = workspacePath == null ? null : workspacePath.asFragment();
return new ExperimentalEventHandler(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,21 +79,23 @@ public static class Options extends OptionsBase {
public double showProgressRateLimit;

@Option(
name = "color",
defaultValue = "auto",
converter = UseColorConverter.class,
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
effectTags = {OptionEffectTag.UNKNOWN},
help = "Use terminal controls to colorize output.")
name = "color",
defaultValue = "auto",
converter = UseColorConverter.class,
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
effectTags = {OptionEffectTag.UNKNOWN},
help = "Use terminal controls to colorize output going to stderr."
)
public UseColor useColorEnum;

@Option(
name = "curses",
defaultValue = "auto",
converter = UseCursesConverter.class,
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
effectTags = {OptionEffectTag.UNKNOWN},
help = "Use terminal cursor controls to minimize scrolling output.")
name = "curses",
defaultValue = "auto",
converter = UseCursesConverter.class,
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
effectTags = {OptionEffectTag.UNKNOWN},
help = "Use terminal cursor controls to minimize scrolling output going to stderr."
)
public UseCurses useCursesEnum;

@Option(
Expand All @@ -107,20 +109,21 @@ public static class Options extends OptionsBase {
public int terminalColumns;

@Option(
name = "isatty",
// TODO(b/137881511): Old name should be removed after 2020-01-01, or whenever is
// reasonable.
oldName = "is_stderr_atty",
defaultValue = "false",
metadataTags = {OptionMetadataTag.HIDDEN},
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
effectTags = {OptionEffectTag.UNKNOWN},
help =
"A system-generated parameter which is used to notify the "
+ "server whether this client is running in a terminal. "
+ "If this is set to false, then '--color=auto' will be treated as '--color=no'. "
+ "If this is set to true, then '--color=auto' will be treated as '--color=yes'.")
public boolean isATty;
name = "is_stderr_atty",
// TODO(b/63386499): Old name should be removed after 2019-02-28.
oldName = "isatty",
defaultValue = "false",
metadataTags = {OptionMetadataTag.HIDDEN},
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
effectTags = {OptionEffectTag.UNKNOWN},
help =
"A system-generated parameter which is used to notify the server whether this client is"
+ " running in a terminal. If this is set to false, then '--color=auto' will be"
+ " treated as '--color=no'. If this is set to true, then '--color=auto' will be"
+ " treated as '--color=yes'. As we only treat the stderr as a terminal, we only"
+ " care if that file descriptor is connected to a TTY."
)
public boolean isStderrATty;

// This lives here (as opposed to the more logical BuildRequest.Options)
// because the client passes it to the server *always*. We don't want the
Expand Down Expand Up @@ -253,11 +256,11 @@ public static class Options extends OptionsBase {
public boolean experimentalUiDeduplicate;

public boolean useColor() {
return useColorEnum == UseColor.YES || (useColorEnum == UseColor.AUTO && isATty);
return useColorEnum == UseColor.YES || (useColorEnum == UseColor.AUTO && isStderrATty);
}

public boolean useCursorControl() {
return useCursesEnum == UseCurses.YES || (useCursesEnum == UseCurses.AUTO && isATty);
return useCursesEnum == UseCurses.YES || (useCursesEnum == UseCurses.AUTO && isStderrATty);
}
}
}

0 comments on commit 75b435b

Please sign in to comment.