From 6a8ddb7630ab5bf8446da2e4816b81557ff837c0 Mon Sep 17 00:00:00 2001
From: Googler <wyv@google.com>
Date: Thu, 13 Oct 2022 09:55:07 -0700
Subject: [PATCH] Bzlmod: change main repo runfiles/execpath prefix to "_main"
See code comment for more explanation.
PiperOrigin-RevId: 480917215
Change-Id: I3a86d2bf45aeaa964cc8b101a6ecef0aab7238d7
---
.../google/devtools/build/lib/skyframe/BUILD | 3 ++
.../lib/skyframe/WorkspaceNameFunction.java | 19 +++++++++++++
.../skyframe/WorkspaceNameFunctionTest.java | 9 ++++++
src/test/shell/bazel/BUILD | 10 +++++--
src/test/shell/bazel/execroot_test.sh | 26 +++++++++++++++++
src/test/shell/bazel/runfiles_test.sh | 28 +++++++++++++++++++
6 files changed, 93 insertions(+), 2 deletions(-)
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
index 583040e2a3c964..18c7e3f8d80085 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
@@ -2898,11 +2898,14 @@ java_library(
srcs = ["WorkspaceNameFunction.java"],
deps = [
":package_value",
+ ":precomputed_value",
":workspace_name_value",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/packages",
+ "//src/main/java/com/google/devtools/build/lib/packages/semantics",
"//src/main/java/com/google/devtools/build/skyframe",
"//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
+ "//src/main/java/net/starlark/java/eval",
"//third_party:jsr305",
],
)
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceNameFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceNameFunction.java
index 27db9a71e89672..725d040f1540b6 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceNameFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceNameFunction.java
@@ -16,11 +16,13 @@
import com.google.devtools.build.lib.cmdline.LabelConstants;
import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException;
import com.google.devtools.build.lib.packages.Package;
+import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunctionException;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import javax.annotation.Nullable;
+import net.starlark.java.eval.StarlarkSemantics;
/**
* {@link SkyFunction} for {@link WorkspaceNameValue}s.
@@ -33,6 +35,23 @@ public class WorkspaceNameFunction implements SkyFunction {
@Nullable
public SkyValue compute(SkyKey skyKey, Environment env)
throws InterruptedException, WorkspaceNameFunctionException {
+ StarlarkSemantics starlarkSemantics = PrecomputedValue.STARLARK_SEMANTICS.get(env);
+ if (starlarkSemantics == null) {
+ return null;
+ }
+ if (starlarkSemantics.getBool(BuildLanguageOptions.ENABLE_BZLMOD)) {
+ // When Bzlmod is enabled, we don't care what the "workspace name" specified in the WORKSPACE
+ // file is, and always use the static string "_main" instead. The workspace name returned by
+ // this SkyFunction is only used as the runfiles/execpath prefix for the main repo; for other
+ // repos, the canonical repo name is used. The canonical name of the main repo is the empty
+ // string, so we can't use that; instead, we just use a static string.
+ //
+ // "_main" was chosen because it's not a valid apparent repo name, which, coupled with the
+ // fact that no Bzlmod-generated canonical repo names are valid apparent repo names, means
+ // that a path passed to rlocation can go through repo mapping multiple times without any
+ // danger (i.e. going through repo mapping is idempotent).
+ return WorkspaceNameValue.withName("_main");
+ }
SkyKey externalPackageKey = PackageValue.key(LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER);
PackageValue externalPackageValue = (PackageValue) env.getValue(externalPackageKey);
if (externalPackageValue == null) {
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceNameFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceNameFunctionTest.java
index f4c60aea8c7a5c..04dcbc9f264f51 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceNameFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceNameFunctionTest.java
@@ -54,6 +54,15 @@ public void testNormal() throws Exception {
.isEqualTo(WorkspaceNameValue.withName("good"));
}
+ @Test
+ public void bzlmod() throws Exception {
+ setBuildLanguageOptions("--enable_bzlmod");
+ scratch.overwriteFile("WORKSPACE", "workspace(name = 'good')");
+ assertThatEvaluationResult(eval())
+ .hasEntryThat(key)
+ .isEqualTo(WorkspaceNameValue.withName("_main"));
+ }
+
@Test
public void testErrorInExternalPkg() throws Exception {
reporter.removeHandler(failFastHandler);
diff --git a/src/test/shell/bazel/BUILD b/src/test/shell/bazel/BUILD
index 57b14aba2391b1..7fb9e679008127 100644
--- a/src/test/shell/bazel/BUILD
+++ b/src/test/shell/bazel/BUILD
@@ -832,7 +832,10 @@ sh_test(
size = "medium",
srcs = ["runfiles_test.sh"],
data = [":test-deps"],
- tags = ["no_windows"],
+ tags = [
+ "no_windows",
+ "requires-network",
+ ],
)
sh_test(
@@ -1013,7 +1016,10 @@ sh_test(
size = "medium",
srcs = ["execroot_test.sh"],
data = [":test-deps"],
- tags = ["no_windows"],
+ tags = [
+ "no_windows",
+ "requires-network",
+ ],
)
sh_test(
diff --git a/src/test/shell/bazel/execroot_test.sh b/src/test/shell/bazel/execroot_test.sh
index 9c312e8635ba74..228718291476a7 100755
--- a/src/test/shell/bazel/execroot_test.sh
+++ b/src/test/shell/bazel/execroot_test.sh
@@ -44,6 +44,32 @@ EOF
assert_contains "$(dirname $execroot)/${ws_name}/bazel-out" out
}
+function test_execroot_structure_with_bzlmod() {
+ cat > WORKSPACE <<EOF
+workspace(name = "whatever_doesnt_matter")
+EOF
+ cat > MODULE.bazel <<EOF
+module(name="this_also_doesnt_matter")
+EOF
+
+ mkdir dir
+ cat > dir/BUILD <<'EOF'
+genrule(
+ name = "use-srcs",
+ srcs = ["BUILD"],
+ cmd = "cp $< $@",
+ outs = ["used-srcs"],
+)
+EOF
+
+ bazel build --enable_bzlmod -s //dir:use-srcs &> $TEST_log \
+ || fail "expected success"
+ execroot="$(bazel info --enable_bzlmod execution_root)"
+ test -e "$execroot/../_main"
+ ls -l bazel-out | tee out
+ assert_contains "$(dirname $execroot)/_main/bazel-out" out
+}
+
function test_sibling_repository_layout() {
touch WORKSPACE
diff --git a/src/test/shell/bazel/runfiles_test.sh b/src/test/shell/bazel/runfiles_test.sh
index 99918ce0059a4a..04cfda90c80f7b 100755
--- a/src/test/shell/bazel/runfiles_test.sh
+++ b/src/test/shell/bazel/runfiles_test.sh
@@ -50,6 +50,34 @@ EOF
[[ -x bazel-bin/foo/foo.runfiles/$name/foo/foo ]] || fail "No foo executable under $name"
}
+function test_runfiles_bzlmod() {
+ create_workspace_with_default_repos WORKSPACE "blorp_malorp"
+ cat > MODULE.bazel <<EOF
+module(name="blep")
+EOF
+
+ mkdir foo
+ cat > foo/BUILD <<EOF
+java_test(
+ name = "foo",
+ srcs = ["Noise.java"],
+ test_class = "Noise",
+)
+EOF
+ cat > foo/Noise.java <<EOF
+public class Noise {
+ public static void main(String[] args) {
+ System.err.println(System.getenv("I'm a test."));
+ }
+}
+EOF
+
+ bazel build --enable_bzlmod //foo:foo >& $TEST_log || fail "Build failed"
+ [[ -d bazel-bin/foo/foo.runfiles/_main ]] || fail "_main runfiles directory not created"
+ [[ -d bazel-bin/foo/foo.runfiles/_main/foo ]] || fail "No foo subdirectory under _main"
+ [[ -x bazel-bin/foo/foo.runfiles/_main/foo/foo ]] || fail "No foo executable under _main"
+}
+
function test_legacy_runfiles_change() {
create_workspace_with_default_repos WORKSPACE foo
cat >> WORKSPACE <<EOF