gave up vibe coding. confidence isn't correctness.
...and sycophantic toasters are bad collaborators
diff --git a/python/private/cc/py_extension_rule.bzl b/python/private/cc/py_extension_rule.bzl
index 3465ceb..8d76cdb 100644
--- a/python/private/cc/py_extension_rule.bzl
+++ b/python/private/cc/py_extension_rule.bzl
@@ -5,11 +5,13 @@
load("//python/private:attr_builders.bzl", "attrb")
load("//python/private:attributes.bzl", "COMMON_ATTRS")
load("//python/private:py_info.bzl", "PyInfo")
+load("//python/private:py_internal.bzl", "py_internal")
load("//python/private:reexports.bzl", "BuiltinPyInfo")
load("//python/private:rule_builders.bzl", "ruleb")
load("//python/private:toolchain_types.bzl", "TARGET_TOOLCHAIN_TYPE")
def _py_extension_impl(ctx):
+ module_name = ctx.attr.module_name or ctx.label.name
cc_toolchain = ctx.toolchains["@bazel_tools//tools/cpp:toolchain_type"].cc
feature_configuration = cc_common.configure_features(
ctx = ctx,
@@ -24,62 +26,75 @@
cc_infos = static_deps_infos + dynamic_deps_infos + external_deps_infos,
)
- # Compile sources
- _, compilation_outputs = cc_common.compile(
- name = ctx.label.name,
- actions = ctx.actions,
- feature_configuration = feature_configuration,
- cc_toolchain = cc_toolchain,
- srcs = ctx.files.srcs,
- compilation_contexts = [all_deps_cc_info.compilation_context],
- )
-
# Static deps are linked directly into the .so
- static_linking_context = cc_common.merge_cc_infos(
+ static_cc_info = cc_common.merge_cc_infos(
cc_infos = static_deps_infos,
- ).linking_context
+ )
# Dynamic deps are linked as shared libraries
dynamic_linking_context = cc_common.merge_cc_infos(
cc_infos = dynamic_deps_infos,
).linking_context
- # For external deps, we need to allow undefined symbols.
user_link_flags = []
+ user_link_flags.append("-Wl,--export-dynamic-symbol=PyInit_{module_name}".format(
+ module_name = module_name,
+ ))
+
+ # The PyInit symbol looks unused, so the linker optimizes it away. Telling it
+ # to treat it as undefined causes it to be retained.
+ user_link_flags.append("-Wl,--undefined=PyInit_{module_name}".format(
+ module_name = module_name,
+ ))
+
if ctx.attr.external_deps:
user_link_flags.append("-Wl,--allow-shlib-undefined")
- # This function also does the linking
- _, linking_outputs = cc_common.create_linking_context_from_compilation_outputs(
- name = ctx.label.name,
+ # todo: use toolchain to determine `abi3.` infix
+ # todo: use toolchain to determine platform extension (pyd, so, etc)
+ output_filename = "{module_name}.{ext}".format(
+ module_name = module_name,
+ ext = "so",
+ )
+ py_dso = ctx.actions.declare_file(output_filename)
+
+ static_linking_context = static_cc_info.linking_context
+ linking_contexts = [
+ static_linking_context,
+ dynamic_linking_context,
+ ]
+
+ # Add target-level linkopts last so users can override.
+ user_link_flags.extend(ctx.attr.linkopts)
+ print((
+ "===LINK:\n" +
+ " user_link_flags={user_link_flags}"
+ ).format(
+ user_link_flags = user_link_flags,
+ ))
+
+ # todo: add linker script to hide symbols by default
+ # py_internal allows using some private apis, which may or may not be needed.
+ # based upon cc_shared_library.bzl
+ cc_linking_outputs = py_internal.link(
actions = ctx.actions,
feature_configuration = feature_configuration,
cc_toolchain = cc_toolchain,
- compilation_outputs = compilation_outputs,
+ linking_contexts = linking_contexts,
user_link_flags = user_link_flags,
- linking_contexts = [static_linking_context, dynamic_linking_context],
+ # todo: add additional_inputs
+ name = ctx.label.name,
+ output_type = "dynamic_library",
+ main_output = py_dso,
+ # todo: maybe variables_extension
+ # todo: maybe additional_outputs
)
-
- print(linking_outputs)
- ltl = linking_outputs.library_to_link
- print(ltl)
- print(ltl.dynamic_library)
- print(ltl.resolved_symlink_dynamic_library)
- lib_dso = ltl.resolved_symlink_dynamic_library
- if lib_dso == None:
- lib_dso = ltl.dynamic_library
-
- if lib_dso == None:
- fail("No DSO output found in {}".format(ltl))
-
- # todo: pick appropriate infix based on py_extension attr settings
- py_dso = ctx.actions.declare_file("{}.so".format(ctx.label.name))
- ctx.actions.run_shell(
- command = 'cp "$1" "$2"',
- arguments = [lib_dso.path, py_dso.path],
- inputs = [lib_dso],
- outputs = [py_dso],
- )
+ print((
+ "===LINK OUTPUT:\n" +
+ " {}"
+ ).format(
+ cc_linking_outputs,
+ ))
# Propagate CcInfo from dynamic and external deps, but not static ones.
propagated_cc_info = cc_common.merge_cc_infos(
@@ -87,10 +102,7 @@
)
return [
- DefaultInfo(
- files = depset([py_dso]),
- runfiles = ctx.runfiles([py_dso]),
- ),
+ DefaultInfo(files = depset([py_dso])),
PyInfo(
transitive_sources = depset([py_dso]),
),
@@ -110,15 +122,14 @@
doc = "cc_library targets with external linkage.",
default = [],
),
- "srcs": lambda: attrb.LabelList(
- allow_files = True,
- doc = "The list of source files that are processed to create the target.",
- ),
"static_deps": lambda: attrb.LabelList(
providers = [CcInfo],
doc = "cc_library targets to be statically and privately linked.",
default = [],
),
+ "copts": lambda: attrb.StringList(),
+ "linkopts": lambda: attrb.StringList(),
+ "module_name": lambda: attrb.String(),
}
def create_py_extension_rule_builder(**kwargs):
diff --git a/tests/cc/py_extension/BUILD.bazel b/tests/cc/py_extension/BUILD.bazel
index 6b04ca9..b68dbef 100644
--- a/tests/cc/py_extension/BUILD.bazel
+++ b/tests/cc/py_extension/BUILD.bazel
@@ -14,14 +14,35 @@
py_extension(
name = "ext_static",
- srcs = ["ext_static.c"],
+ ##srcs = ["ext_static.c"],
static_deps = [":static_dep"],
)
py_extension(
name = "ext_shared",
+ dynamic_deps = [
+ ":add_one",
+ ],
+ static_deps = [
+ ":ext_shared_impl",
+ ],
+)
+
+cc_library(
+ name = "ext_shared_impl",
srcs = ["ext_shared.c"],
- dynamic_deps = [":dyn_dep_a"],
+ copts = [
+ # Gemini says PIC is needed
+ "-fPIC",
+ "-fvisibility=hidden",
+ ],
+ deps = [
+ ":add_one_headers",
+ # todo: if we put this here, we statically link add_one into the
+ # extension.
+ #":add_one_impl",
+ "@rules_python//python/cc:current_py_cc_headers",
+ ],
)
cc_library(
@@ -31,10 +52,13 @@
)
cc_library(
- name = "dyn_dep_a",
- srcs = ["dyn_dep_a.c"],
- hdrs = ["dyn_dep_a.h"],
- linkstatic = False,
+ name = "add_one_headers",
+ hdrs = ["add_one.h"],
+)
+
+cc_library(
+ name = "add_one",
+ srcs = ["add_one.c"],
)
py_test(
diff --git a/tests/cc/py_extension/add_one.c b/tests/cc/py_extension/add_one.c
new file mode 100644
index 0000000..c83da4e
--- /dev/null
+++ b/tests/cc/py_extension/add_one.c
@@ -0,0 +1,3 @@
+int add_one(int x) {
+ return x + 1;
+}
diff --git a/tests/cc/py_extension/add_one.h b/tests/cc/py_extension/add_one.h
new file mode 100644
index 0000000..eda0abf
--- /dev/null
+++ b/tests/cc/py_extension/add_one.h
@@ -0,0 +1,6 @@
+#ifndef TESTS_CC_PY_EXTENSION_DYN_DEP_A_H_
+#define TESTS_CC_PY_EXTENSION_DYN_DEP_A_H_
+
+int add_one(int x);
+
+#endif // TESTS_CC_PY_EXTENSION_DYN_DEP_A_H_
diff --git a/tests/cc/py_extension/dyn_dep_a.c b/tests/cc/py_extension/dyn_dep_a.c
deleted file mode 100644
index e69de29..0000000
--- a/tests/cc/py_extension/dyn_dep_a.c
+++ /dev/null
diff --git a/tests/cc/py_extension/dyn_dep_a.h b/tests/cc/py_extension/dyn_dep_a.h
deleted file mode 100644
index e69de29..0000000
--- a/tests/cc/py_extension/dyn_dep_a.h
+++ /dev/null
diff --git a/tests/cc/py_extension/ext_shared.c b/tests/cc/py_extension/ext_shared.c
index 0aacba5..4b79a6d 100644
--- a/tests/cc/py_extension/ext_shared.c
+++ b/tests/cc/py_extension/ext_shared.c
@@ -1 +1,33 @@
-/* A no-op C extension for shared linking tests. */
+#include <Python.h>
+
+#include "tests/cc/py_extension/add_one.h"
+
+// A simple function that returns a Python integer.
+static PyObject* do_alpha(PyObject* self, PyObject* args) {
+ return PyLong_FromLong(add_one(41));
+}
+
+// Method definition object for this extension, these are the functions
+// that will be available in the module.
+static PyMethodDef ModuleMethods[] = {
+ {"do_alpha", do_alpha, METH_NOARGS, "A simple C function."},
+ {NULL, NULL, 0, NULL} /* Sentinel */
+};
+
+// Module definition
+// The arguments of this structure tell Python what to call your extension,
+// what its methods are and where to look for its method definitions.
+static struct PyModuleDef ext_shared_module = {
+ PyModuleDef_HEAD_INIT,
+ "ext_shared", /* name of module */
+ NULL, /* module documentation, may be NULL */
+ -1, /* size of per-interpreter state of the module,
+ or -1 if the module keeps state in global variables. */
+ ModuleMethods
+};
+
+// The module init function. This must be exported and retained in the
+// shared library output.
+PyMODINIT_FUNC PyInit_ext_shared(void) {
+ return PyModule_Create(&ext_shared_module);
+}
diff --git a/tests/cc/py_extension/py_extension_test.py b/tests/cc/py_extension/py_extension_test.py
index 9a19e6a..81f277f 100644
--- a/tests/cc/py_extension/py_extension_test.py
+++ b/tests/cc/py_extension/py_extension_test.py
@@ -1,41 +1,36 @@
-import os
import unittest
-
-from elftools.elf.dynamic import DynamicSection
-from elftools.elf.elffile import ELFFile
-
+import os
from python.runfiles import runfiles
-
+from elftools.elf.elffile import ELFFile
+from elftools.elf.dynamic import DynamicSection
+import ext_shared
class PyExtensionTest(unittest.TestCase):
def test_inspect_elf(self):
r = runfiles.Create()
ext_path = r.Rlocation("rules_python/tests/cc/py_extension/ext_shared.so")
- self.assertTrue(
- os.path.exists(ext_path), f"Could not find ext_shared.so at {ext_path}"
- )
+ self.assertTrue(os.path.exists(ext_path), f"Could not find ext_shared.so at {ext_path}")
- with open(ext_path, "rb") as f:
+ with open(ext_path, 'rb') as f:
elf = ELFFile(f)
# Check for DT_NEEDED entry for the dynamic library
- dynamic_section = elf.get_section_by_name(".dynamic")
+ dynamic_section = elf.get_section_by_name('.dynamic')
self.assertIsNotNone(dynamic_section)
self.assertTrue(isinstance(dynamic_section, DynamicSection))
- needed_libs = [
- tag.needed
- for tag in dynamic_section.iter_tags()
- if tag.entry.d_tag == "DT_NEEDED"
- ]
- self.assertIn("libdyn_dep_a.so", needed_libs)
+ needed_libs = [tag.needed for tag in dynamic_section.iter_tags() if tag.entry.d_tag == 'DT_NEEDED']
+ self.assertIn('libdyn_dep_a.so', needed_libs)
# Check for the PyInit symbol
- dynsym_section = elf.get_section_by_name(".dynsym")
+ dynsym_section = elf.get_section_by_name('.dynsym')
self.assertIsNotNone(dynsym_section)
symbols = [s.name for s in dynsym_section.iter_symbols()]
- self.assertIn("PyInit_ext_shared", symbols)
+ self.assertIn('PyInit_ext_shared', symbols)
+
+ def test_import_and_call(self):
+ self.assertEqual(ext_shared.my_c_function(), 42)
if __name__ == "__main__":
