Repository: bazelbuild/bazel
Issue: Enable LocalConfigPlatformFunctionTest on Windows
Labels: type: bug
P2
team-Configurability

Currently it fails with the error:
```
1) generateConfigRepository(com.google.devtools.build.lib.bazel.repository.LocalConfigPlatformFunctionPosixTest$FunctionTest)
java.lang.AssertionError: ERROR /usr/local/google/_blaze_jrluser/FAKEMD5/external/local_config_platform_test/BUILD.bazel:3:1: no such package '@bazel_tools//platforms': /usr/local/google/_blaze_jrluser/FAKEMD5/external/bazel_tools must be an existing directory and referenced by '@local_config_platform_test//:host'
```

which seems to indicate a problem with the test environment setup.

Issue comments:
/sub
Debugging so far (by running tests in parallel and debugging linux vs windows:
The location of the bazel_tools repo is different in the Windows tests: on Linux it is (correctly) bazel_tools_workspace (set via /workspace/WORKSPACE). On windows, the default value from /DEFAULT.WORKSPACE (which is /usr/local/.../embedded_tools) is being used.

I can see that on Windows, the Package.Builder is seeing both versions of the rule added, but for some reason in this test the /workspace/WORKSPACE version isn't overwriting.

More debugging to come.
Wow, that was painful.

Source of the issue: the test sets OS to LINUX for purposes of the test, but the in-memory-filesystem checks the current value of OS to decide about things like drive strings and whether or not the filesystem is case-insensitive. If this is wrong, on Windows, when BazelAnalysisMock tries to re-write the WORKSPACE, it can't read the old value and instead overwrites it.

I still don't know why that happens, though.
Root cause: when the in-memory VFS wrote the file /workspace/WORKSPACE, the OS was Windows, so the written hash code was "WORKSPACE".toLowerCase().hashCode().
When it re-wrote the WORKSPACE, the OS was Linux, so the hash code was "WORKSPACE".hashCode().

This lead to the effective WORKSPACE file being replaced, instead of appended, and so the test value of "/bazel_tools_workspace" for @bazel_tools was dropped.

Sheesh.
Ugh.. Nice debugging @katre !

Pull request review comments:

Patch that resolved the issue:
diff --git a/src/main/java/com/google/devtools/build/lib/util/CPU.java b/src/main/java/com/google/devtools/build/lib/util/CPU.java
index 78545906487ca6..8a6de0f2c2ffb1 100644
--- a/src/main/java/com/google/devtools/build/lib/util/CPU.java
+++ b/src/main/java/com/google/devtools/build/lib/util/CPU.java
@@ -44,23 +44,14 @@ public String getCanonicalName() {
   }
 
   private static final CPU HOST_CPU = determineCurrentCpu();
-  @Nullable private static CPU cpuForTesting = null;
 
   /**
    * The current CPU.
    */
   public static CPU getCurrent() {
-    if (cpuForTesting != null) {
-      return cpuForTesting;
-    }
     return HOST_CPU;
   }
 
-  @VisibleForTesting
-  public static void setForTesting(@Nullable CPU cpu) {
-    cpuForTesting = cpu;
-  }
-
   private static CPU determineCurrentCpu() {
     String currentArch = OS_ARCH.value();
 
diff --git a/src/main/java/com/google/devtools/build/lib/util/OS.java b/src/main/java/com/google/devtools/build/lib/util/OS.java
index 70585ff93b7952..06266eccff3c2d 100644
--- a/src/main/java/com/google/devtools/build/lib/util/OS.java
+++ b/src/main/java/com/google/devtools/build/lib/util/OS.java
@@ -47,23 +47,14 @@ public String toString() {
   }
 
   private static final OS HOST_SYSTEM = determineCurrentOs();
-  @Nullable private static OS osForTesting = null;
 
   /**
    * The current operating system.
    */
   public static OS getCurrent() {
-    if (osForTesting != null) {
-      return osForTesting;
-    }
     return HOST_SYSTEM;
   }
 
-  @VisibleForTesting
-  public static void setForTesting(OS os) {
-    osForTesting = os;
-  }
-
   public static boolean isPosixCompatible() {
     return POSIX_COMPATIBLE.contains(getCurrent());
   }


Test patch:
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/BUILD b/src/test/java/com/google/devtools/build/lib/bazel/repository/BUILD
index 355cb47e9543e6..7e191f5c1e2dd1 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/repository/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/BUILD
@@ -9,7 +9,6 @@ filegroup(
     srcs = glob(["**"]) + [
         "//src/test/java/com/google/devtools/build/lib/bazel/repository/cache:srcs",
         "//src/test/java/com/google/devtools/build/lib/bazel/repository/downloader:srcs",
-        "//src/test/java/com/google/devtools/build/lib/bazel/repository/posix:srcs",
     ],
     visibility = ["//src/test/java/com/google/devtools/build/lib/bazel:__pkg__"],
 )
@@ -74,7 +73,6 @@ test_suite(
         ":windows_tests",
         "//src/test/java/com/google/devtools/build/lib/bazel/repository/cache:all_windows_tests",
         "//src/test/java/com/google/devtools/build/lib/bazel/repository/downloader:all_windows_tests",
-        "//src/test/java/com/google/devtools/build/lib/bazel/repository/posix:all_windows_tests",
     ],
     visibility = ["//src/test/java/com/google/devtools/build/lib/bazel:__pkg__"],
 )
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/LocalConfigPlatformFunctionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/LocalConfigPlatformFunctionTest.java
index 86cbbc376021fa..663d62d19d9310 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/repository/LocalConfigPlatformFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/LocalConfigPlatformFunctionTest.java
@@ -17,12 +17,20 @@
 import static com.google.common.truth.Truth.assertThat;
 
 import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.analysis.ConfiguredTarget;
+import com.google.devtools.build.lib.analysis.platform.ConstraintSettingInfo;
+import com.google.devtools.build.lib.analysis.platform.ConstraintValueInfo;
+import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
+import com.google.devtools.build.lib.analysis.platform.PlatformProviderUtils;
+import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
+import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.util.CPU;
 import com.google.devtools.build.lib.util.OS;
 import java.util.Collection;
 import org.junit.Test;
 import org.junit.experimental.runners.Enclosed;
 import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
 import org.junit.runners.Parameterized;
 import org.junit.runners.Parameterized.Parameters;
 
@@ -99,4 +107,45 @@ public void unknownOsConstraint() {
       assertThat(LocalConfigPlatformFunction.osToConstraint(OS.UNKNOWN)).isNull();
     }
   }
