aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorGravatar Janak Ramakrishnan <janakr@google.com>2016-04-19 13:13:29 +0000
committerGravatar Damien Martin-Guillerez <dmarting@google.com>2016-04-19 13:52:13 +0000
commit68e5335b1ec05cffcb08cef552d9d0a2c5d41397 (patch)
treef81804c1a4f4047338f91c3f7975e428d1375d93
parent10993fe27a62d5a4e683a206291c1bd44a492daf (diff)
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
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java7
-rw-r--r--src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonConfiguredTargetTest.java47
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;
@@ -534,6 +537,50 @@ public class CcCommonConfiguredTargetTest extends BuildViewTestCase {
}
@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(
"third_party/root",