Feature Request: local_includes for cc_library, making an easy path for good cross-platform hygiene

[cpsauer]

Hi wonderful Bazelers,

This is a small-scope cc feature request.

As background: cc_library lets you specify include directories for its headers, automatically handling the package. This is very nice for interface headers but bad problematic for implementation headers you don't want to propagate to dependents. The docs steer you to add -I flags to copts to handle that case.

The problem is that this leads people to write code that won't work with toolchains where the flags are different. And needing to figure out that you have to use package_name() (which doesn't handle the external workspace case) tends to lead users to just make all includes public, propagating them to all dependencies. Bad times.

A fairly simple new parameter, local_includes could solve this well, I'd propose. What do you think?

Thanks,
Chris
(ex-Googler)
P.S. Edited from the template for clarity, brevity, and friendliness.

[cameron-martin]

This same idea was also mentioned here: #13803 (comment)

[kon72]

+1 for local_includes.

I think this can be achived with implementation_deps

def custom_cc_library(
        name,
        local_includes = [],
        implementation_deps = [],
        **kwargs):
    native.cc_library(
        name = name + "_local_includes",
        includes = local_includes,
        visibility = ["//visibility:private"],
    )

    native.cc_library(
        name = name,
        implementation_deps = implementation_deps + [name + "_local_includes"],
        **kwargs
    )

[afoxley]

It seems like the behavior people are asking for here was recently added as "strip_include_prefix" https://bazel.build/reference/be/c-cpp#cc_library.strip_include_prefix

This name is non-intuitive and IMO it would be much better if "local_includes" was used as suggested in this issue. Is there a way an alias could be added?

[cameron-martin]

I thought that attribute has been there for ages, and I'm missing how it's the same as local_includes

[afoxley]

I had thought it was added about a year ago? Maybe not.

From a bazel user perspective, when exposing headers from a cc_library often people reach for includes to express the include path to downstream users. It's the intuitively named option but is usually not what you actually want (Uses -isystem).

strip_include_prefix provides a cross platform way to expose include paths to library users as it creates a virtual include folder and symlinks the exposed headers in.

If it was named local_includes, people may make the mistake less often was my thought.

[cameron-martin]

Oh I see, and you take advantage of the workspace root by default being on the include path?

[afoxley]

No these are all relative paths, not directly on the workspace root. See an example of the pigweed project recently working through migrating off extensive use of includes. https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/248233

[afoxley]

Thinking about this more, local_includes isn't quite the right name either since the path does propagate to downstream library users. So it's not really like local_defines which doesn't propagate.

Really you want control over whether -isystem or -I (or even -iquote) equivalent is used on the headers exposed by the library. Ideally in a cross platform way.

[shahms]

Non.-propagation is a key feature of the request. This isn't about the headers exposed by the library, but the internal headers used by it.

[afoxley]

Got it, makes sense to leave local_includes for that purpose. I guess I wish there was a better name for what strip_include_prefix does.