bazel-contrib / rules_go

Go rules for Bazel
Apache License 2.0
1.39k stars 663 forks source link

C++ interop and swig #719

Open ajbouh opened 7 years ago

ajbouh commented 7 years ago

I see in the top-level README.md that there is no support at the moment for C++ interop. However, I also see that there is support for cgo.

What work is required to properly compile .swigcxx files in a package?

I'm hacking together a workaround for the package I'm using, but I'm willing to do this properly and submit a PR. Any pointers about the "right" way to do this?

jayconrod commented 7 years ago

As you said, we do have support for cgo. You can embed C code in comments above import "C", and you can also include C sources and headers in your go_library srcs attribute. Additionally, you can depend on cc_library rules (which may include C++ code) via the cdeps attribute. So you should be able to have some C glue code that bridges between C++ and Go.

We haven't given a great deal of thought to supporting SWIG yet, although it's something we're interested in doing eventually. A fairly simple design would be to have a rule which takes a .swig file and some cc_library dependencies and produces something that looks like a go_library with a cgo object. It's not clear to me that such a rule should be part of rules_go. Maybe there should be a rules_swig repository that handles more cross-language interop.

ajbouh commented 7 years ago

I've written my own shell-script-based rule that takes in .go, .swigcxx, .h, and .cpp files and emits a go binary created with -buildmode=c-shared.

My makeshift rule works for my needs and, while a bit of a hack, also supports the sort of bidirectional interop that I need between Go and C++.

Can I share anything else about my use case that would help identify requirements or design parameters for better swig support?

ajbouh commented 7 years ago

@jayconrod I don't think SWIG rules in a separate repository is the right answer for end users.

In the README for this project there's this:

If your project can be built with go build, you can generate your BUILD files using Gazelle. ...

If this claim is meant to be taken literally, SWIG support should eventually become part of Gazelle (and therefore rules_go). This is becausego build already does all the heavy lifting when it comes to SWIG support, automatically passing along .swigcxx and .swigc files it encounters.

ajbouh commented 7 years ago

@jayconrod Here's what I put together. It doesn't have exhaustive coverage for all types of files supported by cgo or cc_inc_library, but it's working for my specific use case (building extensions for TensorFlow in golang).

If you'd like to me to convert this to a PR, I'm happy to do that.

https://gist.github.com/ajbouh/1143ce82996143f073c488e53bb55dce

jayconrod commented 7 years ago

@ajbouh Thanks for sharing that gist. That seems like a good temporary solution, but I'd like to hold off on adding this to rules_go for now.

Ultimately, I think the best design for this is to have a rules_swig repo with support for Go and other languages. It would generate a .go file that could be picked up in a go_library.

As you correctly pointed out, the charter of Gazelle is to map go build to Bazel, so once that's in place, Gazelle could instantiate SWIG rules and add an appropriate WORKSPACE dependency.

ajbouh commented 7 years ago

Makes sense. How can I track the progress of that effort?

On Aug 28, 2017 7:35 AM, "Jay Conrod" notifications@github.com wrote:

@ajbouh https://github.com/ajbouh Thanks for sharing that gist. That seems like a good temporary solution, but I'd like to hold off on adding this to rules_go for now.

Ultimately, I think the best design for this is to have a rules_swig repo with support for Go and other languages. It would generate a .go file that could be picked up in a go_library.

As you correctly pointed out, the charter of Gazelle is to map go build to Bazel, so once that's in place, Gazelle could instantiate SWIG rules and add an appropriate WORKSPACE dependency.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bazelbuild/rules_go/issues/719#issuecomment-325371205, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAcnSBRdaLHVMT52Rw5IcfEJiLLBRBLks5sctAcgaJpZM4O2q6E .

jayconrod commented 7 years ago

Simplest way to track progress is just to leave this issue open. We'll update this when we start working on SWIG support.

This is a feature that we want to work on, but we have a lot of other high-priority features earlier on our roadmap, so it may be a few months before we get to this.

ajbouh commented 7 years ago

That works for me.

As a small usability note, I have found that it's best to make a swig/c/c++ package as small as possible and move all non-essential go code to its own package because of the difference in compilation speed and quality of error messages of cgo vs pure go packages.

There's also some weirdness with how go encourages you to use swig includes (refer to them by basename in the same directory) and how Bazel wants you to do it (relative to workspace root).

On Aug 28, 2017 9:23 AM, "Jay Conrod" notifications@github.com wrote:

Simplest way to track progress is just to leave this issue open. We'll update this when we start working on SWIG support.

This is a feature that we want to work on, but we have a lot of other high-priority features earlier on our roadmap, so it may be a few months before we get to this.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bazelbuild/rules_go/issues/719#issuecomment-325401996, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAcnScxm9rBQOC0OHXVlrHc-hI4h9Suks5sculzgaJpZM4O2q6E .