Skip to content

Commit

Permalink
Allow modules to specify their own repo names
Browse files Browse the repository at this point in the history
With the `repo_name` attribute in the `module` directive, a module can specify its own repo name for itself and any extensions it contains. This is particularly useful during migration.

PiperOrigin-RevId: 472299124
Change-Id: I0febecfcfe73343e8bd52ce13e5dbb22581858e4
  • Loading branch information
Wyverald authored and copybara-github committed Sep 5, 2022
1 parent 4a8f756 commit 73ed74e
Show file tree
Hide file tree
Showing 6 changed files with 166 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import java.util.Map;
import java.util.Optional;
import java.util.function.UnaryOperator;
import javax.annotation.Nullable;

Expand Down Expand Up @@ -74,6 +75,13 @@ public final RepositoryName getCanonicalRepoName() {
*/
public abstract int getCompatibilityLevel();

/**
* The name of the repository representing this module, as seen by the module itself. By default,
* the name of the repo is the name of the module. This can be specified to ease migration for
* projects that have been using a repo name for itself that differs from its module name.
*/
public abstract String getRepoName();

/**
* Target patterns identifying execution platforms to register when this module is selected. Note
* that these are what was written in module files verbatim, and don't contain canonical repo
Expand Down Expand Up @@ -112,10 +120,10 @@ public final RepositoryMapping getRepoMappingWithBazelDepsOnly() {
if (getKey().equals(ModuleKey.ROOT)) {
mapping.put("", RepositoryName.MAIN);
}
// Every module should be able to reference itself as @<module name>.
// If this is the root module, this perfectly falls into @<module name> => @
if (!getName().isEmpty()) {
mapping.put(getName(), getCanonicalRepoName());
// Every module should be able to reference itself as @<module repo name>.
// If this is the root module, this perfectly falls into @<module repo name> => @
if (!getRepoName().isEmpty()) {
mapping.put(getRepoName(), getCanonicalRepoName());
}
for (Map.Entry<String, ModuleKey> dep : getDeps().entrySet()) {
// Special note: if `dep` is actually the root module, its ModuleKey would be ROOT whose
Expand Down Expand Up @@ -172,6 +180,9 @@ public abstract static class Builder {
/** Optional; defaults to {@code 0}. */
public abstract Builder setCompatibilityLevel(int value);

/** Optional; defaults to {@link #setName}. */
public abstract Builder setRepoName(String value);

abstract ImmutableList.Builder<String> executionPlatformsToRegisterBuilder();

@CanIgnoreReturnValue
Expand Down Expand Up @@ -220,6 +231,17 @@ public Builder addExtensionUsage(ModuleExtensionUsage value) {
return this;
}

abstract Module build();
abstract String getName();

abstract Optional<String> getRepoName();

abstract Module autoBuild();

final Module build() {
if (getRepoName().isEmpty()) {
setRepoName(getName());
}
return autoBuild();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,16 @@ static void validateModuleName(String moduleName) throws EvalException {
named = true,
positional = false,
defaultValue = "0"),
@Param(
name = "repo_name",
doc =
"The name of the repository representing this module, as seen by the module itself."
+ " By default, the name of the repo is the name of the module. This can be"
+ " specified to ease migration for projects that have been using a repo name"
+ " for itself that differs from its module name.",
named = true,
positional = false,
defaultValue = "''"),
@Param(
name = "execution_platforms_to_register",
doc =
Expand Down Expand Up @@ -187,6 +197,7 @@ public void module(
String name,
String version,
StarlarkInt compatibilityLevel,
String repoName,
Iterable<?> executionPlatformsToRegister,
Iterable<?> toolchainsToRegister,
StarlarkThread thread)
Expand All @@ -198,6 +209,13 @@ public void module(
if (!name.isEmpty()) {
validateModuleName(name);
}
if (repoName.isEmpty()) {
repoName = name;
addRepoNameUsage(name, "as the current module name", thread.getCallerLocation());
} else {
RepositoryName.validateUserProvidedRepoName(repoName);
addRepoNameUsage(repoName, "as the module's own repo name", thread.getCallerLocation());
}
Version parsedVersion;
try {
parsedVersion = Version.parse(version);
Expand All @@ -210,12 +228,12 @@ public void module(
.setName(name)
.setVersion(parsedVersion)
.setCompatibilityLevel(compatibilityLevel.toInt("compatibility_level"))
.setRepoName(repoName)
.addExecutionPlatformsToRegister(
checkAllAbsolutePatterns(
executionPlatformsToRegister, "execution_platforms_to_register"))
.addToolchainsToRegister(
checkAllAbsolutePatterns(toolchainsToRegister, "toolchains_to_register"));
addRepoNameUsage(name, "as the current module name", thread.getCallerLocation());
}

private static ImmutableList<String> checkAllAbsolutePatterns(Iterable<?> iterable, String where)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,12 @@ public ModuleBuilder setKey(ModuleKey value) {
return this;
}

@CanIgnoreReturnValue
public ModuleBuilder setRepoName(String value) {
this.builder.setRepoName(value);
return this;
}

@CanIgnoreReturnValue
public ModuleBuilder setRegistry(FakeRegistry value) {
this.builder.setRegistry(value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -894,8 +894,7 @@ public void generatedReposHaveCorrectMappings() throws Exception {
"def _ext_impl(ctx):",
" internal_repo(name='internal')",
" ext_repo(name='ext')",
"tag=tag_class(attrs={'file':attr.label()})",
"ext=module_extension(implementation=_ext_impl,tag_classes={'tag':tag})");
"ext=module_extension(implementation=_ext_impl)");

registry.addModule(createModuleKey("foo", "1.0"), "module(name='foo',version='1.0')");
scratch.file(modulesRoot.getRelative("foo~1.0/WORKSPACE").getPathString());
Expand All @@ -912,6 +911,42 @@ public void generatedReposHaveCorrectMappings() throws Exception {
.isEqualTo("foo: foo-stuff internal: internal-stuff");
}

@Test
public void generatedReposHaveCorrectMappings_moduleOwnRepoName() throws Exception {
// tests that things work correctly when the module specifies its own repo name (via
// `module(repo_name=...)`).
scratch.file(
workspaceRoot.getRelative("MODULE.bazel").getPathString(),
"module(name='foo',version='1.0',repo_name='bar')",
"ext = use_extension('//:defs.bzl','ext')",
"use_repo(ext,'ext')");
scratch.file(workspaceRoot.getRelative("BUILD").getPathString());
scratch.file(workspaceRoot.getRelative("data.bzl").getPathString(), "data='hello world'");
scratch.file(
workspaceRoot.getRelative("defs.bzl").getPathString(),
"def _ext_repo_impl(ctx):",
" ctx.file('WORKSPACE')",
" ctx.file('BUILD')",
" ctx.file('data.bzl', \"\"\"load('@bar//:data.bzl', bar_data='data')",
"data = 'bar: '+bar_data",
"\"\"\")",
"ext_repo = repository_rule(implementation=_ext_repo_impl)",
"",
"ext=module_extension(implementation=lambda ctx: ext_repo(name='ext'))");
scratch.file(
workspaceRoot.getRelative("ext_data.bzl").getPathString(),
"load('@ext//:data.bzl', ext_data='data')",
"data='ext: ' + ext_data");

SkyKey skyKey = BzlLoadValue.keyForBuild(Label.parseCanonical("//:ext_data.bzl"));
EvaluationResult<BzlLoadValue> result =
evaluator.evaluate(ImmutableList.of(skyKey), evaluationContext);
if (result.hasError()) {
throw result.getError().getException();
}
assertThat(result.get(skyKey).getModule().getGlobal("data")).isEqualTo("ext: bar: hello world");
}

@Test
public void generatedReposHaveCorrectMappings_internalRepoWins() throws Exception {
scratch.file(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -729,6 +729,18 @@ public void badModuleName_bazelDep() throws Exception {
assertContainsEvent("invalid module name 'f.'");
}

@Test
public void badRepoName_module() throws Exception {
scratch.file(
rootDirectory.getRelative("MODULE.bazel").getPathString(),
"module(name='foo',version='0.1',repo_name='_foo')");

reporter.removeHandler(failFastHandler); // expect failures
evaluator.evaluate(ImmutableList.of(ModuleFileValue.KEY_FOR_ROOT_MODULE), evaluationContext);

assertContainsEvent("invalid user-provided repo name '_foo'");
}

@Test
public void badRepoName_bazelDep() throws Exception {
scratch.file(
Expand Down Expand Up @@ -833,4 +845,39 @@ public void testBuiltinModules_forBuiltinModules() throws Exception {
.addDep("foo", createModuleKey("foo", "2.0"))
.build());
}

@Test
public void moduleRepoName() throws Exception {
scratch.file(
rootDirectory.getRelative("MODULE.bazel").getPathString(),
"module(name='aaa',version='0.1',repo_name='bbb')");
FakeRegistry registry = registryFactory.newFakeRegistry("/foo");
ModuleFileFunction.REGISTRIES.set(differencer, ImmutableList.of(registry.getUrl()));

EvaluationResult<RootModuleFileValue> result =
evaluator.evaluate(
ImmutableList.of(ModuleFileValue.KEY_FOR_ROOT_MODULE), evaluationContext);
if (result.hasError()) {
fail(result.getError().toString());
}
RootModuleFileValue rootModuleFileValue = result.get(ModuleFileValue.KEY_FOR_ROOT_MODULE);
assertThat(rootModuleFileValue.getModule())
.isEqualTo(
ModuleBuilder.create("aaa", "0.1").setKey(ModuleKey.ROOT).setRepoName("bbb").build());
}

@Test
public void moduleRepoName_conflict() throws Exception {
scratch.file(
rootDirectory.getRelative("MODULE.bazel").getPathString(),
"module(name='aaa',version='0.1',repo_name='bbb')",
"bazel_dep(name='bbb',version='1.0')");
FakeRegistry registry = registryFactory.newFakeRegistry("/foo");
ModuleFileFunction.REGISTRIES.set(differencer, ImmutableList.of(registry.getUrl()));

reporter.removeHandler(failFastHandler); // expect failures
evaluator.evaluate(ImmutableList.of(ModuleFileValue.KEY_FOR_ROOT_MODULE), evaluationContext);

assertContainsEvent("The repo name 'bbb' is already being used as the module's own repo name");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ public void testSimpleMapping() throws Exception {
}

@Test
public void testRepoNameMapping_asMainModule() throws Exception {
public void testRepoNameMapping_asRootModule() throws Exception {
scratch.overwriteFile(
"MODULE.bazel",
"module(name='aaa',version='0.1')",
Expand Down Expand Up @@ -173,6 +173,35 @@ public void testRepoNameMapping_asMainModule() throws Exception {
name));
}

@Test
public void testRepoNameMapping_asRootModule_withOwnRepoName() throws Exception {
scratch.overwriteFile(
"MODULE.bazel",
"module(name='aaa',version='0.1',repo_name='haha')",
"bazel_dep(name='bbb',version='1.0', repo_name = 'com_foo_bar_b')");
registry.addModule(createModuleKey("bbb", "1.0"), "module(name='bbb', version='1.0')");

RepositoryName name = RepositoryName.MAIN;
SkyKey skyKey = RepositoryMappingValue.key(name);
EvaluationResult<RepositoryMappingValue> result = eval(skyKey);

assertThat(result.hasError()).isFalse();
assertThatEvaluationResult(result)
.hasEntryThat(skyKey)
.isEqualTo(
withMappingForRootModule(
ImmutableMap.of(
"",
RepositoryName.MAIN,
"haha",
RepositoryName.MAIN,
TestConstants.WORKSPACE_NAME,
RepositoryName.MAIN,
"com_foo_bar_b",
RepositoryName.create("bbb~1.0")),
name));
}

@Test
public void testRepoNameMapping_asDependency() throws Exception {
scratch.overwriteFile(
Expand Down

0 comments on commit 73ed74e

Please sign in to comment.