pybind / pybind11_protobuf

Pybind11 bindings for Google's Protocol Buffers
Other
54 stars 33 forks source link

Create a release package to enable adding pybind11_protobuf to Bazel Central Registry #139

Open phaedon opened 9 months ago

phaedon commented 9 months ago

Hi, Could you please add a complete set of instructions on how to include and use with Bazel?

I added the following stanza to my WORKSPACE file:

git_repository(
    name = "pybind11_protobuf",
    branch = "main",
    remote = "https://github.com/pybind/pybind11_protobuf.git",
    strip_prefix = "pybind11_protobuf",
)

However, I cannot get the requirement for a com_google_protobuf dependency to work. I believe the current practice would be to add this dep to MODULE.bazel: https://registry.bazel.build/modules/protobuf/21.7

and then add this to the BUILD rule: "@protobuf//:protos_python",

This however gives me this error: no such package '@com_google_protobuf//': The repository '@com_google_protobuf' could not be resolved: Repository '@com_google_protobuf' is not defined and referenced by '@pybind11_protobuf//:native_proto_caster'

What's the correct way to get this to work? (for future compatibility, I have filed a BCR request here https://github.com/bazelbuild/bazel-central-registry/issues/1121 )

phaedon commented 9 months ago

As a follow-up, please note that copy-pasting the deps exactly as listed in the WORKSPACE in this repo, to my own client repo, generates a big list of errors along these lines:

duration_proto/google/protobuf/duration.pb.h:127:5: error: 'GOOGLE_DCHECK' was not declared in this scope; did you mean 'ABSL_DCHECK'?
  127 |     GOOGLE_DCHECK(GetOwningArena() == other->GetOwningArena());
      |     ^~~~~~~~~~~~~
      |     ABSL_DCHECK
rwgk commented 9 months ago

Trying to help, but note that I don't actually use this externally myself, and this repo just has "best effort" support (IOW people's spare time).

copy-pasting the deps exactly as listed in the WORKSPACE

Sounds like the way to go.

generates a big list of errors along these lines

I don't see GOOGLE_DCHECK anywhere in this repo.

Really just guessing:

Do you see this somewhere in your sources (and error messages)?

#include "glog/logging.h"

Maybe try apt install libgoogle-glog-dev or similar.

Or probably better, add to your WORKSPACE:

"@com_github_google_glog//:glog"
urls = ["https://github.com/google/glog/archive/<hex hash here>.zip"]
phaedon commented 7 months ago

Ok, it looks like these issues are fixed by: https://github.com/pybind/pybind11_protobuf/pull/149

The client repo simply needs to add the following to WORKSPACE (note reference to temp branch prior to code review):

git_repository(
    name = "pybind11_protobuf",
    branch = "fs-bzlmod",
    remote = "https://github.com/phaedon/pybind11_protobuf.git",
)

and then add this dep to the pybind_extension rule: "@pybind11_protobuf//pybind11_protobuf:native_proto_caster",

david-crouse commented 6 months ago

Hi! I'm working on cleaning up this repo and want to make progress on this as well. I'll work on getting this in soon.

phaedon commented 6 months ago

Great! It looks like a release package will need to be published also, so that we can start to add it to https://registry.bazel.build/

david-crouse commented 5 months ago

Yup! I just added MODULE.bazel, but as of now it's not fully ready since some packages in the BCR are pretty out of date, leading to out-of-sync versions and/or http_archive rules in the MODULE file. Abseil-py's 2.x update to the BCR is being discussed here:https://github.com/abseil/abseil-py/issues/263, and Protobuf's is being discussed here: https://github.com/protocolbuffers/protobuf/issues/14564.

eguiraud commented 2 months ago

Hi, I'm using a mix of modules and WORKSPACE because some dependencies still don't have modules available in the BCR. I pull in pybind11_protobuf with a http_archive rule in the WORKSPACE.bazel file, but e.g. abseil as a bazel module.

However the bazel module name for abseil is abseil-cpp while pybind11_protobuf looks for com_google_absl.

How do people usually set up pybind11_protobuf in bazel? Is it only easily possible in fully WORKSPACE-based projects?

mering commented 2 months ago

@eguiraud you can use repo_mapping in WORKSPACE dependencies. You can add repo aliases in MODULE.bazel dependencies.

FYI I am also working on adding it to BCR: https://github.com/bazelbuild/bazel-central-registry/pull/2074

Note that as the release process doesn't create any tags (and it is not planned to change this), I think we are better of in just using a version based on date and commit hash such that we can upload to BCR as needed. Otherwise future updates would be blocked on some maintainer manually creating a tag which does not follow any QA or compatibility tests anyways.

phaedon commented 2 months ago

@eguiraud I have for example this which works:

In WORKSPACE:

git_repository(
    name = "pybind11_protobuf",
    commit = "b4a2e87a10cd5f6309e4ff67c040a470d7ec2373",
    remote = "https://github.com/pybind/pybind11_protobuf.git",
)

In MODULE.bazel:

bazel_dep(name = "pybind11_bazel", version = "2.11.1.bzl.1")
bazel_dep(name = "abseil-cpp", version = "20230802.0.bcr.1", repo_name = "com_google_absl")
eguiraud commented 2 months ago

Hi, thank you both for your quick replies!

repo_mapping in the pybind11_protobuf WORKSPACE entry

I tried adding a repo_mapping to the abseil-cpp bazel module in the WORKSPACE entry and it seems to do the job:

http_archive(
    name = "pybind11_protobuf",
    strip_prefix = "pybind11_protobuf-main",
    url = "https://github.com/pybind/pybind11_protobuf/archive/refs/heads/main.zip",
    repo_mapping = {"@com_google_absl": "@abseil-cpp"},
)

(same for other dependencies of pybind11_protobuf that I bring in as modules)

repo_name in the abseil-cpp MODULE entry

That seems to also do the job but it also requires changing @abseil-cpp to @com_google_absl everywhere, as that's the one allowed name now?

pybind11_protobuf in the bazel central registry

I saw it was just merged, this is the simplest solution. Thank you for pushing for it @mering !

I have a lot of new fun issues with dependency version mismatches between rules_python, protobuf and pybind11_protobuf but that's a whole new, fresh can of worms :)

mering commented 2 months ago

@eguiraud I am working on upgrading rules_python in protobuf in https://github.com/protocolbuffers/protobuf/pull/16942 but this is blocked by some CI upgrade which will hopefully be finished this week.

eguiraud commented 2 months ago

Thank you @mering !

I ran out of time to try and figure this out now, I got stuck at a problem with pybind11_bazel building the module for python 3.11 instead of the desired 3.10, like here: https://github.com/pybind/pybind11_bazel/issues/82. I'm mentioning it here because somehow it only happens when I add pybind11_protobuf into the mix -- we have been using pybind11_bazel without pybind11_protobuf for a while and it works fine (no python version mismatches).

For now I'm pivoting to writing the pybind11 bindings just for the couple of things I actually need by hand, but I'll try pybind11_protobuf again when the bazel protobuf module will pick up a more recent rules_python.