Normalize C++ action config tool paths based on exec platform OS

RELNOTES: Tool paths specified in `cc_toolchain` action configs are now normalized based on the current execution platform's OS rather than the host OS. In particular, Windows-style absolute paths are now treated as absolute paths when building on a Windows executor from a non-Windows host.

src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java

@@ -1469,6 +1469,17 @@ public boolean isExecutedOnWindows() {
         .hasConstraintValue(OS_TO_CONSTRAINTS.get(OS.WINDOWS));
   }
 
+  /** Returns the OS of the execution platform. */
+  public OS getExecutionPlatformOs() {
+    for (var osToConstraint : OS_TO_CONSTRAINTS.entrySet()) {
+      if (getExecutionPlatform().constraints().hasConstraintValue(osToConstraint.getValue())) {
+        return osToConstraint.getKey();
+      }
+    }
+    // Fall back to assuming exec OS == host OS.
+    return OS.getCurrent();
+  }
+
   /**
    * @return the set of features applicable for the current rule.
    */

src/main/java/com/google/devtools/build/lib/analysis/constraints/ConstraintConstants.java

@@ -30,33 +30,25 @@ public final class ConstraintConstants {
 
   // Standard mapping between OS and the corresponding platform constraints.
   public static final ImmutableMap<OS, ConstraintValueInfo> OS_TO_CONSTRAINTS =
-      ImmutableMap.<OS, ConstraintValueInfo>builder()
-          .put(
-              OS.DARWIN,
-              ConstraintValueInfo.create(
-                  OS_CONSTRAINT_SETTING,
-                  Label.parseCanonicalUnchecked("@platforms//os:osx")))
-          .put(
-              OS.WINDOWS,
-              ConstraintValueInfo.create(
-                  OS_CONSTRAINT_SETTING,
-                  Label.parseCanonicalUnchecked("@platforms//os:windows")))
-          .put(
-              OS.FREEBSD,
-              ConstraintValueInfo.create(
-                  OS_CONSTRAINT_SETTING,
-                  Label.parseCanonicalUnchecked("@platforms//os:freebsd")))
-          .put(
-              OS.OPENBSD,
-              ConstraintValueInfo.create(
-                  OS_CONSTRAINT_SETTING,
-                  Label.parseCanonicalUnchecked("@platforms//os:openbsd")))
-          .put(
-              OS.UNKNOWN,
-              ConstraintValueInfo.create(
-                  OS_CONSTRAINT_SETTING,
-                  Label.parseCanonicalUnchecked("@platforms//os:none")))
-          .buildOrThrow();
+      ImmutableMap.of(
+          OS.LINUX,
+          ConstraintValueInfo.create(
+              OS_CONSTRAINT_SETTING, Label.parseCanonicalUnchecked("@platforms//os:linux")),
+          OS.DARWIN,
+          ConstraintValueInfo.create(
+              OS_CONSTRAINT_SETTING, Label.parseCanonicalUnchecked("@platforms//os:osx")),
+          OS.WINDOWS,
+          ConstraintValueInfo.create(
+              OS_CONSTRAINT_SETTING, Label.parseCanonicalUnchecked("@platforms//os:windows")),
+          OS.FREEBSD,
+          ConstraintValueInfo.create(
+              OS_CONSTRAINT_SETTING, Label.parseCanonicalUnchecked("@platforms//os:freebsd")),
+          OS.OPENBSD,
+          ConstraintValueInfo.create(
+              OS_CONSTRAINT_SETTING, Label.parseCanonicalUnchecked("@platforms//os:openbsd")),
+          OS.UNKNOWN,
+          ConstraintValueInfo.create(
+              OS_CONSTRAINT_SETTING, Label.parseCanonicalUnchecked("@platforms//os:none")));
 
   // No-op constructor to keep this from being instantiated.
   private ConstraintConstants() {}

src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java

@@ -75,6 +75,7 @@
 import com.google.devtools.build.lib.starlarkbuildapi.cpp.CcModuleApi;
 import com.google.devtools.build.lib.starlarkbuildapi.cpp.ExtraLinkTimeLibraryApi;
 import com.google.devtools.build.lib.util.FileTypeSet;
+import com.google.devtools.build.lib.util.OS;
 import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.lib.util.StringUtil;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -1205,10 +1206,11 @@ public CcToolchainConfigInfo ccToolchainConfigInfoFromStarlark(
     ImmutableSet<String> featureNames =
         featureList.stream().map(Feature::getName).collect(toImmutableSet());
 
+    OS execOs = starlarkRuleContext.getRuleContext().getExecutionPlatformOs();
     ImmutableList.Builder<ActionConfig> actionConfigBuilder = ImmutableList.builder();
     for (Object actionConfig : actionConfigs) {
       checkRightStarlarkInfoProvider(actionConfig, "action_configs", "ActionConfigInfo");
-      actionConfigBuilder.add(actionConfigFromStarlark((StarlarkInfo) actionConfig));
+      actionConfigBuilder.add(actionConfigFromStarlark((StarlarkInfo) actionConfig, execOs));
     }
     ImmutableList<ActionConfig> actionConfigList = actionConfigBuilder.build();
 
@@ -1676,7 +1678,8 @@ static FlagSet flagSetFromStarlark(StarlarkInfo flagSetStruct, String actionName
    * {@link StarlarkInfo}.
    */
   @VisibleForTesting
-  static CcToolchainFeatures.Tool toolFromStarlark(StarlarkInfo toolStruct) throws EvalException {
+  static CcToolchainFeatures.Tool toolFromStarlark(StarlarkInfo toolStruct, OS execOs)
+      throws EvalException {
     checkRightProviderType(toolStruct, "tool");
 
     String toolPathString = getOptionalFieldFromStarlarkProvider(toolStruct, "path", String.class);
@@ -1690,7 +1693,12 @@ static CcToolchainFeatures.Tool toolFromStarlark(StarlarkInfo toolStruct) throws
         throw infoError(toolStruct, "\"tool\" and \"path\" cannot be set at the same time.");
       }
 
-      toolPath = PathFragment.create(toolPathString);
+      try {
+        toolPath = PathFragment.createForOs(toolPathString, execOs);
+      } catch (PathFragment.PathUnsupportedOnThisOs e) {
+        throw infoError(
+            toolStruct, "The 'path' field of tool is not a valid path: %s", e.getMessage());
+      }
       if (toolPath.isEmpty()) {
         throw infoError(toolStruct, "The 'path' field of tool must be a nonempty string.");
       }
@@ -1723,7 +1731,7 @@ static CcToolchainFeatures.Tool toolFromStarlark(StarlarkInfo toolStruct) throws
 
   /** Creates an {@link ActionConfig} from a {@link StarlarkInfo}. */
   @VisibleForTesting
-  static ActionConfig actionConfigFromStarlark(StarlarkInfo actionConfigStruct)
+  static ActionConfig actionConfigFromStarlark(StarlarkInfo actionConfigStruct, OS execOs)
       throws EvalException {
     checkRightProviderType(actionConfigStruct, "action_config");
     String actionName =
@@ -1748,7 +1756,7 @@ static ActionConfig actionConfigFromStarlark(StarlarkInfo actionConfigStruct)
     ImmutableList<StarlarkInfo> toolStructs =
         getStarlarkProviderListFromStarlarkField(actionConfigStruct, "tools");
     for (StarlarkInfo toolStruct : toolStructs) {
-      toolBuilder.add(toolFromStarlark(toolStruct));
+      toolBuilder.add(toolFromStarlark(toolStruct, execOs));
     }
 
     ImmutableList.Builder<FlagSet> flagSetBuilder = ImmutableList.builder();

src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java

@@ -979,21 +979,16 @@ private static void checkToolPath(PathFragment toolPath, CToolchain.Tool.PathOri
 
     /** Returns the path to this action's tool relative to the provided crosstool path. */
     String getToolPathString(PathFragment ccToolchainPath) {
-      switch (toolPathOrigin) {
-        case CROSSTOOL_PACKAGE:
+      return switch (toolPathOrigin) {
+        case CROSSTOOL_PACKAGE -> {
           // Legacy behavior.
           if (toolPathString == null) {
             toolPathString = ccToolchainPath.getRelative(toolPathFragment).getSafePathString();
           }
-          return toolPathString;
-
-        case FILESYSTEM_ROOT: // fallthrough.
-        case WORKSPACE_ROOT:
-          return toolPathFragment.getSafePathString();
-      }
-
-      // Unreached.
-      throw new IllegalStateException();
+          yield toolPathString;
+        }
+        case FILESYSTEM_ROOT, WORKSPACE_ROOT -> toolPathFragment.getSafePathString();
+      };
     }
 
     /**

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

@@ -51,6 +51,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/skyframe/serialization",
         "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec:serialization-constant",
         "//src/main/java/com/google/devtools/build/lib/util:filetype",
+        "//src/main/java/com/google/devtools/build/lib/util:os",
         "//third_party:error_prone_annotations",
         "//third_party:guava",
         "//third_party:jsr305",

src/main/java/com/google/devtools/build/lib/vfs/OsPathPolicy.java

@@ -86,14 +86,23 @@ public interface OsPathPolicy {
 
   boolean isCaseSensitive();
 
-  // We *should* use a case-insensitive policy for OS.DARWIN, but we currently don't handle this.
-  OsPathPolicy HOST_POLICY =
-      OS.getCurrent() == OS.WINDOWS ? WindowsOsPathPolicy.INSTANCE : UnixOsPathPolicy.INSTANCE;
+  OsPathPolicy HOST_POLICY = getFilePathOs(OS.getCurrent());
 
   static OsPathPolicy getFilePathOs() {
     return HOST_POLICY;
   }
 
+  static OsPathPolicy getFilePathOs(OS os) {
+    if (os != OS.WINDOWS) {
+      // We *should* use a case-insensitive policy for OS.DARWIN, but we currently don't handle
+      // this.
+      return UnixOsPathPolicy.INSTANCE;
+    }
+    return os == OS.getCurrent()
+        ? WindowsOsPathPolicy.INSTANCE
+        : WindowsOsPathPolicy.CROSS_PLATFORM_INSTANCE;
+  }
+
   /** Utilities for implementations of {@link OsPathPolicy}. */
   class Utils {
     /**
@@ -135,4 +144,15 @@ static int removeRelativePaths(String[] segments, int starti, boolean isAbsolute
       return segmentCount;
     }
   }
+
+  /**
+   * Unchecked exception thrown by {@link OsPathPolicy} implementations when a path cannot be
+   * normalized on the current host OS.
+   */
+  final class UncheckedPathUnsupportedOnThisOsException
+      extends UnsupportedOperationException {
+    UncheckedPathUnsupportedOnThisOsException(String message) {
+      super(message);
+    }
+  }
 }

src/main/java/com/google/devtools/build/lib/vfs/PathFragment.java

@@ -68,15 +68,28 @@ public abstract class PathFragment
 
   /** Creates a new normalized path fragment. */
   public static PathFragment create(String path) {
+    return createInternal(path, OS);
+  }
+
+  public static PathFragment createForOs(String path, com.google.devtools.build.lib.util.OS os)
+      throws PathUnsupportedOnThisOs {
+    try {
+      return createInternal(path, OsPathPolicy.getFilePathOs(os));
+    } catch (OsPathPolicy.UncheckedPathUnsupportedOnThisOsException e) {
+      throw new PathUnsupportedOnThisOs(e);
+    }
+  }
+
+  private static PathFragment createInternal(String path, OsPathPolicy osPathPolicy) {
     if (path.isEmpty()) {
       return EMPTY_FRAGMENT;
     }
-    int normalizationLevel = OS.needsToNormalize(path);
+    int normalizationLevel = osPathPolicy.needsToNormalize(path);
     String normalizedPath =
         normalizationLevel != OsPathPolicy.NORMALIZED
-            ? OS.normalize(path, normalizationLevel)
+            ? osPathPolicy.normalize(path, normalizationLevel)
             : path;
-    int driveStrLength = OS.getDriveStrLength(normalizedPath);
+    int driveStrLength = osPathPolicy.getDriveStrLength(normalizedPath);
     return makePathFragment(normalizedPath, driveStrLength);
   }
 
@@ -126,6 +139,16 @@ public boolean isEmpty() {
    */
   public abstract int getDriveStrLength();
 
+  /**
+   * Thrown by {@link #createForOs(String, com.google.devtools.build.lib.util.OS)} * when a path
+   * cannot be normalized on the current host OS.
+   */
+  public static final class PathUnsupportedOnThisOs extends Exception {
+    private PathUnsupportedOnThisOs(OsPathPolicy.UncheckedPathUnsupportedOnThisOsException e) {
+      super(e.getMessage(), e);
+    }
+  }
+
   private static final class RelativePathFragment extends PathFragment {
     // DON'T add any fields here unless you know what you are doing. Adding another field will
     // increase the shallow heap of a RelativePathFragment instance beyond the current value of 16

src/main/java/com/google/devtools/build/lib/vfs/WindowsOsPathPolicy.java

@@ -23,7 +23,11 @@
 @VisibleForTesting
 class WindowsOsPathPolicy implements OsPathPolicy {
 
-  static final WindowsOsPathPolicy INSTANCE = new WindowsOsPathPolicy();
+  static final WindowsOsPathPolicy INSTANCE =
+      new WindowsOsPathPolicy(new DefaultShortPathResolver());
+
+  static final WindowsOsPathPolicy CROSS_PLATFORM_INSTANCE =
+      new WindowsOsPathPolicy(new CrossPlatformShortPathResolver());
 
   static final int NEEDS_SHORT_PATH_NORMALIZATION = NEEDS_NORMALIZE + 1;
 
@@ -33,7 +37,7 @@ class WindowsOsPathPolicy implements OsPathPolicy {
   private final ShortPathResolver shortPathResolver;
 
   interface ShortPathResolver {
-    String resolveShortPath(String path);
+    String resolveShortPath(String path) throws UncheckedPathUnsupportedOnThisOsException;
   }
 
   static class DefaultShortPathResolver implements ShortPathResolver {
@@ -47,10 +51,15 @@ public String resolveShortPath(String path) {
     }
   }
 
-  WindowsOsPathPolicy() {
-    this(new DefaultShortPathResolver());
+  static class CrossPlatformShortPathResolver implements ShortPathResolver {
+    @Override
+    public String resolveShortPath(String path) {
+      throw new UncheckedPathUnsupportedOnThisOsException(
+          "Windows short paths can only be resolved on a Windows host: " + path);
+    }
   }
 
+  @VisibleForTesting
   WindowsOsPathPolicy(ShortPathResolver shortPathResolver) {
     this.shortPathResolver = shortPathResolver;
   }

src/test/java/com/google/devtools/build/lib/rules/cpp/BUILD

@@ -674,6 +674,7 @@ java_test(
         "//src/main/java/com/google/devtools/build/lib/rules/cpp",
         "//src/main/java/com/google/devtools/build/lib/skyframe:bzl_load_value",
         "//src/main/java/com/google/devtools/build/lib/util",
+        "//src/main/java/com/google/devtools/build/lib/util:os",
         "//src/main/java/com/google/devtools/build/lib/vfs",
         "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
         "//src/main/java/net/starlark/java/eval",