Optimize _virtual_includes when paths are only stripped

In this case it's not neccessary to create _virtual_includes, just setting the correct include path is enough.

This should alleviate Windows problems with too long paths.

Related: bazelbuild#18683
PiperOrigin-RevId: 626360114
Change-Id: I4e7d8ee39e58dd27601f18c4a624c1d206785abe

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

@@ -96,14 +96,45 @@ def _compute_public_headers(
     if include_prefix and include_prefix.startswith("/"):
         include_prefix = include_prefix[1:]
 
-    if not strip_prefix and not include_prefix:
+    if (not strip_prefix and not include_prefix) or not public_headers_artifacts:
         return struct(
             headers = public_headers_artifacts + non_module_map_headers,
             module_map_headers = public_headers_artifacts,
             virtual_include_path = None,
             virtual_to_original_headers = depset(),
         )
 
+    # When only stripping, we don't need to use _virtual_include path
+    if strip_prefix and not include_prefix:
+        module_map_headers = []
+        virtual_to_original_headers_list = []
+        include_paths = {}
+        for original_header in public_headers_artifacts:
+            repo_relative_path = _repo_relative_path(original_header)
+            if not repo_relative_path.startswith(strip_prefix):
+                fail("header '{}' is not under the specified strip prefix '{}'".format(repo_relative_path, strip_prefix))
+            include_path = original_header.root.path
+            workspace_root = original_header.owner.workspace_root
+            if not include_path:  # this is a source/non-generated file
+                include_path = workspace_root
+            elif workspace_root.startswith("external/"):
+                # in non-sibling repo layout, we need to add workspace root as well
+                include_path = include_path + "/" + workspace_root
+            include_path = paths.get_relative(include_path, strip_prefix)
+            include_paths[include_path] = True
+            module_map_headers.append(original_header)
+
+        virtual_headers = module_map_headers + non_module_map_headers
+
+        # We can only handle 1 include path. In case there are more, fallback to _virtual_imports.
+        if len(include_paths.keys()) == 1:
+            return struct(
+                headers = virtual_headers,
+                module_map_headers = module_map_headers,
+                virtual_include_path = include_paths.keys()[0],
+                virtual_to_original_headers = depset(virtual_to_original_headers_list),
+            )
+
     module_map_headers = []
     virtual_to_original_headers_list = []
     for original_header in public_headers_artifacts:

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

@@ -927,10 +927,9 @@ public void testAbsoluteAndRelativeStripPrefix() throws Exception {
             .getCcCompilationContext();
 
     assertThat(ActionsTestUtil.prettyArtifactNames(relative.getDeclaredIncludeSrcs()))
-        .containsExactly("third_party/a/_virtual_includes/relative/b.h", "third_party/a/v1/b.h");
+        .containsExactly("third_party/a/v1/b.h");
     assertThat(ActionsTestUtil.prettyArtifactNames(absolute.getDeclaredIncludeSrcs()))
-        .containsExactly(
-            "third_party/a/_virtual_includes/absolute/a/v1/b.h", "third_party/a/v1/b.h");
+        .containsExactly("third_party/a/v1/b.h");
   }
 
   @Test

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

@@ -5569,7 +5569,9 @@ public void testStripIncludePrefix() throws Exception {
     assertThat(target).isNotNull();
     CcInfo ccInfo = target.get(CcInfo.PROVIDER);
     assertThat(artifactsToStrings(ccInfo.getCcCompilationContext().getDirectPublicHdrs()))
-        .contains("bin third_party/bar/_virtual_includes/starlark_lib_suffix/starlark_lib.h");
+        .contains("src third_party/bar/v1/starlark_lib.h");
+    assertThat(ccInfo.getCcCompilationContext().getIncludeDirs())
+        .contains(PathFragment.create("third_party/bar/v1"));
   }
 
   @Test

src/test/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryTest.java

@@ -1321,8 +1321,7 @@ public void testIncludesDirsOfTransitiveCcIncDepsGetPassedToCompileAction() thro
     CommandAction compileAction = compileAction("//package:objc_lib", "b.o");
     // We remove spaces, since the crosstool rules do not use spaces for include paths.
     String compileActionArgs = Joiner.on("").join(compileAction.getArguments()).replace(" ", "");
-    assertThat(compileActionArgs)
-        .matches(".*-iquote.*/third_party/cc_lib/_virtual_includes/cc_lib.*");
+    assertThat(compileActionArgs).matches(".*-iquote.*third_party/cc_lib/.*");
   }
 
   @Test

