diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml index 09af992bf6..3e16b249cd 100644 --- 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 index f06d343d95..e668a68822 100644 --- 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 index fe883d1c0d..6ef0d6e5e7 100644 --- a/docs/BUILD +++ b/docs/BUILD @@ -78,10 +78,19 @@ 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"], ) @@ -89,6 +98,7 @@ 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 index c28accdc09..ffdc6409e4 100644 --- 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 index e8097e19d7..f1b72543d2 100644 --- 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 index 41fd3a6124..2b0a91fa3d 100644 --- 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 index 8e9519fd4a..e880c20381 100644 --- 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,6 +307,7 @@ 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'. @@ -313,7 +315,7 @@ def extract_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 index 3803fba9ce..85bc95830d 100644 --- 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 index 5bf5f7a805..f5577136ff 100644 --- 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 index a5c76d3a3b..9619af5f0b 100644 --- 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 index 8e69da7612..1dc49c7240 100644 --- a/python/pip_install/pip_repository.bzl +++ b/python/pip_install/pip_repository.bzl @@ -24,6 +24,23 @@ 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 @@ -31,7 +48,8 @@ def _resolve_python_interpreter(rctx): 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) @@ -39,7 +57,7 @@ def _resolve_python_interpreter(rctx): 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 index eb5cabdc28..f10d86ac6a 100644 --- a/tools/bazel_integration_test/test_runner.py +++ b/tools/bazel_integration_test/test_runner.py @@ -5,6 +5,7 @@ import shutil import sys import tempfile +import textwrap from pathlib import Path from subprocess import Popen @@ -12,6 +13,7 @@ r = runfiles.Create() + def main(conf_file): with open(conf_file) as j: config = json.load(j) @@ -28,26 +30,50 @@ def main(conf_file): if workspacePath.startswith("external/"): workspacePath = ".." + workspacePath[len("external") :] - with tempfile.TemporaryDirectory(dir=os.environ["TEST_TMPDIR"]) as tmpdir: - workdir = os.path.join(tmpdir, "wksp") - print("copying workspace under test %s to %s" % (workspacePath, workdir)) - shutil.copytree(workspacePath, workdir) - - for command in config['bazelCommands']: - bazel_args = command.split(' ') - bazel_args.append("--override_repository=rules_python=%s/rules_python" % os.environ['TEST_SRCDIR']) - - # Bazel's wrapper script needs this or you get - # 2020/07/13 21:58:11 could not get the user's cache directory: $HOME is not defined - os.environ["HOME"] = str(Path.home()) - - bazel_args.insert(0, bazelBinary) - bazel_process = Popen(bazel_args, cwd=workdir) - bazel_process.wait() - if bazel_process.returncode != 0: - # Test failure in Bazel is exit 3 - # https://github.com/bazelbuild/bazel/blob/486206012a664ecb20bdb196a681efc9a9825049/src/main/java/com/google/devtools/build/lib/util/ExitCode.java#L44 - sys.exit(3) + with tempfile.TemporaryDirectory(dir=os.environ["TEST_TMPDIR"]) as tmp_homedir: + home_bazel_rc = Path(tmp_homedir) / ".bazelrc" + home_bazel_rc.write_text( + textwrap.dedent( + """\ + startup --max_idle_secs=1 + common --announce_rc + """ + ) + ) + + with tempfile.TemporaryDirectory(dir=os.environ["TEST_TMPDIR"]) as tmpdir: + workdir = os.path.join(tmpdir, "wksp") + print("copying workspace under test %s to %s" % (workspacePath, workdir)) + shutil.copytree(workspacePath, workdir) + + for command in config["bazelCommands"]: + bazel_args = command.split(" ") + bazel_args.append( + "--override_repository=rules_python=%s/rules_python" + % os.environ["TEST_SRCDIR"] + ) + + # Bazel's wrapper script needs this or you get + # 2020/07/13 21:58:11 could not get the user's cache directory: $HOME is not defined + os.environ["HOME"] = str(tmp_homedir) + + bazel_args.insert(0, bazelBinary) + bazel_process = Popen(bazel_args, cwd=workdir) + bazel_process.wait() + + if platform.system() == "Windows": + # Cleanup any bazel files + bazel_process = Popen([bazelBinary, "clean"], cwd=workdir) + bazel_process.wait() + + # Shutdown the bazel instance to avoid issues cleaning up the workspace + bazel_process = Popen([bazelBinary, "shutdown"], cwd=workdir) + bazel_process.wait() + + if bazel_process.returncode != 0: + # Test failure in Bazel is exit 3 + # https://github.com/bazelbuild/bazel/blob/486206012a664ecb20bdb196a681efc9a9825049/src/main/java/com/google/devtools/build/lib/util/ExitCode.java#L44 + sys.exit(3) if __name__ == "__main__":