diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkAttrModule.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkAttrModule.java index 0839e3fcde9452..5ddcc30bba79cf 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkAttrModule.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkAttrModule.java @@ -462,7 +462,7 @@ public Descriptor labelAttribute( Label label = ((BazelModuleContext) Module.ofInnermostEnclosingStarlarkFunction(thread).getClientData()) .label(); - if (!label.getPackageIdentifier().getRepository().toString().equals("@_builtins")) { + if (!label.getPackageIdentifier().getRepository().getName().equals("_builtins")) { throw Starlark.errorf("Rule in '%s' cannot use private API", label.getPackageName()); } } 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 061ac56bc661cc..1a7e72b30beeb3 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 @@ -369,7 +369,7 @@ public String getCanonicalForm() { } public String getUnambiguousCanonicalForm() { - return packageIdentifier.getRepository() + return packageIdentifier.getRepository().getNameWithAt() + "//" + packageIdentifier.getPackageFragment() + ":" diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java b/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java index 01c7350d3d0ef8..c64f2befea5681 100644 --- a/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java +++ b/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java @@ -169,8 +169,7 @@ public PathFragment getRunfilesPath() { */ // TODO(bazel-team): Maybe rename to "getDefaultForm"? public String getCanonicalForm() { - String repository = getRepository().getCanonicalForm(); - return repository + "//" + getPackageFragment(); + return repository.getCanonicalForm() + "//" + getPackageFragment(); } /** @@ -183,9 +182,10 @@ public String getCanonicalForm() { // that disparity? @Override public String toString() { - // Avoid creating a new String object in the common case of the main repository (PathFragment - // stores its toString). - return repository.isMain() ? pkgName.toString() : (repository + "//" + pkgName); + if (repository.isMain()) { + return getPackageFragment().getPathString(); + } + return getCanonicalForm(); } @Override @@ -218,7 +218,7 @@ public int compareTo(PackageIdentifier that) { return pkgName.compareTo(that.pkgName); } return ComparisonChain.start() - .compare(repository.toString(), that.repository.toString()) + .compare(repository.getName(), that.repository.getName()) .compare(pkgName, that.pkgName) .result(); } 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 ce1ce5601fee3b..f23a0b5d83c90c 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 @@ -108,7 +108,7 @@ public static Pair fromPathFragment( } try { - RepositoryName repoName = RepositoryName.create(path.getSegment(1)); + RepositoryName repoName = create(path.getSegment(1)); PathFragment subPath = path.subFragment(2); return Pair.of(repoName, subPath); } catch (LabelSyntaxException e) { @@ -188,14 +188,13 @@ public boolean isMain() { } /** Returns the repository name, with leading "{@literal @}". */ - // TODO(bazel-team): Use this over toString()- easier to track its usage. public String getNameWithAt() { return name; } /** - * Returns the repository name with leading '@' except for the main repo, which is just the empty - * string. + * Returns the repository name with leading "{@literal @}" except for the main repo, which is just + * the empty string. */ // TODO(bazel-team): Consider renaming to "getDefaultForm". public String getCanonicalForm() { diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java index 68edc9942538e9..190ebfc15baf0c 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java +++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java @@ -535,7 +535,7 @@ public String repositoryName(StarlarkThread thread) throws EvalException { BazelStarlarkContext.from(thread).checkLoadingPhase("native.repository_name"); PackageIdentifier packageId = PackageFactory.getContext(thread).getBuilder().getPackageIdentifier(); - return packageId.getRepository().toString(); + return packageId.getRepository().getNameWithAt(); } private static Dict getRuleDict(Rule rule, Mutability mu) throws EvalException { diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java index 13169225f6de08..831d6311879583 100755 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java @@ -1936,7 +1936,7 @@ public static boolean isBuiltIn(StarlarkThread thread) { Label label = ((BazelModuleContext) Module.ofInnermostEnclosingStarlarkFunction(thread).getClientData()) .label(); - return label.getPackageIdentifier().getRepository().toString().equals("@_builtins"); + return label.getPackageIdentifier().getRepository().getName().equals("_builtins"); } protected Language parseLanguage(String string) throws EvalException { diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaStarlarkCommon.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaStarlarkCommon.java index 66e3171cd8c442..7c2b874f6ccc90 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaStarlarkCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaStarlarkCommon.java @@ -338,7 +338,7 @@ static void checkPrivateAccess(StarlarkThread thread) throws EvalException { ((BazelModuleContext) Module.ofInnermostEnclosingStarlarkFunction(thread).getClientData()) .label(); if (!PRIVATE_STARLARKIFACTION_ALLOWLIST.contains(label.getPackageName()) - && !label.getPackageIdentifier().getRepository().toString().equals("@_builtins")) { + && !label.getPackageIdentifier().getRepository().getName().equals("_builtins")) { throw Starlark.errorf("Rule in '%s' cannot use private API", label.getPackageName()); } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java index a49a67cfbb34e0..4aa4b902c69298 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java @@ -374,8 +374,10 @@ && managedDirectoriesExist(directories.getWorkspace(), managedDirectories)) { if (!repoRoot.exists()) { // The repository isn't on the file system, there is nothing we can do. throw new RepositoryFunctionException( - new IOException("to fix, run\n\tbazel fetch //...\nExternal repository " + repositoryName - + " not found and fetching repositories is disabled."), + new IOException( + "to fix, run\n\tbazel fetch //...\nExternal repository " + + repositoryName.getNameWithAt() + + " not found and fetching repositories is disabled."), Transience.TRANSIENT); } @@ -677,9 +679,7 @@ private static class RepositoryFetching implements FetchProgress { final String message; RepositoryFetching(String name, boolean finished) { - this.id = name; - this.finished = finished; - this.message = finished ? "finished." : "fetching"; + this(name, finished, finished ? "finished." : "fetching"); } RepositoryFetching(String name, boolean finished, String message) { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/LocalRepositoryLookupValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/LocalRepositoryLookupValue.java index c4c784a75c3b39..a822a68142cbd1 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/LocalRepositoryLookupValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/LocalRepositoryLookupValue.java @@ -158,7 +158,7 @@ public PathFragment getPath() { @Override public String toString() { - return "SuccessfulLocalRepositoryLookupValue(" + repositoryName + ")"; + return "SuccessfulLocalRepositoryLookupValue(" + repositoryName.getNameWithAt() + ")"; } @Override diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java index 2c472732a2d50a..dcd80121f9558f 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java @@ -146,7 +146,7 @@ public static String explainNoBuildFileValue(PackageIdentifier packageKey, Envir return "BUILD file not found in directory '" + packageKey.getPackageFragment() + "' of external repository " - + packageKey.getRepository() + + packageKey.getRepository().getNameWithAt() + ". " + educationalMessage; } diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/SymlinkForestTest.java b/src/test/java/com/google/devtools/build/lib/buildtool/SymlinkForestTest.java index 1c54091d5f8b7b..c88628117a5365 100644 --- a/src/test/java/com/google/devtools/build/lib/buildtool/SymlinkForestTest.java +++ b/src/test/java/com/google/devtools/build/lib/buildtool/SymlinkForestTest.java @@ -163,7 +163,7 @@ private static PackageIdentifier createExternalPkg(Root root, String repo, Strin repoRoot.getRelative(pkg).createDirectoryAndParents(); FileSystemUtils.createEmptyFile(repoRoot.getRelative(pkg).getChild("file")); } - return PackageIdentifier.create(RepositoryName.create("" + repo), PathFragment.create(pkg)); + return PackageIdentifier.create(RepositoryName.create(repo), PathFragment.create(pkg)); } // Create package for main repo @@ -173,7 +173,7 @@ private static PackageIdentifier createMainPkg(Root repoRoot, String pkg) repoRoot.getRelative(pkg).createDirectoryAndParents(); FileSystemUtils.createEmptyFile(repoRoot.getRelative(pkg).getChild("file")); } - return PackageIdentifier.create(RepositoryName.create(""), PathFragment.create(pkg)); + return PackageIdentifier.createInMainRepo(PathFragment.create(pkg)); } private static void assertLinksTo(Path fromRoot, Root toRoot, String relpart) throws IOException { 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 c50014e3ce923d..f338a44e849ff0 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 @@ -187,7 +187,7 @@ public void testGetRelativeWithRemoteRepoToDefaultRepo() throws Exception { Label relative = base.getRelativeWithRemapping("@//x:y", ImmutableMap.of()); - assertThat(relative.getRepository()).isEqualTo(RepositoryName.create("")); + assertThat(relative.getRepository()).isEqualTo(RepositoryName.MAIN); assertThat(relative.getPackageFragment()).isEqualTo(PathFragment.create("x")); assertThat(relative.getName()).isEqualTo("y"); } diff --git a/src/test/java/com/google/devtools/build/lib/cmdline/RepositoryNameTest.java b/src/test/java/com/google/devtools/build/lib/cmdline/RepositoryNameTest.java index 1a0002a90e10b8..9a8743f3300041 100644 --- a/src/test/java/com/google/devtools/build/lib/cmdline/RepositoryNameTest.java +++ b/src/test/java/com/google/devtools/build/lib/cmdline/RepositoryNameTest.java @@ -36,15 +36,15 @@ public void assertNotValid(String name, String expectedMessage) { @Test public void testValidateRepositoryName() throws Exception { - assertThat(RepositoryName.create("foo").toString()).isEqualTo("@foo"); - assertThat(RepositoryName.create("").toString()).isEqualTo("@"); + assertThat(RepositoryName.create("foo").getNameWithAt()).isEqualTo("@foo"); + assertThat(RepositoryName.create("").getNameWithAt()).isEqualTo("@"); assertThat(RepositoryName.create("")).isSameInstanceAs(RepositoryName.MAIN); - assertThat(RepositoryName.create("foo_bar").toString()).isEqualTo("@foo_bar"); - assertThat(RepositoryName.create("foo-bar").toString()).isEqualTo("@foo-bar"); - assertThat(RepositoryName.create("foo.bar").toString()).isEqualTo("@foo.bar"); - assertThat(RepositoryName.create("..foo").toString()).isEqualTo("@..foo"); - assertThat(RepositoryName.create("foo..").toString()).isEqualTo("@foo.."); - assertThat(RepositoryName.create(".foo").toString()).isEqualTo("@.foo"); + assertThat(RepositoryName.create("foo_bar").getNameWithAt()).isEqualTo("@foo_bar"); + assertThat(RepositoryName.create("foo-bar").getNameWithAt()).isEqualTo("@foo-bar"); + assertThat(RepositoryName.create("foo.bar").getNameWithAt()).isEqualTo("@foo.bar"); + assertThat(RepositoryName.create("..foo").getNameWithAt()).isEqualTo("@..foo"); + assertThat(RepositoryName.create("foo..").getNameWithAt()).isEqualTo("@foo.."); + assertThat(RepositoryName.create(".foo").getNameWithAt()).isEqualTo("@.foo"); assertNotValid(".", "repo names are not allowed to be '@.'"); assertNotValid("..", "repo names are not allowed to be '@..'"); @@ -58,13 +58,10 @@ public void testRunfilesDir() throws Exception { assertThat(RepositoryName.create("foo").getRunfilesPath()) .isEqualTo(PathFragment.create("../foo")); assertThat(RepositoryName.create("").getRunfilesPath()).isEqualTo(PathFragment.EMPTY_FRAGMENT); - assertThat(RepositoryName.create("").getRunfilesPath()) - .isEqualTo(PathFragment.EMPTY_FRAGMENT); } @Test public void testGetDefaultCanonicalForm() throws Exception { - assertThat(RepositoryName.create("").getCanonicalForm()).isEqualTo(""); assertThat(RepositoryName.create("").getCanonicalForm()).isEqualTo(""); assertThat(RepositoryName.create("foo").getCanonicalForm()).isEqualTo("@foo"); }