Skip to content

Commit 26216ac

Browse files
[8.4.0] Introduce shorten_virtual_includes cc toolchain feature (#26532)
Fixes #18683 Related: protocolbuffers/protobuf#20085 This change is a rework of #26005 to enable short virtual includes based on a cc feature, therefore we can limit the change to only MSVC compiler on Windows. RELNOTES: If a cc toolchain feature named `shorten_virtual_includes` is enabled, virtual include header files are linked under `bin/_virtual_includes/<hash of target path>` instead of `bin/<target package path>/_virtual_includes/<target name>`. This shortens the virtual include paths which is critical for mitigating long path issue with MSVC on Windows. Closes #26528. PiperOrigin-RevId: 781975309 Change-Id: Ia573a5f25707ad2462aa3a4459fc66db1779df36 --------- Co-authored-by: Googler <[email protected]>
1 parent 609cbad commit 26216ac

File tree

7 files changed

+113
-25
lines changed

7 files changed

+113
-25
lines changed

site/en/docs/cc-toolchain-config-reference.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1093,6 +1093,13 @@ conditions.
10931093
features below.
10941094
</td>
10951095
</tr>
1096+
<tr>
1097+
<td><strong><code>shorten_virtual_includes</code></strong>
1098+
</td>
1099+
<td>
1100+
If enabled, virtual include header files are linked under <code>bin/_virtual_includes/&lt;hash of target path&gt;</code> instead of <code>bin/&lt;target package path&gt;/_virtual_includes/&lt;target name&gt;</code>. Useful on Windows to avoid long path issue with MSVC.
1101+
</td>
1102+
</tr>
10961103
</table>
10971104

10981105
#### Legacy features patching logic {:#legacy-features-patching-logic}

src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -387,6 +387,12 @@ public static ToolchainTypeRequirement ccToolchainTypeRequirement(RuleDefinition
387387
/** A string constant for a feature that, if enabled, disables .d file handling. */
388388
public static final String NO_DOTD_FILE = "no_dotd_file";
389389

390+
/**
391+
* A string constant for a feature that, if enabled, shortens the virtual include paths via
392+
* hashing.
393+
*/
394+
public static final String SHORTEN_VIRTUAL_INCLUDES = "shorten_virtual_includes";
395+
390396
/*
391397
* A string constant for the fdo_instrument feature.
392398
*/

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

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,11 @@
1515
"""Compilation helper for C++ rules."""
1616

1717
load(":common/cc/cc_common.bzl", "cc_common")
18-
load(":common/cc/cc_helper.bzl", "cc_helper")
18+
load(
19+
":common/cc/cc_helper_internal.bzl",
20+
"package_source_root",
21+
"repository_exec_path",
22+
)
1923
load(":common/cc/semantics.bzl", "USE_EXEC_ROOT_FOR_VIRTUAL_INCLUDES_SYMLINKS")
2024
load(":common/paths.bzl", "paths")
2125

@@ -55,7 +59,8 @@ def _compute_public_headers(
5559
label,
5660
binfiles_dir,
5761
non_module_map_headers,
58-
is_sibling_repository_layout):
62+
is_sibling_repository_layout,
63+
shorten_virtual_includes):
5964
if include_prefix:
6065
if not paths.is_normalized(include_prefix, False):
6166
fail("include prefix should not contain uplevel references: " + include_prefix)
@@ -107,7 +112,11 @@ def _compute_public_headers(
107112

108113
module_map_headers = []
109114
virtual_to_original_headers_list = []
110-
virtual_include_dir = paths.join(paths.join(cc_helper.package_source_root(label.workspace_name, label.package, is_sibling_repository_layout), _VIRTUAL_INCLUDES_DIR), label.name)
115+
source_package_path = package_source_root(label.workspace_name, label.package, is_sibling_repository_layout)
116+
if shorten_virtual_includes:
117+
virtual_include_dir = paths.join(_VIRTUAL_INCLUDES_DIR, "%x" % hash(paths.join(source_package_path, label.name)))
118+
else:
119+
virtual_include_dir = paths.join(source_package_path, _VIRTUAL_INCLUDES_DIR, label.name)
111120
for original_header in public_headers_artifacts:
112121
repo_relative_path = _repo_relative_path(original_header)
113122
if not repo_relative_path.startswith(strip_prefix):
@@ -144,7 +153,7 @@ def _generates_header_module(feature_configuration, public_headers, private_head
144153
generate_action
145154

146155
def _header_module_artifact(actions, label, is_sibling_repository_layout, suffix, extension):
147-
object_dir = paths.join(paths.join(cc_helper.package_source_root(label.workspace_name, label.package, is_sibling_repository_layout), "_objs"), label.name)
156+
object_dir = paths.join(paths.join(package_source_root(label.workspace_name, label.package, is_sibling_repository_layout), "_objs"), label.name)
148157
base_name = label.name.split("/")[-1]
149158
output_path = paths.join(object_dir, base_name + suffix + extension)
150159
return actions.declare_shareable_artifact(output_path)
@@ -219,11 +228,12 @@ def _init_cc_compilation_context(
219228
# we might pick up stale generated files.
220229
sibling_repo_layout = config.is_sibling_repository_layout()
221230
repo_name = label.workspace_name
222-
repo_path = cc_helper.repository_exec_path(repo_name, sibling_repo_layout)
231+
repo_path = repository_exec_path(repo_name, sibling_repo_layout)
223232
gen_include_dir = _include_dir(genfiles_dir, repo_path, sibling_repo_layout)
224233
bin_include_dir = _include_dir(binfiles_dir, repo_path, sibling_repo_layout)
225234
quote_include_dirs_for_context = [repo_path, gen_include_dir, bin_include_dir] + quote_include_dirs
226235
external = repo_name != "" and _enabled(feature_configuration, "external_include_paths")
236+
shorten_virtual_includes = _enabled(feature_configuration, "shorten_virtual_includes")
227237
external_include_dirs = []
228238
declared_include_srcs = []
229239

@@ -252,6 +262,7 @@ def _init_cc_compilation_context(
252262
binfiles_dir,
253263
non_module_map_headers,
254264
sibling_repo_layout,
265+
shorten_virtual_includes,
255266
)
256267
if public_headers.virtual_include_path:
257268
if external:
@@ -289,6 +300,7 @@ def _init_cc_compilation_context(
289300
binfiles_dir,
290301
non_module_map_headers,
291302
sibling_repo_layout,
303+
shorten_virtual_includes,
292304
)
293305

294306
separate_module = None

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

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ load(
2020
"is_versioned_shared_library_extension_valid",
2121
"should_create_per_object_debug_info",
2222
_artifact_category = "artifact_category",
23+
_package_source_root = "package_source_root",
24+
_repository_exec_path = "repository_exec_path",
2325
)
2426
load(":common/cc/cc_info.bzl", "CcInfo")
2527
load(":common/objc/objc_common.bzl", "objc_common")
@@ -1039,29 +1041,9 @@ def _report_invalid_options(cc_toolchain, cpp_config):
10391041
if cpp_config.grte_top() != None and cc_toolchain.sysroot == None:
10401042
fail("The selected toolchain does not support setting --grte_top (it doesn't specify builtin_sysroot).")
10411043

1042-
def _is_repository_main(repository):
1043-
return repository == ""
1044-
1045-
def _repository_exec_path(repository, sibling_repository_layout):
1046-
if _is_repository_main(repository):
1047-
return ""
1048-
prefix = "external"
1049-
if sibling_repository_layout:
1050-
prefix = ".."
1051-
if repository.startswith("@"):
1052-
repository = repository[1:]
1053-
return paths.get_relative(prefix, repository)
1054-
10551044
def _package_exec_path(ctx, package, sibling_repository_layout):
10561045
return paths.get_relative(_repository_exec_path(ctx.label.workspace_name, sibling_repository_layout), package)
10571046

1058-
def _package_source_root(repository, package, sibling_repository_layout):
1059-
if _is_repository_main(repository) or sibling_repository_layout:
1060-
return package
1061-
if repository.startswith("@"):
1062-
repository = repository[1:]
1063-
return paths.get_relative(paths.get_relative("external", repository), package)
1064-
10651047
def _system_include_dirs(ctx, additional_make_variable_substitutions):
10661048
result = []
10671049
sibling_repository_layout = ctx.configuration.is_sibling_repository_layout()

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

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ Utility functions for C++ rules that don't depend on cc_common.
1818
Only use those within C++ implementation. The others need to go through cc_common.
1919
"""
2020

21+
load(":common/paths.bzl", "paths")
22+
2123
cc_common_internal = _builtins.internal.cc_common
2224

2325
CREATE_COMPILE_ACTION_API_ALLOWLISTED_PACKAGES = [("", "devtools/rust/cc_interop"), ("", "third_party/crubit")]
@@ -126,6 +128,47 @@ def is_versioned_shared_library(file):
126128
return False
127129
return is_versioned_shared_library_extension_valid(file.basename)
128130

131+
def _is_repository_main(repository):
132+
return repository == ""
133+
134+
def package_source_root(repository, package, sibling_repository_layout):
135+
"""
136+
Determines the source root for a given repository and package.
137+
138+
Args:
139+
repository: The repository to get the source root for.
140+
package: The package to get the source root for.
141+
sibling_repository_layout: Whether the repository layout is a sibling repository layout.
142+
143+
Returns:
144+
The source root for the given repository and package.
145+
"""
146+
if _is_repository_main(repository) or sibling_repository_layout:
147+
return package
148+
if repository.startswith("@"):
149+
repository = repository[1:]
150+
return paths.get_relative(paths.get_relative("external", repository), package)
151+
152+
def repository_exec_path(repository, sibling_repository_layout):
153+
"""
154+
Determines the exec path for a given repository.
155+
156+
Args:
157+
repository: The repository to get the exec path for.
158+
sibling_repository_layout: Whether the repository layout is a sibling repository layout.
159+
160+
Returns:
161+
The exec path for the given repository.
162+
"""
163+
if _is_repository_main(repository):
164+
return ""
165+
prefix = "external"
166+
if sibling_repository_layout:
167+
prefix = ".."
168+
if repository.startswith("@"):
169+
repository = repository[1:]
170+
return paths.get_relative(prefix, repository)
171+
129172
def use_pic_for_binaries(cpp_config, feature_configuration):
130173
"""
131174
Returns whether binaries must be compiled with position independent code.

src/test/java/com/google/devtools/build/lib/analysis/mock/cc_toolchain_config.bzl

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@ _FEATURE_NAMES = struct(
122122
cpp_compile_with_requirements = "cpp_compile_with_requirements",
123123
no_copts_tokenization = "no_copts_tokenization",
124124
generate_linkmap = "generate_linkmap",
125+
shorten_virtual_includes = "shorten_virtual_includes",
125126
)
126127

127128
_cpp_modules_feature = feature(
@@ -142,6 +143,11 @@ _supports_dynamic_linker_feature = feature(
142143
enabled = True,
143144
)
144145

146+
_shorten_virtual_includes_feature = feature(
147+
name = _FEATURE_NAMES.shorten_virtual_includes,
148+
enabled = True,
149+
)
150+
145151
_supports_interface_shared_libraries_feature = feature(
146152
name = _FEATURE_NAMES.supports_interface_shared_libraries,
147153
enabled = True,
@@ -1451,6 +1457,7 @@ _feature_name_to_feature = {
14511457
_FEATURE_NAMES.cpp_compile_with_requirements: _cpp_compile_with_requirements,
14521458
_FEATURE_NAMES.generate_pdb_file: _generate_pdb_file_feature,
14531459
_FEATURE_NAMES.generate_linkmap: _generate_linkmap_feature,
1460+
_FEATURE_NAMES.shorten_virtual_includes: _shorten_virtual_includes_feature,
14541461
"header_modules_feature_configuration": _header_modules_feature_configuration,
14551462
"env_var_feature_configuration": _env_var_feature_configuration,
14561463
"host_and_nonhost_configuration": _host_and_nonhost_configuration,

src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1079,6 +1079,37 @@ public void testIncludeManglingSmoke() throws Exception {
10791079
.getRelative("third_party/a/_virtual_includes/a"));
10801080
}
10811081

1082+
@Test
1083+
public void testIncludeManglingSmokeWithShortendVirtualIncludes() throws Exception {
1084+
getAnalysisMock()
1085+
.ccSupport()
1086+
.setupCcToolchainConfig(
1087+
mockToolsConfig,
1088+
CcToolchainConfig.builder().withFeatures(CppRuleClasses.SHORTEN_VIRTUAL_INCLUDES));
1089+
scratch.file(
1090+
"third_party/a/BUILD",
1091+
"""
1092+
licenses(["notice"])
1093+
1094+
cc_library(
1095+
name = "a",
1096+
hdrs = ["v1/b/c.h"],
1097+
include_prefix = "lib",
1098+
strip_include_prefix = "v1",
1099+
)
1100+
""");
1101+
1102+
ConfiguredTarget lib = getConfiguredTarget("//third_party/a");
1103+
CcCompilationContext ccCompilationContext = lib.get(CcInfo.PROVIDER).getCcCompilationContext();
1104+
assertThat(ActionsTestUtil.prettyArtifactNames(ccCompilationContext.getDeclaredIncludeSrcs()))
1105+
.containsExactly("_virtual_includes/207132b2/lib/b/c.h", "third_party/a/v1/b/c.h");
1106+
assertThat(ccCompilationContext.getIncludeDirs())
1107+
.containsExactly(
1108+
getTargetConfiguration()
1109+
.getBinFragment(RepositoryName.MAIN)
1110+
.getRelative("_virtual_includes/207132b2"));
1111+
}
1112+
10821113
@Test
10831114
public void testUpLevelReferencesInIncludeMangling() throws Exception {
10841115
scratch.file(

0 commit comments

Comments
 (0)