Skip to content

Commit

Permalink
Fix inconsistent virtual include paths
Browse files Browse the repository at this point in the history
So far only the generated header in the virtual includes is tested but the Include path pointing to that directory isnt

This is a testcase to reproduce the suspected error reported here:
- bazelbuild#23061

Assuming that the location of the file is asserted:
```Java
    assertThat(artifactsToStrings(ccInfo.getCcCompilationContext().getDirectPublicHdrs()))
        .contains("bin third_party/bar/_virtual_includes/starlark_lib_suffix/starlark_lib.h");
```
Adding a testcase for the include path seems reasonable.

A local run of the testcase shows the error:
```
1) testStripIncludePrefixIncludePath(com.google.devtools.build.lib.rules.cpp.StarlarkCcCommonTest)
value of: getIncludeDirs().onlyElement()
expected: bazel-out/k8-fastbuild/bin/third_party/bar/_virtual_includes/starlark_lib_suffix
but was : bazel-out/k8-fastbuild/bin/third_party/bar/_virtual_includes/starlark_lib
        at com.google.devtools.build.lib.rules.cpp.StarlarkCcCommonTest.testStripIncludePrefixIncludePath(StarlarkCcCommonTest.java:5902)

FAILURES!!!
Tests run: 73,  Failures: 1
```

The change in behaviour was introduced in bazelbuild@7e0df68#diff-403c46ec3075b8e9e6d490ce955db88dae2d457b8046608884039b18b10ab6ccR774

Fixes: bazelbuild#23061

Closes bazelbuild#23071.

PiperOrigin-RevId: 660681269
Change-Id: Ia3f8a8a6cf8bf0e093416a247e348e6de6719584
  • Loading branch information
FaBrand authored and fmeum committed Aug 20, 2024
1 parent 0e5d7ff commit 98af4c0
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -529,26 +529,6 @@ public StarlarkProvider ccTestRunnerInfo() throws EvalException {
return starlarkCcTestRunnerInfo;
}

// This looks ugly, however it is necessary. Good thing is we are planning to get rid of genfiles
// directory altogether so this method has a bright future(of being removed).
@StarlarkMethod(
name = "bin_or_genfiles_relative_to_unique_directory",
documented = false,
parameters = {
@Param(name = "actions", positional = false, named = true),
@Param(name = "unique_directory", positional = false, named = true),
})
public String binOrGenfilesRelativeToUniqueDirectory(
StarlarkActionFactory actions, String uniqueDirectory) {
ActionConstructionContext actionConstructionContext = actions.getActionConstructionContext();
return actionConstructionContext
.getBinOrGenfilesDirectory()
.getExecPath()
.getRelative(
actionConstructionContext.getUniqueDirectory(PathFragment.create(uniqueDirectory)))
.getPathString();
}

