Skip to content

Commit 2178c68

Browse files
committed
Use includes instead of system_includes for includes attr
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
1 parent 106903d commit 2178c68

File tree

5 files changed

+9
-6
lines changed

5 files changed

+9
-6
lines changed

src/main/starlark/builtins_bzl/common/cc/cc_binary.bzl

+1-1
Original file line numberDiff line numberDiff line change
@@ -510,7 +510,7 @@ def cc_binary_impl(ctx, additional_linkopts, force_linkstatic = False):
510510
cxx_flags = cc_helper.get_copts(ctx, feature_configuration, additional_make_variable_substitutions, attr = "cxxopts"),
511511
defines = cc_helper.defines(ctx, additional_make_variable_substitutions),
512512
local_defines = cc_helper.local_defines(ctx, additional_make_variable_substitutions) + cc_helper.get_local_defines_for_runfiles_lookup(ctx, ctx.attr.deps),
513-
system_includes = cc_helper.system_include_dirs(ctx, additional_make_variable_substitutions),
513+
includes = cc_helper.include_dirs(ctx, additional_make_variable_substitutions),
514514
private_hdrs = cc_helper.get_private_hdrs(ctx),
515515
public_hdrs = cc_helper.get_public_hdrs(ctx),
516516
copts_filter = cc_helper.copts_filter(ctx, additional_make_variable_substitutions),

src/main/starlark/builtins_bzl/common/cc/cc_helper.bzl

+2-2
Original file line numberDiff line numberDiff line change
@@ -1062,7 +1062,7 @@ def _package_source_root(repository, package, sibling_repository_layout):
10621062
repository = repository[1:]
10631063
return paths.get_relative(paths.get_relative("external", repository), package)
10641064

1065-
def _system_include_dirs(ctx, additional_make_variable_substitutions):
1065+
def _include_dirs(ctx, additional_make_variable_substitutions):
10661066
result = []
10671067
sibling_repository_layout = ctx.configuration.is_sibling_repository_layout()
10681068
package = ctx.label.package
@@ -1264,7 +1264,7 @@ cc_helper = struct(
12641264
get_private_hdrs = _get_private_hdrs,
12651265
get_public_hdrs = _get_public_hdrs,
12661266
report_invalid_options = _report_invalid_options,
1267-
system_include_dirs = _system_include_dirs,
1267+
include_dirs = _include_dirs,
12681268
get_coverage_environment = _get_coverage_environment,
12691269
create_cc_instrumented_files_info = _create_cc_instrumented_files_info,
12701270
linkopts = _linkopts,

src/main/starlark/builtins_bzl/common/cc/cc_library.bzl

+1-1
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ def _cc_library_impl(ctx):
6363
cxx_flags = cc_helper.get_copts(ctx, feature_configuration, additional_make_variable_substitutions, attr = "cxxopts"),
6464
defines = cc_helper.defines(ctx, additional_make_variable_substitutions),
6565
local_defines = cc_helper.local_defines(ctx, additional_make_variable_substitutions) + cc_helper.get_local_defines_for_runfiles_lookup(ctx, ctx.attr.deps + ctx.attr.implementation_deps),
66-
system_includes = cc_helper.system_include_dirs(ctx, additional_make_variable_substitutions),
66+
includes = cc_helper.include_dirs(ctx, additional_make_variable_substitutions),
6767
copts_filter = cc_helper.copts_filter(ctx, additional_make_variable_substitutions),
6868
purpose = "cc_library-compile",
6969
srcs = cc_helper.get_srcs(ctx),

src/main/starlark/builtins_bzl/common/cc/compile/cc_compilation_helper.bzl

+4-1
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,10 @@ def _init_cc_compilation_context(
227227
external_include_dirs = []
228228
declared_include_srcs = []
229229

230-
if not external:
230+
if not external and _enabled(feature_configuration, "system_include_paths"):
231+
system_include_dirs_for_context = system_include_dirs + include_dirs
232+
include_dirs_for_context = []
233+
elif not external:
231234
system_include_dirs_for_context = list(system_include_dirs)
232235
include_dirs_for_context = list(include_dirs)
233236
else:

src/main/starlark/builtins_bzl/common/objc/compilation_support.bzl

+1-1
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ def _build_common_variables(
105105
has_module_map = has_module_map,
106106
attr_linkopts = attr_linkopts,
107107
direct_cc_compilation_contexts = direct_cc_compilation_contexts,
108-
includes = cc_helper.system_include_dirs(ctx, {}) if hasattr(ctx.attr, "includes") else [],
108+
includes = cc_helper.include_dirs(ctx, {}) if hasattr(ctx.attr, "includes") else [],
109109
)
110110

111111
return struct(

0 commit comments

Comments
 (0)