Do not expose external repo source root as system include

[DoDoENT]

Fixes #801.

When external_include_paths is enabled for a target in an external repository, rules_cc reclassifies public include paths as external/system includes. Including the external repository source root (repo_path) in that set emits -isystem external/<repo> on downstream compile actions and exposes unrelated root files to angle-bracket lookup.

On case-insensitive filesystems, a repository containing a root-level VERSION file can therefore break libc++’s #include <version>. The issue originally surfaced in DoDoENT/bazel-playground through its OpenCV/libtiff dependency graph.

Keep the external repository root propagated as a quote-only include (-iquote). This preserves repository-relative quoted includes for downstream dependents without making the root eligible for angle-bracket includes. Generated roots and explicit public include directories remain external/system includes.

The external_include_paths analysis regression inspects a downstream compile action and asserts that the exact external repository root:

• is absent from -isystem and -I, reproducing the original header-shadowing failure;
• remains present in -iquote, reproducing the downstream include failure caused by keeping the root target-local.

Other public paths from the external dependency are still required to appear as system includes.

Validation:

• bazel test //tests/cc/common:cc_binary_configured_target_tests --test_output=errors

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[keith]

There's a tough call here because this is potentially breaking external_include_paths intent of only importing through system paths to avoid warnings for repos that actually do use the "import from the root of the repo" scheme. That's probably pretty rare in BCR repos though. Putting this behind yet another feature would feel bad tool.

anyone have bright ideas?

[DoDoENT]

There's a tough call here because this is potentially breaking external_include_paths intent of only importing through system paths to avoid warnings for repos that actually do use the "import from the root of the repo" scheme. That's probably pretty rare in BCR repos though. Putting this behind yet another feature would feel bad tool.

anyone have bright ideas?

But shouldn't be expected that repos that do use "import from the root of the repo" should have includes = ["."] in their main public target? In that case, nothing should break.

The change just removes the implicit include paths that were never mentioned anywhere, yet caused problems. libtiff and marisa-trie are examples of BCR packages that need to patch upstream sources to remove the VERSION file to avoid this issue.

As C++ gains more STL headers in newer revisions, more conflicts like these will arise, some even on case-sensitive filesystems.

[keith]

But shouldn't be expected that repos that do use "import from the root of the repo" should have includes = ["."] in their main public target? In that case, nothing should break.

Today that isn't possible #625

[DoDoENT]

But shouldn't be expected that repos that do use "import from the root of the repo" should have includes = ["."] in their main public target? In that case, nothing should break.

Today that isn't possible #625

I wasn't aware of that. But don't you think that this PR actually fits nicely together with yours?

[keith]

the annoyance I guess is then users would have to add a dep on a module in the root of the repo to every nested cc_library. not in the simple BCR case, but in the more complex case where they follow that pattern