Enabled CI for Windows (#585)

UebelAndre · alexeagle · web-flow · commit 628d40fc82af · 2022-01-02T21:55:36.000-08:00
Co-authored-by: Alex Eagle &lt;eagle@post.harvard.edu&gt;
diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml
@@ -4,17 +4,33 @@ buildifier:
   # keep this argument in sync with .pre-commit-config.yaml
   warnings: "all"
 all_targets: &all_targets
-    build_targets:
+  build_targets:
     - "..."
     # As a regression test for #225, check that wheel targets still build when
     # their package path is qualified with the repo name.
     - "@rules_python//examples/wheel/..."
-    # We control Bazel version in integration tests, so we don't need USE_BAZEL_VERSION for tests.
-    skip_use_bazel_version_for_test: true
-    test_targets:
+  # We control Bazel version in integration tests, so we don't need USE_BAZEL_VERSION for tests.
+  skip_use_bazel_version_for_test: true
+  test_targets:
     - "..."
 platforms:
   ubuntu1804:
     <<: *all_targets
   macos:
     <<: *all_targets
+  windows:
+    build_targets:
+      - "--" # Allows negative patterns; hack for https://github.com/bazelbuild/continuous-integration/pull/245
+      - "..."
+      # Gazelle is not fully Windows compatible: https://github.com/bazelbuild/bazel-gazelle/issues/1122
+      - "-//gazelle/..."
+      # As a regression test for #225, check that wheel targets still build when
+      # their package path is qualified with the repo name.
+      - "@rules_python//examples/wheel/..."
+      # We control Bazel version in integration tests, so we don't need USE_BAZEL_VERSION for tests.
+    skip_use_bazel_version_for_test: true
+    test_targets:
+      - "--" # Allows negative patterns; hack for https://github.com/bazelbuild/continuous-integration/pull/245
+      - "..."
+      # Gazelle is not fully Windows compatible: https://github.com/bazelbuild/bazel-gazelle/issues/1122
+      - "-//gazelle/..."
diff --git a/.bazelrc b/.bazelrc
@@ -15,3 +15,6 @@ test --test_output=errors
 # creating (possibly empty) __init__.py files and adding them to the srcs of
 # Python targets as required.
 build --incompatible_default_to_explicit_init_py
+
+# Windows makes use of runfiles for some rules
+build --enable_runfiles
diff --git a/docs/BUILD b/docs/BUILD
@@ -78,17 +78,27 @@ bzl_library(
     ],
 )
 
+# TODO: Stardoc does not guarantee consistent outputs accross platforms (Unix/Windows).
+# As a result we do not build or test docs on Windows.
+_NOT_WINDOWS = select({
+    "@platforms//os:linux": [],
+    "@platforms//os:macos": [],
+    "//conditions:default": ["@platforms//:incompatible"],
+})
+
 stardoc(
     name = "core-docs",
     out = "python.md_",
     input = "//python:defs.bzl",
+    target_compatible_with = _NOT_WINDOWS,
     deps = [":defs"],
 )
 
 stardoc(
     name = "pip-docs",
     out = "pip.md_",
     input = "//python:pip.bzl",
+    target_compatible_with = _NOT_WINDOWS,
     deps = [
         ":bazel_repo_tools",
         ":pip_install_bzl",
@@ -100,6 +110,7 @@ stardoc(
     name = "packaging-docs",
     out = "packaging.md_",
     input = "//python:packaging.bzl",
+    target_compatible_with = _NOT_WINDOWS,
     deps = [":packaging_bzl"],
 )
 
@@ -109,6 +120,7 @@ stardoc(
         failure_message = "Please run:   bazel run //docs:update",
         file1 = k + ".md",
         file2 = k + ".md_",
+        target_compatible_with = _NOT_WINDOWS,
     )
     for k in _DOCS.keys()
 ]
@@ -123,10 +135,12 @@ write_file(
         "cp -fv bazel-bin/docs/{0}.md_ docs/{0}.md".format(k)
         for k in _DOCS.keys()
     ],
+    target_compatible_with = _NOT_WINDOWS,
 )
 
 sh_binary(
     name = "update",
     srcs = ["update.sh"],
     data = _DOCS.values(),
+    target_compatible_with = _NOT_WINDOWS,
 )
