From 68e5335b1ec05cffcb08cef552d9d0a2c5d41397 Mon Sep 17 00:00:00 2001
From: Janak Ramakrishnan <janakr@google.com>
Date: Tue, 19 Apr 2016 13:13:29 +0000
Subject: Allow external repositories to have includes attributes that point to
 packages not in third_party. We don't need to police users' external
 repositories.

Fixes #1151

--
MOS_MIGRATED_REVID=120222222
---
 .../devtools/build/lib/rules/cpp/CcCommon.java     |  7 +++-
 .../rules/cpp/CcCommonConfiguredTargetTest.java    | 47 ++++++++++++++++++++++
 2 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
index f9a935a9eb..6190ad10a5 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
@@ -27,6 +27,7 @@ import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.cmdline.PackageIdentifier;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.packages.BuildType;
@@ -398,7 +399,8 @@ public final class CcCommon {
 
   private List<PathFragment> getIncludeDirsFromIncludesAttribute() {
     List<PathFragment> result = new ArrayList<>();
-    PathFragment packageFragment = ruleContext.getLabel().getPackageIdentifier().getPathFragment();
+    PackageIdentifier packageIdentifier = ruleContext.getLabel().getPackageIdentifier();
+    PathFragment packageFragment = packageIdentifier.getPathFragment();
     for (String includesAttr : ruleContext.attributes().get("includes", Type.STRING_LIST)) {
       includesAttr = ruleContext.expandMakeVariables("includes", includesAttr);
       if (includesAttr.startsWith("/")) {
@@ -430,7 +432,8 @@ public final class CcCommon {
                 + packageFragment
                 + "'. This will be an error in the future");
         // TODO(janakr): Add a link to a page explaining the problem and fixes?
-      } else if (!packageFragment.startsWith(RuleClass.THIRD_PARTY_PREFIX)) {
+      } else if (packageIdentifier.getRepository().isMain()
+          && !includesPath.startsWith(RuleClass.THIRD_PARTY_PREFIX)) {
         ruleContext.attributeWarning(
             "includes",
             "'"
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonConfiguredTargetTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonConfiguredTargetTest.java
index ca261344a0..f7e181a79d 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonConfiguredTargetTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonConfiguredTargetTest.java
@@ -34,9 +34,12 @@ import com.google.devtools.build.lib.analysis.OutputGroupProvider;
 import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
 import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
 import com.google.devtools.build.lib.cmdline.PackageIdentifier;
+import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.testutil.MoreAsserts;
 import com.google.devtools.build.lib.testutil.TestConstants;
 import com.google.devtools.build.lib.util.FileType;
+import com.google.devtools.build.lib.vfs.FileSystemUtils;
+import com.google.devtools.build.lib.vfs.ModifiedFileSet;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig;
 
@@ -533,6 +536,50 @@ public class CcCommonConfiguredTargetTest extends BuildViewTestCase {
         "           includes = ['./'])");
   }
 
+  @Test
+  public void testCcLibraryThirdPartyIncludesNotWarned() throws Exception {
+    eventCollector.clear();
+    ConfiguredTarget target =
+        scratchConfiguredTarget(
+            "third_party/pkg",
+            "lib",
+            "licenses(['unencumbered'])",
+            "cc_library(name = 'lib',",
+            "           srcs = ['foo.cc'],",
+            "           includes = ['./'])");
+    assertThat(view.hasErrors(target)).isFalse();
+    assertNoEvents();
+  }
+
+  @Test
+  public void testCcLibraryExternalIncludesNotWarned() throws Exception {
+    eventCollector.clear();
+    FileSystemUtils.appendIsoLatin1(
+        scratch.resolve("WORKSPACE"),
+        "local_repository(",
+        "    name = 'pkg',",
+        "    path = '/foo')");
+    getSkyframeExecutor()
+        .invalidateFilesUnderPathForTesting(
+            eventCollector,
+            new ModifiedFileSet.Builder().modify(new PathFragment("WORKSPACE")).build(),
+            rootDirectory);
+    FileSystemUtils.createDirectoryAndParents(scratch.resolve("/foo/bar"));
+    scratch.file("/foo/WORKSPACE", "workspace(name = 'pkg')");
+    scratch.file(
+        "/foo/bar/BUILD",
+        "cc_library(name = 'lib',",
+        "           srcs = ['foo.cc'],",
+        "           includes = ['./'])");
+    Target target = getTarget("@pkg//bar:lib");
+    ensureTargetsVisited(target.getLabel());
+    assertThat(
+            view.hasErrors(
+                view.getConfiguredTargetForTesting(reporter, target.getLabel(), targetConfig)))
+        .isFalse();
+    assertNoEvents();
+  }
+
   @Test
   public void testCcLibraryRootIncludesError() throws Exception {
     checkError(
-- 
cgit v1.2.3