From 248ed86f2bfe6cf3dc7ffc69fa55cb488fd4a4ea Mon Sep 17 00:00:00 2001 From: Mike Kruskal Date: Wed, 30 Nov 2022 19:54:51 -0800 Subject: [PATCH] Add better handling for systems without python3 installed. The current behavior will crash any Bazel command immediately, due to our declared pip dependencies in WORKSPACE, if python3 can't be found. The new behavior will mock out these workspace dependencies and allow any non-python targets to run. Python targets will be skipped by wildcard expressions if there's no system python3, and will fail when run directly, due to compatibility mismatch. PiperOrigin-RevId: 492085254 --- .bazelrc | 2 - .github/workflows/bazel_tests.yml | 35 ++- WORKSPACE | 16 +- bazel/BUILD | 4 + bazel/system_python.bzl | 209 ++++++++++++++++-- bazel/workspace_deps.bzl | 8 +- cmake/make_cmakelists.py | 3 + python/BUILD | 20 +- python/dist/BUILD.bazel | 12 + python/pb_unit_tests/pyproto_test_wrapper.bzl | 4 + python/requirements.txt | 1 + python/requirements_311.txt | 1 + 12 files changed, 276 insertions(+), 39 deletions(-) create mode 100644 python/requirements.txt create mode 100644 python/requirements_311.txt diff --git a/.bazelrc b/.bazelrc index 982a1543c4..9f61850dd5 100644 --- a/.bazelrc +++ b/.bazelrc @@ -1,8 +1,6 @@ # temporary fix for https://github.com/bazelbuild/bazel/issues/12905 on macOS build --features=-debug_prefix_map_pwd_is_dot -build --extra_toolchains=@system_python//:python_toolchain - # Pin to C++17 build --cxxopt=-std=c++17 --host_cxxopt=-std=c++17 diff --git a/.github/workflows/bazel_tests.yml b/.github/workflows/bazel_tests.yml index 90a6438ce4..efe6fe5e69 100644 --- a/.github/workflows/bazel_tests.yml +++ b/.github/workflows/bazel_tests.yml @@ -60,11 +60,40 @@ jobs: run: echo "BAZEL_CACHE_AUTH=--remote_upload_local_results=false" >> $GITHUB_ENV if: ${{ github.event.pull_request.head.repo.full_name != 'protocolbuffers/upb' }} - name: Setup Python venv - run: rm -rf /tmp/venv && python3 -m venv /tmp/venv + run: rm -rf /tmp/venv && python3 -m venv /tmp/venv && source /tmp/venv/bin/activate && python3 --version - name: Install dependencies run: sudo apt update && sudo apt install -y ${{ matrix.install }} if: matrix.install != '' - name: Install numpy - run: pip install numpy + run: source /tmp/venv/bin/activate && pip3 install numpy - name: Run tests - run: cd ${{ github.workspace }} && PATH=/tmp/venv/bin:$PATH CC=${{ matrix.CC }} ${{ matrix.BAZEL }} test --test_output=errors $BAZEL_CACHE $BAZEL_CACHE_AUTH ... ${{ matrix.flags }} + run: cd ${{ github.workspace }} && source /tmp/venv/bin/activate && CC=${{ matrix.CC }} ${{ matrix.BAZEL }} test --test_output=errors $BAZEL_CACHE $BAZEL_CACHE_AUTH ... ${{ matrix.flags }} + + no-python: + runs-on: ubuntu-20.04 + env: + BAZEL_CACHE: --remote_cache=https://storage.googleapis.com/protobuf-bazel-cache/upb + + strategy: + fail-fast: false # Don't cancel all jobs if one fails. + + name: "No System Python" + + steps: + - uses: actions/checkout@v2 + - name: Set up Cloud SDK + uses: google-github-actions/auth@v0 + with: + credentials_json: ${{ secrets.GOOGLE_CREDENTIALS }} + export_environment_variables: true + if: ${{ github.event.pull_request.head.repo.full_name == 'protocolbuffers/upb' }} + - name: Set up Bazel read/write caching + run: echo "BAZEL_CACHE_AUTH=--google_default_credentials" >> $GITHUB_ENV + if: ${{ github.event.pull_request.head.repo.full_name == 'protocolbuffers/upb' }} + - name: Set up Bazel read-only caching + run: echo "BAZEL_CACHE_AUTH=--remote_upload_local_results=false" >> $GITHUB_ENV + if: ${{ github.event.pull_request.head.repo.full_name != 'protocolbuffers/upb' }} + - name: Uninstall python + run: which python3 && sudo mv `which python3` /tmp && ! which python3 + - name: Run tests + run: cd ${{ github.workspace }} && bazel test --test_output=errors $BAZEL_CACHE $BAZEL_CACHE_AUTH //python/... diff --git a/WORKSPACE b/WORKSPACE index 20dfc57ad3..dba443348e 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -4,7 +4,6 @@ load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") load("//bazel:workspace_deps.bzl", "upb_deps") upb_deps() -register_toolchains("@system_python//:python_toolchain") load("@com_google_protobuf//:protobuf_deps.bzl", "protobuf_deps") protobuf_deps() @@ -69,9 +68,20 @@ load("@rules_fuzzing//fuzzing:init.bzl", "rules_fuzzing_init") rules_fuzzing_init() -load("@rules_python//python:pip.bzl", "pip_install") +load("//bazel:system_python.bzl", "system_python") +system_python( + name = "system_python", + minimum_python_version = "3.7", +) +load("@system_python//:register.bzl", "register_system_python") +register_system_python() + +load("@system_python//:pip.bzl", "pip_install") pip_install( name="pip_deps", - requirements = "@com_google_protobuf//python:requirements.txt" + requirements = "//python:requirements.txt", + requirements_overrides = { + "3.11": "//python:requirements_311.txt", + }, ) diff --git a/bazel/BUILD b/bazel/BUILD index 148f4a0bee..d767c8f013 100644 --- a/bazel/BUILD +++ b/bazel/BUILD @@ -30,6 +30,10 @@ licenses(["notice"]) py_binary( name = "amalgamate", srcs = ["amalgamate.py"], + target_compatible_with = select({ + "@system_python//:exists": [], + "//conditions:default": ["@platforms//:incompatible"], + }), visibility = ["//:__pkg__"], ) diff --git a/bazel/system_python.bzl b/bazel/system_python.bzl index 7e2dac92b4..89cef1589b 100644 --- a/bazel/system_python.bzl +++ b/bazel/system_python.bzl @@ -25,7 +25,60 @@ """Repository rule for using Python 3.x headers from the system.""" +# Mock out rules_python's pip.bzl for cases where no system python is found. +_mock_pip = """ +def _pip_install_impl(repository_ctx): + repository_ctx.file("BUILD.bazel", ''' +py_library( + name = "noop", + visibility = ["//visibility:public"], +) +''') + repository_ctx.file("requirements.bzl", ''' +def install_deps(*args, **kwargs): + print("WARNING: could not install pip dependencies") + +def requirement(*args, **kwargs): + return "@{}//:noop" +'''.format(repository_ctx.attr.name)) +pip_install = repository_rule( + implementation = _pip_install_impl, + attrs = { + "requirements": attr.string(), + "requirements_overrides": attr.string_dict(), + "python_interpreter_target": attr.string(), + }, +) +pip_parse = pip_install +""" + +# Alias rules_python's pip.bzl for cases where a system pythong is found. +_alias_pip = """ +load("@rules_python//python:pip.bzl", _pip_install = "pip_install", _pip_parse = "pip_parse") +def _get_requirements(requirements, requirements_overrides): + for version, override in requirements_overrides.items(): + if version in "{python_version}": + requirements = override + break + return requirements + +def pip_install(requirements, requirements_overrides={{}}, **kwargs): + _pip_install( + python_interpreter_target = "@{repo}//:interpreter", + requirements = _get_requirements(requirements, requirements_overrides), + **kwargs, + ) +def pip_parse(requirements, requirements_overrides={{}}, **kwargs): + _pip_parse( + python_interpreter_target = "@{repo}//:interpreter", + requirements = _get_requirements(requirements, requirements_overrides), + **kwargs, + ) +""" + _build_file = """ +load("@bazel_skylib//lib:selects.bzl", "selects") +load("@bazel_skylib//rules:common_settings.bzl", "string_flag") load("@bazel_tools//tools/python:toolchain.bzl", "py_runtime_pair") cc_library( @@ -35,9 +88,55 @@ cc_library( visibility = ["//visibility:public"], ) +string_flag( + name = "internal_python_support", + build_setting_default = "{support}", + values = [ + "None", + "Supported", + "Unsupported", + ] +) + +config_setting( + name = "none", + flag_values = {{ + ":internal_python_support": "None", + }}, + visibility = ["//visibility:public"], +) + +config_setting( + name = "supported", + flag_values = {{ + ":internal_python_support": "Supported", + }}, + visibility = ["//visibility:public"], +) + +config_setting( + name = "unsupported", + flag_values = {{ + ":internal_python_support": "Unsupported", + }}, + visibility = ["//visibility:public"], +) + +selects.config_setting_group( + name = "exists", + match_any = [":supported", ":unsupported"], + visibility = ["//visibility:public"], +) + +sh_binary( + name = "interpreter", + srcs = ["interpreter"], + visibility = ["//visibility:public"], +) + py_runtime( name = "py3_runtime", - interpreter_path = "{}", + interpreter_path = "{interpreter}", python_version = "PY3", ) @@ -53,40 +152,105 @@ toolchain( ) """ -def _get_python_version(repository_ctx): - py_program = "import sys; print(str(sys.version_info.major) + str(sys.version_info.minor))" - result = repository_ctx.execute(["python3", "-c", py_program]) - return (result.stdout).strip() +_register = """ +def register_system_python(): + native.register_toolchains("@{}//:python_toolchain") +""" -def _get_config_var(repository_ctx, name): +_mock_register = """ +def register_system_python(): + pass +""" + +def _get_python_version(repository_ctx): + py_program = "import sys; print(str(sys.version_info.major) + '.' + str(sys.version_info.minor) + '.' + str(sys.version_info.micro))" + result = repository_ctx.execute(["python3", "-c", py_program]) + return (result.stdout).strip().split(".") + +def _get_python_path(repository_ctx): py_program = "import sysconfig; print(sysconfig.get_config_var('%s'), end='')" - result = repository_ctx.execute(["python3", "-c", py_program % (name)]) + result = repository_ctx.execute(["python3", "-c", py_program % ("INCLUDEPY")]) if result.return_code != 0: return None return result.stdout -def _python_headers_impl(repository_ctx): - path = _get_config_var(repository_ctx, "INCLUDEPY") - if not path: - # buildifier: disable=print - print("WARNING: no system python available, builds against system python will fail") - repository_ctx.file("BUILD.bazel", "") - repository_ctx.file("version.bzl", "SYSTEM_PYTHON_VERSION = None") - return - repository_ctx.symlink(path, "python") +def _populate_package(ctx, path, python3, python_version): + ctx.symlink(path, "python") + support = "Supported" + for idx, v in enumerate(ctx.attr.minimum_python_version.split(".")): + if int(python_version[idx]) < int(v): + support = "Unsupported" + break + + build_file = _build_file.format( + interpreter = python3, + support = support, + ) + + ctx.file("interpreter", "exec {} \"$@\"".format(python3)) + ctx.file("BUILD.bazel", build_file) + ctx.file("version.bzl", "SYSTEM_PYTHON_VERSION = '{}{}'".format(python_version[0], python_version[1])) + ctx.file("register.bzl", _register.format(ctx.attr.name)) + ctx.file("pip.bzl", _alias_pip.format( + python_version = ".".join(python_version), + repo = ctx.attr.name, + )) + +def _populate_empty_package(ctx): + # Mock out all the entrypoints we need to run from WORKSPACE. Targets that + # actually need python should use `target_compatible_with` and the generated + # @system_python//:exists or @system_python//:supported constraints. + ctx.file( + "BUILD.bazel", + _build_file.format( + interpreter = "", + support = "None", + ), + ) + ctx.file("version.bzl", "SYSTEM_PYTHON_VERSION = 'None'") + ctx.file("register.bzl", _mock_register) + ctx.file("pip.bzl", _mock_pip) + +def _system_python_impl(repository_ctx): + path = _get_python_path(repository_ctx) python3 = repository_ctx.which("python3") python_version = _get_python_version(repository_ctx) - repository_ctx.file("BUILD.bazel", _build_file.format(python3)) - repository_ctx.file("version.bzl", "SYSTEM_PYTHON_VERSION = '{}'".format(python_version)) -# The system_python() repository rule exposes Python headers from the system. + if path and python_version[0] == "3": + _populate_package(repository_ctx, path, python3, python_version) + else: + # buildifier: disable=print + print("WARNING: no system python available, builds against system python will fail") + _populate_empty_package(repository_ctx) + +# The system_python() repository rule exposes information from the version of python installed in the current system. # # In WORKSPACE: # system_python( # name = "system_python_repo", +# minimum_python_version = "3.7", # ) # -# This repository exposes a single rule that you can depend on from BUILD: +# This repository exposes some repository rules for configuring python in Bazel. The python toolchain +# *must* be registered in your WORKSPACE: +# load("@system_python_repo//:register.bzl", "register_system_python") +# register_system_python() +# +# Pip dependencies can optionally be specified using a wrapper around rules_python's repository rules: +# load("@system_python//:pip.bzl", "pip_install") +# pip_install( +# name="pip_deps", +# requirements = "@com_google_protobuf//python:requirements.txt", +# ) +# An optional argument `requirements_overrides` takes a dictionary mapping python versions to alternate +# requirements files. This works around the requirement for fully pinned dependencies in python_rules. +# +# Four config settings are exposed from this repository to help declare target compatibility in Bazel. +# For example, `@system_python_repo//:exists` will be true if a system python version has been found. +# The `none` setting will be true only if no python version was found, and `supported`/`unsupported` +# correspond to whether or not the system version is compatible with `minimum_python_version`. +# +# This repository also exposes a header rule that you can depend on from BUILD files: # cc_library( # name = "foobar", # srcs = ["foobar.cc"], @@ -96,6 +260,9 @@ def _python_headers_impl(repository_ctx): # The headers should correspond to the version of python obtained by running # the `python3` command on the system. system_python = repository_rule( - implementation = _python_headers_impl, + implementation = _system_python_impl, local = True, + attrs = { + "minimum_python_version": attr.string(default = "3.7"), + }, ) diff --git a/bazel/workspace_deps.bzl b/bazel/workspace_deps.bzl index 2e78baf782..f857acb22d 100644 --- a/bazel/workspace_deps.bzl +++ b/bazel/workspace_deps.bzl @@ -1,7 +1,6 @@ load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") load("@bazel_tools//tools/build_defs/repo:utils.bzl", "maybe") load("//bazel:python_downloads.bzl", "python_nuget_package", "python_source_archive") -load("//bazel:system_python.bzl", "system_python") def _github_archive(repo, commit, **kwargs): repo_name = repo.split("/")[-1] @@ -24,7 +23,8 @@ def upb_deps(): _github_archive, name = "com_google_protobuf", repo = "https://github.com/protocolbuffers/protobuf", - commit = "c79832bddc3931d798d31d417238e4377f869c79", + commit = "016ef2393e163cb8a49186adab8e2981476eec37", + sha256 = "ed91f723ba5db42190ca4a2734d1fdd449ceccf259c1112d3e4da14b322680f5", patches = ["@upb//bazel:protobuf.patch"], ) @@ -45,10 +45,6 @@ def upb_deps(): urls = ["https://github.com/bazelbuild/bazel-skylib/archive/main.tar.gz"], ) - system_python( - name = "system_python", - ) - #Python Downloads python_source_archive( diff --git a/cmake/make_cmakelists.py b/cmake/make_cmakelists.py index 2d5901e100..bf124209df 100755 --- a/cmake/make_cmakelists.py +++ b/cmake/make_cmakelists.py @@ -239,6 +239,9 @@ class WorkspaceFileFunctions(object): def system_python(self, **kwargs): pass + def register_system_python(self, **kwargs): + pass + def register_toolchains(self, toolchain): pass diff --git a/python/BUILD b/python/BUILD index b937be1166..31bbae3d77 100644 --- a/python/BUILD +++ b/python/BUILD @@ -172,6 +172,21 @@ selects.config_setting_group( ], ) +# begin:github_only +_message_target_compatible_with = { + "@platforms//os:windows": ["@platforms//:incompatible"], + "@system_python//:none": ["@platforms//:incompatible"], + "@system_python//:unsupported": ["@platforms//:incompatible"], + "//conditions:default": [], +} +# end:github_only +# begin:google_only +# _message_target_compatible_with = { +# "@platforms//os:windows": ["//third_party/bazel_platforms:incompatible"], +# "//conditions:default": [], +# } +# end:google_only + py_extension( name = "_message", srcs = [ @@ -202,10 +217,7 @@ py_extension( # casts between function pointers and object pointers. "-Wno-pedantic", ], - target_compatible_with = select({ - "@platforms//os:windows": ["//third_party/bazel_platforms:incompatible"], - "//conditions:default": [], - }), + target_compatible_with = select(_message_target_compatible_with), deps = [ "//:collections", "//:descriptor_upb_proto_reflection", diff --git a/python/dist/BUILD.bazel b/python/dist/BUILD.bazel index a8fcbf689a..8ac6479bd9 100644 --- a/python/dist/BUILD.bazel +++ b/python/dist/BUILD.bazel @@ -230,6 +230,10 @@ py_wheel( "python/", "src/", ], + target_compatible_with = select({ + "@system_python//:none": ["@platforms//:incompatible"], + "//conditions:default": [], + }), version = PROTOBUF_PYTHON_VERSION, deps = [ ":message_mod", @@ -261,6 +265,10 @@ py_wheel( "python/", "src/", ], + target_compatible_with = select({ + "@system_python//:none": ["@platforms//:incompatible"], + "//conditions:default": [], + }), version = PROTOBUF_PYTHON_VERSION, deps = [ ":well_known_proto_py_pb2", @@ -279,6 +287,10 @@ py_wheel( "python/", "src/", ], + target_compatible_with = select({ + "@system_python//:none": ["@platforms//:incompatible"], + "//conditions:default": [], + }), version = PROTOBUF_PYTHON_VERSION, deps = [ "//python/pb_unit_tests:test_files", diff --git a/python/pb_unit_tests/pyproto_test_wrapper.bzl b/python/pb_unit_tests/pyproto_test_wrapper.bzl index bda44e95d7..e550873e0a 100644 --- a/python/pb_unit_tests/pyproto_test_wrapper.bzl +++ b/python/pb_unit_tests/pyproto_test_wrapper.bzl @@ -15,6 +15,10 @@ def pyproto_test_wrapper(name, deps = []): "@com_google_protobuf//:python_test_srcs", "@com_google_protobuf//:python_srcs", ] + deps, + target_compatible_with = select({ + "@system_python//:supported": [], + "//conditions:default": ["@platforms//:incompatible"], + }), ) # end:github_only diff --git a/python/requirements.txt b/python/requirements.txt new file mode 100644 index 0000000000..ee6dfb448a --- /dev/null +++ b/python/requirements.txt @@ -0,0 +1 @@ +numpy==1.21.6 diff --git a/python/requirements_311.txt b/python/requirements_311.txt new file mode 100644 index 0000000000..1f2c987a7e --- /dev/null +++ b/python/requirements_311.txt @@ -0,0 +1 @@ +numpy==1.23.5