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

Expose strip_include_prefix to cc_common.compile #11856

Closed
lirp opened this issue Jul 28, 2020 · 2 comments
Closed

Expose strip_include_prefix to cc_common.compile #11856

lirp opened this issue Jul 28, 2020 · 2 comments
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Rules-CPP Issues for C++ rules type: feature request

Comments

@lirp
Copy link

lirp commented Jul 28, 2020

Description of the problem / feature request:

cc_library has a strip_include_prefix facility, cc_common.compile does not.

Feature requests: what underlying problem are you trying to solve with this feature?

We have a "fat" C++ code generator target that emits multiple C++ files from a single source based on some configurations. The output files are written to multiple directories. For example:

generator(name = "foo", configs = ["bar", "baz"]) in //some/directory writes out files to some/directory/bar/foo.h and some/directory/baz/foo.h.

We then allow downstream targets to include those headers with the configuration omitted, e.g. for some target:
generator(name = "qux", deps = ["//some/directory:foo"], configs = ["bar", "baz"], srcs = ["qux.cc"])
qux.cc can contain:
#include "some/directory/foo.h"

Right now, we have a bunch of macros to make this work; "foo" expands to several cc_library targets with strip_include_prefix set appropriately, and then generator() elaborates all dependencies to depend on the right cc_library targets for the given configuration. For example,
"qux" elaborates to "qux_bar" and "qux_baz"
"foo" elaborates to "foo_bar" and "foo_baz"
"qux_bar" has deps = "foo_bar"
"qux_baz" has deps = "foo_baz"

We'd like to be able to configure those dependencies in a more fine-grained way though; our current set of macros/rules requires that every target build in every configuration. However, we'd like to have a way to express:
new_generator(name = "foo") in //some/directory writes out some/directory/foo.h.
generator(name = "qux", deps = ["//some/directory:foo"], configs = ["bar", "baz"], srcs = ["qux.cc"])
"qux" elaborates to "qux_bar" and "qux_baz"
"qux_bar" has deps = "foo"
"qux_baz" has deps = "foo"

This seems like a reasonable candidate for the C++ sandwich, except strip_include_prefix is missing.

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

Try to cc_common.compile with strip_include_prefix = something.

What operating system are you running Bazel on?

Linux

What's the output of bazel info release?

release 3.4.0

If bazel info release returns "development version" or "(@non-git)", tell us how you built Bazel.

n/a

What's the output of git remote get-url origin ; git rev-parse master ; git rev-parse HEAD ?

n/a

Have you found anything relevant by searching the web?

No

Any other information, logs, or outputs that you want to share?

No

I'm happy to send a change for review to address this; it looks like it's straightforward to plumb strip_include_prefix into cc_common.compile. I'd like confirmation that this is acceptable before writing up the change though.

@oquenchil oquenchil added P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Rules-CPP Issues for C++ rules type: feature request labels Jul 28, 2020
@oquenchil
Copy link
Contributor

We wanted to completely re-do how includes works in the C++ rules but that work was put on hold. Until we have a clear idea on how to proceed with the whole redesign I don't think we will be adding this to the API.

@lirp
Copy link
Author

lirp commented Jul 28, 2020

Does that have an ETA? Does this make sense on the interim? Right now this is at least an inconsistency between cc_common.compile and cc_library.

aiuto pushed a commit that referenced this issue Aug 20, 2020
This brings cc_common.compile a bit closer to parity with cc_library.

Fixes #11856

RELNOTES[NEW]: cc_common.compile support for include_prefix/strip_include_prefix

PiperOrigin-RevId: 325850902
michaeleisel pushed a commit to michaeleisel/bazel that referenced this issue Sep 3, 2020
This brings cc_common.compile a bit closer to parity with cc_library.

Fixes bazelbuild#11856

RELNOTES[NEW]: cc_common.compile support for include_prefix/strip_include_prefix

PiperOrigin-RevId: 325850902
aiuto pushed a commit that referenced this issue Sep 12, 2020
This brings cc_common.compile a bit closer to parity with cc_library.

Fixes #11856

RELNOTES[NEW]: cc_common.compile support for include_prefix/strip_include_prefix

PiperOrigin-RevId: 325850902
aiuto pushed a commit that referenced this issue Sep 25, 2020
This brings cc_common.compile a bit closer to parity with cc_library.

Fixes #11856

RELNOTES[NEW]: cc_common.compile support for include_prefix/strip_include_prefix

PiperOrigin-RevId: 325850902
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Rules-CPP Issues for C++ rules type: feature request
Projects
None yet
Development

No branches or pull requests

2 participants