diff options
3 files changed, 94 insertions, 32 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkImport.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkImport.java index 5aa744a2ba..95ddf96ffb 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkImport.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkImport.java @@ -28,11 +28,21 @@ public interface SkylarkImport extends Serializable { * Returns the string originally used to specify the import (may represent a label or a path). */ String getImportString(); - + + /** + * Returns the import in the form of a path fragment for use by tools. Label-based imports are + * converted to paths as follows: Imports using absolute labels or paths yield an absolute path + * (whose root corresponds to the containing depot). Imports using relative labels yield a + * package-relate path, and imports using relative paths yield a directory (of the importing-file) + * relative path. All paths reference file names ending in '.bzl'. If there is an external + * repository prefix, it is ignored. + */ + PathFragment asPathFragment(); + /** * Given a {@link Label} representing the file that contains this import, returns a {@link Label} * representing the .bzl file to be imported. - * + * * @throws IllegalStateException if this import takes the form of an absolute path. */ Label getLabel(Label containingFileLabel); diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkImports.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkImports.java index 6c82a56b65..2edd38fc3b 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkImports.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkImports.java @@ -39,6 +39,9 @@ public class SkylarkImports { } @Override + public abstract PathFragment asPathFragment(); + + @Override public abstract Label getLabel(Label containingFileLabel); @Override @@ -61,6 +64,11 @@ public class SkylarkImports { } @Override + public PathFragment asPathFragment() { + return importPath; + } + + @Override public Label getLabel(Label containingFileLabel) { throw new IllegalStateException("can't request a label from an absolute path import"); } @@ -85,6 +93,11 @@ public class SkylarkImports { } @Override + public PathFragment asPathFragment() { + return new PathFragment(importFile); + } + + @Override public Label getLabel(Label containingFileLabel) { // The twistiness of the code below is due to the fact that the containing file may be in // a subdirectory of the package that contains it. We need to construct a Label with @@ -111,6 +124,11 @@ public class SkylarkImports { } @Override + public PathFragment asPathFragment() { + return new PathFragment(PathFragment.ROOT_DIR).getRelative(importLabel.toPathFragment()); + } + + @Override public Label getLabel(Label containingFileLabel) { // When the import label contains no explicit repository identifier, we resolve it relative // to the repo of the containing file. @@ -127,6 +145,11 @@ public class SkylarkImports { } @Override + public PathFragment asPathFragment() { + return new PathFragment(importTarget); + } + + @Override public Label getLabel(Label containingFileLabel) { // Unlike a relative path import, the import target is relative to the containing package, // not the containing directory within the package. diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkImportTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkImportTest.java index de3452fe11..ae0f98ef06 100644 --- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkImportTest.java +++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkImportTest.java @@ -34,15 +34,19 @@ public class SkylarkImportTest { @Rule public ExpectedException thrown = ExpectedException.none(); - private void validAbsoluteLabelTest(String labelString) throws Exception { + private void validAbsoluteLabelTest(String labelString, String expectedLabelString, + String expectedPathString) throws Exception { SkylarkImport importForLabel = SkylarkImports.create(labelString); - assertThat(importForLabel.hasAbsolutePath()).isFalse(); - assertThat(importForLabel.getImportString()).isEqualTo(labelString); + assertThat(importForLabel.hasAbsolutePath()).named("hasAbsolutePath()").isFalse(); + assertThat(importForLabel.getImportString()).named("getIMportString()").isEqualTo(labelString); Label irrelevantContainingFile = Label.parseAbsoluteUnchecked("//another/path:BUILD"); - assertThat(importForLabel.getLabel(irrelevantContainingFile)) - .isEqualTo(Label.parseAbsoluteUnchecked(labelString)); + assertThat(importForLabel.getLabel(irrelevantContainingFile)).named("getLabel()") + .isEqualTo(Label.parseAbsoluteUnchecked(expectedLabelString)); + + assertThat(importForLabel.asPathFragment()).named("asPathFragment()") + .isEqualTo(new PathFragment(expectedPathString)); thrown.expect(IllegalStateException.class); importForLabel.getAbsolutePath(); @@ -50,12 +54,16 @@ public class SkylarkImportTest { @Test public void testValidAbsoluteLabel() throws Exception { - validAbsoluteLabelTest("//some/skylark:file.bzl"); + validAbsoluteLabelTest("//some/skylark:file.bzl", + /*expected label*/ "//some/skylark:file.bzl", + /*expected path*/ "/some/skylark/file.bzl"); } @Test public void testValidAbsoluteLabelWithRepo() throws Exception { - validAbsoluteLabelTest("@my_repo//some/skylark:file.bzl"); + validAbsoluteLabelTest("@my_repo//some/skylark:file.bzl", + /*expected label*/ "@my_repo//some/skylark:file.bzl", + /*expected path*/ "/some/skylark/file.bzl"); } @Test @@ -63,68 +71,85 @@ public class SkylarkImportTest { String pathToTest = "/some/skylark/file"; SkylarkImport importForPath = SkylarkImports.create(pathToTest); - assertThat(importForPath.hasAbsolutePath()).isTrue(); - assertThat(importForPath.getImportString()).isEqualTo(pathToTest); + assertThat(importForPath.hasAbsolutePath()).named("hasAbsolutePath()").isTrue(); + assertThat(importForPath.getImportString()).named("getImportString()").isEqualTo(pathToTest); Label irrelevantContainingFile = Label.parseAbsoluteUnchecked("//another/path:BUILD"); - assertThat(importForPath.getAbsolutePath()).isEqualTo(new PathFragment(pathToTest + ".bzl")); + assertThat(importForPath.getAbsolutePath()).named("getAbsolutePath()") + .isEqualTo(new PathFragment("//some/skylark/file.bzl")); + + assertThat(importForPath.asPathFragment()).named("asPathFragment()") + .isEqualTo(new PathFragment("/some/skylark/file.bzl")); thrown.expect(IllegalStateException.class); importForPath.getLabel(irrelevantContainingFile); } private void validRelativeLabelTest(String labelString, - String containingLabelString, String expectedLabelString) throws Exception { + String containingLabelString, String expectedLabelString, String expectedPathString) + throws Exception { SkylarkImport importForLabel = SkylarkImports.create(labelString); - assertThat(importForLabel.hasAbsolutePath()).isFalse(); - assertThat(importForLabel.getImportString()).isEqualTo(labelString); + assertThat(importForLabel.hasAbsolutePath()).named("hasAbsolutePath()").isFalse(); + assertThat(importForLabel.getImportString()).named("getImportString()").isEqualTo(labelString); // The import label is relative to the parent's package, not the parent's directory. Label containingLabel = Label.parseAbsolute(containingLabelString); - assertThat(importForLabel.getLabel(containingLabel)) - .isEqualTo(Label.parseAbsolute(expectedLabelString)); + assertThat(importForLabel.getLabel(containingLabel)).named("getLabel()") + .isEqualTo(Label.parseAbsolute(expectedLabelString)); - thrown.expect(IllegalStateException.class); + assertThat(importForLabel.asPathFragment()).named("asPathFragment()") + .isEqualTo(new PathFragment(expectedPathString)); + + thrown.expect(IllegalStateException.class); importForLabel.getAbsolutePath(); } @Test public void testValidRelativeSimpleLabelInPackageDir() throws Exception { - validRelativeLabelTest(":file.bzl", /*containing*/ "//some/skylark:BUILD", - /*expected*/ "//some/skylark:file.bzl"); + validRelativeLabelTest(":file.bzl", + /*containing*/ "//some/skylark:BUILD", + /*expected label*/ "//some/skylark:file.bzl", + /*expected path*/ "file.bzl"); } @Test public void testValidRelativeSimpleLabelInPackageSubdir() throws Exception { - validRelativeLabelTest(":file.bzl", /*containing*/ "//some/path/to:skylark/parent.bzl", - /*expected*/ "//some/path/to:file.bzl"); + validRelativeLabelTest(":file.bzl", + /*containing*/ "//some/path/to:skylark/parent.bzl", + /*expected label*/ "//some/path/to:file.bzl", + /*expected path*/ "file.bzl"); } @Test public void testValidRelativeComplexLabelInPackageDir() throws Exception { validRelativeLabelTest(":subdir/containing/file.bzl", /*containing*/ "//some/skylark:BUILD", - /*expected*/ "//some/skylark:subdir/containing/file.bzl"); + /*expected label*/ "//some/skylark:subdir/containing/file.bzl", + /*expected path*/ "subdir/containing/file.bzl"); } @Test public void testValidRelativeComplexLabelInPackageSubdir() throws Exception { validRelativeLabelTest(":subdir/containing/file.bzl", /*containing*/ "//some/path/to:skylark/parent.bzl", - /*expected*/ "//some/path/to:subdir/containing/file.bzl"); + /*expected label*/ "//some/path/to:subdir/containing/file.bzl", + /*expected path*/ "subdir/containing/file.bzl"); } private void validRelativePathTest(String pathString, String containingLabelString, - String expectedLabelString) throws Exception { + String expectedLabelString, String expectedPathString) throws Exception { SkylarkImport importForPath = SkylarkImports.create(pathString); - assertThat(importForPath.hasAbsolutePath()).isFalse(); + assertThat(importForPath.hasAbsolutePath()).named("hasAbsolutePath()").isFalse(); // The import label is relative to the parent's directory not the parent's package. Label containingLabel = Label.parseAbsolute(containingLabelString); - assertThat(importForPath.getLabel(containingLabel)) - .isEqualTo(Label.parseAbsolute(expectedLabelString)); + assertThat(importForPath.getLabel(containingLabel)).named("getLabel()") + .isEqualTo(Label.parseAbsolute(expectedLabelString)); + + assertThat(importForPath.asPathFragment()).named("asPathFragment()") + .isEqualTo(new PathFragment(expectedPathString)); thrown.expect(IllegalStateException.class); importForPath.getAbsolutePath(); @@ -132,14 +157,18 @@ public class SkylarkImportTest { @Test public void testValidRelativePathInPackageDir() throws Exception { - validRelativePathTest("file", /*containing*/ "//some/skylark:BUILD", - /*expected*/ "//some/skylark:file.bzl"); + validRelativePathTest("file", + /*containing*/ "//some/skylark:BUILD", + /*expected label*/ "//some/skylark:file.bzl", + /*expected path*/ "file.bzl"); } @Test public void testValidRelativePathInPackageSubdir() throws Exception { - validRelativePathTest("file", /*containing*/ "//some/path/to:skylark/parent.bzl", - /*expected*/ "//some/path/to:skylark/file.bzl"); + validRelativePathTest("file", + /*containing*/ "//some/path/to:skylark/parent.bzl", + /*expected label*/ "//some/path/to:skylark/file.bzl", + /*expected path*/ "file.bzl"); } private void invalidImportTest(String importString, String expectedMsgPrefix) throws Exception { |