@StarlarkMethod(
name = "create_umbrella_header_action",
documented = false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ def _compute_public_headers(
include_prefix,
strip_include_prefix,
label,
binfiles_dir,
non_module_map_headers,
is_sibling_repository_layout):
if include_prefix:
Expand Down Expand Up @@ -106,6 +107,7 @@ def _compute_public_headers(

module_map_headers = []
virtual_to_original_headers_list = []
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)
for original_header in public_headers_artifacts:
repo_relative_path = _repo_relative_path(original_header)
if not repo_relative_path.startswith(strip_prefix):
Expand All @@ -115,7 +117,6 @@ def _compute_public_headers(
include_path = paths.get_relative(include_prefix, include_path)

if not original_header.path == include_path:
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)
virtual_header = actions.declare_shareable_artifact(paths.join(virtual_include_dir, include_path))
actions.symlink(
output = virtual_header,
Expand All @@ -130,11 +131,10 @@ def _compute_public_headers(
module_map_headers.append(original_header)

virtual_headers = module_map_headers + non_module_map_headers

return struct(
headers = virtual_headers,
module_map_headers = module_map_headers,
virtual_include_path = cc_internal.bin_or_genfiles_relative_to_unique_directory(actions = actions, unique_directory = _VIRTUAL_INCLUDES_DIR),
virtual_include_path = paths.join(binfiles_dir, virtual_include_dir),
virtual_to_original_headers = depset(virtual_to_original_headers_list),
)

Expand Down Expand Up @@ -249,6 +249,7 @@ def _init_cc_compilation_context(
include_prefix,
strip_include_prefix,
label,
binfiles_dir,
non_module_map_headers,
sibling_repo_layout,
)
Expand Down Expand Up @@ -285,6 +286,7 @@ def _init_cc_compilation_context(
include_prefix,
strip_include_prefix,
label,
binfiles_dir,
non_module_map_headers,
sibling_repo_layout,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.collect.nestedset.Depset;
import com.google.devtools.build.lib.packages.Provider;
import com.google.devtools.build.lib.packages.StarlarkInfo;
Expand Down Expand Up @@ -5573,6 +5574,33 @@ public void testStripIncludePrefix() throws Exception {
.contains("bin third_party/bar/_virtual_includes/starlark_lib_suffix/starlark_lib.h");
}

@Test
public void testStripIncludePrefixIncludePath() throws Exception {
createFilesForTestingCompilation(
scratch, "third_party/tools/build_defs/foo", "strip_include_prefix='v1'");
scratch.file(
"third_party/bar/BUILD",
"""
load("//third_party/tools/build_defs/foo:extension.bzl", "cc_starlark_library")
cc_starlark_library(
name = "starlark_lib",
srcs = ["starlark_lib.cc"],
private_hdrs = ["v1/private_starlark_lib.h"],
public_hdrs = ["v1/starlark_lib.h"],
)
""");
ConfiguredTarget target = getConfiguredTarget("//third_party/bar:starlark_lib");
assertThat(target).isNotNull();
CcInfo ccInfo = target.get(CcInfo.PROVIDER);

assertThat(ccInfo.getCcCompilationContext().getIncludeDirs())
.containsExactly(
getTargetConfiguration()
.getBinFragment(RepositoryName.MAIN)
.getRelative("third_party/bar/_virtual_includes/starlark_lib_suffix"));
}

@Test
public void testStripIncludePrefixAndIncludePrefix() throws Exception {
createFilesForTestingCompilation(
Expand All @@ -5596,6 +5624,31 @@ public void testStripIncludePrefixAndIncludePrefix() throws Exception {
"bin third_party/bar/_virtual_includes/starlark_lib_suffix/prefix/starlark_lib.h");
}

@Test
public void testStripIncludePrefixAndIncludePrefixIncludePath() throws Exception {
createFilesForTestingCompilation(
scratch,
"third_party/tools/build_defs/foo",
"strip_include_prefix='v1', include_prefix='prefix'");
scratch.file(
"third_party/bar/BUILD",
"load('//third_party/tools/build_defs/foo:extension.bzl', 'cc_starlark_library')",
"cc_starlark_library(",
" name = 'starlark_lib',",
" srcs = ['starlark_lib.cc'],",
" public_hdrs = ['v1/starlark_lib.h'],",
" private_hdrs = ['v1/private_starlark_lib.h'],",
")");
ConfiguredTarget target = getConfiguredTarget("//third_party/bar:starlark_lib");
assertThat(target).isNotNull();
CcInfo ccInfo = target.get(CcInfo.PROVIDER);
assertThat(ccInfo.getCcCompilationContext().getIncludeDirs())
.containsExactly(
getTargetConfiguration()
.getBinFragment(RepositoryName.MAIN)
.getRelative("third_party/bar/_virtual_includes/starlark_lib_suffix"));
}

@Test
public void testHeaders() throws Exception {
createFilesForTestingCompilation(
Expand Down

0 comments on commit 98af4c0

Please sign in to comment.