src/test/shell/bazel/cc_integration_test.sh

@@ -103,6 +103,119 @@ EOF
     || fail "Should have found include at repository-relative path"
 }
 
+function test_cc_library_strip_only() {
+  r="$TEST_TMPDIR/r"
+  mkdir -p foo/v1
+  mkdir -p foo/v2
+  mkdir -p "$TEST_TMPDIR/r/foo/v1"
+  mkdir -p "$TEST_TMPDIR/r/foo/v2"
+  create_workspace_with_default_repos "$TEST_TMPDIR/r/WORKSPACE"
+  echo "#define FOO 42" | tee foo/v1/foo.h > "$TEST_TMPDIR/r/foo/v1/foo.h"
+  echo "#define BAR 42" | tee foo/v2/bar.h > "$TEST_TMPDIR/r/foo/v2/bar.h"
+  cat | tee foo/BUILD > "$TEST_TMPDIR/r/foo/BUILD" <<EOF
+cc_library(
+  name = "foo",
+  hdrs = ["v1/foo.h"],
+  strip_include_prefix = "v1",
+  visibility = ["//visibility:public"],
+)
+
+genrule(
+  name = "foo_h",
+  cmd = 'echo "#define FOO 42" > "\$@"',
+  outs = ["v2/foo_gen.h"],
+)
+
+cc_library(
+  name = "foo_generated",
+  hdrs = [":foo_h"],
+  strip_include_prefix = "v2",
+  visibility = ["//visibility:public"],
+)
+
+cc_library(
+  name = "foo_mixed",
+  hdrs = [":foo_h", "v2/bar.h"],
+  strip_include_prefix = "v2",
+  visibility = ["//visibility:public"],
+)
+EOF
+  cat >> $(create_workspace_with_default_repos WORKSPACE) <<EOF
+local_repository(
+  name = "foo",
+  path = "$TEST_TMPDIR/r",
+)
+EOF
+
+  cat > BUILD <<EOF
+cc_binary(
+  name = "repo",
+  srcs = ["ok.cc"],
+  deps = ["@foo//foo"],
+)
+cc_binary(
+  name = "repo_gen",
+  srcs = ["gen.cc"],
+  deps = ["@foo//foo:foo_generated"],
+)
+cc_binary(
+  name = "repo_mixed",
+  srcs = ["mixed.cc"],
+  deps = ["@foo//foo:foo_mixed"],
+)
+cc_binary(
+  name = "no_repo",
+  srcs = ["ok.cc"],
+  deps = ["//foo"],
+)
+cc_binary(
+  name = "no_repo_gen",
+  srcs = ["gen.cc"],
+  deps = ["//foo:foo_generated"],
+)
+cc_binary(
+  name = "no_repo_mixed",
+  srcs = ["mixed.cc"],
+  deps = ["//foo:foo_mixed"],
+)
+EOF
+
+  cat > ok.cc <<EOF
+#include <stdio.h>
+#include "foo.h"
+int main() {
+  printf("FOO is %d\n", FOO);
+}
+EOF
+
+  cat > gen.cc <<EOF
+#include <stdio.h>
+#include "foo_gen.h"
+int main() {
+  printf("FOO is %d\n", FOO);
+}
+EOF
+
+  cat > mixed.cc <<EOF
+#include <stdio.h>
+#include "foo_gen.h"
+#include "bar.h"
+int main() {
+  printf("FOO is %d\n", FOO);
+  printf("BAR is %d\n", BAR);
+}
+EOF
+  bazel build --noexperimental_sibling_repository_layout :repo || fail "Should have found include at synthetic path"
+  bazel build --noexperimental_sibling_repository_layout :repo_gen || fail "Should have found include at synthetic path"
+  bazel build --noexperimental_sibling_repository_layout :repo_mixed || fail "Should have found include at synthetic path"
+  bazel build --experimental_sibling_repository_layout :repo || fail "Should have found include at synthetic path"
+  bazel build --experimental_sibling_repository_layout :repo_gen || fail "Should have found include at synthetic path"
+  bazel build --experimental_sibling_repository_layout :repo_mixed || fail "Should have found include at synthetic path"
+  bazel build :no_repo || fail "Should have found include at synthetic path"
+  bazel build :no_repo_gen || fail "Should have found include at synthetic path"
+  bazel build :no_repo_mixed || fail "Should have found include at synthetic path"
+}
+
 
 function test_include_validation_sandbox_disabled() {
   local workspace="${FUNCNAME[0]}"