Skip to content

Commit e8422d3

Browse files
ma-olicopybara-github
authored andcommitted
Add a raw_allowlist_include_directories cc_toolchain top argument
Copybara Import from #450 BEGIN_PUBLIC Add a raw_allowlist_include_directories cc_toolchain top argument (#450) Apple toolchain declaration in bazel are inherently non-hermetic from a bazel perspective, as they rely on the system's Xcode installation. As a result, apple_support currently make use of cxx_builtin_include_directories to add known system directories, as seen in the toolchain declaration here: https://github.com/bazelbuild/apple_support/blob/master/crosstool/osx_cc_configure.bzl#L136 This currently cannot be done in rules based toolchain, as the allowlist_include_directories argument in cc_args() requires a DirectoryInfo provider. In this change, we're adding a new top-level raw_allowlist_include_directories parameter to the cc_toolchain() macro and to the underlying cc_toolchain_config() rule. This parameter will act just like the existing one, but will accept raw strings that will be added directly to cxx_builtin_include_directories. Fixes #242 Closes #450 END_PUBLIC COPYBARA_INTEGRATE_REVIEW=#450 from ma-oli:raw_allowlist_include_directories d4076e4 PiperOrigin-RevId: 794358089 Change-Id: I92e622b549fb38fd1f798efaeb119afcefc4ccb7
1 parent 879d73b commit e8422d3

File tree

9 files changed

+45
-3
lines changed

9 files changed

+45
-3
lines changed

cc/toolchains/args.bzl

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,10 @@ def _cc_args_impl(ctx):
4747
format = {k: v for v, k in ctx.attr.format.items()},
4848
)
4949

50+
for path in ctx.attr.allowlist_absolute_include_directories:
51+
if not path.startswith("/"):
52+
fail("`{}` is not an absolute paths".format(path))
53+
5054
nested = None
5155
if ctx.attr.args or ctx.attr.nested:
5256
# Forward the format variables used by the env formatting so they don't trigger
@@ -74,6 +78,9 @@ def _cc_args_impl(ctx):
7478
allowlist_include_directories = depset(
7579
direct = [d[DirectoryInfo] for d in ctx.attr.allowlist_include_directories],
7680
),
81+
allowlist_absolute_include_directories = depset(
82+
direct = ctx.attr.allowlist_absolute_include_directories,
83+
),
7784
)
7885

7986
return [
@@ -87,6 +94,7 @@ def _cc_args_impl(ctx):
8794
for action in actions.to_list()
8895
]),
8996
allowlist_include_directories = args.allowlist_include_directories,
97+
allowlist_absolute_include_directories = args.allowlist_absolute_include_directories,
9098
),
9199
]
92100

