diff options
7 files changed, 45 insertions, 76 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/PlatformOptions.java b/src/main/java/com/google/devtools/build/lib/analysis/PlatformOptions.java index c1d0877254..445f8e7684 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/PlatformOptions.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/PlatformOptions.java @@ -45,19 +45,6 @@ public class PlatformOptions extends FragmentOptions { ) public Label hostPlatform; - @Option( - name = "host_platform_remote_properties_override", - oldName = "experimental_remote_platform_override", - defaultValue = "null", - category = "remote", - documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, - effectTags = {OptionEffectTag.UNKNOWN}, - help = - "Manually set the remote_execution_properties for the host platform" - + " if it is not already set." - ) - public String hostPlatformRemotePropertiesOverride; - // TODO(katre): Add execution platforms. @Option( @@ -128,9 +115,6 @@ public class PlatformOptions extends FragmentOptions { host.hostPlatform = this.hostPlatform; host.extraToolchains = this.extraToolchains; host.enabledToolchainTypes = this.enabledToolchainTypes; - host.hostPlatformRemotePropertiesOverride = this.hostPlatformRemotePropertiesOverride; - host.toolchainResolutionDebug = this.toolchainResolutionDebug; - host.toolchainResolutionOverrides = this.toolchainResolutionOverrides; return host; } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/platform/PlatformInfo.java b/src/main/java/com/google/devtools/build/lib/analysis/platform/PlatformInfo.java index bfee814b6c..a5f29fc4f4 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/platform/PlatformInfo.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/platform/PlatformInfo.java @@ -208,11 +208,6 @@ public class PlatformInfo extends NativeInfo { return this; } - /** Returns the data being sent to a potential remote executor. */ - public String getRemoteExecutionProperties() { - return remoteExecutionProperties; - } - /** * Sets the data being sent to a potential remote executor. * diff --git a/src/main/java/com/google/devtools/build/lib/remote/BUILD b/src/main/java/com/google/devtools/build/lib/remote/BUILD index 51c33841ae..8ac275eea1 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/BUILD +++ b/src/main/java/com/google/devtools/build/lib/remote/BUILD @@ -22,7 +22,6 @@ java_library( "//src/main/java/com/google/devtools/build/lib:runtime", "//src/main/java/com/google/devtools/build/lib:util", "//src/main/java/com/google/devtools/build/lib/actions", - "//src/main/java/com/google/devtools/build/lib/analysis/platform", "//src/main/java/com/google/devtools/build/lib/authandtls", "//src/main/java/com/google/devtools/build/lib/buildeventstream", "//src/main/java/com/google/devtools/build/lib/concurrent", diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteOptions.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteOptions.java index b9b1f55d79..4b257d7ac4 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteOptions.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteOptions.java @@ -20,6 +20,9 @@ import com.google.devtools.common.options.Option; import com.google.devtools.common.options.OptionDocumentationCategory; import com.google.devtools.common.options.OptionEffectTag; import com.google.devtools.common.options.OptionsBase; +import com.google.devtools.remoteexecution.v1test.Platform; +import com.google.protobuf.TextFormat; +import com.google.protobuf.TextFormat.ParseException; /** Options for remote execution and distributed caching. */ public final class RemoteOptions extends OptionsBase { @@ -110,6 +113,16 @@ public final class RemoteOptions extends OptionsBase { public boolean remoteUploadLocalResults; @Option( + name = "experimental_remote_platform_override", + defaultValue = "null", + category = "remote", + documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, + effectTags = {OptionEffectTag.UNKNOWN}, + help = "Temporary, for testing only. Manually set a Platform to pass to remote execution." + ) + public String experimentalRemotePlatformOverride; + + @Option( name = "remote_instance_name", defaultValue = "", category = "remote", @@ -211,4 +224,19 @@ public final class RemoteOptions extends OptionsBase { help = "A file path to a local disk cache." ) public PathFragment experimentalLocalDiskCachePath; + + public Platform parseRemotePlatformOverride() { + if (experimentalRemotePlatformOverride != null) { + Platform.Builder platformBuilder = Platform.newBuilder(); + try { + TextFormat.getParser().merge(experimentalRemotePlatformOverride, platformBuilder); + } catch (ParseException e) { + throw new IllegalArgumentException( + "Failed to parse --experimental_remote_platform_override", e); + } + return platformBuilder.build(); + } else { + return null; + } + } } diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java index e8af197907..9cf68d114c 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java @@ -32,6 +32,7 @@ import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.remoteexecution.v1test.Action; import com.google.devtools.remoteexecution.v1test.ActionResult; import com.google.devtools.remoteexecution.v1test.Command; +import com.google.devtools.remoteexecution.v1test.Platform; import io.grpc.Context; import java.io.IOException; import java.util.Collection; @@ -51,6 +52,8 @@ import javax.annotation.Nullable; final class RemoteSpawnCache implements SpawnCache { private final Path execRoot; private final RemoteOptions options; + // TODO(olaola): This will be set on a per-action basis instead. + private final Platform platform; private final AbstractRemoteActionCache remoteCache; private final String buildRequestId; @@ -75,6 +78,7 @@ final class RemoteSpawnCache implements SpawnCache { DigestUtil digestUtil) { this.execRoot = execRoot; this.options = options; + this.platform = options.parseRemotePlatformOverride(); this.remoteCache = remoteCache; this.verboseFailures = verboseFailures; this.cmdlineReporter = cmdlineReporter; @@ -99,7 +103,7 @@ final class RemoteSpawnCache implements SpawnCache { spawn.getOutputFiles(), digestUtil.compute(command), repository.getMerkleDigest(inputRoot), - spawn.getExecutionPlatform(), + platform, policy.getTimeout(), Spawns.mayBeCached(spawn)); 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 55a64dfb1d..87a3058f1a 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 @@ -26,8 +26,6 @@ import com.google.devtools.build.lib.actions.SpawnResult; import com.google.devtools.build.lib.actions.SpawnResult.Status; import com.google.devtools.build.lib.actions.Spawns; import com.google.devtools.build.lib.actions.cache.VirtualActionInput; -import com.google.devtools.build.lib.analysis.platform.PlatformInfo; -import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.Reporter; @@ -48,8 +46,6 @@ import com.google.devtools.remoteexecution.v1test.Digest; import com.google.devtools.remoteexecution.v1test.ExecuteRequest; import com.google.devtools.remoteexecution.v1test.ExecuteResponse; import com.google.devtools.remoteexecution.v1test.Platform; -import com.google.protobuf.TextFormat; -import com.google.protobuf.TextFormat.ParseException; import io.grpc.Context; import io.grpc.Status.Code; import java.io.IOException; @@ -74,6 +70,8 @@ class RemoteSpawnRunner implements SpawnRunner { private final Path execRoot; private final RemoteOptions options; + // TODO(olaola): This will be set on a per-action basis instead. + private final Platform platform; private final SpawnRunner fallbackRunner; private final boolean verboseFailures; @@ -100,6 +98,7 @@ class RemoteSpawnRunner implements SpawnRunner { DigestUtil digestUtil) { this.execRoot = execRoot; this.options = options; + this.platform = options.parseRemotePlatformOverride(); this.fallbackRunner = fallbackRunner; this.remoteCache = remoteCache; this.remoteExecutor = remoteExecutor; @@ -131,7 +130,7 @@ class RemoteSpawnRunner implements SpawnRunner { spawn.getOutputFiles(), digestUtil.compute(command), repository.getMerkleDigest(inputRoot), - spawn.getExecutionPlatform(), + platform, policy.getTimeout(), Spawns.mayBeCached(spawn)); @@ -265,10 +264,9 @@ class RemoteSpawnRunner implements SpawnRunner { Collection<? extends ActionInput> outputs, Digest command, Digest inputRoot, - @Nullable PlatformInfo executionPlatform, + Platform platform, Duration timeout, boolean cacheable) { - Action.Builder action = Action.newBuilder(); action.setCommandDigest(command); action.setInputRootDigest(inputRoot); @@ -285,14 +283,9 @@ class RemoteSpawnRunner implements SpawnRunner { Collections.sort(outputPaths); Collections.sort(outputDirectoryPaths); action.addAllOutputFiles(outputPaths); - - // Get the remote platform properties. - if (executionPlatform != null) { - Platform platform = - parsePlatform(executionPlatform.label(), executionPlatform.remoteExecutionProperties()); + if (platform != null) { action.setPlatform(platform); } - if (!timeout.isZero()) { action.setTimeout(com.google.protobuf.Duration.newBuilder().setSeconds(timeout.getSeconds())); } @@ -302,21 +295,6 @@ class RemoteSpawnRunner implements SpawnRunner { return action.build(); } - static Platform parsePlatform(Label platformLabel, @Nullable String platformDescription) { - Platform.Builder platformBuilder = Platform.newBuilder(); - try { - if (platformDescription != null) { - TextFormat.getParser().merge(platformDescription, platformBuilder); - } - } catch (ParseException e) { - throw new IllegalArgumentException( - String.format( - "Failed to parse remote_execution_properties from platform %s", platformLabel), - e); - } - return platformBuilder.build(); - } - static Command buildCommand(List<String> arguments, ImmutableMap<String, String> env) { Command.Builder command = Command.newBuilder(); command.addAllArguments(arguments); diff --git a/src/main/java/com/google/devtools/build/lib/rules/platform/Platform.java b/src/main/java/com/google/devtools/build/lib/rules/platform/Platform.java index 07196bb41d..99cc90b436 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/platform/Platform.java +++ b/src/main/java/com/google/devtools/build/lib/rules/platform/Platform.java @@ -17,7 +17,6 @@ package com.google.devtools.build.lib.rules.platform; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.FileProvider; import com.google.devtools.build.lib.analysis.FilesToRunProvider; -import com.google.devtools.build.lib.analysis.PlatformOptions; import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder; import com.google.devtools.build.lib.analysis.RuleConfiguredTargetFactory; import com.google.devtools.build.lib.analysis.RuleContext; @@ -40,40 +39,22 @@ public class Platform implements RuleConfiguredTargetFactory { PlatformInfo.Builder platformBuilder = PlatformInfo.builder().setLabel(ruleContext.getLabel()); - Boolean isHostPlatform = - ruleContext.attributes().get(PlatformRule.HOST_PLATFORM_ATTR, Type.BOOLEAN); - Boolean isTargetPlatform = - ruleContext.attributes().get(PlatformRule.TARGET_PLATFORM_ATTR, Type.BOOLEAN); - if (isHostPlatform && isTargetPlatform) { - ruleContext.attributeError( - PlatformRule.HOST_PLATFORM_ATTR, - "A single platform cannot have both host_platform and target_platform set."); - return null; - } else if (isHostPlatform) { + if (ruleContext.attributes().get(PlatformRule.HOST_PLATFORM_ATTR, Type.BOOLEAN)) { // Create default constraints based on the current host OS and CPU values. String cpuOption = ruleContext.getConfiguration().getHostCpu(); autodetectConstraints(cpuOption, ruleContext, platformBuilder); - } else if (isTargetPlatform) { + } else if (ruleContext.attributes().get(PlatformRule.TARGET_PLATFORM_ATTR, Type.BOOLEAN)) { // Create default constraints based on the current OS and CPU values. String cpuOption = ruleContext.getConfiguration().getCpu(); autodetectConstraints(cpuOption, ruleContext, platformBuilder); + } else { + platformBuilder.addConstraints( + PlatformProviderUtils.constraintValues( + ruleContext.getPrerequisites(PlatformRule.CONSTRAINT_VALUES_ATTR, Mode.DONT_CHECK))); } - // Add the declared constraints. Because setting the host_platform or target_platform attribute - // to true on a platform automatically includes the detected CPU and OS constraints, if the - // constraint_values attribute tries to add those, this will throw an error. - platformBuilder.addConstraints( - PlatformProviderUtils.constraintValues( - ruleContext.getPrerequisites(PlatformRule.CONSTRAINT_VALUES_ATTR, Mode.DONT_CHECK))); - String remoteExecutionProperties = ruleContext.attributes().get(PlatformRule.REMOTE_EXECUTION_PROPS_ATTR, Type.STRING); - if (platformBuilder.getRemoteExecutionProperties() == null && isHostPlatform) { - // Use the default override. - PlatformOptions platformOptions = - ruleContext.getConfiguration().getOptions().get(PlatformOptions.class); - remoteExecutionProperties = platformOptions.hostPlatformRemotePropertiesOverride; - } platformBuilder.setRemoteExecutionProperties(remoteExecutionProperties); PlatformInfo platformInfo; |