Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
phaedon opened this issue Nov 13, 2023 · 12 comments

Comments

@phaedon
Copy link

phaedon commented Nov 13, 2023

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 bazelbuild/bazel-central-registry#1121 )

@phaedon
Copy link
Author

phaedon commented Nov 13, 2023

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
Copy link
Contributor

rwgk commented Nov 13, 2023

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
Copy link
Author

phaedon commented Jan 8, 2024

Ok, it looks like these issues are fixed by: #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
Copy link

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
Copy link
Author

phaedon commented Feb 23, 2024

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
Copy link

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:abseil/abseil-py#263, and Protobuf's is being discussed here: protocolbuffers/protobuf#14564.

@phaedon phaedon changed the title Documentation request: Bazel build instructions Create a release package to enable adding pybind11_protobuf to Bazel Central Registry Apr 4, 2024
@eguiraud
Copy link

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
Copy link

mering commented May 29, 2024

@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: bazelbuild/bazel-central-registry#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
Copy link
Author

phaedon commented May 29, 2024

@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
Copy link

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
Copy link

mering commented May 30, 2024

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

@eguiraud
Copy link

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: pybind/pybind11_bazel#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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants