Simplify

bazelbuild · Dec 1, 2023 · f66a696 · f66a696
1 parent 4b1ae36
commit f66a696
Show file tree

Hide file tree

Showing 7 changed files with 32 additions and 27 deletions.
diff --git a/...ava/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java b/...ava/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java
@@ -44,7 +44,6 @@
 import com.google.devtools.build.lib.packages.RuleFactory.InvalidRuleException;
 import com.google.devtools.build.lib.packages.RuleFunction;
 import com.google.devtools.build.lib.packages.StarlarkExportable;
-import com.google.devtools.build.lib.packages.WorkspaceFactory;
 import com.google.devtools.build.lib.packages.WorkspaceFactoryHelper;
 import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
 import com.google.devtools.build.lib.starlarkbuildapi.repository.RepositoryModuleApi;
@@ -260,8 +259,7 @@ private Object createRuleLegacy(StarlarkThread thread, Dict<String, Object> kwar
             ruleClass,
             /* bindRuleClass= */ null,
             WorkspaceFactoryHelper.getFinalKwargs(kwargs),
-            thread.getCallStack(),
-            thread.getSemantics().getBool(WorkspaceFactory.EVALUATE_FOR_WORKSPACE_SUFFIX));
+            thread.getCallStack());
       } catch (InvalidRuleException | NameConflictException | LabelSyntaxException e) {
         throw Starlark.errorf("%s", e.getMessage());
       }

diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
@@ -44,9 +44,6 @@
 /** Parser for WORKSPACE files. Fills in an ExternalPackage.Builder */
 public class WorkspaceFactory {
 
-  public static final String DEFAULT_WORKSPACE_SUFFIX_FILE = "/DEFAULT.WORKSPACE.SUFFIX";
-  public static final String EVALUATE_FOR_WORKSPACE_SUFFIX = "-evaluate_for_workspace_suffix";
-
   private final Package.Builder builder;
 
   private final Path installDir;
@@ -120,12 +117,7 @@ public void execute(
       Program prog = Program.compileFile(file, module);
 
       // create thread
-      StarlarkSemantics fileStarlarkSemantics = starlarkSemantics;
-      if (file.getName().equals(DEFAULT_WORKSPACE_SUFFIX_FILE)) {
-        fileStarlarkSemantics =
-            fileStarlarkSemantics.toBuilder().setBool(EVALUATE_FOR_WORKSPACE_SUFFIX, true).build();
-      }
-      StarlarkThread thread = new StarlarkThread(mutability, fileStarlarkSemantics);
+      StarlarkThread thread = new StarlarkThread(mutability, starlarkSemantics);
       thread.setLoader(loadedModules::get);
       thread.setPrintHandler(Event.makeDebugPrintHandler(localReporter));
       thread.setThreadLocal(
@@ -281,8 +273,7 @@ public Object call(StarlarkThread thread, Tuple args, Dict<String, Object> kwarg
                   ruleClass,
                   bindRuleClass,
                   WorkspaceFactoryHelper.getFinalKwargs(kwargs),
-                  thread.getCallStack(),
-                  thread.getSemantics().getBool(EVALUATE_FOR_WORKSPACE_SUFFIX));
+                  thread.getCallStack());
           RepositoryName.validateUserProvidedRepoName(rule.getName());
         } catch (RuleFactory.InvalidRuleException
             | Package.NameConflictException

diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactoryHelper.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactoryHelper.java
@@ -36,14 +36,20 @@
 /** A helper for the {@link WorkspaceFactory} to create repository rules */
 public final class WorkspaceFactoryHelper {
 
+  public static final String DEFAULT_WORKSPACE_SUFFIX_FILE = "/DEFAULT.WORKSPACE.SUFFIX";
+
+  public static boolean originatesInWorkspaceSuffix(
+      ImmutableList<StarlarkThread.CallStackEntry> callstack) {
+    return callstack.get(0).location.file().equals(DEFAULT_WORKSPACE_SUFFIX_FILE);
+  }
+
   @CanIgnoreReturnValue
   public static Rule createAndAddRepositoryRule(
       Package.Builder pkg,
       RuleClass ruleClass,
       RuleClass bindRuleClass,
       Map<String, Object> kwargs,
-      ImmutableList<StarlarkThread.CallStackEntry> callstack,
-      boolean forWorkspaceSuffix)
+      ImmutableList<StarlarkThread.CallStackEntry> callstack)
       throws RuleFactory.InvalidRuleException,
           Package.NameConflictException,
           LabelSyntaxException,
@@ -71,7 +77,7 @@ public static Rule createAndAddRepositoryRule(
         throw new LabelSyntaxException(e.getMessage());
       }
     }
-    pkg.addRegisteredToolchains(toolchains.build(), forWorkspaceSuffix);
+    pkg.addRegisteredToolchains(toolchains.build(), originatesInWorkspaceSuffix(callstack));
     return rule;
   }
 

diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceGlobals.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceGlobals.java
@@ -14,6 +14,7 @@
 
 package com.google.devtools.build.lib.packages;
 
