bazel: handle proto_src_root when dealing with proto includes, generated files and C++ headers

This change completes the handling of proto_src_root when it comes to inclusion of protos, generating the proto files in the right place and adding the generated headers to the include paths.

WANT_LGTM=elenairina
RELNOTES: None.
PiperOrigin-RevId: 208457740

3 files changed, 35 insertions, 20 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java
index 474ea556fc..22ddbbfddf 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java
@@ -51,7 +51,6 @@ import com.google.devtools.build.lib.rules.cpp.CcToolchainVariables.VariablesExt
 import com.google.devtools.build.lib.rules.cpp.CppConfiguration.HeadersCheckingMode;
 import com.google.devtools.build.lib.skylarkbuildapi.cpp.CompilationInfoApi;
 import com.google.devtools.build.lib.syntax.SkylarkNestedSet;
-import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.util.FileTypeSet;
 import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
@@ -232,6 +231,8 @@ public final class CcCompilationHelper {
   private boolean generateNoPicAction;
   private boolean generatePicAction;
   private boolean allowCoverageInstrumentation = true;
+  private String stripIncludePrefix = null;
+  private String includePrefix = null;
 
   // TODO(plf): Pull out of class.
   private CcCompilationContext ccCompilationContext;
@@ -705,6 +706,18 @@ public final class CcCompilationHelper {
     return this;
   }
 
+  /** Sets the include prefix to append to the public headers. */
+  public CcCompilationHelper setIncludePrefix(@Nullable String includePrefix) {
+    this.includePrefix = includePrefix;
+    return this;
+  }
+
+  /** Sets the include prefix to remove from the public headers. */
+  public CcCompilationHelper setStripIncludePrefix(@Nullable String stripIncludePrefix) {
+    this.stripIncludePrefix = stripIncludePrefix;
+    return this;
+  }
+
   public void setAllowCoverageInstrumentation(boolean allowCoverageInstrumentation) {
     this.allowCoverageInstrumentation = allowCoverageInstrumentation;
   }
@@ -811,33 +824,23 @@ public final class CcCompilationHelper {
   }
 
   private PublicHeaders computePublicHeaders() {
-    if (!ruleContext.attributes().has("strip_include_prefix", Type.STRING)
-        || !ruleContext.attributes().has("include_prefix", Type.STRING)) {
-      return new PublicHeaders(
-          ImmutableList.copyOf(Iterables.concat(publicHeaders, nonModuleMapHeaders)),
-          ImmutableList.copyOf(publicHeaders),
-          null);
-    }
-
     PathFragment prefix = null;
-    if (ruleContext.attributes().isAttributeValueExplicitlySpecified("include_prefix")) {
-      String prefixAttr = ruleContext.attributes().get("include_prefix", Type.STRING);
-      prefix = PathFragment.create(prefixAttr);
-      if (PathFragment.containsUplevelReferences(prefixAttr)) {
-        ruleContext.attributeError("include_prefix", "should not contain uplevel references");
+    if (includePrefix != null) {
+      prefix = PathFragment.create(includePrefix);
+      if (PathFragment.containsUplevelReferences(includePrefix)) {
+        ruleContext.ruleError("include prefix should not contain uplevel references");
       }
       if (prefix.isAbsolute()) {
-        ruleContext.attributeError("include_prefix", "should be a relative path");
+        ruleContext.ruleError("include prefix should be a relative path");
       }
     }
 
     PathFragment stripPrefix;
-    if (ruleContext.attributes().isAttributeValueExplicitlySpecified("strip_include_prefix")) {
-      String stripPrefixAttr = ruleContext.attributes().get("strip_include_prefix", Type.STRING);
-      if (PathFragment.containsUplevelReferences(stripPrefixAttr)) {
-        ruleContext.attributeError("strip_include_prefix", "should not contain uplevel references");
+    if (stripIncludePrefix != null) {
+      if (PathFragment.containsUplevelReferences(stripIncludePrefix)) {
+        ruleContext.ruleError("strip include prefix should not contain uplevel references");
       }
-      stripPrefix = PathFragment.create(stripPrefixAttr);
+      stripPrefix = PathFragment.create(stripIncludePrefix);
       if (stripPrefix.isAbsolute()) {
         stripPrefix =
             ruleContext
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java
index 4266f667f1..319f7d84f0 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java
@@ -175,6 +175,16 @@ public abstract class CcLibrary implements RuleConfiguredTargetFactory {
       compilationHelper.addPublicTextualHeaders(
           ruleContext.getPrerequisiteArtifacts("textual_hdrs", Mode.TARGET).list());
     }
+    if (ruleContext.getRule().isAttrDefined("include_prefix", Type.STRING)
+        && ruleContext.attributes().isAttributeValueExplicitlySpecified("include_prefix")) {
+      compilationHelper.setIncludePrefix(
+          ruleContext.attributes().get("include_prefix", Type.STRING));
+    }
+    if (ruleContext.getRule().isAttrDefined("strip_include_prefix", Type.STRING)
+        && ruleContext.attributes().isAttributeValueExplicitlySpecified("strip_include_prefix")) {
+      compilationHelper.setStripIncludePrefix(
+          ruleContext.attributes().get("strip_include_prefix", Type.STRING));
+    }
 
     if (common.getLinkopts().contains("-static")) {
       ruleContext.attributeWarning("linkopts", "Using '-static' here won't work. "
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java
index f48e1de8f9..83d0c7d288 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java
@@ -321,6 +321,7 @@ public abstract class CcProtoAspect extends NativeAspectClass implements Configu
     }
 
     private void createProtoCompileAction(SupportData supportData, Collection<Artifact> outputs) {
+      String protoRoot = supportData.getProtoSourceRoot();
       String genfilesPath =
           ruleContext
               .getConfiguration()
@@ -331,6 +332,7 @@ public abstract class CcProtoAspect extends NativeAspectClass implements Configu
                       .getPackageIdentifier()
                       .getRepository()
                       .getPathUnderExecRoot())
+              .getRelative(protoRoot == null ? "" : protoRoot)
               .getPathString();
 
       ImmutableList.Builder<ToolchainInvocation> invocations = ImmutableList.builder();