Flip --enable_workspace to false

- Most test cases have migrated from WORKSPACE to MODULE.bazel - Some test cases still enable WORKSPACE because they test WORKSPACE functionalities. Part of #23253 Fixes #19824 Working towards #23023 PiperOrigin-RevId: 667901985 Change-Id: I35ee8050d835a62fed4cd7e6637bd0616766f3c4
bazelbuild · Aug 27, 2024 · 5881c38 · 5881c38
1 parent 9341288
commit 5881c38
Show file tree

Hide file tree

Showing 64 changed files with 597 additions and 528 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/LockFileModuleExtension.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/LockFileModuleExtension.java
@@ -20,6 +20,7 @@
 import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable;
 import com.google.devtools.build.lib.rules.repository.RepoRecordedInput;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import com.ryanharter.auto.value.gson.GenerateTypeAdapter;
 import java.util.Optional;
 
@@ -93,6 +94,7 @@ public abstract Builder setRecordedRepoMappingEntries(
    * A {@link LockFileModuleExtension} together with its {@link ModuleExtensionEvalFactors},
    * comprising a single lockfile entry for a certain extension.
    */
+  @AutoCodec
   public record WithFactors(
       ModuleExtensionEvalFactors extensionFactors, LockFileModuleExtension moduleExtension) {}
 }
diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryMapping.java b/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryMapping.java
@@ -113,7 +113,8 @@ public RepositoryMapping withAdditionalMappings(Map<String, RepositoryName> addi
    * repo of the given additional mappings is ignored.
    */
   public RepositoryMapping withAdditionalMappings(RepositoryMapping additionalMappings) {
-    return withAdditionalMappings(additionalMappings.entries());
+    return withAdditionalMappings(
+        (additionalMappings == null) ? ImmutableMap.of() : additionalMappings.entries());
   }
 
   /**

diff --git a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java
@@ -263,7 +263,7 @@ public final class BuildLanguageOptions extends OptionsBase {
 
   @Option(
       name = "enable_workspace",
-      defaultValue = "true",
+      defaultValue = "false",
       documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
       effectTags = OptionEffectTag.LOADING_AND_ANALYSIS,
       help =
@@ -914,7 +914,7 @@ public StarlarkSemantics toStarlarkSemantics() {
       "-experimental_enable_first_class_macros";
   public static final String EXPERIMENTAL_ENABLE_SCL_DIALECT = "-experimental_enable_scl_dialect";
   public static final String ENABLE_BZLMOD = "+enable_bzlmod";
-  public static final String ENABLE_WORKSPACE = "+enable_workspace";
+  public static final String ENABLE_WORKSPACE = "-enable_workspace";
   public static final String EXPERIMENTAL_ISOLATED_EXTENSION_USAGES =
       "-experimental_isolated_extension_usages";
   public static final String EXPERIMENTAL_GOOGLE_LEGACY_API = "-experimental_google_legacy_api";

diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/LocalRepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/rules/repository/LocalRepositoryFunction.java
@@ -37,6 +37,11 @@ public boolean isLocal(Rule rule) {
     return true;
   }
 
+  @Override
+  protected void setupRepoRootBeforeFetching(Path repoRoot) throws RepositoryFunctionException {
+    // Repo setup is already handled in RepositoryDelegatorFunction.symlinkRepoRoot
+  }
+
   @Override
   public RepositoryDirectoryValue.Builder fetch(
       Rule rule,

diff --git a/...main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java b/...main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java
@@ -51,6 +51,7 @@
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.lib.vfs.Root;
 import com.google.devtools.build.lib.vfs.RootedPath;
+import com.google.devtools.build.lib.vfs.Symlinks;
 import com.google.devtools.build.skyframe.SkyFunction;
 import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
 import com.google.devtools.build.skyframe.SkyKey;
@@ -486,7 +487,6 @@ private RepositoryDirectoryValue setupOverride(
       PathFragment sourcePath, Environment env, Path repoRoot, RepositoryName repoName)
       throws RepositoryFunctionException, InterruptedException {
     DigestWriter.clearMarkerFile(directories, repoName);
-    RepositoryFunction.setupRepoRoot(repoRoot);
     RepositoryDirectoryValue.Builder directoryValue =
         symlinkRepoRoot(
             directories,
@@ -509,8 +509,15 @@ public static RepositoryDirectoryValue.Builder symlinkRepoRoot(
       String userDefinedPath,
       Environment env)
       throws RepositoryFunctionException, InterruptedException {
+    if (source.isDirectory(Symlinks.NOFOLLOW)) {
+      try {
+        source.deleteTree();
+      } catch (IOException e) {
+        throw new RepositoryFunctionException(e, Transience.TRANSIENT);
+      }
+    }
     try {
-      source.createSymbolicLink(destination);
+      FileSystemUtils.ensureSymbolicLink(source, destination);
     } catch (IOException e) {
       throw new RepositoryFunctionException(
           new IOException(

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RepositoryMappingValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/RepositoryMappingValue.java
@@ -144,9 +144,9 @@ public abstract static class Key implements SkyKey {
      * Whether the root module should see repos defined in WORKSPACE. This only takes effect when
      * {@link #repoName} is the main repo.
      */
-    abstract boolean rootModuleShouldSeeWorkspaceRepos();
+    public abstract boolean rootModuleShouldSeeWorkspaceRepos();
 
-    static Key create(RepositoryName repoName, boolean rootModuleShouldSeeWorkspaceRepos) {
+    public static Key create(RepositoryName repoName, boolean rootModuleShouldSeeWorkspaceRepos) {
       return interner.intern(
           new AutoValue_RepositoryMappingValue_Key(repoName, rootModuleShouldSeeWorkspaceRepos));
     }

diff --git a/src/main/starlark/builtins_bzl/common/cc/cc_helper.bzl b/src/main/starlark/builtins_bzl/common/cc/cc_helper.bzl
@@ -178,7 +178,7 @@ def _get_dynamic_library_for_runtime_or_none(library, linking_statically):
 
     return library.dynamic_library
 
-_CPP_TOOLCHAIN_TYPE = "@" + objc_semantics.get_repo() + "//tools/cpp:toolchain_type"
+_CPP_TOOLCHAIN_TYPE = "@@" + objc_semantics.get_repo() + "//tools/cpp:toolchain_type"
 
 def _find_cpp_toolchain(ctx, *, mandatory = True):
     """

diff --git a/src/test/java/com/google/devtools/build/lib/analysis/DeprecationValidatorTest.java b/src/test/java/com/google/devtools/build/lib/analysis/DeprecationValidatorTest.java
@@ -15,8 +15,6 @@
 package com.google.devtools.build.lib.analysis;
 
 import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
-import java.io.OutputStream;
-import java.nio.charset.StandardCharsets;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -84,35 +82,35 @@ public void deprecationWarningForDifferentPackage() throws Exception {
 
   @Test
   public void deprecationWarningForSamePackageInDifferentRepository() throws Exception {
-    try (OutputStream output = scratch.resolve("WORKSPACE").getOutputStream(/* append= */ true)) {
-      output.write(
-          "\nlocal_repository(name = 'r', path = '/r')\n".getBytes(StandardCharsets.UTF_8));
-    }
-    scratch.file("/r/WORKSPACE", "workspace(name = 'r')");
+    scratch.appendFile(
+        "MODULE.bazel",
+        "bazel_dep(name = 'r')",
+        "local_path_override(module_name = 'r', path = '/r')");
+    scratch.file("/r/MODULE.bazel", "module(name = 'r')");
     scratch.file("/r/a/BUILD", "filegroup(name='b', deprecation='deprecation warning printed')");
     invalidatePackages();
     checkWarning(
         "a",
         "a",
-        "target '//a:a' depends on deprecated target '@@r//a:b': deprecation warning printed",
+        "target '//a:a' depends on deprecated target '@@r+//a:b': deprecation warning printed",
         "filegroup(name='a', srcs=['@r//a:b'])");
   }
 
   @Test
   public void deprecationWarningForJavatestsCompanionOfJavaPackageInDifferentRepository()
       throws Exception {
-    try (OutputStream output = scratch.resolve("WORKSPACE").getOutputStream(/* append= */ true)) {
-      output.write(
-          "\nlocal_repository(name = 'r', path = '/r')\n".getBytes(StandardCharsets.UTF_8));
-    }
-    scratch.file("/r/WORKSPACE", "workspace(name = 'r')");
+    scratch.appendFile(
+        "MODULE.bazel",
+        "bazel_dep(name = 'r')",
+        "local_path_override(module_name = 'r', path = '/r')");
+    scratch.file("/r/MODULE.bazel", "module(name = 'r')");
     scratch.file(
         "/r/java/a/BUILD", "filegroup(name='b', deprecation='deprecation warning printed')");
     invalidatePackages();
     checkWarning(
         "javatests/a",
         "a",
-        "target '//javatests/a:a' depends on deprecated target '@@r//java/a:b': "
+        "target '//javatests/a:a' depends on deprecated target '@@r+//java/a:b': "
             + "deprecation warning printed",
         "filegroup(name='a', srcs=['@r//java/a:b'])");
   }

diff --git a/src/test/java/com/google/devtools/build/lib/analysis/LocationExpanderIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/analysis/LocationExpanderIntegrationTest.java
@@ -170,8 +170,7 @@ public void otherPathExpansion() throws Exception {
             srcs = [":foo"],
         )
         """);
-
-    FileSystemUtils.appendIsoLatin1(scratch.resolve("WORKSPACE"), "workspace(name='workspace')");
+    scratch.overwriteFile("MODULE.bazel", "module(name='workspace')");
     // Invalidate WORKSPACE to pick up the name.
     getSkyframeExecutor()
         .invalidateFilesUnderPathForTesting(
@@ -209,92 +208,102 @@ public void otherPathExternalExpansionLegacyExternalRunfiles() throws Exception
     scratch.file(
         "expansion/BUILD",
         "sh_library(name='lib', srcs=['@r//p:foo'])");
-    FileSystemUtils.appendIsoLatin1(
-        scratch.resolve("WORKSPACE"), "local_repository(name='r', path='/r')");
+    scratch.appendFile(
+        "MODULE.bazel",
+        "bazel_dep(name = 'r')",
+        "local_path_override(module_name = 'r', path = '/r')");
 
     // Invalidate WORKSPACE so @r can be resolved.
     getSkyframeExecutor()
         .invalidateFilesUnderPathForTesting(
             reporter, ModifiedFileSet.EVERYTHING_MODIFIED, Root.fromPath(rootDirectory));
 
     FileSystemUtils.createDirectoryAndParents(scratch.resolve("/foo/bar"));
-    scratch.file("/r/WORKSPACE", "workspace(name = 'r')");
+    scratch.file("/r/MODULE.bazel", "module(name = 'r')");
     scratch.file("/r/p/BUILD", "genrule(name='foo', outs=['foo.txt'], cmd='never executed')");
 
     useConfiguration("--legacy_external_runfiles");
     LocationExpander expander = makeExpander("//expansion:lib");
     assertThat(expander.expand("foo $(execpath @r//p:foo) bar"))
-        .matches("foo .*-out/.*/external/r/p/foo\\.txt bar");
+        .matches("foo .*-out/.*/external/r\\+/p/foo\\.txt bar");
     assertThat(expander.expand("foo $(execpaths @r//p:foo) bar"))
-        .matches("foo .*-out/.*/external/r/p/foo\\.txt bar");
+        .matches("foo .*-out/.*/external/r\\+/p/foo\\.txt bar");
     assertThat(expander.expand("foo $(rootpath @r//p:foo) bar"))
-        .matches("foo external/r/p/foo.txt bar");
+        .matches("foo external/r\\+/p/foo.txt bar");
     assertThat(expander.expand("foo $(rootpaths @r//p:foo) bar"))
-        .matches("foo external/r/p/foo.txt bar");
+        .matches("foo external/r\\+/p/foo.txt bar");
     assertThat(expander.expand("foo $(rlocationpath @r//p:foo) bar"))
-        .isEqualTo("foo r/p/foo.txt bar");
+        .isEqualTo("foo r+/p/foo.txt bar");
     assertThat(expander.expand("foo $(rlocationpath @r//p:foo) bar"))
-        .isEqualTo("foo r/p/foo.txt bar");
+        .isEqualTo("foo r+/p/foo.txt bar");
   }
 
   @Test
   public void otherPathExternalExpansionNoLegacyExternalRunfiles() throws Exception {
     scratch.file("expansion/BUILD", "sh_library(name='lib', srcs=['@r//p:foo'])");
-    FileSystemUtils.appendIsoLatin1(
-        scratch.resolve("WORKSPACE"), "local_repository(name='r', path='/r')");
+    scratch.appendFile(
+        "MODULE.bazel",
+        "bazel_dep(name = 'r')",
+        "local_path_override(module_name = 'r', path = '/r')");
 
     // Invalidate WORKSPACE so @r can be resolved.
     getSkyframeExecutor()
         .invalidateFilesUnderPathForTesting(
             reporter, ModifiedFileSet.EVERYTHING_MODIFIED, Root.fromPath(rootDirectory));
 
     FileSystemUtils.createDirectoryAndParents(scratch.resolve("/foo/bar"));
-    scratch.file("/r/WORKSPACE", "workspace(name = 'r')");
+    scratch.file("/r/MODULE.bazel", "module(name = 'r')");
     scratch.file("/r/p/BUILD", "genrule(name='foo', outs=['foo.txt'], cmd='never executed')");
 
     useConfiguration("--nolegacy_external_runfiles");
     LocationExpander expander = makeExpander("//expansion:lib");
     assertThat(expander.expand("foo $(execpath @r//p:foo) bar"))
-        .matches("foo .*-out/.*/external/r/p/foo\\.txt bar");
+        .matches("foo .*-out/.*/external/r\\+/p/foo\\.txt bar");
     assertThat(expander.expand("foo $(execpaths @r//p:foo) bar"))
-        .matches("foo .*-out/.*/external/r/p/foo\\.txt bar");
-    assertThat(expander.expand("foo $(rootpath @r//p:foo) bar")).matches("foo ../r/p/foo.txt bar");
-    assertThat(expander.expand("foo $(rootpaths @r//p:foo) bar")).matches("foo ../r/p/foo.txt bar");
+        .matches("foo .*-out/.*/external/r\\+/p/foo\\.txt bar");
+    assertThat(expander.expand("foo $(rootpath @r//p:foo) bar"))
+        .matches("foo ../r\\+/p/foo.txt bar");
+    assertThat(expander.expand("foo $(rootpaths @r//p:foo) bar"))
+        .matches("foo ../r\\+/p/foo.txt bar");
     assertThat(expander.expand("foo $(rlocationpath @r//p:foo) bar"))
-        .isEqualTo("foo r/p/foo.txt bar");
+        .isEqualTo("foo r+/p/foo.txt bar");
     assertThat(expander.expand("foo $(rlocationpath @r//p:foo) bar"))
-        .isEqualTo("foo r/p/foo.txt bar");
+        .isEqualTo("foo r+/p/foo.txt bar");
   }
 
   @Test
   public void otherPathExternalExpansionNoLegacyExternalRunfilesSiblingRepositoryLayout()
       throws Exception {
     scratch.file("expansion/BUILD", "sh_library(name='lib', srcs=['@r//p:foo'])");
-    FileSystemUtils.appendIsoLatin1(
-        scratch.resolve("WORKSPACE"), "local_repository(name='r', path='/r')");
+    scratch.appendFile(
+        "MODULE.bazel",
+        "bazel_dep(name = 'r')",
+        "local_path_override(module_name = 'r', path = '/r')");
 
     // Invalidate WORKSPACE so @r can be resolved.
     getSkyframeExecutor()
         .invalidateFilesUnderPathForTesting(
             reporter, ModifiedFileSet.EVERYTHING_MODIFIED, Root.fromPath(rootDirectory));
 
     FileSystemUtils.createDirectoryAndParents(scratch.resolve("/foo/bar"));
-    scratch.file("/r/WORKSPACE", "workspace(name = 'r')");
+    scratch.file("/r/MODULE.bazel", "module(name = 'r')");
     scratch.file("/r/p/BUILD", "genrule(name='foo', outs=['foo.txt'], cmd='never executed')");
 
     useConfiguration("--nolegacy_external_runfiles");
     setBuildLanguageOptions("--experimental_sibling_repository_layout");
     LocationExpander expander = makeExpander("//expansion:lib");
     assertThat(expander.expand("foo $(execpath @r//p:foo) bar"))
-        .matches("foo .*-out/r/.*/p/foo\\.txt bar");
+        .matches("foo .*-out/r\\+/.*/p/foo\\.txt bar");
     assertThat(expander.expand("foo $(execpaths @r//p:foo) bar"))
-        .matches("foo .*-out/r/.*/p/foo\\.txt bar");
-    assertThat(expander.expand("foo $(rootpath @r//p:foo) bar")).matches("foo ../r/p/foo.txt bar");
-    assertThat(expander.expand("foo $(rootpaths @r//p:foo) bar")).matches("foo ../r/p/foo.txt bar");
+        .matches("foo .*-out/r\\+/.*/p/foo\\.txt bar");
+    assertThat(expander.expand("foo $(rootpath @r//p:foo) bar"))
+        .matches("foo ../r\\+/p/foo.txt bar");
+    assertThat(expander.expand("foo $(rootpaths @r//p:foo) bar"))
+        .matches("foo ../r\\+/p/foo.txt bar");
     assertThat(expander.expand("foo $(rlocationpath @r//p:foo) bar"))
-        .isEqualTo("foo r/p/foo.txt bar");
+        .isEqualTo("foo r+/p/foo.txt bar");
     assertThat(expander.expand("foo $(rlocationpaths @r//p:foo) bar"))
-        .isEqualTo("foo r/p/foo.txt bar");
+        .isEqualTo("foo r+/p/foo.txt bar");
   }
 
   @Test

diff --git a/src/test/java/com/google/devtools/build/lib/analysis/RunfilesRepoMappingManifestTest.java b/src/test/java/com/google/devtools/build/lib/analysis/RunfilesRepoMappingManifestTest.java
@@ -145,12 +145,11 @@ public void diamond() throws Exception {
     }
 
     // Called last as it triggers package invalidation, which requires a valid MODULE.bazel setup.
-    rewriteWorkspace("workspace(name='aaa_ws')");
+    invalidatePackages();
 
     assertThat(getRepoMappingManifestForTarget("//:aaa"))
         .containsExactly(
             ",aaa," + getRuleClassProvider().getRunfilesPrefix(),
-            ",aaa_ws," + getRuleClassProvider().getRunfilesPrefix(),
             ",bbb,bbb+",
             "bbb+,bbb,bbb+",
             "bbb+,ddd,ddd+",
@@ -215,18 +214,18 @@ public void runfilesFromToolchain() throws Exception {
         "tooled_binary(name='tooled')");
 
     // Called last as it triggers package invalidation, which requires a valid MODULE.bazel setup.
-    rewriteWorkspace("workspace(name='main')");
+    invalidatePackages();
 
     assertThat(getRepoMappingManifestForTarget("//:tooled"))
         .containsExactly(
-            ",main," + getRuleClassProvider().getRunfilesPrefix(),
             "bare_rule+,bare_rule,bare_rule+",
             "tooled_rule+,bare_rule,bare_rule+")
         .inOrder();
   }
 
   @Test
   public void actionRerunsOnRepoMappingChange_workspaceName() throws Exception {
+    setBuildLanguageOptions("--enable_workspace");
     overwriteWorkspaceFile("workspace(name='aaa_ws')");
     scratch.overwriteFile(
         "MODULE.bazel",

diff --git a/src/test/java/com/google/devtools/build/lib/analysis/mock/BUILD b/src/test/java/com/google/devtools/build/lib/analysis/mock/BUILD
@@ -29,8 +29,10 @@ java_library(
         "//tools/cpp:cc_toolchain_config_lib.bzl",
     ],
     deps = [
+        "//src/main/java/com/google/devtools/build/lib:runtime",
         "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
         "//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",
+        "//src/main/java/com/google/devtools/build/lib/bazel:repository_module",
         "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:resolution",
         "//src/main/java/com/google/devtools/build/lib/bazel/repository",
         "//src/main/java/com/google/devtools/build/lib/bazel/rules",