Don't print stack traces when repository rules have the wrong type

This also entirely disallows select() in repository rules. All repository rules should now error out if the wrong type is given, instead of printing a stack trace.

Fixes #1307.

--
MOS_MIGRATED_REVID=132872804

13 files changed, 275 insertions, 103 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/GitCloner.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/GitCloner.java
index 49126f903f..12ff3966ee 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/GitCloner.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/GitCloner.java
@@ -16,9 +16,9 @@ package com.google.devtools.build.lib.bazel.repository;
 
 import com.google.devtools.build.lib.bazel.repository.downloader.ProxyHelper;
 import com.google.devtools.build.lib.events.EventHandler;
-import com.google.devtools.build.lib.packages.AggregatingAttributeMapper;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.rules.repository.RepositoryFunction.RepositoryFunctionException;
+import com.google.devtools.build.lib.rules.repository.WorkspaceAttributeMapper;
 import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
@@ -96,27 +96,31 @@ public class GitCloner {
       Rule rule, Path outputDirectory, EventHandler eventHandler,
       Map<String, String> clientEnvironment)
       throws RepositoryFunctionException {
-    AggregatingAttributeMapper mapper = AggregatingAttributeMapper.of(rule);
-    if ((mapper.has("commit", Type.STRING) == mapper.has("tag", Type.STRING))
-        && (mapper.get("commit", Type.STRING).isEmpty()
-            == mapper.get("tag", Type.STRING).isEmpty())) {
+    WorkspaceAttributeMapper mapper = WorkspaceAttributeMapper.of(rule);
+    if (mapper.isAttributeValueExplicitlySpecified("commit")
+        == mapper.isAttributeValueExplicitlySpecified("tag")) {
       throw new RepositoryFunctionException(
           new EvalException(rule.getLocation(), "One of either commit or tag must be defined"),
           Transience.PERSISTENT);
     }
 
+    GitRepositoryDescriptor descriptor;
     String startingPoint;
-    if (mapper.has("commit", Type.STRING) && !mapper.get("commit", Type.STRING).isEmpty()) {
-      startingPoint = mapper.get("commit", Type.STRING);
-    } else {
-      startingPoint = "tags/" + mapper.get("tag", Type.STRING);
-    }
+    try {
+      if (mapper.isAttributeValueExplicitlySpecified("commit")) {
+        startingPoint = mapper.get("commit", Type.STRING);
+      } else {
+        startingPoint = "tags/" + mapper.get("tag", Type.STRING);
+      }
 
-    GitRepositoryDescriptor descriptor = new GitRepositoryDescriptor(
-        mapper.get("remote", Type.STRING),
-        startingPoint,
-        mapper.get("init_submodules", Type.BOOLEAN),
-        outputDirectory);
+      descriptor = new GitRepositoryDescriptor(
+          mapper.get("remote", Type.STRING),
+          startingPoint,
+          mapper.get("init_submodules", Type.BOOLEAN),
+          outputDirectory);
+    } catch (EvalException e) {
+      throw new RepositoryFunctionException(e, Transience.PERSISTENT);
+    }
 
     // Setup proxy if remote is http or https
     if (descriptor.remote != null && descriptor.remote.startsWith("http")) {
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/HttpArchiveFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/HttpArchiveFunction.java
index 56522c7312..873061e0c9 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/HttpArchiveFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/HttpArchiveFunction.java
@@ -18,10 +18,11 @@ import com.google.devtools.build.lib.analysis.BlazeDirectories;
 import com.google.devtools.build.lib.analysis.RuleDefinition;
 import com.google.devtools.build.lib.bazel.repository.downloader.HttpDownloader;
 import com.google.devtools.build.lib.bazel.rules.workspace.HttpArchiveRule;
-import com.google.devtools.build.lib.packages.AggregatingAttributeMapper;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue;
 import com.google.devtools.build.lib.rules.repository.RepositoryFunction;
+import com.google.devtools.build.lib.rules.repository.WorkspaceAttributeMapper;
+import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
@@ -76,9 +77,13 @@ public class HttpArchiveFunction extends RepositoryFunction {
         .setTargetName(rule.getName())
         .setArchivePath(downloadPath)
         .setRepositoryPath(outputDirectory);
-    AggregatingAttributeMapper mapper = AggregatingAttributeMapper.of(rule);
-    if (mapper.has("strip_prefix", Type.STRING)) {
-      builder.setPrefix(mapper.get("strip_prefix", Type.STRING));
+    WorkspaceAttributeMapper mapper = WorkspaceAttributeMapper.of(rule);
+    if (mapper.isAttributeValueExplicitlySpecified("strip_prefix")) {
+      try {
+        builder.setPrefix(mapper.get("strip_prefix", Type.STRING));
+      } catch (EvalException e) {
+        throw new RepositoryFunctionException(e, Transience.PERSISTENT);
+      }
     }
     return builder.build();
   }
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/HttpFileFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/HttpFileFunction.java
index 25216ed2c2..1ded55400b 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/HttpFileFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/HttpFileFunction.java
@@ -16,10 +16,12 @@ package com.google.devtools.build.lib.bazel.repository;
 
 import com.google.devtools.build.lib.analysis.RuleDefinition;
 import com.google.devtools.build.lib.bazel.rules.workspace.HttpFileRule;
-import com.google.devtools.build.lib.packages.AggregatingAttributeMapper;
 import com.google.devtools.build.lib.packages.Rule;
+import com.google.devtools.build.lib.rules.repository.WorkspaceAttributeMapper;
+import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
 
 /**
  * Downloads a jar file from a URL.
@@ -28,9 +30,14 @@ public class HttpFileFunction extends HttpArchiveFunction {
   @Override
   protected DecompressorDescriptor getDescriptor(Rule rule, Path downloadPath, Path outputDirectory)
       throws RepositoryFunctionException {
-    AggregatingAttributeMapper mapper = AggregatingAttributeMapper.of(rule);
-    boolean executable = (mapper.has("executable", Type.BOOLEAN)
-        && mapper.get("executable", Type.BOOLEAN));
+    WorkspaceAttributeMapper mapper = WorkspaceAttributeMapper.of(rule);
+    boolean executable = false;
+    try {
+      executable = (mapper.isAttributeValueExplicitlySpecified("executable")
+          && mapper.get("executable", Type.BOOLEAN));
+    } catch (EvalException e) {
+      throw new RepositoryFunctionException(e, Transience.PERSISTENT);
+    }
     return DecompressorDescriptor.builder()
         .setDecompressor(FileDecompressor.INSTANCE)
         .setTargetKind(rule.getTargetKind())
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/MavenJarFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/MavenJarFunction.java
index a18b4e089c..a41f3ab076 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/MavenJarFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/MavenJarFunction.java
@@ -23,10 +23,9 @@ import com.google.devtools.build.lib.analysis.BlazeDirectories;
 import com.google.devtools.build.lib.analysis.RuleDefinition;
 import com.google.devtools.build.lib.bazel.repository.downloader.HttpDownloader;
 import com.google.devtools.build.lib.bazel.rules.workspace.MavenJarRule;
-import com.google.devtools.build.lib.packages.AggregatingAttributeMapper;
-import com.google.devtools.build.lib.packages.AttributeMap;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue;
+import com.google.devtools.build.lib.rules.repository.WorkspaceAttributeMapper;
 import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.util.Fingerprint;
@@ -77,53 +76,59 @@ public class MavenJarFunction extends HttpArchiveFunction {
 
   private static MavenServerValue getServer(Rule rule, Environment env)
       throws RepositoryFunctionException, InterruptedException {
-    AggregatingAttributeMapper mapper = AggregatingAttributeMapper.of(rule);
-    boolean hasRepository =
-        mapper.has("repository", Type.STRING) && !mapper.get("repository", Type.STRING).isEmpty();
-    boolean hasServer = mapper.has("server", Type.STRING)
-        && !mapper.get("server", Type.STRING).isEmpty();
+    WorkspaceAttributeMapper mapper = WorkspaceAttributeMapper.of(rule);
+    boolean hasRepository = mapper.isAttributeValueExplicitlySpecified("repository");
+    boolean hasServer = mapper.isAttributeValueExplicitlySpecified("server");
 
     if (hasRepository && hasServer) {
       throw new RepositoryFunctionException(new EvalException(
           rule.getLocation(), rule + " specifies both "
-              + "'repository' and 'server', which are mutually exclusive options"),
+          + "'repository' and 'server', which are mutually exclusive options"),
           Transience.PERSISTENT);
-    } else if (hasRepository) {
-      return MavenServerValue.createFromUrl(mapper.get("repository", Type.STRING));
-    } else {
-      String serverName = DEFAULT_SERVER;
-      if (hasServer) {
-        serverName = mapper.get("server", Type.STRING);
-      }
+    }
 
-      return (MavenServerValue) env.getValue(MavenServerValue.key(serverName));
+    try {
+      if (hasRepository) {
+        return MavenServerValue.createFromUrl(mapper.get("repository", Type.STRING));
+      } else {
+        String serverName = DEFAULT_SERVER;
+        if (hasServer) {
+          serverName = mapper.get("server", Type.STRING);
+        }
+        return (MavenServerValue) env.getValue(MavenServerValue.key(serverName));
+      }
+    } catch (EvalException e) {
+      throw new RepositoryFunctionException(e, Transience.PERSISTENT);
     }
+
   }
 
   @Override
   public SkyValue fetch(
       Rule rule, Path outputDirectory, BlazeDirectories directories, Environment env)
           throws RepositoryFunctionException, InterruptedException {
-    AggregatingAttributeMapper mapper = AggregatingAttributeMapper.of(rule);
     MavenServerValue serverValue = getServer(rule, env);
     if (env.valuesMissing()) {
       return null;
     }
     MavenDownloader downloader;
     try {
-      downloader = createMavenDownloader(directories, mapper, serverValue);
+      downloader = createMavenDownloader(directories, rule, serverValue);
     } catch (IOException e) {
       throw new RepositoryFunctionException(e, Transience.PERSISTENT);
+    } catch (EvalException e) {
+      throw new RepositoryFunctionException(e, Transience.PERSISTENT);
     }
     return createOutputTree(downloader);
   }
 
   private MavenDownloader createMavenDownloader(
-      BlazeDirectories directories, AttributeMap mapper, MavenServerValue serverValue)
-      throws IOException {
-    String name = mapper.getName();
+      BlazeDirectories directories, Rule rule, MavenServerValue serverValue)
+      throws IOException, EvalException {
+    String name = rule.getName();
     Path outputDirectory = getExternalRepositoryDirectory(directories).getRelative(name);
-    return new MavenDownloader(name, mapper, outputDirectory, serverValue);
+    return new MavenDownloader(
+        name, WorkspaceAttributeMapper.of(rule), outputDirectory, serverValue);
   }
 
   private SkyValue createOutputTree(MavenDownloader downloader)
@@ -169,8 +174,9 @@ public class MavenJarFunction extends HttpArchiveFunction {
     private final Server server;
 
     public MavenDownloader(
-        String name, AttributeMap mapper, Path outputDirectory, MavenServerValue serverValue)
-        throws IOException {
+        String name, WorkspaceAttributeMapper mapper, Path outputDirectory,
+        MavenServerValue serverValue)
+        throws IOException, EvalException {
       this.name = name;
       this.outputDirectory = outputDirectory;
 
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/MavenServerFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/MavenServerFunction.java
index ea85f50c80..2111aedc82 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/MavenServerFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/MavenServerFunction.java
@@ -18,13 +18,14 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
 import com.google.devtools.build.lib.bazel.rules.workspace.MavenServerRule;
-import com.google.devtools.build.lib.packages.AggregatingAttributeMapper;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.rules.repository.RepositoryFunction;
 import com.google.devtools.build.lib.rules.repository.RepositoryFunction.RepositoryFunctionException;
 import com.google.devtools.build.lib.rules.repository.RepositoryFunction.RepositoryNotFoundException;
+import com.google.devtools.build.lib.rules.repository.WorkspaceAttributeMapper;
 import com.google.devtools.build.lib.skyframe.FileValue;
 import com.google.devtools.build.lib.skyframe.PrecomputedValue;
+import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.util.Fingerprint;
 import com.google.devtools.build.lib.vfs.Path;
@@ -90,28 +91,33 @@ public class MavenServerFunction implements SkyFunction {
             Transience.TRANSIENT);
       }
     } else {
-      AggregatingAttributeMapper mapper = AggregatingAttributeMapper.of(repositoryRule);
+      WorkspaceAttributeMapper mapper = WorkspaceAttributeMapper.of(repositoryRule);
       serverName = repositoryRule.getName();
-      url = mapper.get("url", Type.STRING);
-      if (!mapper.has("settings_file", Type.STRING)
-          || mapper.get("settings_file", Type.STRING).isEmpty()) {
-        settingsFiles = getDefaultSettingsFile(directories, env);
-      } else {
-        PathFragment settingsFilePath = new PathFragment(mapper.get("settings_file", Type.STRING));
-        RootedPath settingsPath = RootedPath.toRootedPath(
-            directories.getWorkspace().getRelative(settingsFilePath), PathFragment.EMPTY_FRAGMENT);
-        FileValue fileValue = (FileValue) env.getValue(FileValue.key(settingsPath));
-        if (fileValue == null) {
-          return null;
-        }
-
-        if (!fileValue.exists()) {
-          throw new RepositoryFunctionException(
-              new IOException("Could not find settings file " + settingsPath),
-              Transience.TRANSIENT);
+      try {
+        url = mapper.get("url", Type.STRING);
+        if (!mapper.isAttributeValueExplicitlySpecified("settings_file")) {
+          settingsFiles = getDefaultSettingsFile(directories, env);
+        } else {
+          PathFragment settingsFilePath = new PathFragment(
+              mapper.get("settings_file", Type.STRING));
+          RootedPath settingsPath = RootedPath.toRootedPath(
+              directories.getWorkspace().getRelative(settingsFilePath),
+              PathFragment.EMPTY_FRAGMENT);
+          FileValue fileValue = (FileValue) env.getValue(FileValue.key(settingsPath));
+          if (fileValue == null) {
+            return null;
+          }
+
+          if (!fileValue.exists()) {
+            throw new RepositoryFunctionException(
+                new IOException("Could not find settings file " + settingsPath),
+                Transience.TRANSIENT);
+          }
+          settingsFiles = ImmutableMap.<String, FileValue>builder().put(
+              USER_KEY, fileValue).build();
         }
-        settingsFiles = ImmutableMap.<String, FileValue>builder().put(
-            USER_KEY, fileValue).build();
+      } catch (EvalException e) {
+        throw new RepositoryFunctionException(e, Transience.PERSISTENT);
       }
     }
 
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/NewHttpArchiveFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/NewHttpArchiveFunction.java
index 29f20e771d..04ad0b708c 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/NewHttpArchiveFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/NewHttpArchiveFunction.java
@@ -16,10 +16,11 @@ package com.google.devtools.build.lib.bazel.repository;
 
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
 import com.google.devtools.build.lib.bazel.repository.downloader.HttpDownloader;
-import com.google.devtools.build.lib.packages.AggregatingAttributeMapper;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.rules.repository.NewRepositoryBuildFileHandler;
 import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue;
+import com.google.devtools.build.lib.rules.repository.WorkspaceAttributeMapper;
+import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
@@ -61,11 +62,14 @@ public class NewHttpArchiveFunction extends HttpArchiveFunction {
 
     // Decompress.
     Path decompressed;
-    AggregatingAttributeMapper mapper = AggregatingAttributeMapper.of(rule);
+    WorkspaceAttributeMapper mapper = WorkspaceAttributeMapper.of(rule);
     String prefix = null;
-    if (mapper.has("strip_prefix", Type.STRING)
-        && !mapper.get("strip_prefix", Type.STRING).isEmpty()) {
-      prefix = mapper.get("strip_prefix", Type.STRING);
+    if (mapper.isAttributeValueExplicitlySpecified("strip_prefix")) {
+      try {
+        prefix = mapper.get("strip_prefix", Type.STRING);
+      } catch (EvalException e) {
+        throw new RepositoryFunctionException(e, Transience.PERSISTENT);
+      }
     }
     decompressed = DecompressorValue.decompress(DecompressorDescriptor.builder()
         .setTargetKind(rule.getTargetKind())
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloader.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloader.java
index 13ade1b6c8..5cf1b58415 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloader.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloader.java
@@ -18,14 +18,16 @@ import com.google.common.hash.Hasher;
 import com.google.common.hash.Hashing;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventHandler;
-import com.google.devtools.build.lib.packages.AggregatingAttributeMapper;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.rules.repository.RepositoryFunction.RepositoryFunctionException;
+import com.google.devtools.build.lib.rules.repository.WorkspaceAttributeMapper;
+import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.skyframe.SkyFunctionException;
 
+import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
@@ -72,10 +74,18 @@ public class HttpDownloader {
   public static Path download(
       Rule rule, Path outputDirectory, EventHandler eventHandler, Map<String, String> clientEnv)
       throws RepositoryFunctionException, InterruptedException {
-    AggregatingAttributeMapper mapper = AggregatingAttributeMapper.of(rule);
-    String url = mapper.get("url", Type.STRING);
-    String sha256 = mapper.get("sha256", Type.STRING);
-    String type = mapper.has("type", Type.STRING) ? mapper.get("type", Type.STRING) : "";
+    WorkspaceAttributeMapper mapper = WorkspaceAttributeMapper.of(rule);
+    String url;
+    String sha256;
+    String type;
+    try {
+      url = mapper.get("url", Type.STRING);
+      sha256 = mapper.get("sha256", Type.STRING);
+      type = mapper.isAttributeValueExplicitlySpecified("type")
+          ? mapper.get("type", Type.STRING) : "";
+    } catch (EvalException e) {
+      throw new RepositoryFunctionException(e, Transience.PERSISTENT);
+    }
 
     try {
       return new HttpDownloader(eventHandler, url, sha256, outputDirectory, type, clientEnv)
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java
index eca0e2434c..4614b21d0a 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java
@@ -23,11 +23,11 @@ import com.google.devtools.build.lib.bazel.repository.downloader.HttpDownloader;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
 import com.google.devtools.build.lib.events.Location;
-import com.google.devtools.build.lib.packages.AggregatingAttributeMapper;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.SkylarkClassObject;
 import com.google.devtools.build.lib.packages.SkylarkClassObjectConstructor;
 import com.google.devtools.build.lib.rules.repository.RepositoryFunction.RepositoryFunctionException;
+import com.google.devtools.build.lib.rules.repository.WorkspaceAttributeMapper;
 import com.google.devtools.build.lib.skyframe.FileSymlinkException;
 import com.google.devtools.build.lib.skyframe.FileValue;
 import com.google.devtools.build.lib.skyframe.InconsistentFilesystemException;
@@ -42,7 +42,6 @@ import com.google.devtools.build.lib.syntax.Runtime;
 import com.google.devtools.build.lib.syntax.SkylarkDict;
 import com.google.devtools.build.lib.syntax.SkylarkList;
 import com.google.devtools.build.lib.syntax.SkylarkType;
-import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.util.StringUtilities;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
@@ -93,17 +92,17 @@ public class SkylarkRepositoryContext {
    * argument).
    */
   SkylarkRepositoryContext(
-      Rule rule, Path outputDirectory, Environment environment, Map<String, String> env) {
+      Rule rule, Path outputDirectory, Environment environment, Map<String, String> env)
+      throws EvalException {
     this.rule = rule;
     this.outputDirectory = outputDirectory;
     this.env = environment;
     this.osObject = new SkylarkOS(env);
-    AggregatingAttributeMapper attrs = AggregatingAttributeMapper.of(rule);
+    WorkspaceAttributeMapper attrs = WorkspaceAttributeMapper.of(rule);
     ImmutableMap.Builder<String, Object> attrBuilder = new ImmutableMap.Builder<>();
     for (String name : attrs.getAttributeNames()) {
       if (!name.equals("$local")) {
-        Type<?> type = attrs.getAttributeType(name);
-        Object val = attrs.get(name, type);
+        Object val = attrs.getObject(name);
         attrBuilder.put(
             attributeToSkylark(name),
             val == null
diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/NewRepositoryBuildFileHandler.java b/src/main/java/com/google/devtools/build/lib/rules/repository/NewRepositoryBuildFileHandler.java
index 18654e1da1..658c957690 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/repository/NewRepositoryBuildFileHandler.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/repository/NewRepositoryBuildFileHandler.java
@@ -17,7 +17,6 @@ package com.google.devtools.build.lib.rules.repository;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
 import com.google.devtools.build.lib.cmdline.LabelValidator;
-import com.google.devtools.build.lib.packages.AggregatingAttributeMapper;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.rules.repository.RepositoryFunction.RepositoryFunctionException;
 import com.google.devtools.build.lib.skyframe.FileSymlinkException;
@@ -60,7 +59,7 @@ public class NewRepositoryBuildFileHandler {
   public boolean prepareBuildFile(Rule rule, Environment env)
       throws RepositoryFunctionException, InterruptedException {
 
-    AggregatingAttributeMapper mapper = AggregatingAttributeMapper.of(rule);
+    WorkspaceAttributeMapper mapper = WorkspaceAttributeMapper.of(rule);
     boolean hasBuildFile = mapper.isAttributeValueExplicitlySpecified("build_file");
     boolean hasBuildFileContent = mapper.isAttributeValueExplicitlySpecified("build_file_content");
 
@@ -78,9 +77,13 @@ public class NewRepositoryBuildFileHandler {
         return false;
       }
 
-    } else if (hasBuildFileContent) { 
+    } else if (hasBuildFileContent) {
 
-      buildFileContent = mapper.get("build_file_content", Type.STRING);
+      try {
+        buildFileContent = mapper.get("build_file_content", Type.STRING);
+      } catch (EvalException e) {
+        throw new RepositoryFunctionException(e, Transience.PERSISTENT);
+      }
 
     } else {
 
@@ -115,8 +118,13 @@ public class NewRepositoryBuildFileHandler {
 
   private FileValue getBuildFileValue(Rule rule, Environment env)
       throws RepositoryFunctionException, InterruptedException {
-    AggregatingAttributeMapper mapper = AggregatingAttributeMapper.of(rule);
-    String buildFileAttribute = mapper.get("build_file", Type.STRING);
+    WorkspaceAttributeMapper mapper = WorkspaceAttributeMapper.of(rule);
+    String buildFileAttribute;
+    try {
+      buildFileAttribute = mapper.get("build_file", Type.STRING);
+    } catch (EvalException e) {
+      throw new RepositoryFunctionException(e, Transience.PERSISTENT);
+    }
     RootedPath rootedBuild;
 
     if (LabelValidator.isAbsolute(buildFileAttribute)) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java
index 4e6628d482..901e4154a9 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java
@@ -22,7 +22,6 @@ import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
 import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
 import com.google.devtools.build.lib.events.Event;
-import com.google.devtools.build.lib.packages.AggregatingAttributeMapper;
 import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException;
 import com.google.devtools.build.lib.packages.NoSuchPackageException;
 import com.google.devtools.build.lib.packages.Package;
@@ -208,9 +207,15 @@ public abstract class RepositoryFunction {
   }
 
   @VisibleForTesting
-  protected static PathFragment getTargetPath(Rule rule, Path workspace) {
-    AggregatingAttributeMapper mapper = AggregatingAttributeMapper.of(rule);
-    String path = mapper.get("path", Type.STRING);
+  protected static PathFragment getTargetPath(Rule rule, Path workspace)
+      throws RepositoryFunctionException {
+    WorkspaceAttributeMapper mapper = WorkspaceAttributeMapper.of(rule);
+    String path;
+    try {
+      path = mapper.get("path", Type.STRING);
+    } catch (EvalException e) {
+      throw new RepositoryFunctionException(e, Transience.PERSISTENT);
+    }
     PathFragment pathFragment = new PathFragment(path);
     return workspace.getRelative(pathFragment).asFragment();
   }
@@ -397,8 +402,13 @@ public abstract class RepositoryFunction {
     // reflected in the SkyFrame tree, since it's not symlinked to it or anything.
     if (repositoryRule.getRuleClass().equals(NewLocalRepositoryRule.NAME)
         && repositoryPath.segmentCount() == 1) {
-      PathFragment pathDir = RepositoryFunction.getTargetPath(
-          repositoryRule, directories.getWorkspace());
+      PathFragment pathDir;
+      try {
+        pathDir = RepositoryFunction.getTargetPath(
+            repositoryRule, directories.getWorkspace());
+      } catch (RepositoryFunctionException e) {
+        throw new IOException(e.getMessage());
+      }
       FileSystem fs = directories.getWorkspace().getFileSystem();
       SkyKey dirKey = DirectoryListingValue.key(
           RootedPath.toRootedPath(fs.getRootDirectory(), fs.getPath(pathDir)));
diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/WorkspaceAttributeMapper.java b/src/main/java/com/google/devtools/build/lib/rules/repository/WorkspaceAttributeMapper.java
new file mode 100644
index 0000000000..b2edf3d96c
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/rules/repository/WorkspaceAttributeMapper.java
@@ -0,0 +1,76 @@
+// Copyright 2016 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.rules.repository;
+
+import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.packages.AggregatingAttributeMapper;
+import com.google.devtools.build.lib.packages.BuildType.SelectorList;
+import com.google.devtools.build.lib.packages.Rule;
+import com.google.devtools.build.lib.syntax.EvalException;
+import com.google.devtools.build.lib.syntax.Type;
+
+/**
+ * An attribute mapper for workspace rules. Similar to NonconfigurableAttributeWrapper, but throws
+ * a wrapped EvalException if select() is used.
+ */
+public class WorkspaceAttributeMapper {
+
+  public static WorkspaceAttributeMapper of(Rule rule) {
+    return new WorkspaceAttributeMapper(rule);
+  }
+
+  private final Rule rule;
+
+  private WorkspaceAttributeMapper(Rule rule) {
+    this.rule = rule;
+  }
+
+  public <T> T get(String attributeName, Type<T> type) throws EvalException {
+    Object value = getObject(attributeName);
+    try {
+      return type.cast(value);
+    } catch (ClassCastException e) {
+      throw new EvalException(
+          rule.getAttributeContainer().getAttributeLocation(attributeName), e.getMessage());
+    }
+  }
+
+  /**
+   * Returns the value for an attribute without casting it to any particular type.
+   */
+  public Object getObject(String attributeName) throws EvalException {
+    Object value = rule.getAttributeContainer().getAttr(attributeName);
+    if (value instanceof SelectorList) {
+      String message;
+      if (rule.getLocation().getPath().getBaseName().equals(
+          Label.EXTERNAL_PACKAGE_FILE_NAME.getPathString())) {
+        message = "select() cannot be used in WORKSPACE files";
+      } else {
+        message = "select() cannot be used in macros called from WORKSPACE files";
+      }
+      throw new EvalException(
+          rule.getAttributeContainer().getAttributeLocation(attributeName), message);
+    }
+    return value;
+  }
+
+  public boolean isAttributeValueExplicitlySpecified(String attr) {
+    return rule.getAttributeContainer().isAttributeValueExplicitlySpecified(attr);
+  }
+
+  public Iterable<String> getAttributeNames() {
+    return AggregatingAttributeMapper.of(rule).getAttributeNames();
+  }
+}
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/MavenJarFunctionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/MavenJarFunctionTest.java
index 6ad732e9d7..6492eeee11 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/repository/MavenJarFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/MavenJarFunctionTest.java
@@ -19,9 +19,9 @@ import static org.junit.Assert.fail;
 
 import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
 import com.google.devtools.build.lib.bazel.repository.MavenJarFunction.MavenDownloader;
-import com.google.devtools.build.lib.packages.AggregatingAttributeMapper;
 import com.google.devtools.build.lib.packages.Rule;
 
+import com.google.devtools.build.lib.rules.repository.WorkspaceAttributeMapper;
 import org.apache.maven.settings.Server;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -45,9 +45,9 @@ public class MavenJarFunctionTest extends BuildViewTestCase {
         "    artifact = 'x',",
         "    sha1 = '12345',",
         ")");
-    AggregatingAttributeMapper map = AggregatingAttributeMapper.of(rule);
     try {
-      new MavenDownloader("foo", map, scratch.dir("/whatever"), TEST_SERVER);
+      new MavenDownloader(
+          "foo", WorkspaceAttributeMapper.of(rule), scratch.dir("/whatever"), TEST_SERVER);
       fail("Invalid sha1 should have thrown.");
     } catch (IOException expected) {
       assertThat(expected.getMessage()).contains("Invalid SHA-1 for maven_jar foo");
@@ -62,8 +62,8 @@ public class MavenJarFunctionTest extends BuildViewTestCase {
         "    artifact = 'x',",
         "    sha1 = 'da39a3ee5e6b4b0d3255bfef95601890afd80709',",
         ")");
-    AggregatingAttributeMapper map = AggregatingAttributeMapper.of(rule);
-    new MavenDownloader("foo", map, scratch.dir("/whatever"), TEST_SERVER);
+    new MavenDownloader(
+        "foo", WorkspaceAttributeMapper.of(rule), scratch.dir("/whatever"), TEST_SERVER);
   }
 
   @Test
@@ -73,7 +73,7 @@ public class MavenJarFunctionTest extends BuildViewTestCase {
         "    name = 'foo',",
         "    artifact = 'x',",
         ")");
-    AggregatingAttributeMapper map = AggregatingAttributeMapper.of(rule);
-    new MavenDownloader("foo", map, scratch.dir("/whatever"), TEST_SERVER);
+    new MavenDownloader(
+        "foo", WorkspaceAttributeMapper.of(rule), scratch.dir("/whatever"), TEST_SERVER);
   }
 }
diff --git a/src/test/shell/bazel/workspace_test.sh b/src/test/shell/bazel/workspace_test.sh
index 9c63e381fb..3e6d6ed43f 100755
--- a/src/test/shell/bazel/workspace_test.sh
+++ b/src/test/shell/bazel/workspace_test.sh
@@ -120,4 +120,41 @@ EOF
   expect_not_log "Exception"
 }
 
+function test_no_select() {
+  cat > WORKSPACE <<EOF
+new_local_repository(
+    name = "foo",
+    path = "/path/to/foo",
+    build_file = select({
+        "//x:y" : "BUILD.1",
+        "//conditions:default" : "BUILD.2"}),
+)
+EOF
+
+  bazel build @foo//... &> $TEST_log && fail "Failure expected" || true
+  expect_log "select() cannot be used in WORKSPACE files"
+}
+
+function test_macro_select() {
+  cat > WORKSPACE <<EOF
+load('//:foo.bzl', 'foo_repo')
+foo_repo()
+EOF
+
+  touch BUILD
+  cat > foo.bzl <<EOF
+def foo_repo():
+  native.new_local_repository(
+      name = "foo",
+      path = "/path/to/foo",
+      build_file = select({
+          "//x:y" : "BUILD.1",
+          "//conditions:default" : "BUILD.2"}),
+  )
+EOF
+
+  bazel build @foo//... &> $TEST_log && fail "Failure expected" || true
+  expect_log "select() cannot be used in macros called from WORKSPACE files"
+}
+
 run_suite "workspace tests"