googleapis/google-cloud-cpp-spanner

Consider supporting include with angle brackets in Bazel.

coryan opened this issue · 2 comments

coryan commented

Some Bazel users may want to use #include <google/cloud/spanner/client.h> (note the angle brackes), which requires adding a includes=(["."]) directive to our Bazel commands. This should be optional because some users of Bazel do not like angle brackets.

devjgm commented

A few comments. Since our BUILD file lives in google/cloud/spanner/BUILD, the option we'd need to add to our cc_library build rule is includes = ["../../.."],. That would result in a -I <...> option being added to all dependents of our build rule so they can treat our whole project root as a system include directory.

The problem w/ this is that ../../.. resolves to our project root, which has our WORKSPACE file, and bazel doesn't like this, giving the following error:

ERROR: /v/google/cloud/spanner/BUILD:37:1: in includes attribute of cc_library rule //google/cloud/spanner:spanner_client: '../../../' resolves to the workspace root, which would allow this rule and all of its transitive dependents to include any file in your workspace. Please include only what you need

The reason this works for other projects like googletest, grpc, and protobuf is because their code lives in their repo at a different path than the one they want users to include. Usually, they put their code in a top-level src/ dir.

I think they use the includes = ... option in their build rule to make the code compile at all. I doubt their goal is to make angle-bracket includes work (I suspect that's just a side effect). Note: including all the googletest, grpc, and protobuf includes with double quotes works too.

Bazel's recommendation with include path is to use double quotes, not angle brackets.

Options

Here are some options.

  1. Not support angle-bracket includes if using bazel. This matche's Bazel's advice, and our code works with this policy today.

  2. Create a top-level src/ directory and mv google/ src/. This should allow us to use the includes = ["../../.."] trick in our BUILD files. However, it will

  3. Create a top-level BUILD file w/ cc_library target that users are supposed to use. This will let us add the includes = ["."] option that will let users include using quotes or angle brackets. This also has the nice feature of providing users with a much simpler user-facing BUILD file that will contain all of the build targets that they're allowed to use. A draft/example of this is #1396

devjgm commented

Fixed in #1396