From a97e17f4a5b6662b96ab4e14b804ac75d6f76ad4 Mon Sep 17 00:00:00 2001 From: John Field Date: Fri, 13 Nov 2015 02:19:52 +0000 Subject: Use Labels, rather than PathFragments, to represent Skylark loads internally. The load location for a Skylark Aspect is specified via a PathFragment, for consistency with current non-Aspect Skylark loads. This should be a semantics-preserving change for users. In a subsequent CL, I'll change the Skylark syntax to allow load statements to use labels as well as paths, with the goal of eventually deprecating the latter. Also: - Removed the hack for handling relative loads in the prelude file. - Refactored some redundant functionality in PackageFunction and SkylarkImportLookupFunction for handling loads. - Removed the ability to put the BUILD file for the package containing a Skylark file under a different package root than the Skylark file itself. This functionality isn't currently used and is inconsistent with Blaze's handling of the package path elsewhere. - Added BUILD files to a number of tests that load Skylark files; this is consistent with the requirement that all Skylark files need to be part of some package. - Changed the constants used to set the location of the prelude file from paths to labels. -- MOS_MIGRATED_REVID=107741568 --- .../skyframe/SkylarkImportLookupFunctionTest.java | 75 +++++++++++++--------- 1 file changed, 44 insertions(+), 31 deletions(-) (limited to 'src/test/java/com/google/devtools/build/lib/skyframe') diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunctionTest.java index fef00d867c..6b60f076c9 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunctionTest.java @@ -15,12 +15,11 @@ package com.google.devtools.build.lib.skyframe; import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; -import com.google.devtools.build.lib.cmdline.PackageIdentifier; +import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.packages.ConstantRuleVisibility; import com.google.devtools.build.lib.pkgcache.PathPackageLocator; import com.google.devtools.build.lib.skyframe.util.SkyframeExecutorTestUtils; import com.google.devtools.build.lib.vfs.Path; -import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.skyframe.ErrorInfo; import com.google.devtools.build.skyframe.EvaluationResult; import com.google.devtools.build.skyframe.SkyKey; @@ -49,47 +48,35 @@ public class SkylarkImportLookupFunctionTest extends BuildViewTestCase { public void testSkylarkImportLabels() throws Exception { scratch.file("pkg1/BUILD"); scratch.file("pkg1/ext.bzl"); - checkLabel("pkg1/ext.bzl", "//pkg1:ext.bzl"); + checkLabel("//pkg1:ext.bzl", "//pkg1:ext.bzl"); scratch.file("pkg2/BUILD"); scratch.file("pkg2/dir/ext.bzl"); - checkLabel("pkg2/dir/ext.bzl", "//pkg2:dir/ext.bzl"); + checkLabel("//pkg2:dir/ext.bzl", "//pkg2:dir/ext.bzl"); scratch.file("dir/pkg3/BUILD"); scratch.file("dir/pkg3/dir/ext.bzl"); - checkLabel("dir/pkg3/dir/ext.bzl", "//dir/pkg3:dir/ext.bzl"); + checkLabel("//dir/pkg3:dir/ext.bzl", "//dir/pkg3:dir/ext.bzl"); } public void testSkylarkImportLabelsAlternativeRoot() throws Exception { scratch.file("/root_2/pkg4/BUILD"); scratch.file("/root_2/pkg4/ext.bzl"); - checkLabel("pkg4/ext.bzl", "//pkg4:ext.bzl"); - } - - public void testSkylarkImportLabelsMultipleRoot_1() throws Exception { - scratch.file("pkg5/BUILD"); - scratch.file("/root_2/pkg5/ext.bzl"); - checkLabel("pkg5/ext.bzl", "//pkg5:ext.bzl"); - } - - public void testSkylarkImportLabelsMultipleRoot_2() throws Exception { - scratch.file("/root_2/pkg6/BUILD"); - scratch.file("pkg6/ext.bzl"); - checkLabel("pkg6/ext.bzl", "//pkg6:ext.bzl"); + checkLabel("//pkg4:ext.bzl", "//pkg4:ext.bzl"); } public void testSkylarkImportLabelsMultipleBuildFiles() throws Exception { scratch.file("dir1/BUILD"); scratch.file("dir1/dir2/BUILD"); scratch.file("dir1/dir2/ext.bzl"); - checkLabel("dir1/dir2/ext.bzl", "//dir1/dir2:ext.bzl"); + checkLabel("//dir1/dir2:ext.bzl", "//dir1/dir2:ext.bzl"); } public void testLoadRelativePath() throws Exception { scratch.file("pkg/BUILD"); scratch.file("pkg/ext1.bzl", "a = 1"); scratch.file("pkg/ext2.bzl", "load('ext1', 'a')"); - get(key("pkg/ext2.bzl")); + get(key("//pkg:ext2.bzl")); } public void testLoadAbsolutePath() throws Exception { @@ -97,7 +84,7 @@ public class SkylarkImportLookupFunctionTest extends BuildViewTestCase { scratch.file("pkg3/BUILD"); scratch.file("pkg2/ext.bzl", "b = 1"); scratch.file("pkg3/ext.bzl", "load('/pkg2/ext', 'b')"); - get(key("pkg3/ext.bzl")); + get(key("//pkg3:ext.bzl")); } private EvaluationResult get(SkyKey skylarkImportLookupKey) @@ -111,9 +98,8 @@ public class SkylarkImportLookupFunctionTest extends BuildViewTestCase { return result; } - private SkyKey key(String file) throws Exception { - return SkylarkImportLookupValue.key( - PackageIdentifier.createInDefaultRepo(new PathFragment(file))); + private SkyKey key(String label) throws Exception { + return SkylarkImportLookupValue.key(Label.parseAbsoluteUnchecked(label)); } private void checkLabel(String file, String label) throws Exception { @@ -125,18 +111,45 @@ public class SkylarkImportLookupFunctionTest extends BuildViewTestCase { public void testSkylarkImportLookupNoBuildFile() throws Exception { scratch.file("pkg/ext.bzl", ""); SkyKey skylarkImportLookupKey = - SkylarkImportLookupValue.key( - PackageIdentifier.createInDefaultRepo(new PathFragment("pkg/ext.bzl"))); + SkylarkImportLookupValue.key(Label.parseAbsoluteUnchecked("//pkg:ext.bzl")); EvaluationResult result = SkyframeExecutorTestUtils.evaluate( getSkyframeExecutor(), skylarkImportLookupKey, /*keepGoing=*/ false, reporter); assertTrue(result.hasError()); ErrorInfo errorInfo = result.getError(skylarkImportLookupKey); String errorMessage = errorInfo.getException().getMessage(); - assertEquals( - "Every .bzl file must have a corresponding package, but 'pkg/ext.bzl' " - + "does not have one. Please create a BUILD file in the same or any parent directory. " - + "Note that this BUILD file does not need to do anything except exist.", - errorMessage); + assertEquals("Extension file not found. Unable to load package for '//pkg:ext.bzl': " + + "BUILD file not found on package path", errorMessage); } + + public void testSkylarkAbsoluteImportFilenameWithControlChars() throws Exception { + scratch.file("pkg/BUILD", ""); + scratch.file("pkg/ext.bzl", "load('/pkg/oops\u0000', 'a')"); + SkyKey skylarkImportLookupKey = + SkylarkImportLookupValue.key(Label.parseAbsoluteUnchecked("//pkg:ext.bzl")); + EvaluationResult result = + SkyframeExecutorTestUtils.evaluate( + getSkyframeExecutor(), skylarkImportLookupKey, /*keepGoing=*/ false, reporter); + assertTrue(result.hasError()); + ErrorInfo errorInfo = result.getError(skylarkImportLookupKey); + String errorMessage = errorInfo.getException().getMessage(); + assertEquals("invalid target name 'oops.bzl': " + + "target names may not contain non-printable characters: '\\x00'", errorMessage); + } + + public void testSkylarkRelativeImportFilenameWithControlChars() throws Exception { + scratch.file("pkg/BUILD", ""); + scratch.file("pkg/ext.bzl", "load('oops\u0000', 'a')"); + SkyKey skylarkImportLookupKey = + SkylarkImportLookupValue.key(Label.parseAbsoluteUnchecked("//pkg:ext.bzl")); + EvaluationResult result = + SkyframeExecutorTestUtils.evaluate( + getSkyframeExecutor(), skylarkImportLookupKey, /*keepGoing=*/ false, reporter); + assertTrue(result.hasError()); + ErrorInfo errorInfo = result.getError(skylarkImportLookupKey); + String errorMessage = errorInfo.getException().getMessage(); + assertEquals("invalid target name 'oops.bzl': " + + "target names may not contain non-printable characters: '\\x00'", errorMessage); + } + } -- cgit v1.2.3