Skip to content

Commit

Permalink
Get rid of the '@' in RepositoryName
Browse files Browse the repository at this point in the history
This is a rather mechanical change that removes the '@' from RepositoryName. So:

- `RepositoryName.create` used to take "@foo", now takes "foo"
- `RepositoryName.createWithValidStrippedName` is renamed `createUnvalidated`; still takes "foo"
- `RepositoryName#getName` is renamed `getNameWithAt`, returning "@foo"
- `RepositoryName#strippedName` is renamed "getName`, returning "foo"
- No other behavior changes

PiperOrigin-RevId: 417379314
  • Loading branch information
Wyverald authored and copybara-github committed Dec 20, 2021
1 parent 18c591a commit 78d0131
Show file tree
Hide file tree
Showing 79 changed files with 400 additions and 493 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ private void propagateTransitiveValidationOutputGroups() {
Label rdeLabel =
ruleContext.getRule().getRuleClassObject().getRuleDefinitionEnvironmentLabel();
// only allow native and builtins to override transitive validation propagation
if (rdeLabel != null && !"@_builtins".equals(rdeLabel.getRepository().getName())) {
if (rdeLabel != null && !"@_builtins".equals(rdeLabel.getRepository().getNameWithAt())) {
ruleContext.ruleError(rdeLabel + " cannot access the _transitive_validation private API");
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,7 @@ public boolean isStrictFilesetOutput() {
}

public String getMainRepositoryName() {
return mainRepositoryName.strippedName();
return mainRepositoryName.getName();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ public enum OutputDirectory {
public ArtifactRoot getRoot(
String outputDirName, BlazeDirectories directories, RepositoryName mainRepositoryName) {
// e.g., execroot/repo1
Path execRoot = directories.getExecRoot(mainRepositoryName.strippedName());
Path execRoot = directories.getExecRoot(mainRepositoryName.getName());
// e.g., [[execroot/repo1]/bazel-out/config/bin]
return ArtifactRoot.asDerivedRoot(
execRoot,
Expand Down Expand Up @@ -160,7 +160,7 @@ public ArtifactRoot getRoot(

this.mergeGenfilesDirectory = options.mergeGenfilesDirectory;
this.siblingRepositoryLayout = siblingRepositoryLayout;
this.execRoot = directories.getExecRoot(mainRepositoryName.strippedName());
this.execRoot = directories.getExecRoot(mainRepositoryName.getName());
}

private static void addMnemonicPart(
Expand Down Expand Up @@ -265,7 +265,7 @@ private ArtifactRoot buildDerivedRoot(
execRoot,
rootType,
directories.getRelativeOutputPath(),
repository.strippedName(),
repository.getName(),
outputDirName,
nameFragment);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ public boolean starlarkMatches(Label label, StarlarkThread starlarkThread) throw
BazelModuleContext.of(ofInnermostEnclosingStarlarkFunction(starlarkThread))
.label()
.getRepository();
if (!"@_builtins".equals(repository.getName())) {
if (!"@_builtins".equals(repository.getNameWithAt())) {
throw Starlark.errorf("private API only for use by builtins");
}
return Allowlist.isAvailableFor(getPackageSpecifications(), label);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,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());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ public StarlarkCallable rule(
}

if (compileOneFiletype instanceof Sequence) {
if (!bzlModule.label().getRepository().getName().equals("@_builtins")) {
if (!bzlModule.label().getRepository().getNameWithAt().equals("@_builtins")) {
throw Starlark.errorf(
"Rule in '%s' cannot use private API", bzlModule.label().getPackageName());
}
Expand Down Expand Up @@ -903,7 +903,7 @@ public void export(EventHandler handler, Label starlarkLabel, String ruleClassNa
}
// TODO(b/121385274): remove when we stop allowlisting starlark transitions
if (hasStarlarkDefinedTransition) {
if (!starlarkLabel.getRepository().getName().equals("@_builtins")) {
if (!starlarkLabel.getRepository().getNameWithAt().equals("@_builtins")) {
if (!hasFunctionTransitionAllowlist) {
errorf(
handler,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ public BazelRepositoryModule() {
Set<String> conflicting =
overrides.keySet().stream()
.filter(repositoryNamesWithManagedDirs::contains)
.map(RepositoryName::getName)
.map(RepositoryName::getNameWithAt)
.collect(Collectors.toSet());
if (!conflicting.isEmpty()) {
String message =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ static BazelModuleResolutionValue createValue(
BiMap<String, ModuleExtensionId> extensionUniqueNames = HashBiMap.create();
for (ModuleExtensionId id : extensionUsagesById.rowKeySet()) {
String bestName =
id.getBzlFileLabel().getRepository().strippedName() + "." + id.getExtensionName();
id.getBzlFileLabel().getRepository().getName() + "." + id.getExtensionName();
if (extensionUniqueNames.putIfAbsent(bestName, id) == null) {
continue;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,8 @@ public final RepositoryMapping getFullRepoMapping(ModuleKey key) {
String canonicalRepoName =
getExtensionUniqueNames().get(extensionId) + "." + entry.getValue();
mapping.put(
RepositoryName.createFromValidStrippedName(entry.getKey()),
RepositoryName.createFromValidStrippedName(canonicalRepoName));
RepositoryName.createUnvalidated(entry.getKey()),
RepositoryName.createUnvalidated(canonicalRepoName));
}
}
return getDepGraph()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,15 +106,15 @@ public final RepositoryMapping getRepoMappingWithBazelDepsOnly() {
// If this is the root module, this perfectly falls into @<module name> => @
if (!getName().isEmpty()) {
mapping.put(
RepositoryName.createFromValidStrippedName(getName()),
RepositoryName.createFromValidStrippedName(getCanonicalRepoName()));
RepositoryName.createUnvalidated(getName()),
RepositoryName.createUnvalidated(getCanonicalRepoName()));
}
for (Map.Entry<String, ModuleKey> dep : getDeps().entrySet()) {
// Special note: if `dep` is actually the root module, its ModuleKey would be ROOT whose
// canonicalRepoName is the empty string. This perfectly maps to the main repo ("@").
mapping.put(
RepositoryName.createFromValidStrippedName(dep.getKey()),
RepositoryName.createFromValidStrippedName(dep.getValue().getCanonicalRepoName()));
RepositoryName.createUnvalidated(dep.getKey()),
RepositoryName.createUnvalidated(dep.getValue().getCanonicalRepoName()));
}
return RepositoryMapping.create(mapping.build(), getCanonicalRepoName());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ private GetModuleFileResult getModuleFile(
(RepositoryDirectoryValue)
env.getValue(
RepositoryDirectoryValue.key(
RepositoryName.createFromValidStrippedName(canonicalRepoName)));
RepositoryName.createUnvalidated(canonicalRepoName)));
if (repoDir == null) {
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,7 @@ private StarlarkBazelModule(String name, String version, Tags tags) {
static Label createModuleRootLabel(String canonicalRepoName) {
return Label.createUnvalidated(
PackageIdentifier.create(
RepositoryName.createFromValidStrippedName(canonicalRepoName),
PathFragment.EMPTY_FRAGMENT),
RepositoryName.createUnvalidated(canonicalRepoName), PathFragment.EMPTY_FRAGMENT),
"unused_dummy_target_name");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ public BlazeCommandResult exec(CommandEnvironment env, OptionsParsingResult opti
// TODO(aehlig): avoid the detour of serializing and then parsing the repository name
try {
repositoriesToFetch.add(
RepositoryDirectoryValue.key(RepositoryName.create("@" + rule.getName())));
RepositoryDirectoryValue.key(RepositoryName.create(rule.getName())));
} catch (LabelSyntaxException e) {
String errorMessage =
String.format(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

import com.google.common.eventbus.Subscribe;
import com.google.devtools.build.lib.bazel.repository.cache.RepositoryCacheHitEvent;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.Reporter;
import com.google.devtools.build.lib.repository.RepositoryFailedEvent;
Expand Down Expand Up @@ -57,7 +56,7 @@ public synchronized void cacheHit(RepositoryCacheHitEvent event) {

@Subscribe
public void failed(RepositoryFailedEvent event) {
String repo = RepositoryName.stripName(event.getRepo());
String repo = event.getRepo();
Set<Pair<String, URL>> cacheHits = cacheHitsByRepo.get(repo);
if (cacheHits != null && !cacheHits.isEmpty()) {
StringBuilder info = new StringBuilder();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ public RepositoryOverride convert(String input) throws OptionsParsingException {
"Repository override directory must be an absolute path", input);
}
try {
return RepositoryOverride.create(RepositoryName.create("@" + pieces[0]), path);
return RepositoryOverride.create(RepositoryName.create(pieces[0]), path);
} catch (LabelSyntaxException e) {
throw new OptionsParsingException("Invalid repository name given to override", input);
}
Expand Down
1 change: 0 additions & 1 deletion src/main/java/com/google/devtools/build/lib/cmdline/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/vfs:ospathpolicy",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//third_party:auto_value",
"//third_party:caffeine",
"//third_party:guava",
"//third_party:jsr305",
],
Expand Down
16 changes: 6 additions & 10 deletions src/main/java/com/google/devtools/build/lib/cmdline/Label.java
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,7 @@ private static Label parseCanonical(String raw) throws LabelSyntaxException {
Parts parts = Parts.parse(raw);
parts.checkPkgIsAbsolute();
RepositoryName repoName =
parts.repo == null
? RepositoryName.MAIN
: RepositoryName.createFromValidStrippedName(parts.repo);
parts.repo == null ? RepositoryName.MAIN : RepositoryName.createUnvalidated(parts.repo);
return createUnvalidated(
PackageIdentifier.create(repoName, PathFragment.create(parts.pkg)), parts.target);
}
Expand All @@ -105,7 +103,7 @@ private static RepositoryName computeRepoNameWithRepoContext(
return ABSOLUTE_PACKAGE_NAMES.contains(parts.pkg) ? RepositoryName.MAIN : currentRepo;
}
// TODO(b/200024947): Make repo mapping take a string and return a RepositoryName.
return repoMapping.get(RepositoryName.createFromValidStrippedName(parts.repo));
return repoMapping.get(RepositoryName.createUnvalidated(parts.repo));
}

// TODO(b/200024947): Make this public.
Expand Down Expand Up @@ -277,9 +275,7 @@ public static Label parseCommandLineLabel(String raw, PathFragment workspaceRela
}
// TODO(b/200024947): This method will eventually need to take a repo mapping too.
RepositoryName repoName =
parts.repo == null
? RepositoryName.MAIN
: RepositoryName.createFromValidStrippedName(parts.repo);
parts.repo == null ? RepositoryName.MAIN : RepositoryName.createUnvalidated(parts.repo);
return create(PackageIdentifier.create(repoName, pathFragment), parts.target);
}

Expand Down Expand Up @@ -406,7 +402,7 @@ public String getCanonicalForm() {
}

public String getUnambiguousCanonicalForm() {
return packageIdentifier.getRepository()
return packageIdentifier.getRepository().getNameWithAt()
+ "//"
+ packageIdentifier.getPackageFragment()
+ ":"
Expand All @@ -422,7 +418,7 @@ public String getUnambiguousCanonicalForm() {
+ "<pre class=language-python>Label(\"@foo//bar:baz\").workspace_name"
+ " == \"foo\"</pre>")
public String getWorkspaceName() {
return packageIdentifier.getRepository().strippedName();
return packageIdentifier.getRepository().getName();
}

/**
Expand All @@ -439,7 +435,7 @@ public String toShorthandString() {
if (packageIdentifier.getRepository().isMain()) {
repository = "";
} else {
repository = packageIdentifier.getRepository().getName();
repository = packageIdentifier.getRepository().getNameWithAt();
}
return repository + "//" + getPackageFragment();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,14 +117,9 @@ static Parts parse(String rawLabel) throws LabelSyntaxException {
return validateAndCreate(repo, pkgIsAbsolute, pkg, target, rawLabel);
}

@Nullable
private static void validateRepoName(@Nullable String repo) throws LabelSyntaxException {
if (repo == null) {
return;
}
String error = RepositoryName.validate('@' + repo);
if (error != null) {
throw syntaxErrorf("invalid repository name '@%s': %s", repo, error);
if (repo != null) {
RepositoryName.validate(repo);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public static PackageIdentifier discoverFromExecPath(
if (tofind.startsWith(prefix)) {
// Using the path prefix can be either "external" or "..", depending on whether the sibling
// repository layout is used.
RepositoryName repository = RepositoryName.createFromValidStrippedName(tofind.getSegment(1));
RepositoryName repository = RepositoryName.createUnvalidated(tofind.getSegment(1));
return PackageIdentifier.create(repository, tofind.subFragment(2));
} else {
return PackageIdentifier.createInMainRepo(tofind);
Expand Down Expand Up @@ -116,9 +116,7 @@ public static PackageIdentifier parse(String input) throws LabelSyntaxException
}
LabelParser.Parts parts = LabelParser.Parts.parse(input + ":dummy_target");
RepositoryName repoName =
parts.repo == null
? RepositoryName.MAIN
: RepositoryName.createFromValidStrippedName(parts.repo);
parts.repo == null ? RepositoryName.MAIN : RepositoryName.createUnvalidated(parts.repo);
return create(repoName, PathFragment.create(parts.pkg));
}

Expand Down Expand Up @@ -148,7 +146,7 @@ public PathFragment getPackagePath(boolean siblingRepositoryLayout) {
return repository.isMain() || siblingRepositoryLayout
? pkgName
: LabelConstants.EXTERNAL_PATH_PREFIX
.getRelative(repository.strippedName())
.getRelative(repository.getName())
.getRelative(pkgName);
}

Expand All @@ -171,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();
}

/**
Expand All @@ -185,7 +182,10 @@ public String getCanonicalForm() {
// that disparity?
@Override
public String toString() {
return (repository.isMain() ? "" : repository + "//") + pkgName;
if (repository.isMain()) {
return getPackageFragment().getPathString();
}
return getCanonicalForm();
}

@Override
Expand Down Expand Up @@ -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();
}
Expand Down
Loading

0 comments on commit 78d0131

Please sign in to comment.