From 7183780b6069bdb704a45278d3a64bd3fdb39505 Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Tue, 24 Aug 2021 07:58:37 -0700 Subject: [PATCH] Added a Valgrind test that works for Python! --- .bazelrc | 11 ++++++++++- .github/workflows/bazel_tests.yml | 1 + WORKSPACE | 6 +++--- bazel/workspace_defs.bzl | 31 +++++++++++++++++++++++++------ cmake/build_defs.bzl | 1 + cmake/make_cmakelists.py | 2 +- python/BUILD | 4 ++-- 7 files changed, 43 insertions(+), 13 deletions(-) diff --git a/.bazelrc b/.bazelrc index 2278545f3b..aec939ea67 100644 --- a/.bazelrc +++ b/.bazelrc @@ -1,11 +1,20 @@ # 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 + # Use our custom-configured c++ toolchain. build:m32 --copt=-m32 --linkopt=-m32 build:asan --copt=-fsanitize=address --linkopt=-fsanitize=address -build:valgrind --run_under='valgrind --leak-check=full --error-exitcode=1' + +# For Valgrind, we have to disable checks of "possible" leaks because the Python +# interpreter does the sorts of things that flag Valgrind "possible" leak checks. +# Ideally we could enforce a stricter check for the non-Python tests, but I don't +# know of an easy way to do that. +# +# We also have to disable pymalloc to avoid triggering Valgrind. +build:valgrind --run_under='valgrind --leak-check=full --trace-children=yes --show-possibly-lost=no --errors-for-leak-kinds=definite --error-exitcode=1' --action_env=PYTHONMALLOC=malloc build:ubsan --copt=-fsanitize=undefined --linkopt=-fsanitize=undefined --action_env=UBSAN_OPTIONS=halt_on_error=1:print_stacktrace=1 # Workaround for the fact that Bazel links with $CC, not $CXX diff --git a/.github/workflows/bazel_tests.yml b/.github/workflows/bazel_tests.yml index edbc05835c..a3d68ee2b5 100644 --- a/.github/workflows/bazel_tests.yml +++ b/.github/workflows/bazel_tests.yml @@ -22,6 +22,7 @@ jobs: - { CC: gcc, os: ubuntu-20.04, flags: "-c opt" } - { CC: clang, os: ubuntu-20.04, flags: "--//:fasttable_enabled=true -- -cmake:test_generated_files" } - { CC: clang, os: ubuntu-20.04, flags: "--config=asan -- -benchmarks:benchmark -python:all" } + - { CC: clang, os: ubuntu-20.04, flags: "--config=valgrind ... -- -cmake:cmake_build" } - { CC: clang, os: macos-11, flags: "" } steps: diff --git a/WORKSPACE b/WORKSPACE index 9e97af4fc8..9de2d4efa4 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -2,7 +2,7 @@ workspace(name = "upb") load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") load("//bazel:workspace_deps.bzl", "upb_deps") -load("//bazel:workspace_defs.bzl", "python_headers") +load("//bazel:workspace_defs.bzl", "system_python") upb_deps() @@ -39,8 +39,8 @@ http_archive( patch_cmds = ["find google -type f -name BUILD.bazel -delete"], ) -python_headers( - name = "python_headers" +system_python( + name = "system_python" ) http_archive( diff --git a/bazel/workspace_defs.bzl b/bazel/workspace_defs.bzl index bb1817d470..86324dddc4 100644 --- a/bazel/workspace_defs.bzl +++ b/bazel/workspace_defs.bzl @@ -26,12 +26,30 @@ """Repository rule for using Python 3.x headers from the system.""" _build_file = """ +load("@bazel_tools//tools/python:toolchain.bzl", "py_runtime_pair") cc_library( name = "python_headers", hdrs = glob(["python/**/*.h"]), includes = ["python"], visibility = ["//visibility:public"], ) + +py_runtime( + name = "py3_runtime", + interpreter_path = "%s", + python_version = "PY3", +) + +py_runtime_pair( + name = "runtime_pair", + py3_runtime = ":py3_runtime", +) + +toolchain( + name = "python_toolchain", + toolchain = ":runtime_pair", + toolchain_type = "@rules_python//python:toolchain_type", +) """ _build_defs_file = """ @@ -49,26 +67,27 @@ def _python_headers_impl(repository_ctx): path = _get_config_var(repository_ctx, "INCLUDEPY") ext_suffix = _get_config_var(repository_ctx, "EXT_SUFFIX") repository_ctx.symlink(path, "python") - repository_ctx.file("BUILD.bazel", _build_file) + python3 = repository_ctx.which("python3") + repository_ctx.file("BUILD.bazel", _build_file % python3) repository_ctx.file("build_defs.bzl", _build_defs_file % ext_suffix) -# The python_headers() repository rule exposes Python headers from the system. +# The system_python() repository rule exposes Python headers from the system. # # In WORKSPACE: -# python_headers( -# name = "python_headers_repo", +# system_python( +# name = "system_python_repo", # ) # # This repository exposes a single rule that you can depend on from BUILD: # cc_library( # name = "foobar", # srcs = ["foobar.cc"], -# deps = ["@python_headers_repo//:python_headers"], +# deps = ["@system_python_repo//:python_headers"], # ) # # The headers should correspond to the version of python obtained by running # the `python3` command on the system. -python_headers = repository_rule( +system_python = repository_rule( implementation = _python_headers_impl, local = True, ) diff --git a/cmake/build_defs.bzl b/cmake/build_defs.bzl index 4a6264b74c..f4ec02f867 100644 --- a/cmake/build_defs.bzl +++ b/cmake/build_defs.bzl @@ -62,6 +62,7 @@ def generated_file_staleness_test(name, outs, generated_pattern): name = name, srcs = [script_name], data = existing_outs + [generated_pattern % file for file in outs], + python_version = "PY3", deps = [ ":staleness_test_lib", ], diff --git a/cmake/make_cmakelists.py b/cmake/make_cmakelists.py index b3266fa18b..dea43974ed 100755 --- a/cmake/make_cmakelists.py +++ b/cmake/make_cmakelists.py @@ -220,7 +220,7 @@ class WorkspaceFileFunctions(object): def rules_fuzzing_init(self): pass - def python_headers(self, **kwargs): + def system_python(self, **kwargs): pass diff --git a/python/BUILD b/python/BUILD index 0df0b1d98a..889db8c95c 100644 --- a/python/BUILD +++ b/python/BUILD @@ -28,7 +28,7 @@ load( "UPB_DEFAULT_COPTS", ) load( - "@python_headers//:build_defs.bzl", + "@system_python//:build_defs.bzl", "EXT_SUFFIX", ) @@ -59,7 +59,7 @@ cc_binary( ":version_script.lds", "//:reflection", "//:upb", - "@python_headers", + "@system_python//:python_headers", ], )