+import static com.google.devtools.build.lib.packages.WorkspaceFactoryHelper.originatesInWorkspaceSuffix;
 import static net.starlark.java.eval.Starlark.NONE;
 
 import com.google.common.collect.ImmutableList;
@@ -51,7 +52,9 @@ public WorkspaceGlobals(
   }
 
   @Override
-  public void workspace(String name, StarlarkThread thread)
+  public void workspace(
+      String name,
+      StarlarkThread thread)
       throws EvalException, InterruptedException {
     if (!allowWorkspaceFunction) {
       throw Starlark.errorf(
@@ -114,7 +117,7 @@ public void registerToolchains(Sequence<?> toolchainLabels, StarlarkThread threa
     List<String> patterns = Sequence.cast(toolchainLabels, String.class, "toolchain_labels");
     builder.addRegisteredToolchains(
         parsePatterns(patterns, builder, thread),
-        thread.getSemantics().getBool(WorkspaceFactory.EVALUATE_FOR_WORKSPACE_SUFFIX));
+        originatesInWorkspaceSuffix(thread.getCallStack()));
   }
 
   @Override

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java
@@ -14,6 +14,7 @@
 
 package com.google.devtools.build.lib.skyframe;
 
+import static com.google.devtools.build.lib.packages.WorkspaceFactoryHelper.DEFAULT_WORKSPACE_SUFFIX_FILE;
 import static com.google.devtools.build.lib.rules.repository.ResolvedFileValue.ATTRIBUTES;
 import static com.google.devtools.build.lib.rules.repository.ResolvedFileValue.NATIVE;
 import static com.google.devtools.build.lib.rules.repository.ResolvedFileValue.REPOSITORIES;
@@ -40,6 +41,7 @@
 import com.google.devtools.build.lib.packages.PackageFactory;
 import com.google.devtools.build.lib.packages.RuleClassProvider;
 import com.google.devtools.build.lib.packages.WorkspaceFactory;
+import com.google.devtools.build.lib.packages.WorkspaceFactoryHelper;
 import com.google.devtools.build.lib.packages.WorkspaceFileValue;
 import com.google.devtools.build.lib.packages.WorkspaceFileValue.WorkspaceFileKey;
 import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
@@ -220,8 +222,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
       StarlarkFile file =
           StarlarkFile.parse(
               ParserInput.fromString(
-                  ruleClassProvider.getDefaultWorkspaceSuffix(),
-                  WorkspaceFactory.DEFAULT_WORKSPACE_SUFFIX_FILE),
+                  ruleClassProvider.getDefaultWorkspaceSuffix(), DEFAULT_WORKSPACE_SUFFIX_FILE),
               // The DEFAULT.WORKSPACE.SUFFIX file breaks through the usual privacy mechanism.
               options.toBuilder().allowLoadPrivateSymbols(true).build());
       if (!file.ok()) {

diff --git a/...om/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContextTest.java b/...om/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContextTest.java
@@ -149,8 +149,7 @@ private void setUpContextForRule(
             buildRuleClass(attributes),
             null,
             kwargs,
-            DUMMY_STACK,
-            /* forWorkspaceSuffix= */ false);
+            DUMMY_STACK);
     DownloadManager downloader = Mockito.mock(DownloadManager.class);
     SkyFunction.Environment environment = Mockito.mock(SkyFunction.Environment.class);
     when(environment.getListener()).thenReturn(listener);

diff --git a/src/test/java/com/google/devtools/build/lib/repository/ExternalPackageHelperTest.java b/src/test/java/com/google/devtools/build/lib/repository/ExternalPackageHelperTest.java
@@ -82,6 +82,7 @@
 import java.util.Optional;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.stream.Stream;
 import javax.annotation.Nullable;
 import org.junit.Before;
 import org.junit.Test;
@@ -411,13 +412,19 @@ private static final class GetRegisteredToolchainsFunction implements SkyFunctio
     @Override
     public SkyValue compute(SkyKey skyKey, Environment env)
         throws SkyFunctionException, InterruptedException {
-      List<TargetPattern> registeredToolchains =
-          RegisteredToolchainsFunction.getWorkspaceToolchains(env);
-      if (registeredToolchains == null) {
+      List<TargetPattern> userRegisteredToolchains =
+          RegisteredToolchainsFunction.getWorkspaceToolchains(env, /* userRegistered= */ true);
+      if (userRegisteredToolchains == null) {
+        return null;
+      }
+      List<TargetPattern> workspaceSuffixRegisteredToolchains =
+          RegisteredToolchainsFunction.getWorkspaceToolchains(env, /* userRegistered= */ false);
+      if (workspaceSuffixRegisteredToolchains == null) {
         return null;
       }
       return GetRegisteredToolchainsValue.create(
-          registeredToolchains.stream()
+          Stream.concat(
+                  userRegisteredToolchains.stream(), workspaceSuffixRegisteredToolchains.stream())
               .map(TargetPattern::getOriginalPattern)
               .collect(toImmutableList()));
     }