diff options
author | 2016-04-19 13:13:29 +0000 | |
---|---|---|
committer | 2016-04-19 13:52:13 +0000 | |
commit | 68e5335b1ec05cffcb08cef552d9d0a2c5d41397 (patch) | |
tree | f81804c1a4f4047338f91c3f7975e428d1375d93 | |
parent | 10993fe27a62d5a4e683a206291c1bd44a492daf (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.java | 7 | ||||
-rw-r--r-- | src/test/java/com/google/devtools/build/lib/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; @@ -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", |