Bazel: Add an option to `cc_*` rules that lets one specify include directories only for the current rule

[lberki]

Currently, this is done by copts=["-I<dir>"] .This has two problems:

• The actual include path depends on whether the rule is in the main repository or not (it shouldn't, but it'll take a lot of time to fix that)
• It encodes knowledge about the compiler used in the BUILD file.

Thus, it'd be better to express this in a compiler-independent way just like we have the includes and defines attributes.

[dflems]

This would solve a lot of problems we're having migrating a large legacy library to build with bazel. Having an option like private_includes or something of that nature on cc_* rules would be enormously beneficial and make our current setup less fragile.

[c-parsons]

Any work going on with this?
I've looked, and it doesn't look like there's a good workaround for this other than an actual native code change.

IIUC, it just looks like we need an alternate attribute which acts identically to "includes" except that it does not propagate these values up to dependents.

[dflems]

@c-parsons: We could work around it pretty readily if we had a way to get the path to the directory represented by the current BUILD file. I know you can get that from a skylark rule, but we use some pretty simple macro wrappers and that feels a bit overkill.

[c-parsons]

Correct -- You can get it from a rule context either natively or in skylark.
But skylark macros don't have a rule context object -- they are not actual rules but simple expansions.
So unfortunately I don't think there's a workaround within a macro.

I agree that writing a skylark rule for this is overkill, especially when it would be infeasible to emulate cc_library except with the added attribute.
We're better off fixing this natively, IMO

[dflems]

I think I was able to figure out a (perhaps less-than-ideal) workaround for this.

In our wrapper macro for objc_library, I added a new field called private_includes. If our macro sees this field, it creates a header-only objc_library and adds it to non_propagated_deps so the includes aren't passed to anyone who depends upon the library:

def my_objc_library(**args):
    include_dirs = args.pop('private_includes', None)
    if include_dirs:
        priv_name = args['name'] + '_incl_private'
        native.objc_library(
            name = priv_name,
            includes = include_dirs,
            hdrs = native.glob([dir + '/**/*.h' for dir in include_dirs]),
            visibility = ['//visibility:private'],
        )
        args['non_propagated_deps'] = args.pop('non_propagated_deps', []) + [':' + priv_name]

    native.objc_library(**args)

@c-parsons Do you think this could serve as an interim solution until we get support for this natively?

[c-parsons]

This is a really nice idea, indeed. This workaround LGTM. Does it work for you?

[dflems]

@c-parsons: Yup, already migrated everything to use this approach 😄

[silvester747]

The workaround using objc_library looks pretty good, but there does not seem to be an equivalent for cc_library, or is there?

[hlopko]

Nope, with cc_library you still have to use copts=["-I<dir>"]. Current plan is that this will be fixed early 2018.

[dflems]

@mhlopko Any updates on this? We have a clunky workaround that works for cc or objc rules, as opposed to the non_propagated_deps approach from above that only works for objc rules. We'd love if this functionality was built into the cc/objc rules in a way that's consistent and portable.

edit: 2018-07-18 added genfiles support

def private_include_copts(includes):
    copts = []
    prefix = ''

    # convert "@" to "external/" unless in the main workspace
    repo_name = native.repository_name()
    package_name = native.package_name()
    if repo_name != '@':
        prefix = 'external/{}/'.format(repo_name[1:])

    for inc in includes:
        copts.append("-I{}{}/{}".format(prefix, package_name, inc))
        copts.append("-I$(GENDIR)/{}{}/{}".format(prefix, package_name, inc))

    return copts

[dflems]

cc @c-parsons

[hlopko]

No progress on this so far. We're doing quite big refactorings for Skylark API for C++ rules, and Skylark API for C++ toolchain, and I don't plan to work on "includes" (there's a bunch of problems) before we're done with apis. Current estimate is ~Q4/2018 for includes.

[dflems]

@mhlopko @c-parsons: I updated my workaround above. Do you see any issues with how I'm using $(GENDIR)? It seems to work fine on my end.

Also, any update on this? Are these new skylark rules being developed openly? I'd love to follow along.

[hlopko]

CC @oquenchil
Tracking bug is https://github.com/bazelbuild/bazel/issues/4570. Bazel@HEAD accepts --experimental_enable_cc_skylark_api that will enable the current WIP version of the API. It's only for experimenting at this point, we break it daily, and we will not support users using it yet. But feedback is more than welcome

[alanfalloon]

Does this work-around still work? I don't see a non_propagated_deps attribute on objc_library. Any chance we will see a real option soon?

Having a makefile variable with the current package path so that I could write copts=["-I$(PACKAGEDIR)/include"] would also work for me.

[meisterT]

cc @oquenchil

[oquenchil]

Duplicate of #6790