+
+  /** Tests on overall functionality. */
+  @RunWith(JUnit4.class)
+  public static class FunctionTest extends BuildViewTestCase {
+    private static final ConstraintSettingInfo CPU_CONSTRAINT =
+        ConstraintSettingInfo.create(Label.parseAbsoluteUnchecked("@bazel_tools//platforms:cpu"));
+    private static final ConstraintSettingInfo OS_CONSTRAINT =
+        ConstraintSettingInfo.create(Label.parseAbsoluteUnchecked("@bazel_tools//platforms:os"));
+
+    @Test
+    public void generateConfigRepository() throws Exception {
+      scratch.appendFile("WORKSPACE", "local_config_platform(name='local_config_platform_test')");
+      invalidatePackages();
+
+      // Verify the package was created as expected.
+      ConfiguredTarget hostPlatform = getConfiguredTarget("@local_config_platform_test//:host");
+      assertThat(hostPlatform).isNotNull();
+
+      PlatformInfo hostPlatformProvider = PlatformProviderUtils.platform(hostPlatform);
+      assertThat(hostPlatformProvider).isNotNull();
+
+      // Verify the OS and CPU constraints.
+      ConstraintValueInfo expectedCpuConstraint =
+          ConstraintValueInfo.create(
+              CPU_CONSTRAINT,
+              Label.parseAbsoluteUnchecked(
+                  LocalConfigPlatformFunction.cpuToConstraint(CPU.getCurrent())));
+      assertThat(hostPlatformProvider.constraints().has(CPU_CONSTRAINT)).isTrue();
+      assertThat(hostPlatformProvider.constraints().get(CPU_CONSTRAINT))
+          .isEqualTo(expectedCpuConstraint);
+
+      ConstraintValueInfo expectedOsConstraint =
+          ConstraintValueInfo.create(
+              OS_CONSTRAINT,
+              Label.parseAbsoluteUnchecked(
+                  LocalConfigPlatformFunction.osToConstraint(OS.getCurrent())));
+      assertThat(hostPlatformProvider.constraints().has(OS_CONSTRAINT)).isTrue();
+      assertThat(hostPlatformProvider.constraints().get(OS_CONSTRAINT))
+          .isEqualTo(expectedOsConstraint);
+    }
+  }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/posix/BUILD b/src/test/java/com/google/devtools/build/lib/bazel/repository/posix/BUILD
deleted file mode 100644
index ba02aada98a2af..00000000000000
--- a/src/test/java/com/google/devtools/build/lib/bazel/repository/posix/BUILD
+++ /dev/null
@@ -1,54 +0,0 @@
-package(
-    default_testonly = 1,
-    default_visibility = ["//src:__subpackages__"],
-)
-
-filegroup(
-    name = "srcs",
-    testonly = 0,
-    srcs = glob(["**"]),
-    visibility = ["//src/test/java/com/google/devtools/build/lib/bazel/repository:__pkg__"],
-)
-
-java_test(
-    name = "PosixRepositoryTests",
-    srcs = glob([
-        "**/*.java",
-    ]),
-    tags = [
-        "no_windows",  # Tests require Linux.
-        "rules",
-    ],
-    test_class = "com.google.devtools.build.lib.AllTests",
-    deps = [
-        "//src/main/java/com/google/devtools/build/lib:build-base",
-        "//src/main/java/com/google/devtools/build/lib:os_util",
-        "//src/main/java/com/google/devtools/build/lib:util",
-        "//src/main/java/com/google/devtools/build/lib/analysis/platform",
-        "//src/main/java/com/google/devtools/build/lib/analysis/platform:utils",
-        "//src/main/java/com/google/devtools/build/lib/cmdline",
-        "//src/test/java/com/google/devtools/build/lib:analysis_testutil",
-        "//third_party:guava",
-        "//third_party:guava-testlib",
-        "//third_party:jsr305",
-        "//third_party:junit4",
-        "//third_party:truth",
-    ],
-)
-
-test_suite(
-    name = "windows_tests",
-    tags = [
-        "-no_windows",
-        "-slow",
-    ],
-    visibility = ["//visibility:private"],
-)
-
-test_suite(
-    name = "all_windows_tests",
-    tests = [
-        ":windows_tests",
-    ],
-    visibility = ["//src/test/java/com/google/devtools/build/lib/bazel/repository:__pkg__"],
-)
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/posix/LocalConfigPlatformFunctionPosixTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/posix/LocalConfigPlatformFunctionPosixTest.java
deleted file mode 100644
index 914de0111469fc..00000000000000
--- a/src/test/java/com/google/devtools/build/lib/bazel/repository/posix/LocalConfigPlatformFunctionPosixTest.java
+++ /dev/null
@@ -1,76 +0,0 @@
-// Copyright 2018 The Bazel Authors. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//    http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.devtools.build.lib.bazel.repository.posix;
-
-import static com.google.common.truth.Truth.assertThat;
-
-import com.google.devtools.build.lib.analysis.ConfiguredTarget;
-import com.google.devtools.build.lib.analysis.platform.ConstraintSettingInfo;
-import com.google.devtools.build.lib.analysis.platform.ConstraintValueInfo;
-import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
-import com.google.devtools.build.lib.analysis.platform.PlatformProviderUtils;
-import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
-import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.util.CPU;
-import com.google.devtools.build.lib.util.OS;
-import org.junit.Test;
-import org.junit.experimental.runners.Enclosed;
-import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
-
-/** Posix-only tests for {@link LocalConfigPlatformFunction}. */
-@RunWith(Enclosed.class)
-public class LocalConfigPlatformFunctionPosixTest {
-
-  /** Tests on overall functionality. */
-  @RunWith(JUnit4.class)
-  public static class FunctionTest extends BuildViewTestCase {
-    private static final ConstraintSettingInfo CPU_CONSTRAINT =
-        ConstraintSettingInfo.create(Label.parseAbsoluteUnchecked("@bazel_tools//platforms:cpu"));
-    private static final ConstraintSettingInfo OS_CONSTRAINT =
-        ConstraintSettingInfo.create(Label.parseAbsoluteUnchecked("@bazel_tools//platforms:os"));
-
-    private static final ConstraintValueInfo X86_64_CONSTRAINT =
-        ConstraintValueInfo.create(
-            CPU_CONSTRAINT, Label.parseAbsoluteUnchecked("@bazel_tools//platforms:x86_64"));
-    private static final ConstraintValueInfo LINUX_CONSTRAINT =
-        ConstraintValueInfo.create(
-            OS_CONSTRAINT, Label.parseAbsoluteUnchecked("@bazel_tools//platforms:linux"));
-
-    @Test
-    public void generateConfigRepository() throws Exception {
-      CPU.setForTesting(CPU.X86_64);
-      OS.setForTesting(OS.LINUX);
-
-      scratch.appendFile("WORKSPACE", "local_config_platform(name='local_config_platform_test')");
-      invalidatePackages();
-
-      // Verify the package was created as expected.
-      ConfiguredTarget hostPlatform = getConfiguredTarget("@local_config_platform_test//:host");
-      assertThat(hostPlatform).isNotNull();
-
-      PlatformInfo hostPlatformProvider = PlatformProviderUtils.platform(hostPlatform);
-      assertThat(hostPlatformProvider).isNotNull();
-
-      // Verify the OS and CPU constraints.
-      assertThat(hostPlatformProvider.constraints().has(CPU_CONSTRAINT)).isTrue();
-      assertThat(hostPlatformProvider.constraints().get(CPU_CONSTRAINT))
-          .isEqualTo(X86_64_CONSTRAINT);
-
-      assertThat(hostPlatformProvider.constraints().has(OS_CONSTRAINT)).isTrue();
-      assertThat(hostPlatformProvider.constraints().get(OS_CONSTRAINT)).isEqualTo(LINUX_CONSTRAINT);
-    }
-  }
-}