diff options
author | 2016-01-28 14:38:31 +0000 | |
---|---|---|
committer | 2016-01-28 15:30:43 +0000 | |
commit | b6fbab7115c1567705e7bddc7b79bdee6313fce3 (patch) | |
tree | 174281e412f825660fe186c95e67b7af863cfaf8 /src | |
parent | 6d44cab8167cfae55cd5a26940018efcbaa43154 (diff) |
Resolve repository-relative labels within the current repository
Using $(location //foo) from an external repository was resolving to @//foo, not
@repo//foo, which generally wouldn't be in the main repository. This may also
fix other cases where getRelative was resolving incorrectly.
Fixes #819.
--
MOS_MIGRATED_REVID=113256854
Diffstat (limited to 'src')
5 files changed, 125 insertions, 29 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/Label.java b/src/main/java/com/google/devtools/build/lib/cmdline/Label.java index 7e1046f8a1..b1bcffa482 100644 --- a/src/main/java/com/google/devtools/build/lib/cmdline/Label.java +++ b/src/main/java/com/google/devtools/build/lib/cmdline/Label.java @@ -49,6 +49,8 @@ public final class Label implements Comparable<Label>, Serializable, SkylarkPrin * things to Bazel. */ private static final ImmutableSet<PathFragment> ABSOLUTE_PACKAGE_NAMES = ImmutableSet.of( + // Used for select + new PathFragment("conditions"), // dependencies that are a function of the configuration new PathFragment("tools/defaults"), // Visibility is labels aren't actually targets @@ -370,18 +372,26 @@ public final class Label implements Comparable<Label>, Serializable, SkylarkPrin */ @SkylarkCallable(name = "relative", doc = "Resolves a label that is either absolute (starts with <code>//</code>) or relative to the" - + " current package.<br>" + + " current package. If this label is in a remote repository, the argument will be resolved " + + "relative to that repository. If the argument contains a repository, it will be returned " + + "as-is. Reserved labels will also be returned as-is.<br>" + "For example:<br>" + "<pre class=language-python>\n" + "Label(\"//foo/bar:baz\").relative(\":quux\") == Label(\"//foo/bar:quux\")\n" + "Label(\"//foo/bar:baz\").relative(\"//wiz:quux\") == Label(\"//wiz:quux\")\n" + + "Label(\"@repo//foo/bar:baz\").relative(\"//wiz:quux\") == Label(\"@repo//wiz:quux\")\n" + + "Label(\"@repo//foo/bar:baz\").relative(\"//visibility:public\") == " + + "Label(\"//visibility:public\")\n" + + "Label(\"@repo//foo/bar:baz\").relative(\"@other//wiz:quux\") == " + + "Label(\"@other//wiz:quux\")\n" + "</pre>") public Label getRelative(String relName) throws LabelSyntaxException { if (relName.length() == 0) { throw new LabelSyntaxException("empty package-relative label"); } + if (LabelValidator.isAbsolute(relName)) { - return parseAbsolute(relName); + return resolveRepositoryRelative(parseAbsolute(relName)); } else if (relName.equals(":")) { throw new LabelSyntaxException("':' is not a valid package-relative label"); } else if (relName.charAt(0) == ':') { @@ -399,18 +409,6 @@ public final class Label implements Comparable<Label>, Serializable, SkylarkPrin * repository would point back to the main repository, which is usually not what is intended. */ public Label resolveRepositoryRelative(Label relative) { - if (relative.packageIdentifier.getRepository().getName().equals("@")) { - try { - return new Label( - PackageIdentifier.create( - PackageIdentifier.DEFAULT_REPOSITORY_NAME, - relative.packageIdentifier.getPackageFragment()), - relative.getName()); - } catch (LabelSyntaxException e) { - throw new IllegalStateException(e); - } - } - if (packageIdentifier.getRepository().isDefault() || !relative.packageIdentifier.getRepository().isDefault() || ABSOLUTE_PACKAGE_NAMES.contains(relative.getPackageIdentifier().getPackageFragment())) { diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryName.java b/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryName.java index 2aa253dcb1..6f5811d58a 100644 --- a/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryName.java +++ b/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryName.java @@ -189,7 +189,7 @@ public final class RepositoryName implements Serializable { * Returns the path at which this repository is mapped within the exec root. */ public PathFragment getPathFragment() { - return isDefault() + return isDefault() || this.equals(PackageIdentifier.MAIN_REPOSITORY_NAME) ? PathFragment.EMPTY_FRAGMENT : new PathFragment(Label.EXTERNAL_PATH_PREFIX).getRelative(strippedName()); } diff --git a/src/main/java/com/google/devtools/build/lib/packages/Package.java b/src/main/java/com/google/devtools/build/lib/packages/Package.java index 51a496dc7a..1cfb443936 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/Package.java +++ b/src/main/java/com/google/devtools/build/lib/packages/Package.java @@ -267,7 +267,8 @@ public class Package { private static Path getSourceRoot(Path buildFile, PathFragment packageFragment) { Path current = buildFile.getParentDirectory(); - for (int i = 0, len = packageFragment.segmentCount(); i < len && current != null; i++) { + for (int i = 0, len = packageFragment.segmentCount(); + i < len && !packageFragment.equals(PathFragment.EMPTY_FRAGMENT); i++) { current = current.getParentDirectory(); } return current; diff --git a/src/test/java/com/google/devtools/build/lib/cmdline/LabelTest.java b/src/test/java/com/google/devtools/build/lib/cmdline/LabelTest.java index 1f3ce8b212..9837861a56 100644 --- a/src/test/java/com/google/devtools/build/lib/cmdline/LabelTest.java +++ b/src/test/java/com/google/devtools/build/lib/cmdline/LabelTest.java @@ -93,19 +93,24 @@ public class LabelTest { } @Test - public void testGetRelative() throws Exception { - Label base = Label - .parseAbsolute("//foo/bar:baz"); - { - Label l = base.getRelative("//p1/p2:target"); - assertEquals("p1/p2", l.getPackageName()); - assertEquals("target", l.getName()); - } - { - Label l = base.getRelative(":quux"); - assertEquals("foo/bar", l.getPackageName()); - assertEquals("quux", l.getName()); - } + public void testGetRelativeWithAbsoluteLabel() throws Exception { + Label base = Label.parseAbsolute("//foo/bar:baz"); + Label l = base.getRelative("//p1/p2:target"); + assertEquals("p1/p2", l.getPackageName()); + assertEquals("target", l.getName()); + } + + @Test + public void testGetRelativeWithRelativeLabel() throws Exception { + Label base = Label.parseAbsolute("//foo/bar:baz"); + Label l = base.getRelative(":quux"); + assertEquals("foo/bar", l.getPackageName()); + assertEquals("quux", l.getName()); + } + + @Test + public void testGetRelativeWithIllegalLabel() throws Exception { + Label base = Label.parseAbsolute("//foo/bar:baz"); try { base.getRelative("/p1/p2:target"); fail(); @@ -133,6 +138,67 @@ public class LabelTest { } @Test + public void testGetRelativeWithDifferentRepo() throws Exception { + PackageIdentifier packageId = PackageIdentifier.create("@repo", new PathFragment("foo")); + Label base = Label.create(packageId, "bar"); + + Label relative = base.getRelative("@remote//x:y"); + + assertEquals(RepositoryName.create("@remote"), relative.getPackageIdentifier().getRepository()); + assertEquals(new PathFragment("x"), relative.getPackageFragment()); + assertEquals("y", relative.getName()); + } + + @Test + public void testGetRelativeWithRepoLocalAbsoluteLabel() throws Exception { + PackageIdentifier packageId = PackageIdentifier.create("@repo", new PathFragment("foo")); + Label base = Label.create(packageId, "bar"); + + Label relative = base.getRelative("//x:y"); + + assertEquals(packageId.getRepository(), relative.getPackageIdentifier().getRepository()); + assertEquals(new PathFragment("x"), relative.getPackageFragment()); + assertEquals("y", relative.getName()); + } + + @Test + public void testGetRelativeWithLocalRepoRelativeLabel() throws Exception { + PackageIdentifier packageId = PackageIdentifier.create("@repo", new PathFragment("foo")); + Label base = Label.create(packageId, "bar"); + + Label relative = base.getRelative(":y"); + + assertEquals(packageId.getRepository(), relative.getPackageIdentifier().getRepository()); + assertEquals(new PathFragment("foo"), relative.getPackageFragment()); + assertEquals("y", relative.getName()); + } + + @Test + public void testGetRelativeWithRepoAndReservedPackage() throws Exception { + PackageIdentifier packageId = PackageIdentifier.create("@repo", new PathFragment("foo")); + Label base = Label.create(packageId, "bar"); + + Label relative = base.getRelative("//conditions:default"); + + PackageIdentifier expected = PackageIdentifier.createInDefaultRepo("conditions"); + assertEquals(expected.getRepository(), relative.getPackageIdentifier().getRepository()); + assertEquals(expected.getPackageFragment(), relative.getPackageFragment()); + assertEquals("default", relative.getName()); + } + + @Test + public void testGetRelativeWithRemoteRepoToDefaultRepo() throws Exception { + PackageIdentifier packageId = PackageIdentifier.create("@repo", new PathFragment("foo")); + Label base = Label.create(packageId, "bar"); + + Label relative = base.getRelative("@//x:y"); + + assertEquals(RepositoryName.create("@"), relative.getPackageIdentifier().getRepository()); + assertEquals(new PathFragment("x"), relative.getPackageFragment()); + assertEquals("y", relative.getName()); + } + + @Test public void testFactory() throws Exception { Label l = Label.create("foo/bar", "quux"); assertEquals("foo/bar", l.getPackageName()); diff --git a/src/test/shell/bazel/bazel_rules_test.sh b/src/test/shell/bazel/bazel_rules_test.sh index 2547ef57d2..bb9e5e2a0f 100755 --- a/src/test/shell/bazel/bazel_rules_test.sh +++ b/src/test/shell/bazel/bazel_rules_test.sh @@ -243,4 +243,35 @@ EOF bazel-genfiles/pkg/test.out } +function test_genrule_remote() { + cat > WORKSPACE <<EOF +local_repository( + name = "r", + path = __workspace_dir__, +) +EOF + mkdir package + cat > package/BUILD <<EOF +genrule( + name = "abs_dep", + srcs = ["//package:in"], + outs = ["abs_dep.out"], + cmd = "echo '\$(locations //package:in)' > \$@", +) + +sh_binary( + name = "in", + srcs = ["in.sh"], +) +EOF + + cat > package/in.sh << EOF +#!/bin/bash +echo "Hi" +EOF + chmod +x package/in.sh + + bazel build @r//package:abs_dep >$TEST_log 2>&1 || fail "Should build" +} + run_suite "rules test" |