Skip to content

Commit

Permalink
WIP: Ensure that repository names don't start with ~
Browse files Browse the repository at this point in the history
  • Loading branch information
fmeum committed Oct 26, 2022
1 parent 6669a4f commit f943c03
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -366,8 +366,16 @@ static BazelModuleResolutionValue createValue(
// Calculate a unique name for each used extension id.
BiMap<String, ModuleExtensionId> extensionUniqueNames = HashBiMap.create();
for (ModuleExtensionId id : extensionUsagesById.rowKeySet()) {
String bestName =
id.getBzlFileLabel().getRepository().getName() + "~" + id.getExtensionName();
// Ensure that the resulting extension name (and thus the repository names derived from it) do
// not start with a tilde.
String repoName = id.getBzlFileLabel().getRepository().getName();
String nonEmptyRepoPart;
if (repoName.isEmpty()) {
nonEmptyRepoPart = "_main";
} else {
nonEmptyRepoPart = repoName;
}
String bestName = nonEmptyRepoPart + "~" + id.getExtensionName();
if (extensionUniqueNames.putIfAbsent(bestName, id) == null) {
continue;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,10 @@ public final class RepositoryName {

@SerializationConstant public static final RepositoryName MAIN = new RepositoryName("");

private static final Pattern VALID_REPO_NAME = Pattern.compile("[\\w\\-.~]*");
// Repository names must not start with a tilde as shells treat unescaped paths starting with them
// specially.
// https://www.gnu.org/software/bash/manual/html_node/Tilde-Expansion.html
private static final Pattern VALID_REPO_NAME = Pattern.compile("|[\\w\\-.][\\w\\-.~]*");

// Must start with a letter. Can contain ASCII letters and digits, underscore, dash, and dot.
private static final Pattern VALID_USER_PROVIDED_NAME = Pattern.compile("[a-zA-Z][-.\\w]*$");
Expand Down Expand Up @@ -156,7 +159,7 @@ static void validate(String name) throws LabelSyntaxException {
if (!VALID_REPO_NAME.matcher(name).matches()) {
throw LabelParser.syntaxErrorf(
"invalid repository name '@%s': repo names may contain only A-Z, a-z, 0-9, '-', '_', '.'"
+ " and '~'",
+ " and '~' and must not start with '~'",
StringUtilities.sanitizeControlChars(name));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1041,7 +1041,7 @@ public void generatedReposHaveCorrectMappings_strictDepsViolation() throws Excep
assertThat(result.hasError()).isTrue();
assertThat(result.getError().getException())
.hasMessageThat()
.contains("No repository visible as '@foo' from repository '@~ext~ext'");
.contains("No repository visible as '@foo' from repository '@_main~ext~ext'");
}

@Test
Expand Down Expand Up @@ -1082,7 +1082,7 @@ public void importNonExistentRepo() throws Exception {
scratch.file(workspaceRoot.getRelative("BUILD").getPathString());
scratch.file(
workspaceRoot.getRelative("data.bzl").getPathString(),
"load('@@~ext~ext//:data.bzl', ext_data='data')",
"load('@@_main~ext~ext//:data.bzl', ext_data='data')",
"data=ext_data");

SkyKey skyKey = BzlLoadValue.keyForBuild(Label.parseCanonical("//:data.bzl"));
Expand Down Expand Up @@ -1216,7 +1216,7 @@ public void extensionRepoCtxReadsFromAnotherExtensionRepo() throws Exception {
workspaceRoot.getRelative("defs.bzl").getPathString(),
"load('@data_repo//:defs.bzl','data_repo')",
"def _ext_impl(ctx):",
" data_file = ctx.read(Label('@@~my_ext2~candy2//:data.bzl'))",
" data_file = ctx.read(Label('@@_main~my_ext2~candy2//:data.bzl'))",
" data_repo(name='candy1',data=data_file)",
"my_ext=module_extension(implementation=_ext_impl)",
"def _ext_impl2(ctx):",
Expand Down Expand Up @@ -1264,7 +1264,7 @@ public void testReportRepoAndBzlCycles_circularExtReposCtxRead() throws Exceptio
SkyKey skyKey =
PackageValue.key(
PackageIdentifier.create(
RepositoryName.createUnvalidated("~my_ext~candy1"), PathFragment.EMPTY_FRAGMENT));
RepositoryName.createUnvalidated("_main~my_ext~candy1"), PathFragment.EMPTY_FRAGMENT));
EvaluationResult<PackageValue> result =
evaluator.evaluate(ImmutableList.of(skyKey), evaluationContext);
assertThat(result.hasError()).isTrue();
Expand All @@ -1275,11 +1275,11 @@ public void testReportRepoAndBzlCycles_circularExtReposCtxRead() throws Exceptio
assertContainsEvent(
"ERROR <no location>: Circular definition of repositories generated by module extensions"
+ " and/or .bzl files:\n"
+ ".-> @~my_ext~candy1\n"
+ ".-> @_main~my_ext~candy1\n"
+ "| extension 'my_ext' defined in //:defs.bzl\n"
+ "| @~my_ext2~candy2\n"
+ "| @_main~my_ext2~candy2\n"
+ "| extension 'my_ext2' defined in //:defs.bzl\n"
+ "`-- @~my_ext~candy1");
+ "`-- @_main~my_ext~candy1");
}

@Test
Expand Down Expand Up @@ -1310,7 +1310,7 @@ public void testReportRepoAndBzlCycles_circularExtReposLoadInDefFile() throws Ex
SkyKey skyKey =
PackageValue.key(
PackageIdentifier.create(
RepositoryName.createUnvalidated("~my_ext~candy1"),
RepositoryName.createUnvalidated("_main~my_ext~candy1"),
PathFragment.create("data.bzl")));
EvaluationResult<PackageValue> result =
evaluator.evaluate(ImmutableList.of(skyKey), evaluationContext);
Expand All @@ -1322,13 +1322,13 @@ public void testReportRepoAndBzlCycles_circularExtReposLoadInDefFile() throws Ex
assertContainsEvent(
"ERROR <no location>: Circular definition of repositories generated by module extensions"
+ " and/or .bzl files:\n"
+ ".-> @~my_ext~candy1\n"
+ ".-> @_main~my_ext~candy1\n"
+ "| extension 'my_ext' defined in //:defs.bzl\n"
+ "| @~my_ext2~candy2\n"
+ "| @_main~my_ext2~candy2\n"
+ "| extension 'my_ext2' defined in //:defs2.bzl\n"
+ "| //:defs2.bzl\n"
+ "| @~my_ext~candy1//:data.bzl\n"
+ "`-- @~my_ext~candy1");
+ "| @_main~my_ext~candy1//:data.bzl\n"
+ "`-- @_main~my_ext~candy1");
}

@Test
Expand All @@ -1350,7 +1350,7 @@ public void testReportRepoAndBzlCycles_extRepoLoadSelfCycle() throws Exception {
SkyKey skyKey =
PackageValue.key(
PackageIdentifier.create(
RepositoryName.createUnvalidated("~my_ext~candy1"),
RepositoryName.createUnvalidated("_main~my_ext~candy1"),
PathFragment.create("data.bzl")));
EvaluationResult<PackageValue> result =
evaluator.evaluate(ImmutableList.of(skyKey), evaluationContext);
Expand All @@ -1362,10 +1362,10 @@ public void testReportRepoAndBzlCycles_extRepoLoadSelfCycle() throws Exception {
assertContainsEvent(
"ERROR <no location>: Circular definition of repositories generated by module extensions"
+ " and/or .bzl files:\n"
+ ".-> @~my_ext~candy1\n"
+ ".-> @_main~my_ext~candy1\n"
+ "| extension 'my_ext' defined in //:defs.bzl\n"
+ "| //:defs.bzl\n"
+ "| @~my_ext~candy1//:data.bzl\n"
+ "`-- @~my_ext~candy1");
+ "| @_main~my_ext~candy1//:data.bzl\n"
+ "`-- @_main~my_ext~candy1");
}
}

0 comments on commit f943c03

Please sign in to comment.