From 07c5c1aa6d0b63605ae793dce78d26122af64a84 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Thu, 27 Oct 2022 06:02:46 -0700 Subject: [PATCH] Ensure repository names don't start with `~` By ensuring that repository names do not start with `~`, runfiles paths are certain not to begin with tildes, which means that they don't need to be shell escaped. Prevents https://github.com/bazelbuild/bazel/pull/16560#discussion_r1005737824 Closes #16564. PiperOrigin-RevId: 484232215 Change-Id: Ie70940aa709f6c7a886564189a3579780731dca6 --- .../bzlmod/BazelModuleResolutionFunction.java | 12 +++++-- .../build/lib/cmdline/RepositoryName.java | 7 ++-- .../bzlmod/ModuleExtensionResolutionTest.java | 33 ++++++++++--------- src/test/py/bazel/bzlmod/bazel_module_test.py | 2 +- 4 files changed, 33 insertions(+), 21 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunction.java index 61d99cae0bbf7b..033f02894dd6d1 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunction.java @@ -366,8 +366,16 @@ static BazelModuleResolutionValue createValue( // Calculate a unique name for each used extension id. BiMap 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. + RepositoryName repository = id.getBzlFileLabel().getRepository(); + String nonEmptyRepoPart; + if (repository.isMain()) { + nonEmptyRepoPart = "_main"; + } else { + nonEmptyRepoPart = repository.getName(); + } + String bestName = nonEmptyRepoPart + "~" + id.getExtensionName(); if (extensionUniqueNames.putIfAbsent(bestName, id) == null) { continue; } 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 88aee8cee76560..2f9f6a0ac8b025 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 @@ -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]*$"); @@ -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)); } } diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java index 6077c231a35e1d..20479b33349a04 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java @@ -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 @@ -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")); @@ -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):", @@ -1264,7 +1264,8 @@ 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 result = evaluator.evaluate(ImmutableList.of(skyKey), evaluationContext); assertThat(result.hasError()).isTrue(); @@ -1275,11 +1276,11 @@ public void testReportRepoAndBzlCycles_circularExtReposCtxRead() throws Exceptio assertContainsEvent( "ERROR : 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 @@ -1310,7 +1311,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 result = evaluator.evaluate(ImmutableList.of(skyKey), evaluationContext); @@ -1322,13 +1323,13 @@ public void testReportRepoAndBzlCycles_circularExtReposLoadInDefFile() throws Ex assertContainsEvent( "ERROR : 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 @@ -1350,7 +1351,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 result = evaluator.evaluate(ImmutableList.of(skyKey), evaluationContext); @@ -1362,10 +1363,10 @@ public void testReportRepoAndBzlCycles_extRepoLoadSelfCycle() throws Exception { assertContainsEvent( "ERROR : 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"); } } diff --git a/src/test/py/bazel/bzlmod/bazel_module_test.py b/src/test/py/bazel/bzlmod/bazel_module_test.py index 4822ff255ac93b..5312a26de2e708 100644 --- a/src/test/py/bazel/bzlmod/bazel_module_test.py +++ b/src/test/py/bazel/bzlmod/bazel_module_test.py @@ -371,7 +371,7 @@ def testRepositoryRuleErrorInModuleExtensionFailsTheBuild(self): allow_failure=True) self.AssertExitCode(exit_code, 48, stderr) self.assertIn( - "ERROR: : //pkg:~module_ext~foo: no such attribute 'invalid_attr' in 'repo_rule' rule", + "ERROR: : //pkg:_main~module_ext~foo: no such attribute 'invalid_attr' in 'repo_rule' rule", stderr) self.assertTrue( any([