bazelbuild / bazelbuild/bazel

Generated headers should work the same as checked-in headers (C++)

Open
#13,803 8 comments 0 reactions 0 assignees View on GitHub
P4 team-Rules-CPP type: support / not a bug (process)
Dominant language
Java
Stars
25.8k
Forks
4.6k
Avg merge
2d 20h
Merged PRs (30d)
72

Description

### Description of the problem / feature request:

When you generate a C++ header it should behave exactly as if you had checked in the header as a source file. Currently, inclusion ordering gets totally messed up in strange and hard-to-debug ways.

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

When processing an `#include` directive, all major compilers (clang, gcc, MSVC) search the directory the current source file before looking in directories provided on the command line. I don't think this is specified in the C++ standard, but it appears to be the defacto standard. In Bazel, when you generate a file, it is generated in the genfiles/binfiles directory `bazel-out//bin`. So if you try to include the generated file it is not found in the same directory. It gets even worse if there's some other header with the same name on the include path, which is what the attached repro demonstrates.

I think I've seen some Bazel documentation that says Bazel symlinks together binfiles, genfiles, and source roots to form one virtual directory (maybe it was the line in [output directory layout](https://docs.bazel.build/versions/main/output_directories.html) about execroot?), but that doesn't seem to be the case.

You can work around this by adding `includes = [""]` to the target to add the directory path to the include search, and that works for a bit. But if your dependency is exporting a public header with the name and switches from `includes` to the preferred `strip_include_prefix` then you're in trouble again. The `-I` for the virtual_includes is searched *before* the `-isystem` for the includes argument on your rule. You have to create a cc_library rule that wraps your generated file and uses `strip_include_prefix` to move it into the top level directory. And then the poison spreads because you've created a header that can be included by any rule with no directory prefix (in fact just such a workaround is I think why I ran into an Options.inc that was being exported with no header in the first place). And it's even worse than that. The order that the virtual_includes are added is dependent on the order of deps. So if your `cc_library` that wraps the generated header sorts after the dependency or transitive dependency through which you get the conflicting header, you lose it again.

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

Download and extract any of the attached archives and run `bazel build Foo`. Note that `bazel build Bar Baz` succeeds because their options.inc are source files instead of generated.

[bazel-include-repro-1.tar.gz](https://github.com/bazelbuild/bazel/files/6934862/bazel-include-repro-1.tar.gz) demonstrates the first case where you end up with a private header from a dependency. You can work around this by adding `includes`.
[bazel-include-repro-2.tar.gz](https://github.com/bazelbuild/bazel/files/6934861/bazel-include-repro-2.tar.gz) demonstrates the second case that can't be worked around using `includes` but can be worked around with `strip_include_prefix` dependent on dependency ordering.
[bazel-include-repro-3.tar.gz](https://github.com/bazelbuild/bazel/files/6934884/bazel-include-repro-3.tar.gz) demonstrates how the `strip_include_prefix` workaround falls over if the dependency that (even transitively) provides the wrong include is sorted before the cc_library wrapper around the genfile.

### What operating system are you running Bazel on?

Linux Debian

### What's the output of `bazel info release`?

4.1.0

### Have you found anything relevant by searching the web?

This looks like an extension of https://github.com/bazelbuild/bazel/issues/4463 which was closed as fixed. That one only covered using full include paths, I think.

### Potential solutions
Use symlinks to make generated files appear exactly like source files in the execroot. I thought this was something Bazel at least claimed to do.

Add the corresponding generated directory of the current file as the first `-I` argument to any C++ compile command. This is what CMake does, based on looking at compile_commands.json

The first option sounds like the right long term fix if it's in Bazel's plans, but the second option would be simpler and relieve a lot of pain right now. I'm not sure what this would do to compile times, but given the number of virtual_includes directories I see on the compile commands right now, it doesn't seem too expensive. One option would be to use another virtual_includes directory to do this and only move over files in the rule's `srcs`.

Contributor guide

Open the contributing guide

Research direction

Extract one of the attached bazel-include-repro archives and run `bazel build Foo`, comparing it with the successful `Bar` and `Baz` builds. Trace how Bazel assembles C++ include paths for generated versus checked-in headers; done means generated headers resolve includes with the same ordering and behavior as checked-in headers without relying on the listed workarounds.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
build-system
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.