@@ -98,6 +106,9 @@ _cc_args = rule(
98106
mandatory = True,
99107
doc = """See documentation for cc_args macro wrapper.""",
100108
),
109+
"allowlist_absolute_include_directories": attr.string_list(
110+
doc = """See documentation for cc_args macro wrapper.""",
111+
),
101112
"allowlist_include_directories": attr.label_list(
102113
providers = [DirectoryInfo],
103114
doc = """See documentation for cc_args macro wrapper.""",
@@ -131,6 +142,7 @@ def cc_args(
131142
name,
132143
actions = None,
133144
allowlist_include_directories = None,
145+
allowlist_absolute_include_directories = None,
134146
args = None,
135147
data = None,
136148
env = None,
@@ -238,6 +250,12 @@ def cc_args(
238250
This can help work around errors like:
239251
`the source file 'main.c' includes the following non-builtin files with absolute paths
240252
(if these are builtin files, make sure these paths are in your toolchain)`.
253+
allowlist_absolute_include_directories: (List[str]) Allowlists absolute include directories,
254+
preventing Bazel from emitting errors when an #include of local system files in the
255+
directory occurs. Be careful when adding directories to this list, as it inherently
256+
causes leaks in hermeticity. Prefer to reserve use of this for cases like Xcode, where
257+
the conventional expectation is to allowlist well-known system-absolute include paths
258+
rather than redistributing the SDK.
241259
args: (List[str]) The command-line arguments that are applied by using this rule. This is
242260
mutually exclusive with [nested](#cc_args-nested).
243261
data: (List[Label]) A list of runtime data dependencies that are required for these
@@ -282,6 +300,7 @@ def cc_args(
282300
name = name,
283301
actions = actions,
284302
allowlist_include_directories = allowlist_include_directories,
303+
allowlist_absolute_include_directories = allowlist_absolute_include_directories,
285304
args = args,
286305
data = data,
287306
env = env,

cc/toolchains/cc_toolchain_info.bzl

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ ArgsInfo = provider(
108108
"files": "(depset[File]) Files required for the args",
109109
"env": "(dict[str, str]) Environment variables to apply",
110110
"allowlist_include_directories": "(depset[DirectoryInfo]) Include directories implied by these arguments that should be allowlisted in Bazel's include checker",
111+
"allowlist_absolute_include_directories": "(depset[str]) Absolute include directories implied by these arguments that should be allowlisted in Bazel's include checker",
111112
},
112113
)
113114
ArgsListInfo = provider(
@@ -119,6 +120,7 @@ ArgsListInfo = provider(
119120
"files": "(depset[File]) The files required for all of the arguments",
120121
"by_action": "(Sequence[struct(action=ActionTypeInfo, args=List[ArgsInfo], files=depset[Files])]) Relevant information about the args keyed by the action type.",
121122
"allowlist_include_directories": "(depset[DirectoryInfo]) Include directories implied by these arguments that should be allowlisted in Bazel's include checker",
123+
"allowlist_absolute_include_directories": "(depset[str]) Absolute include directories implied by these arguments that should be allowlisted in Bazel's include checker",
122124
},
123125
)
124126

@@ -137,6 +139,7 @@ FeatureInfo = provider(
137139
"overridable": "(bool) Whether the feature is an overridable feature.",
138140
"overrides": "(Optional[FeatureInfo]) The feature that this overrides. Must be a known feature",
139141
"allowlist_include_directories": "(depset[DirectoryInfo]) Include directories implied by this feature that should be allowlisted in Bazel's include checker",
142+
"allowlist_absolute_include_directories": "(depset[str]) Absolute include directories implied by these arguments that should be allowlisted in Bazel's include checker",
140143
},
141144
)
142145
FeatureSetInfo = provider(
@@ -221,5 +224,6 @@ ToolchainConfigInfo = provider(
221224
"make_variables": "Sequence[MakeVariableInfo] Make variable substitutions for this toolchain",
222225
"files": "(dict[ActionTypeInfo, depset[File]]) Files required for the toolchain, keyed by the action type.",
223226
"allowlist_include_directories": "(depset[DirectoryInfo]) Built-in include directories implied by this toolchain's args and tools that should be allowlisted in Bazel's include checker",
227+
"allowlist_absolute_include_directories": "(List[str]) Built-in include directories allowed the sandbox. Use with care",
224228
},
225229
)

cc/toolchains/feature.bzl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ def _cc_feature_impl(ctx):
8383
overridable = False,
8484
overrides = overrides,
8585
allowlist_include_directories = args.allowlist_include_directories,
86+
allowlist_absolute_include_directories = args.allowlist_absolute_include_directories,
8687
)
8788

8889
return [

cc/toolchains/impl/collect.bzl

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,9 @@ def collect_args_lists(targets, label):
146146
allowlist_include_directories = depset(
147147
transitive = [a.allowlist_include_directories for a in args],
148148
),
149+
allowlist_absolute_include_directories = depset(
150+
transitive = [a.allowlist_absolute_include_directories for a in args],
151+
),
149152
by_action = tuple([
150153
struct(
151154
action = k,

cc/toolchains/impl/external_feature.bzl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ def _cc_external_feature_impl(ctx):
4444
overridable = ctx.attr.overridable,
4545
overrides = None,
4646
allowlist_include_directories = depset(),
47+
allowlist_absolute_include_directories = depset(),
4748
)
4849
providers = [
4950
feature,

cc/toolchains/impl/legacy_converter.bzl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,7 @@ def convert_toolchain(toolchain):
202202
d.path
203203
for d in toolchain.allowlist_include_directories.to_list()
204204
]
205+
cxx_builtin_include_directories += toolchain.allowlist_absolute_include_directories.to_list()
205206

206207
artifact_name_patterns = [
207208
legacy_artifact_name_pattern(

cc/toolchains/impl/toolchain_config_info.bzl

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,12 @@ def toolchain_config_info(label, known_features = [], enabled_features = [], arg
205205
for src in features + tools.values()
206206
] + [args.allowlist_include_directories],
207207
)
208+
allowlist_absolute_include_directories = depset(
209+
transitive = [
210+
src.allowlist_absolute_include_directories
211+
for src in features
212+
] + [args.allowlist_absolute_include_directories],
213+
)
208214
toolchain_config = ToolchainConfigInfo(
209215
label = label,
210216
features = features,
@@ -213,6 +219,7 @@ def toolchain_config_info(label, known_features = [], enabled_features = [], arg
213219
args = args,
214220
files = files,
215221
allowlist_include_directories = allowlist_include_directories,
222+
allowlist_absolute_include_directories = allowlist_absolute_include_directories,
216223
artifact_name_patterns = _collect_artifact_name_patterns(artifact_name_patterns, fail),
217224
make_variables = _collect_make_variables(make_variables, fail),
218225
)

docs/toolchain_api.md

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -573,9 +573,10 @@ tool that supports PIC.
573573
<pre>
574574
load("@rules_cc//cc/toolchains/impl:documented_api.bzl", "cc_args")
575575

576-
cc_args(*, <a href="#cc_args-name">name</a>, <a href="#cc_args-actions">actions</a>, <a href="#cc_args-allowlist_include_directories">allowlist_include_directories</a>, <a href="#cc_args-args">args</a>, <a href="#cc_args-data">data</a>, <a href="#cc_args-env">env</a>, <a href="#cc_args-format">format</a>, <a href="#cc_args-iterate_over">iterate_over</a>,
577-
<a href="#cc_args-nested">nested</a>, <a href="#cc_args-requires_not_none">requires_not_none</a>, <a href="#cc_args-requires_none">requires_none</a>, <a href="#cc_args-requires_true">requires_true</a>, <a href="#cc_args-requires_false">requires_false</a>, <a href="#cc_args-requires_equal">requires_equal</a>,
578-
<a href="#cc_args-requires_equal_value">requires_equal_value</a>, <a href="#cc_args-requires_any_of">requires_any_of</a>, <a href="#cc_args-kwargs">**kwargs</a>)
576+
cc_args(*, <a href="#cc_args-name">name</a>, <a href="#cc_args-actions">actions</a>, <a href="#cc_args-allowlist_include_directories">allowlist_include_directories</a>, <a href="#cc_args-allowlist_absolute_include_directories">allowlist_absolute_include_directories</a>,
577+
<a href="#cc_args-args">args</a>, <a href="#cc_args-data">data</a>, <a href="#cc_args-env">env</a>, <a href="#cc_args-format">format</a>, <a href="#cc_args-iterate_over">iterate_over</a>, <a href="#cc_args-nested">nested</a>, <a href="#cc_args-requires_not_none">requires_not_none</a>, <a href="#cc_args-requires_none">requires_none</a>,
578+
<a href="#cc_args-requires_true">requires_true</a>, <a href="#cc_args-requires_false">requires_false</a>, <a href="#cc_args-requires_equal">requires_equal</a>, <a href="#cc_args-requires_equal_value">requires_equal_value</a>, <a href="#cc_args-requires_any_of">requires_any_of</a>,
579+
<a href="#cc_args-kwargs">**kwargs</a>)
579580
</pre>
580581

581582
Action-specific arguments for use with a [`cc_toolchain`](#cc_toolchain).
@@ -659,6 +660,7 @@ For more extensive examples, see the usages here:
659660
| <a id="cc_args-name"></a>name | (str) The name of the target. | none |
660661
| <a id="cc_args-actions"></a>actions | (List[Label]) A list of labels of [`cc_action_type`](#cc_action_type) or [`cc_action_type_set`](#cc_action_type_set) rules that dictate which actions these arguments should be applied to. | `None` |
661662
| <a id="cc_args-allowlist_include_directories"></a>allowlist_include_directories | (List[Label]) A list of include paths that are implied by using this rule. These must point to a skylib [directory](https://github.com/bazelbuild/bazel-skylib/tree/main/docs/directory_doc.md#directory) or [subdirectory](https://github.com/bazelbuild/bazel-skylib/tree/main/docs/directory_subdirectory_doc.md#subdirectory) rule. Some flags (e.g. --sysroot) imply certain include paths are available despite not explicitly specifying a normal include path flag (`-I`, `-isystem`, etc.). Bazel checks that all included headers are properly provided by a dependency or allowlisted through this mechanism.<br><br>As a rule of thumb, only use this if Bazel is complaining about absolute paths in your toolchain and you've ensured that the toolchain is compiling with the `-no-canonical-prefixes` and/or `-fno-canonical-system-headers` arguments.<br><br>This can help work around errors like: `the source file 'main.c' includes the following non-builtin files with absolute paths (if these are builtin files, make sure these paths are in your toolchain)`. | `None` |
663+
| <a id="cc_args-allowlist_absolute_include_directories"></a>allowlist_absolute_include_directories | (List[str]) Allowlists absolute include directories, preventing Bazel from emitting errors when an #include of local system files in the directory occurs. Be careful when adding directories to this list, as it inherently causes leaks in hermeticity. Prefer to reserve use of this for cases like Xcode, where the conventional expectation is to allowlist well-known system-absolute include paths rather than redistributing the SDK. | `None` |
662664
| <a id="cc_args-args"></a>args | (List[str]) The command-line arguments that are applied by using this rule. This is mutually exclusive with [nested](#cc_args-nested). | `None` |
663665
| <a id="cc_args-data"></a>data | (List[Label]) A list of runtime data dependencies that are required for these arguments to work as intended. | `None` |
664666
| <a id="cc_args-env"></a>env | (Dict[str, str]) Environment variables that should be set when the tool is invoked. | `None` |

tests/rule_based_toolchain/subjects.bzl

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ _FEATURE_FLAGS = dict(
8484
external = _subjects.bool,
8585
overrides = None,
8686
allowlist_include_directories = _FakeDirectoryDepset,
87+
allowlist_absolute_include_directories = ProviderDepset(_subjects.str),
8788
)
8889

8990
# Break the dependency loop.
@@ -148,6 +149,7 @@ _ArgsFactory = generate_factory(
148149
nested = optional_subject(_NestedArgsFactory.factory),
149150
requires_any_of = ProviderSequence(_FeatureConstraintFactory),
150151
allowlist_include_directories = _FakeDirectoryDepset,
152+
allowlist_absolute_include_directories = ProviderDepset(_subjects.str),
151153
),
152154
)
153155

@@ -163,6 +165,7 @@ _ArgsListFactory = generate_factory(
163165
))({value.action: value for value in values}, meta = meta),
164166
files = _subjects.depset_file,
165167
allowlist_include_directories = _FakeDirectoryDepset,
168+
allowlist_absolute_include_directories = ProviderDepset(_subjects.str),
166169
),
167170
)
168171

@@ -221,6 +224,7 @@ _ToolchainConfigFactory = generate_factory(
221224
args = ProviderSequence(_ArgsFactory),
222225
files = dict_key_subject(_subjects.depset_file),
223226
allowlist_include_directories = _FakeDirectoryDepset,
227+
allowlist_absolute_include_directories = ProviderDepset(_subjects.str),
224228
artifact_name_patterns = [],
225229
make_variables = [],
226230
),

0 commit comments

Comments
 (0)