Prevent load statements in remote repository's WORKSPACE file to break

If a load statements was present in a remote repository's WORKSPACE file,
the parsing of the WORKSPACE file to get the workspace name will break
the build because it tries to load the Skylark extension from the main
repository.

This change replace the parsing of the whole remote repository's WORKSPACE file
by a parsing of only it first chunk (before the first load statement). This
change also enforce that the workpace() function to be called only from the
top of the WORKSPACE file.

--
MOS_MIGRATED_REVID=114528640

2 files changed, 52 insertions, 30 deletions

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
index d300653fb7..62eeb342da 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
@@ -78,6 +78,8 @@ public class WorkspaceFactory {
   private final Path workspaceDir;
   private final Mutability mutability;
 
+  private final boolean allowOverride;
+
   private final ImmutableMap<String, BaseFunction> workspaceFunctions;
   private final ImmutableList<EnvironmentExtension> environmentExtensions;
 
@@ -127,6 +129,7 @@ public class WorkspaceFactory {
     this.mutability = mutability;
     this.installDir = installDir;
     this.workspaceDir = workspaceDir;
+    this.allowOverride = allowOverride;
     this.environmentExtensions = environmentExtensions;
     this.workspaceFunctions = createWorkspaceFunctions(ruleClassProvider, allowOverride);
   }
@@ -256,34 +259,53 @@ public class WorkspaceFactory {
     }
   }
 
-  @SkylarkSignature(name = "workspace", objectType = Object.class, returnType = SkylarkList.class,
-      doc = "Sets the name for this workspace. Workspace names should be a Java-package-style "
-          + "description of the project, using underscores as separators, e.g., "
-          + "github.com/bazelbuild/bazel should use com_github_bazelbuild_bazel. Names must start "
-          + "with a letter and can only contain letters, numbers, and underscores.",
-      mandatoryPositionals = {
-          @SkylarkSignature.Param(name = "name", type = String.class,
-              doc = "the name of the workspace.")},
-      documented = true, useAst = true, useEnvironment = true)
+  @SkylarkSignature(
+    name = "workspace",
+    objectType = Object.class,
+    returnType = SkylarkList.class,
+    doc =
+        "Sets the name for this workspace. Workspace names should be a Java-package-style "
+            + "description of the project, using underscores as separators, e.g., "
+            + "github.com/bazelbuild/bazel should use com_github_bazelbuild_bazel. Names must "
+            + "start with a letter and can only contain letters, numbers, and underscores.",
+    mandatoryPositionals = {
+      @SkylarkSignature.Param(name = "name", type = String.class, doc = "the name of the workspace."
+      )
+    },
+    documented = true,
+    useAst = true,
+    useEnvironment = true
+  )
   private static final BuiltinFunction.Factory newWorkspaceFunction =
       new BuiltinFunction.Factory("workspace") {
-        public BuiltinFunction create() {
-          return new BuiltinFunction(
-              "workspace", FunctionSignature.namedOnly("name"), BuiltinFunction.USE_AST_ENV) {
-            public Object invoke(String name, FuncallExpression ast, Environment env)
-                throws EvalException {
-              if (!isLegalWorkspaceName(name)) {
-                throw new EvalException(
-                    ast.getLocation(), name + " is not a legal workspace name");
+        public BuiltinFunction create(boolean allowOverride) {
+          if (allowOverride) {
+            return new BuiltinFunction(
+                "workspace", FunctionSignature.namedOnly("name"), BuiltinFunction.USE_AST_ENV) {
+              public Object invoke(String name, FuncallExpression ast, Environment env)
+                  throws EvalException {
+                if (!isLegalWorkspaceName(name)) {
+                  throw new EvalException(
+                      ast.getLocation(), name + " is not a legal workspace name");
+                }
+                String errorMessage = LabelValidator.validateTargetName(name);
+                if (errorMessage != null) {
+                  throw new EvalException(ast.getLocation(), errorMessage);
+                }
+                PackageFactory.getContext(env, ast).pkgBuilder.setWorkspaceName(name);
+                return NONE;
               }
-              String errorMessage = LabelValidator.validateTargetName(name);
-              if (errorMessage != null) {
-                throw new EvalException(ast.getLocation(), errorMessage);
+            };
+          } else {
+            return new BuiltinFunction(
+                "workspace", FunctionSignature.namedOnly("name"), BuiltinFunction.USE_AST) {
+              public Object invoke(String name, FuncallExpression ast) throws EvalException {
+                throw new EvalException(
+                    ast.getLocation(),
+                    "workspace() function should be used only at the top of the WORKSPACE file.");
               }
-              PackageFactory.getContext(env, ast).pkgBuilder.setWorkspaceName(name);
-              return NONE;
-            }
-          };
+            };
+          }
         }
       };
 
@@ -379,7 +401,7 @@ public class WorkspaceFactory {
 
   private void addWorkspaceFunctions(Environment workspaceEnv, StoredEventHandler localReporter) {
     try {
-      workspaceEnv.setup("workspace", newWorkspaceFunction.apply());
+      workspaceEnv.setup("workspace", newWorkspaceFunction.apply(allowOverride));
       for (Map.Entry<String, BaseFunction> function : workspaceFunctions.entrySet()) {
         workspaceEnv.update(function.getKey(), function.getValue());
       }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryLoaderFunction.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryLoaderFunction.java
index f4efc06780..08cd23f7f5 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryLoaderFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryLoaderFunction.java
@@ -18,9 +18,8 @@ import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
 import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.Location;
-import com.google.devtools.build.lib.skyframe.ExternalPackageFunction;
-import com.google.devtools.build.lib.skyframe.PackageValue;
 import com.google.devtools.build.lib.skyframe.RepositoryValue;
+import com.google.devtools.build.lib.skyframe.WorkspaceFileValue;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.lib.vfs.RootedPath;
@@ -49,9 +48,10 @@ public class RepositoryLoaderFunction implements SkyFunction {
       return null;
     }
 
-    SkyKey workspaceKey = ExternalPackageFunction.key(
-        RootedPath.toRootedPath(repository.getPath(), new PathFragment("WORKSPACE")));
-    PackageValue workspacePackage = (PackageValue) env.getValue(workspaceKey);
+    SkyKey workspaceKey =
+        WorkspaceFileValue.key(
+            RootedPath.toRootedPath(repository.getPath(), new PathFragment("WORKSPACE")));
+    WorkspaceFileValue workspacePackage = (WorkspaceFileValue) env.getValue(workspaceKey);
     if (workspacePackage == null) {
       return null;
     }