Request: rename includes to system_includes in cc_* functions
#20267
Labels
P3
We're not considering working on this, but happy to review a PR. (No assignee)
team-Rules-CPP
Issues for C++ rules
type: feature request
Comments
carlosgalvezp
changed the title
includes to system_includes in cc_* functions
carlosgalvezp
changed the title
includes to system_includes in cc_* functionsincludes to system_includes in cc_* functions
|
@buildbreaker2021 For context, is |
sgowroji
added
type: feature request
team-Rules-CPP
|
The attribute is used in our third party. Improving docs and renaming the attr could help. |
comius
added
P3
|
A good number of rules in iOS ecosystem add |
|
We use "includes =" a lot for our own includes. We also use it for 3rd party includes that we import via repo rules. It would be good to be able to get Bazel to apply -isystem for 3rd party includes and -I for ours. I can think of a number of solutions. The simplest is perhaps a transitive_copts option that would give a lot of flexibility. |
|
To clarify: I do not want to remove |
|
This is a huge stumbling block, would love to see an attribute rename to make this harder to get wrong. |
keith
added a commit
to keith/bazel
that referenced
this issue
Apr 1, 2025
Previously even though the attribute was named `includes` it was passed through the `system_includes` field of the compilation context. This resulted in toolchains passing these include paths with `-isystem`, which is unexpected if you use this for first party code. Many non bazel-first projects have header directory structures that require custom include paths be propagated throughout the graph, the alternative to `includes` is to use `strip_include_prefix`. The downside of `strip_include_prefix` is that you add 1 include path per `cc_library`, even if the libraries are in the same package. With `includes` these are deduplicated. In the case of LLVM using `includes` reduced the number of search paths on the order of hundreds. If users want to use `-isystem` for third party code that uses `includes`, they can pass `--features=external_include_paths --host_features=external_include_paths` If there are first party libraries users want to use `-isystem` with, they can use `features = ["system_include_paths"]` Fixes bazelbuild#20267
|
I'm proposing an alternative solution to this here: #25750 |
keith
added a commit
to keith/bazel
that referenced
this issue
Apr 1, 2025
Previously even though the attribute was named `includes` it was passed through the `system_includes` field of the compilation context. This resulted in toolchains passing these include paths with `-isystem`, which is unexpected if you use this for first party code. Many non bazel-first projects have header directory structures that require custom include paths be propagated throughout the graph, the alternative to `includes` is to use `strip_include_prefix`. The downside of `strip_include_prefix` is that you add 1 include path per `cc_library`, even if the libraries are in the same package. With `includes` these are deduplicated. In the case of LLVM using `includes` reduced the number of search paths on the order of hundreds. If users want to use `-isystem` for third party code that uses `includes`, they can pass `--features=external_include_paths --host_features=external_include_paths` If there are first party libraries users want to use `-isystem` with, they can use `features = ["system_include_paths"]` Fixes bazelbuild#20267
|
There was an alternate proposal here that suggested fixing this by having |
keith
added a commit
to keith/bazel
that referenced
this issue
Apr 1, 2025
Previously even though the attribute was named `includes`, the paths added there were actually propagated through the `system_includes` attribute of compilation context. This lead to crosstools passing these flags with `-isystem` which was unexpected for first party code. Many non bazel-first projects have header directory structures that require custom include paths be propagated throughout the graph, the alternative to `includes` is to use `strip_include_prefix`. The downside of `strip_include_prefix` is that you add 1 include path per `cc_library`, even if the libraries are in the same package. With `includes` these are deduplicated. In the case of LLVM using `includes` reduced the number of search paths on the order of hundreds. If users want to use `-isystem` for third party code that uses `includes`, they can pass `--features=external_include_paths --host_features=external_include_paths` Fixes bazelbuild#20267
|
thanks I hadn't seen that! here's an option that adds |
keith
added a commit
to keith/bazel
that referenced
this issue
Apr 2, 2025
Previously even though the attribute was named `includes`, the paths added there were actually propagated through the `system_includes` attribute of compilation context. This lead to crosstools passing these flags with `-isystem` which was unexpected for first party code. Many non bazel-first projects have header directory structures that require custom include paths be propagated throughout the graph, the alternative to `includes` is to use `strip_include_prefix`. The downside of `strip_include_prefix` is that you add 1 include path per `cc_library`, even if the libraries are in the same package. With `includes` these are deduplicated. In the case of LLVM using `includes` reduced the number of search paths on the order of hundreds. If users want to use `-isystem` for third party code that uses `includes`, they can pass `--features=external_include_paths --host_features=external_include_paths` Fixes bazelbuild#20267
keith
added a commit
to keith/bazel
that referenced
this issue
Apr 2, 2025
Previously even though the attribute was named `includes` it was passed through the `system_includes` field of the compilation context. This resulted in toolchains passing these include paths with `-isystem`, which is unexpected if you use this for first party code. Many non bazel-first projects have header directory structures that require custom include paths be propagated throughout the graph, the alternative to `includes` is to use `strip_include_prefix`. The downside of `strip_include_prefix` is that you add 1 include path per `cc_library`, even if the libraries are in the same package. With `includes` these are deduplicated. In the case of LLVM using `includes` reduced the number of search paths on the order of hundreds. If users want to use `-isystem` for third party code that uses `includes`, they can pass `--features=external_include_paths --host_features=external_include_paths` If there are first party libraries users want to use `-isystem` with, they can use `features = ["system_include_paths"]` Fixes bazelbuild#20267
keith
added a commit
to keith/bazel
that referenced
this issue
Apr 2, 2025
Previously even though the attribute was named `includes` it was passed through the `system_includes` field of the compilation context. This resulted in toolchains passing these include paths with `-isystem`, which is unexpected if you use this for first party code. Many non bazel-first projects have header directory structures that require custom include paths be propagated throughout the graph, the alternative to `includes` is to use `strip_include_prefix`. The downside of `strip_include_prefix` is that you add 1 include path per `cc_library`, even if the libraries are in the same package. With `includes` these are deduplicated. In the case of LLVM using `includes` reduced the number of search paths on the order of hundreds. If users want to use `-isystem` for third party code that uses `includes`, they can pass `--features=external_include_paths --host_features=external_include_paths` If there are first party libraries users want to use `-isystem` with, they can use `features = ["system_include_paths"]` Fixes bazelbuild#20267
keith
added a commit
to keith/bazel
that referenced
this issue
Apr 2, 2025
Previously even though the attribute was named `includes` it was passed through the `system_includes` field of the compilation context. This resulted in toolchains passing these include paths with `-isystem`, which is unexpected if you use this for first party code. Many non bazel-first projects have header directory structures that require custom include paths be propagated throughout the graph, the alternative to `includes` is to use `strip_include_prefix`. The downside of `strip_include_prefix` is that you add 1 include path per `cc_library`, even if the libraries are in the same package. With `includes` these are deduplicated. In the case of LLVM using `includes` reduced the number of search paths on the order of hundreds. If users want to use `-isystem` for third party code that uses `includes`, they can pass `--features=external_include_paths --host_features=external_include_paths` If there are first party libraries users want to use `-isystem` with, they can use `features = ["system_include_paths"]` Fixes bazelbuild#20267
keith
added a commit
to keith/bazel
that referenced
this issue
Apr 2, 2025
Previously even though the attribute was named `includes` it was passed through the `system_includes` field of the compilation context. This resulted in toolchains passing these include paths with `-isystem`, which is unexpected if you use this for first party code. Many non bazel-first projects have header directory structures that require custom include paths be propagated throughout the graph, the alternative to `includes` is to use `strip_include_prefix`. The downside of `strip_include_prefix` is that you add 1 include path per `cc_library`, even if the libraries are in the same package. With `includes` these are deduplicated. In the case of LLVM using `includes` reduced the number of search paths on the order of hundreds. If users want to use `-isystem` for third party code that uses `includes`, they can pass `--features=external_include_paths --host_features=external_include_paths` If there are first party libraries users want to use `-isystem` with, they can use `features = ["system_include_paths"]` Fixes bazelbuild#20267 RELNOTES[INC]: Use `-I` instead of `-isystem` for `cc_library` / `cc_binary` `includes` attr. To use `-isystem` for only external repositories, you can pass `--features=external_include_paths --host_features=external_include_paths`. To use `-isystem` for a single `cc_library` / `cc_binary` `includes`, you can set `features = ["system_include_paths"],` on the target
keith
added a commit
to keith/bazel
that referenced
this issue
Apr 3, 2025
Previously even though the attribute was named `includes` it was passed through the `system_includes` field of the compilation context. This resulted in toolchains passing these include paths with `-isystem`, which is unexpected if you use this for first party code. Many non bazel-first projects have header directory structures that require custom include paths be propagated throughout the graph, the alternative to `includes` is to use `strip_include_prefix`. The downside of `strip_include_prefix` is that you add 1 include path per `cc_library`, even if the libraries are in the same package. With `includes` these are deduplicated. In the case of LLVM using `includes` reduced the number of search paths on the order of hundreds. If users want to use `-isystem` for third party code that uses `includes`, they can pass `--features=external_include_paths --host_features=external_include_paths` If there are first party libraries users want to use `-isystem` with, they can use `features = ["system_include_paths"]` Fixes bazelbuild#20267 RELNOTES[INC]: Use `-I` instead of `-isystem` for `cc_library` / `cc_binary` `includes` attr. To use `-isystem` for only external repositories, you can pass `--features=external_include_paths --host_features=external_include_paths`. To use `-isystem` for a single `cc_library` / `cc_binary` `includes`, you can set `features = ["system_include_paths"],` on the target
keith
added a commit
to keith/bazel
that referenced
this issue
Apr 3, 2025
Previously even though the attribute was named `includes` it was passed through the `system_includes` field of the compilation context. This resulted in toolchains passing these include paths with `-isystem`, which is unexpected if you use this for first party code. Many non bazel-first projects have header directory structures that require custom include paths be propagated throughout the graph, the alternative to `includes` is to use `strip_include_prefix`. The downside of `strip_include_prefix` is that you add 1 include path per `cc_library`, even if the libraries are in the same package. With `includes` these are deduplicated. In the case of LLVM using `includes` reduced the number of search paths on the order of hundreds. If users want to use `-isystem` for third party code that uses `includes`, they can pass `--features=external_include_paths --host_features=external_include_paths` If there are first party libraries users want to use `-isystem` with, they can use `features = ["system_include_paths"]` Fixes bazelbuild#20267 RELNOTES[INC]: Use `-I` instead of `-isystem` for `cc_library` / `cc_binary` `includes` attr. To use `-isystem` for only external repositories, you can pass `--features=external_include_paths --host_features=external_include_paths`. To use `-isystem` for a single `cc_library` / `cc_binary` `includes`, you can set `features = ["system_include_paths"],` on the target
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Currently,
includesis misleading, since the name does not in any way reflect the fact that the headers will be included as system headers. This goes against developer expectations.System headers should rarely be used on own code, because all C++ tools (compilers, static analyzers, etc) will ignore warnings in such headers.
It would be a lot clearer and easy to spot during code review if
includeswas renamed tosystem_includes.The text was updated successfully, but these errors were encountered: