From 711885db8ffd4ff201a5ff7f00c431f36e6ccdf4 Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Sat, 26 Feb 2022 09:48:05 -0800 Subject: [PATCH 1/7] Added py_extension(). --- bazel/py_extension.bzl | 39 ++++++++++++++++++++++++++++++++++++++ bazel/py_proto_library.bzl | 2 +- bazel/workspace_defs.bzl | 18 +++++++++--------- bazel/workspace_deps.bzl | 4 ++-- python/BUILD | 28 +++++++-------------------- 5 files changed, 58 insertions(+), 33 deletions(-) create mode 100644 bazel/py_extension.bzl diff --git a/bazel/py_extension.bzl b/bazel/py_extension.bzl new file mode 100644 index 0000000000..7c1adf4146 --- /dev/null +++ b/bazel/py_extension.bzl @@ -0,0 +1,39 @@ + +load( + "//bazel:build_defs.bzl", + "UPB_DEFAULT_COPTS", +) + +def py_extension(name, srcs, deps): + version_script = name + "_version_script.lds" + symbol = "PyInit_" + name + native.genrule( + name = "gen_" + version_script, + outs = [version_script], + cmd = "echo 'message { global: " + symbol + "; local: *; };' > $@", + ) + + native.cc_binary( + name = name, + srcs = srcs, + copts = UPB_DEFAULT_COPTS + [ + # The Python API requires patterns that are ISO C incompatible, like + # casts between function pointers and object pointers. + "-Wno-pedantic", + ], + # We use a linker script to hide all symbols except the entry point for + # the module. + linkopts = select({ + "@platforms//os:linux": ["-Wl,--version-script,$(location :" + version_script + ")"], + "@platforms//os:macos": [ + "-Wl,-exported_symbol", + "-Wl,_" + symbol, + ], + }), + linkshared = True, + linkstatic = True, + deps = deps + [ + ":" + version_script, + "@system_python//:python_headers", + ], + ) diff --git a/bazel/py_proto_library.bzl b/bazel/py_proto_library.bzl index 9f99112346..8ef4d9708d 100644 --- a/bazel/py_proto_library.bzl +++ b/bazel/py_proto_library.bzl @@ -103,7 +103,7 @@ def _py_proto_library_aspect_impl(target, ctx): ) outs_depset = depset(srcs) return [ - PyInfo(transitive_sources = outs_depset) + PyInfo(transitive_sources = outs_depset), ] _py_proto_library_aspect = aspect( diff --git a/bazel/workspace_defs.bzl b/bazel/workspace_defs.bzl index 2f596f01fb..03655b2409 100644 --- a/bazel/workspace_defs.bzl +++ b/bazel/workspace_defs.bzl @@ -53,17 +53,17 @@ toolchain( """ def _get_config_var(repository_ctx, name): - py_program = "import sysconfig; print(sysconfig.get_config_var('%s'), end='')" - result = repository_ctx.execute(["python3", "-c", py_program % (name)]) - if result.return_code != 0: - fail("No python3 executable available on the system") - return result.stdout + py_program = "import sysconfig; print(sysconfig.get_config_var('%s'), end='')" + result = repository_ctx.execute(["python3", "-c", py_program % (name)]) + if result.return_code != 0: + fail("No python3 executable available on the system") + return result.stdout def _python_headers_impl(repository_ctx): - path = _get_config_var(repository_ctx, "INCLUDEPY") - repository_ctx.symlink(path, "python") - python3 = repository_ctx.which("python3") - repository_ctx.file("BUILD.bazel", _build_file % python3) + path = _get_config_var(repository_ctx, "INCLUDEPY") + repository_ctx.symlink(path, "python") + python3 = repository_ctx.which("python3") + repository_ctx.file("BUILD.bazel", _build_file % python3) # The system_python() repository rule exposes Python headers from the system. # diff --git a/bazel/workspace_deps.bzl b/bazel/workspace_deps.bzl index a5a3564449..db3afd7a79 100644 --- a/bazel/workspace_deps.bzl +++ b/bazel/workspace_deps.bzl @@ -23,10 +23,10 @@ def upb_deps(): "rm python/google/protobuf/__init__.py", "rm python/google/protobuf/pyext/__init__.py", "rm python/google/protobuf/internal/__init__.py", - ] + ], ) - rules_python_version = "740825b7f74930c62f44af95c9a4c1bd428d2c53" # Latest @ 2021-06-23 + rules_python_version = "740825b7f74930c62f44af95c9a4c1bd428d2c53" # Latest @ 2021-06-23 maybe( http_archive, diff --git a/python/BUILD b/python/BUILD index 8126f52200..687ebe6003 100644 --- a/python/BUILD +++ b/python/BUILD @@ -31,6 +31,10 @@ load( "//bazel:py_proto_library.bzl", "py_proto_library", ) +load( + "//bazel:py_extension.bzl", + "py_extension", +) load( "@rules_python//python:packaging.bzl", "py_wheel", @@ -38,8 +42,8 @@ load( licenses(["notice"]) -cc_binary( - name = "message", +py_extension( + name = "_message", srcs = [ "convert.c", "convert.h", @@ -61,24 +65,7 @@ cc_binary( "repeated.c", "repeated.h", ], - copts = UPB_DEFAULT_COPTS + [ - # The Python API requires patterns that are ISO C incompatible, like - # casts between function pointers and object pointers. - "-Wno-pedantic", - ], - # We use a linker script to hide all symbols except the entry point for - # the module. - linkopts = select({ - "@platforms//os:linux": ["-Wl,--version-script,$(location :version_script.lds)"], - "@platforms//os:macos": [ - "-Wl,-exported_symbol", - "-Wl,_PyInit__message", - ], - }), - linkshared = True, - linkstatic = True, deps = [ - ":version_script.lds", "//:descriptor_upb_proto_reflection", "//:reflection", "//:textformat", @@ -86,7 +73,6 @@ cc_binary( "//upb/util:compare", "//upb/util:def_to_proto", "//upb/util:required_fields", - "@system_python//:python_headers", ], ) @@ -108,7 +94,7 @@ EXT_SUFFIX = ".abi3.so" genrule( name = "copy_message", - srcs = [":message"], + srcs = [":_message"], outs = ["google/protobuf/pyext/_message" + EXT_SUFFIX], cmd = "cp $< $@", ) From 1e75fbf2c2b5457839d3576c9634d2933263c9b2 Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Sat, 26 Feb 2022 09:55:13 -0800 Subject: [PATCH 2/7] Ported api_implementation to py_extension(). --- bazel/py_extension.bzl | 2 +- python/BUILD | 34 +++++++--------------------------- 2 files changed, 8 insertions(+), 28 deletions(-) diff --git a/bazel/py_extension.bzl b/bazel/py_extension.bzl index 7c1adf4146..beee32148f 100644 --- a/bazel/py_extension.bzl +++ b/bazel/py_extension.bzl @@ -4,7 +4,7 @@ load( "UPB_DEFAULT_COPTS", ) -def py_extension(name, srcs, deps): +def py_extension(name, srcs, deps=[]): version_script = name + "_version_script.lds" symbol = "PyInit_" + name native.genrule( diff --git a/python/BUILD b/python/BUILD index 687ebe6003..369f09701e 100644 --- a/python/BUILD +++ b/python/BUILD @@ -23,22 +23,9 @@ # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS # SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -load( - "//bazel:build_defs.bzl", - "UPB_DEFAULT_COPTS", -) -load( - "//bazel:py_proto_library.bzl", - "py_proto_library", -) -load( - "//bazel:py_extension.bzl", - "py_extension", -) -load( - "@rules_python//python:packaging.bzl", - "py_wheel", -) +load("//bazel:py_proto_library.bzl", "py_proto_library") +load("//bazel:py_extension.bzl", "py_extension") # copybara:strip_for_google3_begin +load("@rules_python//python:packaging.bzl", "py_wheel") licenses(["notice"]) @@ -76,16 +63,9 @@ py_extension( ], ) -cc_binary( - name = "api_implementation", - srcs = [ - "api_implementation.c", - ], - linkshared = True, - linkstatic = True, - # Enable once linker script is available. - #copts = ["-fvisibility=hidden"], - deps = ["@system_python//:python_headers"], +py_extension( + name = "_api_implementation", + srcs = ["api_implementation.c"], ) # Copy the extensions into the location recognized by Python. @@ -101,7 +81,7 @@ genrule( genrule( name = "copy_api_implementation", - srcs = [":api_implementation"], + srcs = [":_api_implementation"], outs = ["google/protobuf/internal/_api_implementation" + EXT_SUFFIX], cmd = "cp $< $@", visibility = ["//python:__subpackages__"], From f9c2a1d6ba7caef5570f8ed806fd8189dc75ffa8 Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Sat, 26 Feb 2022 09:56:02 -0800 Subject: [PATCH 3/7] Fixed copybara tag. --- python/BUILD | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/BUILD b/python/BUILD index 369f09701e..700e73c5b1 100644 --- a/python/BUILD +++ b/python/BUILD @@ -24,7 +24,7 @@ # SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. load("//bazel:py_proto_library.bzl", "py_proto_library") -load("//bazel:py_extension.bzl", "py_extension") # copybara:strip_for_google3_begin +load("//bazel:py_extension.bzl", "py_extension") # copybara:strip_for_google3 load("@rules_python//python:packaging.bzl", "py_wheel") licenses(["notice"]) From b1426546fc443ee90687b5b384d78f6cc5baa992 Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Sat, 26 Feb 2022 21:22:16 -0800 Subject: [PATCH 4/7] Mark python packaging rules as stripped for google3. --- python/BUILD | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/python/BUILD b/python/BUILD index 700e73c5b1..23e5327ae0 100644 --- a/python/BUILD +++ b/python/BUILD @@ -68,6 +68,23 @@ py_extension( srcs = ["api_implementation.c"], ) +py_test( + name = "minimal_test", + srcs = [ + "minimal_test.py", + ], + imports = ["."], + legacy_create_init = False, + deps = [ + "//python:message_ext", + "@com_google_protobuf//:python_common_test_protos", + "@com_google_protobuf//:python_specific_test_protos", + "@com_google_protobuf//:python_srcs", + ], +) + +# copybara:strip_for_google3_begin + # Copy the extensions into the location recognized by Python. # .abi3.so indicates use of the limited API, and cross-version ABI compatibility. EXT_SUFFIX = ".abi3.so" @@ -102,20 +119,6 @@ py_library( visibility = ["//python:__subpackages__"], ) -py_test( - name = "minimal_test", - srcs = [ - "minimal_test.py", - ], - imports = ["."], - legacy_create_init = False, - deps = [ - "//python:message_ext", - "@com_google_protobuf//:python_common_test_protos", - "@com_google_protobuf//:python_specific_test_protos", - "@com_google_protobuf//:python_srcs", - ], -) py_proto_library( name = "well_known_proto_pb2", @@ -155,3 +158,5 @@ py_wheel( "@com_google_protobuf//:python_srcs", ], ) + +# copybara:strip_end From cf7dc19b728162437672cd98030ab1918e12f36b Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Sat, 26 Feb 2022 21:48:03 -0800 Subject: [PATCH 5/7] Added explicit cast for freefunc. --- python/protobuf.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/protobuf.h b/python/protobuf.h index a45632b6d5..fdad168b86 100644 --- a/python/protobuf.h +++ b/python/protobuf.h @@ -193,7 +193,7 @@ PyObject* PyUpb_Forbidden_New(PyObject* cls, PyObject* args, PyObject* kwds); static inline void PyUpb_Dealloc(void* self) { PyTypeObject* tp = Py_TYPE(self); assert(PyType_GetFlags(tp) & Py_TPFLAGS_HEAPTYPE); - freefunc tp_free = PyType_GetSlot(tp, Py_tp_free); + freefunc tp_free = (freefunc)PyType_GetSlot(tp, Py_tp_free); tp_free(self); Py_DECREF(tp); } From dfc07a8f8904c244be705b54cfa4b332efd3da52 Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Sat, 26 Feb 2022 21:51:59 -0800 Subject: [PATCH 6/7] Added explicit dep on :table from Python. --- BUILD | 5 ++++- python/BUILD | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/BUILD b/BUILD index 442bab84a3..31e93b798b 100644 --- a/BUILD +++ b/BUILD @@ -444,7 +444,10 @@ cc_library( "upb/table_internal.h", "upb/upb.h", ], - visibility = ["//tests:__pkg__"], + visibility = [ + "//python:__pkg__", + "//tests:__pkg__", + ], deps = [ ":port", ], diff --git a/python/BUILD b/python/BUILD index 23e5327ae0..f6949bdc91 100644 --- a/python/BUILD +++ b/python/BUILD @@ -55,6 +55,7 @@ py_extension( deps = [ "//:descriptor_upb_proto_reflection", "//:reflection", + "//:table", "//:textformat", "//:upb", "//upb/util:compare", From dd28a2a745d39005132b932e0c64641f47591023 Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Sat, 26 Feb 2022 22:32:40 -0800 Subject: [PATCH 7/7] Make minimal_test open-source only for now. --- python/BUILD | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/BUILD b/python/BUILD index f6949bdc91..cb48bc6109 100644 --- a/python/BUILD +++ b/python/BUILD @@ -69,6 +69,8 @@ py_extension( srcs = ["api_implementation.c"], ) +# copybara:strip_for_google3_begin + py_test( name = "minimal_test", srcs = [ @@ -84,8 +86,6 @@ py_test( ], ) -# copybara:strip_for_google3_begin - # Copy the extensions into the location recognized by Python. # .abi3.so indicates use of the limited API, and cross-version ABI compatibility. EXT_SUFFIX = ".abi3.so"