Skip to content

Commit e3b7e17

Browse files
scentinicopybara-github
authored andcommitted
When generating a symlink in _virtual_includes, add the original header to the 'allowed to use' set too
When a cc_library 'a' makes use of strip_include_prefix, bazel creates a symlink for the hdrs at bazel-out/.../bin/_virtual_includes/a/ Later, bazel passes -I bazel-out/.../bin/_virtual_includes/a/ to the command line to tell the compiler where to look for those headers. For each header in hdrs, bazel will either put the header artifact itself in a set of 'allowed to use' headers, or, in the case of strip_include_prefix usage, bazel will add the symlink to the header under bazel-out/.../bin/_virtual_includes/a/ as 'allowed to use'. When searching for headers, the compiler will do the following (Taken from https://gcc.gnu.org/onlinedocs/gcc/Directory-Options.html): 1. For the quote form of the include directive, the directory of the current file is searched first. 2. For the quote form of the include directive, the directories specified by -iquote options are searched in left-to-right order, as they appear on the command line. 3. Directories specified with -I options are scanned in left-to-right order. ... In the case of the following cc_library: ``` cc_library( name = 'foo', hdrs = ["lib/foo.h"], srcs = ["lib/foo.cc"], strip_include_prefix = 'lib', ) ``` if foo.cc includes foo.h as #include "foo.h" the compiler will find the header in step 1 of the search, thus will use the original header and not the symlink. The compiler will note this in the .d file. Bazel however added the symlink in the list of 'allowed to use' headers, so at some point it will error out with "undeclared inclusion(s) in rule" error due to the discrepancy. This cl tells bazel to add the original header in the 'allowed to use' headers even when it generates a symlink in the _virtual_includes directory. Fixes #3828 #6337 RELNOTES:None. PiperOrigin-RevId: 344240730
1 parent 09b135d commit e3b7e17

File tree

3 files changed

+36
-9
lines changed

3 files changed

+36
-9
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -949,9 +949,9 @@ private PublicHeaders computePublicHeaders() throws InterruptedException {
949949
virtualToOriginalHeaders.add(
950950
Pair.of(virtualHeader.getExecPathString(), originalHeader.getExecPathString()));
951951
}
952-
} else {
953-
moduleHeadersBuilder.add(originalHeader);
954952
}
953+
954+
moduleHeadersBuilder.add(originalHeader);
955955
}
956956

957957
ImmutableList<Artifact> moduleMapHeaders = moduleHeadersBuilder.build();

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -840,7 +840,7 @@ public void testIncludeManglingSmoke() throws Exception {
840840
ConfiguredTarget lib = getConfiguredTarget("//third_party/a");
841841
CcCompilationContext ccCompilationContext = lib.get(CcInfo.PROVIDER).getCcCompilationContext();
842842
assertThat(ActionsTestUtil.prettyArtifactNames(ccCompilationContext.getDeclaredIncludeSrcs()))
843-
.containsExactly("third_party/a/_virtual_includes/a/lib/b/c.h");
843+
.containsExactly("third_party/a/_virtual_includes/a/lib/b/c.h", "third_party/a/v1/b/c.h");
844844
assertThat(ccCompilationContext.getIncludeDirs())
845845
.containsExactly(
846846
getTargetConfiguration()
@@ -887,9 +887,10 @@ public void testAbsoluteAndRelativeStripPrefix() throws Exception {
887887
.getCcCompilationContext();
888888

889889
assertThat(ActionsTestUtil.prettyArtifactNames(relative.getDeclaredIncludeSrcs()))
890-
.containsExactly("third_party/a/_virtual_includes/relative/b.h");
890+
.containsExactly("third_party/a/_virtual_includes/relative/b.h", "third_party/a/v1/b.h");
891891
assertThat(ActionsTestUtil.prettyArtifactNames(absolute.getDeclaredIncludeSrcs()))
892-
.containsExactly("third_party/a/_virtual_includes/absolute/a/v1/b.h");
892+
.containsExactly(
893+
"third_party/a/_virtual_includes/absolute/a/v1/b.h", "third_party/a/v1/b.h");
893894
}
894895

895896
@Test

src/test/shell/bazel/cc_integration_test.sh

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,8 @@ cc_binary(
7676
)
7777
7878
cc_binary(
79-
name = "bad",
80-
srcs = ["bad.cc"],
79+
name = "still_ok",
80+
srcs = ["still_ok.cc"],
8181
deps = ["@foo//foo"],
8282
)
8383
EOF
@@ -90,16 +90,42 @@ int main() {
9090
}
9191
EOF
9292

93-
cat > bad.cc <<EOF
93+
cat > still_ok.cc <<EOF
9494
#include <stdio.h>
9595
#include "foo/v1/foo.h"
9696
int main() {
9797
printf("FOO is %d\n", FOO);
9898
}
9999
EOF
100100

101-
bazel build :bad && fail "Should not have found include at repository-relative path"
102101
bazel build :ok || fail "Should have found include at synthetic path"
102+
bazel build :still_ok \
103+
|| fail "Should have found include at repository-relative path"
104+
}
105+
106+
107+
function test_include_validation_sandbox_disabled() {
108+
local workspace="${FUNCNAME[0]}"
109+
mkdir -p "${workspace}"/lib
110+
111+
create_workspace_with_default_repos "${workspace}/WORKSPACE"
112+
cat >> "${workspace}/BUILD" << EOF
113+
cc_library(
114+
name = "foo",
115+
srcs = ["lib/foo.cc"],
116+
hdrs = ["lib/foo.h"],
117+
strip_include_prefix = "lib",
118+
)
119+
EOF
120+
cat >> "${workspace}/lib/foo.cc" << EOF
121+
#include "foo.h"
122+
EOF
123+
124+
touch "${workspace}/lib/foo.h"
125+
126+
cd "${workspace}"
127+
bazel build --spawn_strategy=standalone //:foo &>"$TEST_log" \
128+
|| fail "Build failed but should have succeeded"
103129
}
104130

105131
function test_tree_artifact_headers_are_invalidated() {

0 commit comments

Comments
 (0)