Skip to content

Commit

Permalink
Fix label unambiguous canonical form to correctly report non-visible …
Browse files Browse the repository at this point in the history
…repo names

This also affects Starlark `str(some_label)`.

Fixes bazelbuild#17258

PiperOrigin-RevId: 503413176
Change-Id: I864c3c892233ede19b2478c3d9e1c806e4b43c11
  • Loading branch information
Wyverald authored and copybara-github committed Jan 20, 2023
1 parent 33fed24 commit 911eedc
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ public String getCanonicalForm() {
* package.
*/
public String getUnambiguousCanonicalForm() {
return String.format("@@%s//%s", getRepository().getName(), getPackageFragment());
return String.format("@%s//%s", getRepository().getNameWithAt(), getPackageFragment());
}

/**
Expand Down
59 changes: 32 additions & 27 deletions src/test/java/com/google/devtools/build/lib/cmdline/LabelTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,7 @@
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;

/**
* Tests for {@link Label}.
*/
/** Tests for {@link Label}. */
@RunWith(JUnit4.class)
public class LabelTest {

Expand Down Expand Up @@ -175,10 +173,7 @@ public void testIdentities() throws Exception {
Label l2 = Label.parseCanonical("//foo/bar:baz");
Label l3 = Label.parseCanonical("//foo/bar:quux");

new EqualsTester()
.addEqualityGroup(l1, l2)
.addEqualityGroup(l3)
.testEquals();
new EqualsTester().addEqualityGroup(l1, l2).addEqualityGroup(l3).testEquals();
}

@Test
Expand Down Expand Up @@ -225,6 +220,7 @@ public void testDotDot() throws Exception {

/**
* Asserts that creating a label throws a SyntaxException.
*
* @param label the label to create.
*/
private static void assertSyntaxError(String expectedError, String label) {
Expand All @@ -238,12 +234,9 @@ private static void assertSyntaxError(String expectedError, String label) {

@Test
public void testBadCharacters() throws Exception {
assertSyntaxError("target names may not contain ':'",
"//foo:bar:baz");
assertSyntaxError("target names may not contain ':'",
"//foo:bar:");
assertSyntaxError("target names may not contain ':'",
"//foo/bar::");
assertSyntaxError("target names may not contain ':'", "//foo:bar:baz");
assertSyntaxError("target names may not contain ':'", "//foo:bar:");
assertSyntaxError("target names may not contain ':'", "//foo/bar::");
}

@Test
Expand All @@ -267,9 +260,9 @@ public void testDotAsAPathSegment() throws Exception {
assertSyntaxError(INVALID_TARGET_NAME, "//foo:./bar/baz");
// TODO(bazel-team): enable when we have removed the "Workaround" in Label
// that rewrites broken Labels by removing the trailing '.'
//assertSyntaxError(INVALID_PACKAGE_NAME,
// assertSyntaxError(INVALID_PACKAGE_NAME,
// "//foo:bar/baz/.");
//assertSyntaxError(INVALID_PACKAGE_NAME,
// assertSyntaxError(INVALID_PACKAGE_NAME,
// "//foo:.");
}

Expand All @@ -280,11 +273,9 @@ public void testTrailingDotSegment() throws Exception {

@Test
public void testSomeOtherBadLabels() throws Exception {
assertSyntaxError("package names may not end with '/'",
"//foo/:bar");
assertSyntaxError("package names may not end with '/'", "//foo/:bar");
assertSyntaxError("package names may not start with '/'", "///p:foo");
assertSyntaxError("package names may not contain '//' path separators",
"//a//b:foo");
assertSyntaxError("package names may not contain '//' path separators", "//a//b:foo");
}

@Test
Expand All @@ -305,24 +296,22 @@ public void testSomeGoodLabels() throws Exception {

@Test
public void testDoubleSlashPathSeparator() throws Exception {
assertSyntaxError("package names may not contain '//' path separators",
"//foo//bar:baz");
assertSyntaxError("target names may not contain '//' path separator",
"//foo:bar//baz");
assertSyntaxError("package names may not contain '//' path separators", "//foo//bar:baz");
assertSyntaxError("target names may not contain '//' path separator", "//foo:bar//baz");
}

@Test
public void testNonPrintableCharacters() throws Exception {
assertSyntaxError(
"target names may not contain non-printable characters: '\\x02'",
"//foo:..\002bar");
"target names may not contain non-printable characters: '\\x02'", "//foo:..\002bar");
}

/** Make sure that control characters - such as CR - are escaped on output. */
@Test
public void testInvalidLineEndings() throws Exception {
assertSyntaxError("invalid target name '..bar\\r': "
+ "target names may not end with carriage returns", "//foo:..bar\r");
assertSyntaxError(
"invalid target name '..bar\\r': " + "target names may not end with carriage returns",
"//foo:..bar\r");
}

@Test
Expand Down Expand Up @@ -391,6 +380,22 @@ public void testWorkspaceName() throws Exception {
assertThat(Label.parseCanonical("@//bar:baz").getWorkspaceName()).isEmpty();
}

@Test
public void testUnambiguousCanonicalForm() throws Exception {
assertThat(Label.parseCanonical("//foo/bar:baz").getUnambiguousCanonicalForm())
.isEqualTo("@@//foo/bar:baz");
assertThat(Label.parseCanonical("@foo//bar:baz").getUnambiguousCanonicalForm())
.isEqualTo("@@foo//bar:baz");
assertThat(
Label.create(
PackageIdentifier.create(
RepositoryName.create("foo").toNonVisible(RepositoryName.create("bar")),
PathFragment.create("baz")),
"quux")
.getUnambiguousCanonicalForm())
.isEqualTo("@@[unknown repo 'foo' requested from @bar]//baz:quux");
}

@Test
public void starlarkStrAndRepr() throws Exception {
Label label = Label.parseCanonical("//x");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,22 @@ public void testRunfilesDir() throws Exception {
.isEqualTo(PathFragment.create("bar/baz"));
}

@Test
public void testUnambiguousCanonicalForm() throws Exception {
assertThat(PackageIdentifier.createInMainRepo("foo/bar").getUnambiguousCanonicalForm())
.isEqualTo("@@//foo/bar");
assertThat(
PackageIdentifier.create("foo", PathFragment.create("bar"))
.getUnambiguousCanonicalForm())
.isEqualTo("@@foo//bar");
assertThat(
PackageIdentifier.create(
RepositoryName.create("foo").toNonVisible(RepositoryName.create("bar")),
PathFragment.create("baz"))
.getUnambiguousCanonicalForm())
.isEqualTo("@@[unknown repo 'foo' requested from @bar]//baz");
}

@Test
public void testDisplayFormInMainRepository() throws Exception {
PackageIdentifier pkg =
Expand Down

0 comments on commit 911eedc

Please sign in to comment.