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

Add support for cc_library targets that depend on Nanobind, without nanobind_library. #48

Open
hawkinsp opened this issue Dec 17, 2024 · 6 comments
Labels
enhancement New feature or request

Comments

@hawkinsp
Copy link

Currently nanobind_bazel exposes a nanobind_library bazel macro that wraps cc_library with nanobind deps and copts. But this isn't a compositional way to do it: if I had a cc_library that depended on two such libraries, then I cannot use nanobind_library.

I'm attempting to opensource a package that does something like this, but I found I needed access to the underlying @nanobind target to do this, i.e., I had to add the following to my MODULE.bazel, which seems like a layering violation:

internal_configure = use_extension("@nanobind_bazel//:internal_configure.bzl", "internal_configure_extension")
use_repo(internal_configure, "nanobind")

Can we expose @nanobind directly to users?

@nicholasjng
Copy link
Owner

What is the problem with multiple nanobind_library dependencies?

If you "just" want a way to override the @nanobind target with your own, that's great timing - I asked about how to do this today on the Bazel Slack, and got a response that should be fairly straightforward to implement: https://bazelbuild.slack.com/archives/CA31HN1T3/p1734427331205089

This would (if I understand correctly) result in a label_flag that you could point to your own nanobind target, with nanobind_bazel's internal nanobind build as a fallback.

@hawkinsp
Copy link
Author

What is the problem with multiple nanobind_library dependencies?

The problem is that if you rely on a wrapper rule, only one library can be the "wrapper".

I want to write

cc_library(
  name="foo",
  srcs = ["foo.cc"],
  deps = [
    "@nanobind",
   "@somethingelse",
  ],
)

and not use nanobind_library.

@nicholasjng
Copy link
Owner

nicholasjng commented Dec 17, 2024

Okay. So how can I help you in this instance? If you can't use the nanobind_library macro, I'm unsure what the value of nanobind_bazel is for you. Note that there is also a @nanobind module on the BCR, but that is missing some of the newer releases.

If it's just about forwarding nanobind to, say, @nanobind_bazel//:nanobind or something similar, I'll see what I can do, and follow up in that Slack thread tomorrow.

(Based on what I currently know, I think I could create a module extension containing @nanobind, which you could then consume, pretty much like you already did with the internal configuration extension in your MODULE.bazel.)

@hawkinsp
Copy link
Author

Well, I do want to use nanobind_extension in the same module. So either we'd want nanobind_bazel to work with a separate nanobind, or we'd want it to export its nanobind.

@nicholasjng
Copy link
Owner

FWIW, I think both should be on the agenda (there is an open issue already for custom nanobind targets).

But reading https://bazel.build/external/extension#extension_usage, I'm wondering if you did just the intended thing by loading in the module extension yourself, like mentioned in these docs. The problem would come when nanobind_bazel also loads that extension in its scope, and you get two identical, but differently labeled nanobinds.

I'm guessing the fix is that you use the extension first, and then pass your "custom" nanobind into your cc_library targets, or your nanobind_librarys with a custom nanobind label field, which I would first need to implement for the nanobind_*_library macros.

@nicholasjng
Copy link
Owner

#49 has landed with an alias declaration of the internal @nanobind as @nanobind_bazel//:nanobind.

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

No branches or pull requests

2 participants