diff --git a/src/main/java/com/google/devtools/build/lib/analysis/platform/PlatformUtils.java b/src/main/java/com/google/devtools/build/lib/analysis/platform/PlatformUtils.java index 3366b6b3ab39fe..6f9315a91b6895 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/platform/PlatformUtils.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/platform/PlatformUtils.java @@ -76,7 +76,11 @@ public static Platform getPlatformProto( ? remoteOptions.getRemoteDefaultExecProperties() : ImmutableSortedMap.of(); - if (spawn.getExecutionPlatform() == null + if ((spawn.getExecutionPlatform() == null + || ( + spawn.getExecutionPlatform().execProperties().isEmpty() + && spawn.getExecutionPlatform().remoteExecutionProperties().isEmpty() + )) && spawn.getCombinedExecProperties().isEmpty() && defaultExecProperties.isEmpty() && additionalProperties.isEmpty()) { diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelStrategyModule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelStrategyModule.java index f503180cada615..3a88de5bfeda19 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelStrategyModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelStrategyModule.java @@ -18,6 +18,7 @@ import com.google.devtools.build.lib.analysis.actions.FileWriteActionContext; import com.google.devtools.build.lib.analysis.actions.TemplateExpansionContext; import com.google.devtools.build.lib.buildtool.BuildRequest; +import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.exec.ExecutionOptions; import com.google.devtools.build.lib.exec.ModuleActionContextRegistry; import com.google.devtools.build.lib.exec.SpawnCache; @@ -90,5 +91,9 @@ public void registerSpawnStrategies( for (Map.Entry> entry : options.strategyByRegexp) { registryBuilder.addDescriptionFilter(entry.getKey(), entry.getValue()); } + + for (Map.Entry> strategy : options.allowedStrategiesByExecPlatform) { + registryBuilder.addExecPlatformFilter(Label.parseCanonicalUnchecked(strategy.getKey()), strategy.getValue()); + } } } diff --git a/src/main/java/com/google/devtools/build/lib/exec/BUILD b/src/main/java/com/google/devtools/build/lib/exec/BUILD index 156a5ae39d161b..671efed7819abd 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/BUILD +++ b/src/main/java/com/google/devtools/build/lib/exec/BUILD @@ -106,6 +106,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/util:resource_converter", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", "//src/main/java/com/google/devtools/common/options", + "//src/main/java/com/google/devtools/build/lib/analysis:config/fragment_options", "//third_party:guava", ], ) @@ -364,6 +365,7 @@ java_library( ":remote_local_fallback_registry", ":spawn_strategy_policy", "//src/main/java/com/google/devtools/build/lib/actions", + "//src/main/java/com/google/devtools/build/lib/cmdline", "//src/main/java/com/google/devtools/build/lib/events", "//src/main/java/com/google/devtools/build/lib/util", "//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception", diff --git a/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java b/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java index 811567aea62c74..8b9ee99bd83d0e 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java +++ b/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java @@ -122,6 +122,23 @@ public class ExecutionOptions extends OptionsBase { + "the 'local' strategy, but reversing the order would run it with 'sandboxed'. ") public List>> strategyByRegexp; + @Option( + name = "allowed_strategies_by_exec_platform", + allowMultiple = true, + converter = Converters.StringToStringListConverter.class, + defaultValue = "null", + documentationCategory = OptionDocumentationCategory.EXECUTION_STRATEGY, + effectTags = {OptionEffectTag.EXECUTION}, + help = + """ + Filters spawn strategies by the execution platform. + Example: `--allowed_strategies_by_exec_platform=@platforms//host:host=local,sandboxed,worker` + to prevent actions configured for the host platform from being spawned remotely. + Example: `--allowed_strategies_by_exec_platform=//:linux_amd64=remote` to prevent actions + configured for a platform `//:linux_amd64` from being spawned locally. + """) + public List>> allowedStrategiesByExecPlatform; + @Option( name = "materialize_param_files", defaultValue = "false", diff --git a/src/main/java/com/google/devtools/build/lib/exec/RemoteLocalFallbackRegistry.java b/src/main/java/com/google/devtools/build/lib/exec/RemoteLocalFallbackRegistry.java index bfce3797d3cb36..14a020afa56cbf 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/RemoteLocalFallbackRegistry.java +++ b/src/main/java/com/google/devtools/build/lib/exec/RemoteLocalFallbackRegistry.java @@ -15,6 +15,7 @@ package com.google.devtools.build.lib.exec; import com.google.devtools.build.lib.actions.ActionContext; +import com.google.devtools.build.lib.actions.Spawn; import javax.annotation.Nullable; /** @@ -29,5 +30,5 @@ public interface RemoteLocalFallbackRegistry extends ActionContext { * @return remote fallback strategy or {@code null} if none was registered */ @Nullable - AbstractSpawnStrategy getRemoteLocalFallbackStrategy(); + AbstractSpawnStrategy getRemoteLocalFallbackStrategy(Spawn spawn); } diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnStrategyRegistry.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnStrategyRegistry.java index 775d9578ad56a1..045b6f2ca46e6c 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SpawnStrategyRegistry.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnStrategyRegistry.java @@ -22,6 +22,7 @@ import com.google.common.collect.ImmutableCollection; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableListMultimap; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMultimap; import com.google.common.collect.LinkedListMultimap; import com.google.common.collect.ListMultimap; @@ -34,6 +35,7 @@ import com.google.devtools.build.lib.actions.SandboxedSpawnStrategy; import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.SpawnStrategy; +import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.events.Reporter; @@ -69,6 +71,7 @@ public final class SpawnStrategyRegistry private final ImmutableListMultimap mnemonicToStrategies; private final StrategyRegexFilter strategyRegexFilter; + private final StrategyPlatformFilter strategyPlatformFilter; private final ImmutableList defaultStrategies; private final ImmutableMultimap mnemonicToRemoteDynamicStrategies; private final ImmutableMultimap mnemonicToLocalDynamicStrategies; @@ -77,12 +80,14 @@ public final class SpawnStrategyRegistry private SpawnStrategyRegistry( ImmutableListMultimap mnemonicToStrategies, StrategyRegexFilter strategyRegexFilter, + StrategyPlatformFilter strategyPlatformFilter, ImmutableList defaultStrategies, ImmutableMultimap mnemonicToRemoteDynamicStrategies, ImmutableMultimap mnemonicToLocalDynamicStrategies, @Nullable AbstractSpawnStrategy remoteLocalFallbackStrategy) { this.mnemonicToStrategies = mnemonicToStrategies; this.strategyRegexFilter = strategyRegexFilter; + this.strategyPlatformFilter = strategyPlatformFilter; this.defaultStrategies = defaultStrategies; this.mnemonicToRemoteDynamicStrategies = mnemonicToRemoteDynamicStrategies; this.mnemonicToLocalDynamicStrategies = mnemonicToLocalDynamicStrategies; @@ -105,7 +110,8 @@ private SpawnStrategyRegistry( * using the given {@link Reporter}. */ public List getStrategies(Spawn spawn, @Nullable EventHandler reporter) { - return getStrategies(spawn.getResourceOwner(), spawn.getMnemonic(), reporter); + return strategyPlatformFilter.getStrategies( + spawn, getStrategies(spawn.getResourceOwner(), spawn.getMnemonic(), reporter)); } /** @@ -116,6 +122,8 @@ public List getStrategies(Spawn spawn, @Nullable EventH * *

If the reason for selecting the context is worth mentioning to the user, logs a message * using the given {@link Reporter}. + * + * NOTE: This method is public for Blaze, getStrategies(Spawn, EventHandler) must be used in Bazel. */ public List getStrategies( ActionExecutionMetadata resourceOwner, String mnemonic, @Nullable EventHandler reporter) { @@ -154,7 +162,7 @@ public ImmutableCollection getDynamicSpawnActionContexts ? mnemonicToRemoteDynamicStrategies : mnemonicToLocalDynamicStrategies; if (mnemonicToDynamicStrategies.containsKey(spawn.getMnemonic())) { - return mnemonicToDynamicStrategies.get(spawn.getMnemonic()); + return strategyPlatformFilter.getStrategies(spawn, mnemonicToDynamicStrategies.get(spawn.getMnemonic())); } if (mnemonicToDynamicStrategies.containsKey("")) { return mnemonicToDynamicStrategies.get(""); @@ -164,8 +172,9 @@ public ImmutableCollection getDynamicSpawnActionContexts @Nullable @Override - public AbstractSpawnStrategy getRemoteLocalFallbackStrategy() { - return remoteLocalFallbackStrategy; + public AbstractSpawnStrategy getRemoteLocalFallbackStrategy(Spawn spawn) { + return strategyPlatformFilter.getStrategies(spawn, Lists.newArrayList(remoteLocalFallbackStrategy)) + .getFirst(); } /** @@ -203,9 +212,16 @@ void logSpawnStrategies() { strategyRegexFilter.getFilterToStrategies().asMap().entrySet()) { Collection value = entry.getValue(); logger.atInfo().log( - "FilterToStrategyImplementations: \"%s\" = [%s]", + "FilterDescriptionToStrategyImplementations: \"%s\" = [%s]", entry.getKey(), toImplementationNames(value)); } + for (Map.Entry> entry : + strategyPlatformFilter.getFilterToStrategies().entrySet()) { + Collection value = entry.getValue(); + logger.atInfo().log( + "FilterPlatformToStrategyImplementations: \"%s\" = [%s]", + entry.getKey().getCanonicalName(), toImplementationNames(value)); + } logger.atInfo().log( "DefaultStrategyImplementations: [%s]", toImplementationNames(defaultStrategies)); @@ -287,6 +303,7 @@ public static final class Builder { private final HashMap> mnemonicToRemoteDynamicIdentifiers = new HashMap<>(); private final HashMap> mnemonicToLocalDynamicIdentifiers = new HashMap<>(); + private final HashMap> execPlatformFilters = new HashMap<>(); @Nullable private String remoteLocalFallbackStrategyIdentifier; @@ -314,6 +331,12 @@ public Builder addDescriptionFilter(RegexFilter filter, List identifiers return this; } + @CanIgnoreReturnValue + public Builder addExecPlatformFilter(Label execPlatform, List identifiers) { + this.execPlatformFilters.put(execPlatform, identifiers); + return this; + } + /** * Adds a filter limiting any spawn whose {@linkplain Spawn#getMnemonic() mnemonic} * (case-sensitively) matches the given mnemonic to only use strategies with the given @@ -444,6 +467,14 @@ public SpawnStrategyRegistry build() throws AbruptExitException { } } + ImmutableMap.Builder> platformToStrategies = ImmutableMap.builder(); + for (Map.Entry> entry : execPlatformFilters.entrySet()) { + Label platform = entry.getKey(); + platformToStrategies.put( + platform, + strategyMapper.toStrategies(entry.getValue(), "platform " + platform.getCanonicalName())); + } + ImmutableListMultimap.Builder mnemonicToStrategies = new ImmutableListMultimap.Builder<>(); for (Map.Entry> entry : mnemonicToIdentifiers.entrySet()) { @@ -514,6 +545,7 @@ public SpawnStrategyRegistry build() throws AbruptExitException { mnemonicToStrategies.build(), new StrategyRegexFilter( strategyMapper, strategyPolicy, filterToIdentifiers, filterToStrategies), + new StrategyPlatformFilter(strategyMapper, platformToStrategies.build()), defaultStrategies, mnemonicToRemoteStrategies.build(), mnemonicToLocalStrategies.build(), @@ -595,6 +627,46 @@ public String toString() { } } + private static class StrategyPlatformFilter { + private final StrategyMapper strategyMapper; + private final ImmutableMap> platformToStrategies; + + private StrategyPlatformFilter( + StrategyMapper strategyMapper, + ImmutableMap> platformToStrategies) { + this.strategyMapper = strategyMapper; + this.platformToStrategies = platformToStrategies; + } + + public List getStrategies( + Spawn spawn, List candidateStrategies) { + var platformLabel = spawn.getExecutionPlatformLabel(); + Preconditions.checkNotNull(platformLabel, "Attempting to spawn action without an execution platform."); + + var allowedStrategies = platformToStrategies.get(platformLabel); + if (allowedStrategies != null) { + List filteredStrategies = new ArrayList<>(); + for (var strategy : candidateStrategies) { + if (allowedStrategies.contains(strategy)) { + filteredStrategies.add(strategy); + } + } + return filteredStrategies; + } + + return candidateStrategies; + } + + public ImmutableCollection getStrategies( + Spawn spawn, ImmutableCollection candidateStrategies) { + return ImmutableList.copyOf(getStrategies(spawn, Lists.newCopyOnWriteArrayList(candidateStrategies))); + } + + ImmutableMap> getFilterToStrategies() { + return platformToStrategies; + } + } + /* Maps the strategy identifier (e.g. "local", "worker"..) to the real strategy. */ private static class StrategyMapper { diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnStrategyResolver.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnStrategyResolver.java index 8a8182c2272e2d..830c8ce07feab0 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SpawnStrategyResolver.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnStrategyResolver.java @@ -86,10 +86,12 @@ public List resolve( if (fallbackStrategies.isEmpty()) { String message = String.format( - "%s spawn cannot be executed with any of the available strategies: %s. Your" - + " --spawn_strategy, --genrule_strategy and/or --strategy flags are probably" - + " too strict. Visit https://github.com/bazelbuild/bazel/issues/7480 for" - + " advice", + """ + %s spawn cannot be executed with any of the available strategies: %s. Your \ + --spawn_strategy, --genrule_strategy, strategy and/or \ + --allowed_strategies_by_exec_platform flags are probably too strict. \ + Visit https://github.com/bazelbuild/bazel/issues/7480 for advice. + """, spawn.getMnemonic(), strategies); throw new UserExecException( FailureDetail.newBuilder() diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java index fb4c8b86db1cb0..89b92192703ead 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java @@ -555,7 +555,7 @@ private SpawnResult execLocally(Spawn spawn, SpawnExecutionContext context) context.getContext(RemoteLocalFallbackRegistry.class); checkNotNull(localFallbackRegistry, "Expected a RemoteLocalFallbackRegistry to be registered"); AbstractSpawnStrategy remoteLocalFallbackStrategy = - localFallbackRegistry.getRemoteLocalFallbackStrategy(); + localFallbackRegistry.getRemoteLocalFallbackStrategy(spawn); checkNotNull( remoteLocalFallbackStrategy, "A remote local fallback strategy must be set if using remote fallback."); diff --git a/src/test/java/com/google/devtools/build/lib/exec/BUILD b/src/test/java/com/google/devtools/build/lib/exec/BUILD index bd7978c336e65c..bb4233da23c1e6 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/BUILD +++ b/src/test/java/com/google/devtools/build/lib/exec/BUILD @@ -45,6 +45,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/analysis:config/invalid_configuration_exception", "//src/main/java/com/google/devtools/build/lib/analysis:configured_target", "//src/main/java/com/google/devtools/build/lib/analysis:server_directories", + "//src/main/java/com/google/devtools/build/lib/analysis/platform", "//src/main/java/com/google/devtools/build/lib/bazel/rules/python", "//src/main/java/com/google/devtools/build/lib/buildeventstream/proto:build_event_stream_java_proto", "//src/main/java/com/google/devtools/build/lib/clock", diff --git a/src/test/java/com/google/devtools/build/lib/exec/SpawnStrategyRegistryTest.java b/src/test/java/com/google/devtools/build/lib/exec/SpawnStrategyRegistryTest.java index c6a4b2a79a39fc..84e32d76ab5bf7 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/SpawnStrategyRegistryTest.java +++ b/src/test/java/com/google/devtools/build/lib/exec/SpawnStrategyRegistryTest.java @@ -28,6 +28,8 @@ import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.SpawnResult; import com.google.devtools.build.lib.actions.SpawnStrategy; +import com.google.devtools.build.lib.analysis.platform.PlatformInfo; +import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.events.Event; @@ -300,6 +302,24 @@ public void testDuplicatedDescriptionFilter() throws Exception { .containsExactly(strategy2); } + @Test + public void testPlatformFilter() throws Exception { + NoopStrategy strategy1 = new NoopStrategy("1"); + NoopStrategy strategy2 = new NoopStrategy("2"); + SpawnStrategyRegistry strategyRegistry = + SpawnStrategyRegistry.builder() + .registerStrategy(strategy1, "foo") + .registerStrategy(strategy2, "bar") + .addExecPlatformFilter(PlatformInfo.EMPTY_PLATFORM_INFO.label(), ImmutableList.of("foo")) + .build(); + + assertThat( + strategyRegistry.getStrategies( + createSpawnWithMnemonicAndDescription("", ""), + SpawnStrategyRegistryTest::noopEventHandler)) + .containsExactly(strategy1); + } + @Test public void testMultipleDefaultStrategies() throws Exception { NoopStrategy strategy1 = new NoopStrategy("1"); @@ -537,7 +557,7 @@ public void testRemoteLocalFallback() throws Exception { .setRemoteLocalFallbackStrategyIdentifier("bar") .build(); - assertThat(strategyRegistry.getRemoteLocalFallbackStrategy()).isEqualTo(strategy2); + assertThat(strategyRegistry.getRemoteLocalFallbackStrategy(createSpawnWithMnemonicAndDescription("", ""))).isEqualTo(strategy2); } @Test @@ -561,7 +581,7 @@ public void testRemoteLocalFallbackNotRegistered() throws Exception { SpawnStrategyRegistry strategyRegistry = SpawnStrategyRegistry.builder().registerStrategy(strategy1, "foo").build(); - assertThat(strategyRegistry.getRemoteLocalFallbackStrategy()).isNull(); + assertThat(strategyRegistry.getRemoteLocalFallbackStrategy(createSpawnWithMnemonicAndDescription("", ""))).isNull(); } @Test diff --git a/src/test/java/com/google/devtools/build/lib/exec/util/FakeOwner.java b/src/test/java/com/google/devtools/build/lib/exec/util/FakeOwner.java index 8c581f529a2db7..f01b0a8d19d2cd 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/util/FakeOwner.java +++ b/src/test/java/com/google/devtools/build/lib/exec/util/FakeOwner.java @@ -80,7 +80,11 @@ private FakeOwner( } public FakeOwner(String mnemonic, String progressMessage, String ownerLabel) { - this(mnemonic, progressMessage, checkNotNull(ownerLabel), null); + this( + mnemonic, + progressMessage, + checkNotNull(ownerLabel), + PlatformInfo.EMPTY_PLATFORM_INFO); } @Override diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java index 18357f6c4048a6..94b01de2550ed7 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java @@ -264,7 +264,7 @@ private FakeSpawnExecutionContext getSpawnContext(Spawn spawn) { AbstractSpawnStrategy fakeLocalStrategy = new AbstractSpawnStrategy(execRoot, localRunner, new ExecutionOptions()) {}; ClassToInstanceMap actionContextRegistry = - ImmutableClassToInstanceMap.of(RemoteLocalFallbackRegistry.class, () -> fakeLocalStrategy); + ImmutableClassToInstanceMap.of(RemoteLocalFallbackRegistry.class, (_spawn) -> fakeLocalStrategy); var actionInputFetcher = new RemoteActionInputFetcher(