Skip to content

Commit bbd9187

Browse files
tjgqcopybara-github
authored andcommitted
Simplify service registration and create a way for a service to supply options.
The service registry is not needed: the service component can simply pass in a list of instantiated services into BlazeRuntime.main(), and the lookup can be done by iterating the list (there will only be a few of them and the iteration is done at startup, so performance isn't a concern). The design mirrors the way BlazeModules work. Also introduce OptionsSupplier, a super-interface of BlazeModule and BlazeService containing only the methods related to options. This provides a uniform way to collect them from both components. PiperOrigin-RevId: 823940987 Change-Id: Ifc414b211d4afc79737941987ec1b6e315772175
1 parent 12d6875 commit bbd9187

23 files changed

+283
-245
lines changed

src/main/java/com/google/devtools/build/lib/BUILD

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,22 @@ java_library(
299299
],
300300
)
301301

302+
java_library(
303+
name = "runtime/options_supplier",
304+
srcs = ["runtime/OptionsSupplier.java"],
305+
deps = ["//src/main/java/com/google/devtools/common/options"],
306+
)
307+
308+
java_library(
309+
name = "runtime/blaze_service",
310+
srcs = ["runtime/BlazeService.java"],
311+
deps = [
312+
":runtime/options_supplier",
313+
"//src/main/java/com/google/devtools/common/options",
314+
"//third_party:guava",
315+
],
316+
)
317+
302318
java_library(
303319
name = "runtime",
304320
srcs = glob(
@@ -312,6 +328,7 @@ java_library(
312328
"buildtool/BaselineCLDiffEvent.java",
313329
"buildtool/SymlinkForest.java",
314330
"runtime/BlazeCommandResult.java",
331+
"runtime/BlazeService.java",
315332
"runtime/CommandDispatcher.java",
316333
"runtime/CommandLinePathFactory.java",
317334
"runtime/ConfigFlagDefinitions.java",
@@ -321,6 +338,7 @@ java_library(
321338
"runtime/MemoryPressureEvent.java",
322339
"runtime/MemoryPressureOptions.java",
323340
"runtime/MemoryPressureStatCollector.java",
341+
"runtime/OptionsSupplier.java",
324342
"runtime/StarlarkOptionsParser.java",
325343
"runtime/TestSummaryOptions.java",
326344
],
@@ -332,10 +350,12 @@ java_library(
332350
":keep-going-option",
333351
":loading-phase-threads-option",
334352
":runtime/blaze_command_result",
353+
":runtime/blaze_service",
335354
":runtime/command_dispatcher",
336355
":runtime/command_line_path_factory",
337356
":runtime/config_flag_definitions",
338357
":runtime/memory_pressure",
358+
":runtime/options_supplier",
339359
":runtime/test_summary_options",
340360
":starlark_options_parser",
341361
"//src/main/java/com/google/devtools/build/lib/actions",

src/main/java/com/google/devtools/build/lib/bazel/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,7 @@ java_library(
169169
":repository_module",
170170
":spawn_log_module",
171171
"//src/main/java/com/google/devtools/build/lib:runtime",
172+
"//src/main/java/com/google/devtools/build/lib:runtime/blaze_service",
172173
"//src/main/java/com/google/devtools/build/lib/analysis:blaze_version_info",
173174
"//src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper:credential_module",
174175
"//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:bazel_lockfile_module",

src/main/java/com/google/devtools/build/lib/bazel/Bazel.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import com.google.devtools.build.lib.authandtls.credentialhelper.CredentialModule;
2020
import com.google.devtools.build.lib.runtime.BlazeModule;
2121
import com.google.devtools.build.lib.runtime.BlazeRuntime;
22+
import com.google.devtools.build.lib.runtime.BlazeService;
2223
import java.io.IOException;
2324
import java.io.InputStream;
2425
import java.util.Properties;
@@ -99,9 +100,12 @@ public final class Bazel {
99100
com.google.devtools.build.lib.metrics.PostGCMemoryUseRecorder.GcAfterBuildModule.class,
100101
com.google.devtools.build.lib.metrics.MetricsModule.class);
101102

103+
@SuppressWarnings("UnnecessarilyFullyQualified") // Class names fully qualified for clarity.
104+
public static final ImmutableList<BlazeService> BAZEL_SERVICES = ImmutableList.of();
105+
102106
public static void main(String[] args) {
103107
BlazeVersionInfo.setBuildInfo(tryGetBuildInfo());
104-
BlazeRuntime.main(BAZEL_MODULES, args);
108+
BlazeRuntime.main(BAZEL_MODULES, BAZEL_SERVICES, args);
105109
}
106110

107111
/**

src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ public OpaqueOptionsData load(BlazeCommand command) {
129129
return OptionsParser.getOptionsData(
130130
BlazeCommandUtils.getOptions(
131131
command.getClass(),
132-
runtime.getBlazeModules(),
132+
runtime.getOptionsSuppliers(),
133133
runtime.getRuleClassProvider()));
134134
}
135135
});

src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandUtils.java

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -46,21 +46,21 @@ public class BlazeCommandUtils {
4646
private BlazeCommandUtils() {}
4747

4848
public static ImmutableList<Class<? extends OptionsBase>> getStartupOptions(
49-
Iterable<BlazeModule> modules) {
49+
Iterable<OptionsSupplier> suppliers) {
5050
Set<Class<? extends OptionsBase>> options = new HashSet<>(DEFAULT_STARTUP_OPTIONS);
51-
for (BlazeModule blazeModule : modules) {
52-
Iterables.addAll(options, blazeModule.getStartupOptions());
51+
for (OptionsSupplier supplier : suppliers) {
52+
Iterables.addAll(options, supplier.getStartupOptions());
5353
}
5454

5555
return ImmutableList.copyOf(options);
5656
}
5757

5858
public static ImmutableSet<Class<? extends OptionsBase>> getCommonOptions(
59-
Iterable<BlazeModule> modules) {
59+
Iterable<OptionsSupplier> suppliers) {
6060
ImmutableSet.Builder<Class<? extends OptionsBase>> builder = ImmutableSet.builder();
6161
builder.addAll(COMMON_COMMAND_OPTIONS);
62-
for (BlazeModule blazeModule : modules) {
63-
builder.addAll(blazeModule.getCommonCommandOptions());
62+
for (OptionsSupplier supplier : suppliers) {
63+
builder.addAll(supplier.getCommonCommandOptions());
6464
}
6565
return builder.build();
6666
}
@@ -73,26 +73,26 @@ public static ImmutableSet<Class<? extends OptionsBase>> getCommonOptions(
7373
*/
7474
public static ImmutableList<Class<? extends OptionsBase>> getOptions(
7575
Class<? extends BlazeCommand> clazz,
76-
Iterable<BlazeModule> modules,
76+
Iterable<OptionsSupplier> suppliers,
7777
ConfiguredRuleClassProvider ruleClassProvider) {
7878
Command commandAnnotation = clazz.getAnnotation(Command.class);
7979
if (commandAnnotation == null) {
8080
throw new IllegalStateException("@Command missing for " + clazz.getName());
8181
}
8282

83-
Set<Class<? extends OptionsBase>> options = new HashSet<>(getCommonOptions(modules));
83+
Set<Class<? extends OptionsBase>> options = new HashSet<>(getCommonOptions(suppliers));
8484
Collections.addAll(options, commandAnnotation.options());
8585

8686
if (commandAnnotation.usesConfigurationOptions()) {
8787
options.addAll(ruleClassProvider.getFragmentRegistry().getOptionsClasses());
8888
}
8989

90-
for (BlazeModule blazeModule : modules) {
91-
Iterables.addAll(options, blazeModule.getCommandOptions(commandAnnotation.name()));
90+
for (OptionsSupplier supplier : suppliers) {
91+
Iterables.addAll(options, supplier.getCommandOptions(commandAnnotation.name()));
9292
}
9393

9494
for (Class<? extends BlazeCommand> base : commandAnnotation.inheritsOptionsFrom()) {
95-
options.addAll(getOptions(base, modules, ruleClassProvider));
95+
options.addAll(getOptions(base, suppliers, ruleClassProvider));
9696
}
9797
return ImmutableList.copyOf(options);
9898
}
@@ -159,15 +159,15 @@ public static String expandHelpTopic(
159159
public static String getUsage(
160160
Class<? extends BlazeCommand> commandClass,
161161
OptionsParser.HelpVerbosity verbosity,
162-
Iterable<BlazeModule> blazeModules,
162+
Iterable<OptionsSupplier> optionsSuppliers,
163163
ConfiguredRuleClassProvider ruleClassProvider,
164164
String productName) {
165165
Command commandAnnotation = commandClass.getAnnotation(Command.class);
166166
return BlazeCommandUtils.expandHelpTopic(
167167
commandAnnotation.name(),
168168
commandAnnotation.help(),
169169
commandClass,
170-
getOptions(commandClass, blazeModules, ruleClassProvider),
170+
getOptions(commandClass, optionsSuppliers, ruleClassProvider),
171171
verbosity,
172172
productName);
173173
}

src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java

Lines changed: 13 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -55,21 +55,20 @@
5555
import javax.annotation.Nullable;
5656

5757
/**
58-
* A module Bazel can load at the beginning of its execution. Modules are supplied with extension
59-
* points to augment the functionality at specific, well-defined places.
58+
* Provides the ability to augment the functionality of the Logical Component (LC).
6059
*
61-
* <p>The constructors of individual Bazel modules should be empty. All work should be done in the
62-
* methods (e.g. {@link #blazeStartup}).
60+
* <p>The augmentation is done by implementing one or more of the methods in this class, which are
61+
* called at well-defined points during the server's lifecycle.
62+
*
63+
* <p>The set of modules is passed into {@link BlazeRuntime#main} and is fixed for the lifetime of
64+
* the server. A module can be obtained by calling {@link BlazeRuntime#getBlazeModule}.
65+
*
66+
* <p>The constructors of individual Bazel modules must take no arguments and be empty. All work
67+
* should be done in the methods (e.g. {@link #blazeStartup}).
6368
*/
64-
public abstract class BlazeModule {
69+
public abstract class BlazeModule implements OptionsSupplier {
6570

66-
/**
67-
* Returns the extra startup options this module contributes.
68-
*
69-
* <p>This method will be called at the beginning of Blaze startup (before {@link #globalInit}).
70-
* The startup options need to be parsed very early in the process, which requires this to be
71-
* separate from {@link #serverInit}.
72-
*/
71+
@Override
7372
public Iterable<Class<? extends OptionsBase>> getStartupOptions() {
7473
return ImmutableList.of();
7574
}
@@ -240,37 +239,12 @@ public OutputService getOutputService() throws AbruptExitException {
240239
return null;
241240
}
242241

243-
/**
244-
* Returns extra options this module contributes to a specific command. Note that option
245-
* inheritance applies: if this method returns a non-empty list, then the returned options are
246-
* added to every command that depends on this command.
247-
*
248-
* <p>This method may be called at any time, and the returned value may be cached. Implementations
249-
* must be thread-safe and never return different lists for the same command name. Typical
250-
* implementations look like this:
251-
*
252-
* <pre>
253-
* return commandName.equals("build")
254-
* ? ImmutableList.<Class<? extends OptionsBase>>of(MyOptions.class)
255-
* : ImmutableList.<Class<? extends OptionsBase>>of();
256-
* </pre>
257-
*
258-
* Note that this example adds options to all commands that inherit from the build command.
259-
*
260-
* <p>This method is also used to generate command-line documentation; in order to avoid
261-
* duplicated options descriptions, this method should never return the same options class for two
262-
* different commands if one of them inherits the other.
263-
*
264-
* <p>If you want to add options to all commands, override {@link #getCommonCommandOptions}
265-
* instead.
266-
*
267-
* @param commandName the command name, e.g. "build" or "test".
268-
*/
242+
@Override
269243
public Iterable<Class<? extends OptionsBase>> getCommandOptions(String commandName) {
270244
return ImmutableList.of();
271245
}
272246

273-
/** Returns extra options this module contributes to all commands. */
247+
@Override
274248
public Iterable<Class<? extends OptionsBase>> getCommonCommandOptions() {
275249
return ImmutableList.of();
276250
}

src/main/java/com/google/devtools/build/lib/runtime/BlazeOptionHandler.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ public final class BlazeOptionHandler {
128128
.flatMap(
129129
cmd ->
130130
BlazeCommandUtils.getOptions(
131-
cmd, runtime.getBlazeModules(), runtime.getRuleClassProvider())
131+
cmd, runtime.getOptionsSuppliers(), runtime.getRuleClassProvider())
132132
.stream())
133133
.distinct()
134134
.collect(toImmutableList());

0 commit comments

Comments
 (0)