diff --git a/examples/wheel/wheel_test.py b/examples/wheel/wheel_test.py
@@ -13,6 +13,7 @@
 # limitations under the License.
 
 import os
+import platform
 import unittest
 import zipfile
 
@@ -89,9 +90,24 @@ def test_customized_wheel(self):
                 "example_customized-0.0.1.dist-info/entry_points.txt"
             )
             # The entries are guaranteed to be sorted.
-            self.assertEquals(
-                record_contents,
-                b"""\
+            if platform.system() == "Windows":
+                self.assertEquals(
+                    record_contents,
+                    b"""\
+example_customized-0.0.1.dist-info/METADATA,sha256=pzE96o3Sp63TDzxAZgl0F42EFevm8x15vpDLqDVp_EQ,378
+example_customized-0.0.1.dist-info/RECORD,,
+example_customized-0.0.1.dist-info/WHEEL,sha256=sobxWSyDDkdg_rinUth-jxhXHqoNqlmNMJY3aTZn2Us,91
+example_customized-0.0.1.dist-info/entry_points.txt,sha256=pqzpbQ8MMorrJ3Jp0ntmpZcuvfByyqzMXXi2UujuXD0,137
+examples/wheel/lib/data.txt,sha256=9vJKEdfLu8bZRArKLroPZJh1XKkK3qFMXiM79MBL2Sg,12
+examples/wheel/lib/module_with_data.py,sha256=8s0Khhcqz3yVsBKv2IB5u4l4TMKh7-c_V6p65WVHPms,637
+examples/wheel/lib/simple_module.py,sha256=z2hwciab_XPNIBNH8B1Q5fYgnJvQTeYf0ZQJpY8yLLY,637
+examples/wheel/main.py,sha256=sgg5iWN_9inYBjm6_Zw27hYdmo-l24fA-2rfphT-IlY,909
+""",
+                )
+            else:
+                self.assertEquals(
+                    record_contents,
+                    b"""\
 example_customized-0.0.1.dist-info/METADATA,sha256=TeeEmokHE2NWjkaMcVJuSAq4_AXUoIad2-SLuquRmbg,372
 example_customized-0.0.1.dist-info/RECORD,,
 example_customized-0.0.1.dist-info/WHEEL,sha256=sobxWSyDDkdg_rinUth-jxhXHqoNqlmNMJY3aTZn2Us,91
@@ -101,7 +117,7 @@ def test_customized_wheel(self):
 examples/wheel/lib/simple_module.py,sha256=z2hwciab_XPNIBNH8B1Q5fYgnJvQTeYf0ZQJpY8yLLY,637
 examples/wheel/main.py,sha256=sgg5iWN_9inYBjm6_Zw27hYdmo-l24fA-2rfphT-IlY,909
 """,
-            )
+                )
             self.assertEquals(
                 wheel_contents,
                 b"""\
@@ -111,9 +127,28 @@ def test_customized_wheel(self):
 Tag: py3-none-any
 """,
             )
-            self.assertEquals(
-                metadata_contents,
-                b"""\
+            if platform.system() == "Windows":
+                self.assertEquals(
+                    metadata_contents,
+                    b"""\
+Metadata-Version: 2.1
+Name: example_customized
+Version: 0.0.1
+Author: Example Author with non-ascii characters: \xc3\x85\xc2\xbc\xc3\x83\xc2\xb3\xc3\x85\xc2\x82w
+Author-email: example@example.com
+Home-page: www.example.com
+License: Apache 2.0
+Classifier: License :: OSI Approved :: Apache Software License
+Classifier: Intended Audience :: Developers
+Requires-Dist: pytest
+
+This is a sample description of a wheel.
+""",
+                )
+            else:
+                self.assertEquals(
+                    metadata_contents,
+                    b"""\
 Metadata-Version: 2.1
 Name: example_customized
 Version: 0.0.1
@@ -127,7 +162,7 @@ def test_customized_wheel(self):
 
 This is a sample description of a wheel.
 """,
-            )
+                )
             self.assertEquals(
                 entry_point_contents,
                 b"""\
diff --git a/python/pip_install/extract_wheels/__init__.py b/python/pip_install/extract_wheels/__init__.py
@@ -7,7 +7,6 @@
 """
 import argparse
 import glob
-import json
 import os
 import pathlib
 import subprocess
diff --git a/python/pip_install/extract_wheels/lib/BUILD b/python/pip_install/extract_wheels/lib/BUILD
@@ -73,14 +73,11 @@ py_test(
 py_test(
     name = "whl_filegroup_test",
     size = "small",
-    srcs = [
-        "whl_filegroup_test.py",
-    ],
+    srcs = ["whl_filegroup_test.py"],
     data = ["//examples/wheel:minimal_with_py_package"],
+    main = "whl_filegroup_test.py",
     tags = ["unit"],
-    deps = [
-        ":lib",
-    ],
+    deps = [":lib"],
 )
 
 py_test(
diff --git a/python/pip_install/extract_wheels/lib/bazel.py b/python/pip_install/extract_wheels/lib/bazel.py
@@ -296,6 +296,7 @@ def extract_wheel(
     enable_implicit_namespace_pkgs: bool,
     repo_prefix: str,
     incremental: bool = False,
+    incremental_dir: Path = Path("."),
 ) -> Optional[str]:
     """Extracts wheel into given directory and creates py_library and filegroup targets.
 
@@ -306,14 +307,15 @@ def extract_wheel(
         enable_implicit_namespace_pkgs: if true, disables conversion of implicit namespace packages and will unzip as-is
         incremental: If true the extract the wheel in a format suitable for an external repository. This
             effects the names of libraries and their dependencies, which point to other external repositories.
+        incremental_dir: An optional override for the working directory of incremental builds.
 
     Returns:
         The Bazel label for the extracted wheel, in the form '//path/to/wheel'.
     """
 
     whl = wheel.Wheel(wheel_file)
     if incremental:
-        directory = "."
+        directory = incremental_dir
     else:
         directory = sanitise_name(whl.name, prefix=repo_prefix)
 
diff --git a/python/pip_install/extract_wheels/lib/wheel.py b/python/pip_install/extract_wheels/lib/wheel.py
@@ -55,6 +55,7 @@ def entry_points(self) -> Dict[str, str]:
             # Calculate the location of the entry_points.txt file
             metadata = self.metadata
             name = "{}-{}".format(metadata.name.replace("-", "_"), metadata.version)
+
             # Note that the zipfile module always uses the forward slash as
             # directory separator, even on Windows, so don't use os.path.join
             # here.  Reference for Python 3.10:
diff --git a/python/pip_install/extract_wheels/lib/whl_filegroup_test.py b/python/pip_install/extract_wheels/lib/whl_filegroup_test.py
@@ -2,6 +2,7 @@
 import shutil
 import tempfile
 import unittest
+from pathlib import Path
 
 from python.pip_install.extract_wheels.lib import bazel
 
@@ -12,12 +13,9 @@ def setUp(self) -> None:
         self.wheel_dir = tempfile.mkdtemp()
         self.wheel_path = os.path.join(self.wheel_dir, self.wheel_name)
         shutil.copy(os.path.join("examples", "wheel", self.wheel_name), self.wheel_dir)
-        self.original_dir = os.getcwd()
-        os.chdir(self.wheel_dir)
 
     def tearDown(self):
         shutil.rmtree(self.wheel_dir)
-        os.chdir(self.original_dir)
 
     def _run(
         self,
@@ -31,12 +29,14 @@ def _run(
             enable_implicit_namespace_pkgs=False,
             incremental=incremental,
             repo_prefix=repo_prefix,
+            incremental_dir=Path(self.wheel_dir),
         )
         # Take off the leading // from the returned label.
         # Assert that the raw wheel ends up in the package.
         generated_bazel_dir = (
             generated_bazel_dir[2:] if not incremental else self.wheel_dir
         )
+
         self.assertIn(self.wheel_name, os.listdir(generated_bazel_dir))
         with open("{}/BUILD.bazel".format(generated_bazel_dir)) as build_file:
             build_file_content = build_file.read()
diff --git a/python/pip_install/parse_requirements_to_bzl/parse_requirements_to_bzl_test.py b/python/pip_install/parse_requirements_to_bzl/parse_requirements_to_bzl_test.py
@@ -1,7 +1,8 @@
 import argparse
 import json
+import tempfile
 import unittest
-from tempfile import NamedTemporaryFile
+from pathlib import Path
 
 from python.pip_install.parse_requirements_to_bzl import (
     generate_parsed_requirements_contents,
@@ -10,15 +11,15 @@
 
 class TestParseRequirementsToBzl(unittest.TestCase):
     def test_generated_requirements_bzl(self) -> None:
-        with NamedTemporaryFile() as requirements_lock:
+        with tempfile.TemporaryDirectory() as temp_dir:
+            requirements_lock = Path(temp_dir) / "requirements.txt"
             comments_and_flags = "#comment\n--require-hashes True\n"
             requirement_string = "foo==0.0.0 --hash=sha256:hashofFoowhl"
-            requirements_lock.write(
+            requirements_lock.write_bytes(
                 bytes(comments_and_flags + requirement_string, encoding="utf-8")
             )
-            requirements_lock.flush()
             args = argparse.Namespace()
-            args.requirements_lock = requirements_lock.name
+            args.requirements_lock = str(requirements_lock.resolve())
             args.repo_prefix = "pip_parsed_deps_pypi__"
             extra_pip_args = ["--index-url=pypi.org/simple"]
             pip_data_exclude = ["**.foo"]
diff --git a/python/pip_install/pip_repository.bzl b/python/pip_install/pip_repository.bzl
@@ -24,22 +24,40 @@ def _construct_pypath(rctx):
     pypath = separator.join([str(p) for p in [rules_root] + thirdparty_roots])
     return pypath
 
+def _get_python_interpreter_attr(rctx):
+    """A helper function for getting the `python_interpreter` attribute or it's default
+
+    Args:
+        rctx (repository_ctx): Handle to the rule repository context.
+
+    Returns:
+        str: The attribute value or it's default
+    """
+    if rctx.attr.python_interpreter:
+        return rctx.attr.python_interpreter
+
+    if "win" in rctx.os.name:
+        return "python.exe"
+    else:
+        return "python3"
+
 def _resolve_python_interpreter(rctx):
     """Helper function to find the python interpreter from the common attributes
 
     Args:
         rctx: Handle to the rule repository context.
     Returns: Python interpreter path.
     """
-    python_interpreter = rctx.attr.python_interpreter
+    python_interpreter = _get_python_interpreter_attr(rctx)
+
     if rctx.attr.python_interpreter_target != None:
         target = rctx.attr.python_interpreter_target
         python_interpreter = rctx.path(target)
     else:
         if "/" not in python_interpreter:
             python_interpreter = rctx.which(python_interpreter)
         if not python_interpreter:
-            fail("python interpreter not found")
+            fail("python interpreter `{}` not found in PATH".format(python_interpreter))
     return python_interpreter
 
 def _parse_optional_attrs(rctx, args):
@@ -125,10 +143,10 @@ def _pip_repository_impl(rctx):
             str(rctx.attr.timeout),
         ]
 
-        if rctx.attr.python_interpreter:
-            args += ["--python_interpreter", rctx.attr.python_interpreter]
+        args += ["--python_interpreter", _get_python_interpreter_attr(rctx)]
         if rctx.attr.python_interpreter_target:
             args += ["--python_interpreter_target", str(rctx.attr.python_interpreter_target)]
+
     else:
         args = [
             python_interpreter,
@@ -193,7 +211,15 @@ to control this flag.
     "pip_data_exclude": attr.string_list(
         doc = "Additional data exclusion parameters to add to the pip packages BUILD file.",
     ),
-    "python_interpreter": attr.string(default = "python3"),
+    "python_interpreter": attr.string(
+        doc = """\
+The python interpreter to use. This can either be an absolute path or the name
+of a binary found on the host's `PATH` environment variable. If no value is set
+`python3` is defaulted for Unix systems and `python.exe` for Windows.
+""",
+        # NOTE: This attribute should not have a default. See `_get_python_interpreter_attr`
+        # default = "python3"
+    ),
     "python_interpreter_target": attr.label(
         allow_single_file = True,
         doc = """
diff --git a/tools/bazel_integration_test/test_runner.py b/tools/bazel_integration_test/test_runner.py
