[7.0.0] Unable to load workspace "@myworkspace" where new_local_repository.build_file refers to BUILD file w/ label "@myworkspace//path/to:BUILD.bazel"

src/main/java/com/google/devtools/build/lib/rules/BUILD

@@ -315,6 +315,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",
         "//src/main/java/com/google/devtools/build/lib/bazel:resolved_event",
         "//src/main/java/com/google/devtools/build/lib/bugreport",
+        "//src/main/java/com/google/devtools/build/lib/cmdline",
         "//src/main/java/com/google/devtools/build/lib/io:inconsistent_filesystem_exception",
         "//src/main/java/com/google/devtools/build/lib/packages",
         "//src/main/java/com/google/devtools/build/lib/skyframe:directory_listing_value",
@@ -348,15 +349,13 @@ java_library(
         ":repository/workspace_attribute_mapper",
         "//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
         "//src/main/java/com/google/devtools/build/lib/cmdline",
-        "//src/main/java/com/google/devtools/build/lib/cmdline:LabelValidator",
         "//src/main/java/com/google/devtools/build/lib/packages",
+        "//src/main/java/com/google/devtools/build/lib/skyframe:package_lookup_function",
         "//src/main/java/com/google/devtools/build/lib/skyframe:package_lookup_value",
         "//src/main/java/com/google/devtools/build/lib/vfs",
-        "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
         "//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:guava",
         "//third_party:jsr305",
     ],
 )

src/main/java/com/google/devtools/build/lib/rules/repository/NewLocalRepositoryFunction.java

@@ -21,7 +21,9 @@
 import com.google.devtools.build.lib.analysis.RuleDefinition;
 import com.google.devtools.build.lib.bazel.ResolvedEvent;
 import com.google.devtools.build.lib.bugreport.BugReport;
+import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.io.InconsistentFilesystemException;
+import com.google.devtools.build.lib.packages.BuildType;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.skyframe.DirectoryListingValue;
 import com.google.devtools.build.lib.vfs.FileSystem;
@@ -160,7 +162,7 @@ public RepositoryDirectoryValue.Builder fetch(
     }
 
     fileHandler.finishFile(rule, outputDirectory, markerData);
-    env.getListener().post(resolve(rule, directories));
+    env.getListener().post(resolve(rule));
 
     return RepositoryDirectoryValue.builder()
         .setPath(outputDirectory)
@@ -173,7 +175,7 @@ public Class<? extends RuleDefinition> getRuleDefinition() {
     return NewLocalRepositoryRule.class;
   }
 
-  private static ResolvedEvent resolve(Rule rule, BlazeDirectories directories) {
+  private static ResolvedEvent resolve(Rule rule) {
     String name = rule.getName();
     Object pathObj = rule.getAttr("path");
     ImmutableMap.Builder<String, Object> origAttr =
@@ -186,22 +188,10 @@ private static ResolvedEvent resolve(Rule rule, BlazeDirectories directories) {
             .append(", path = ")
             .append(Starlark.repr(pathObj));
 
-    Object buildFileObj = rule.getAttr("build_file");
-    if ((buildFileObj instanceof String) && !((String) buildFileObj).isEmpty()) {
-      // Build files might refer to an embedded file (as they to for "local_jdk"), so we have to
-      // describe the argument in a portable way.
-      origAttr.put("build_file", buildFileObj);
-      String buildFileArg;
-      PathFragment pathFragment = PathFragment.create((String) buildFileObj);
-      PathFragment embeddedDir = directories.getEmbeddedBinariesRoot().asFragment();
-      if (pathFragment.isAbsolute() && pathFragment.startsWith(embeddedDir)) {
-        buildFileArg =
-            "__embedded_dir__ + \"/\" + "
-                + Starlark.repr(pathFragment.relativeTo(embeddedDir).toString());
-      } else {
-        buildFileArg = Starlark.repr(buildFileObj.toString());
-      }
-      repr.append(", build_file = ").append(buildFileArg);
+    Label buildFile = (Label) rule.getAttr("build_file", BuildType.NODEP_LABEL);
+    if (buildFile != null) {
+      origAttr.put("build_file", buildFile);
+      repr.append(", build_file = ").append(Starlark.repr(buildFile));
     } else {
       Object buildFileContentObj = rule.getAttr("build_file_content");
       if (buildFileContentObj != null) {

src/main/java/com/google/devtools/build/lib/rules/repository/NewLocalRepositoryRule.java

@@ -19,6 +19,7 @@
 
 import com.google.devtools.build.lib.analysis.RuleDefinition;
 import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
+import com.google.devtools.build.lib.packages.BuildType;
 import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
 
@@ -46,7 +47,7 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment envi
         named BUILD, but can be. (Something like BUILD.new-repo-name may work well for
         distinguishing it from the repository's actual BUILD files.)</p>
         <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
-        .add(attr("build_file", STRING))
+        .add(attr("build_file", BuildType.NODEP_LABEL))
         /* <!-- #BLAZE_RULE(new_local_repository).ATTRIBUTE(build_file_content) -->
         The content for the BUILD file for this repository.
 
@@ -62,7 +63,7 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment envi
         named WORKSPACE, but can be. (Something like WORKSPACE.new-repo-name may work well for
         distinguishing it from the repository's actual WORKSPACE files.)</p>
          <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
-        .add(attr("workspace_file", STRING))
+        .add(attr("workspace_file", BuildType.NODEP_LABEL))
         /* <!-- #BLAZE_RULE(new_local_repository).ATTRIBUTE(workspace_file_content) -->
         The content for the WORKSPACE file for this repository.
 

src/main/java/com/google/devtools/build/lib/rules/repository/NewRepositoryFileHandler.java

@@ -16,14 +16,13 @@
 
 import com.google.devtools.build.lib.actions.FileValue;
 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.BuildType;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.Type;
 import com.google.devtools.build.lib.rules.repository.RepositoryFunction.RepositoryFunctionException;
+import com.google.devtools.build.lib.skyframe.PackageLookupFunction;
 import com.google.devtools.build.lib.skyframe.PackageLookupValue;
 import com.google.devtools.build.lib.vfs.Path;
-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.skyframe.SkyFunction.Environment;
@@ -73,13 +72,11 @@ public void finishFile(Rule rule, Path outputDirectory, Map<String, String> mark
    */
   private abstract static class BaseFileHandler {
 
-    private final Path workspacePath;
     private final String filename;
     private FileValue fileValue;
     private String fileContent;
 
-    private BaseFileHandler(Path workspacePath, String filename) {
-      this.workspacePath = workspacePath;
+    private BaseFileHandler(String filename) {
       this.filename = filename;
     }
 
@@ -147,14 +144,7 @@ public void finishFile(Rule rule, Path outputDirectory, Map<String, String> mark
       if (fileValue != null) {
         // Link x/FILENAME to <build_root>/x.FILENAME.
         symlinkFile(fileValue, filename, outputDirectory);
-        String fileAttribute = getFileAttributeValue(rule);
-        String fileKey;
-        if (LabelValidator.isAbsolute(fileAttribute)) {
-          fileKey = getFileAttributeAsLabel(rule).toString();
-        } else {
-          // TODO(pcloudy): Don't add absolute path into markerData once it's not supported
-          fileKey = fileValue.realRootedPath().asPath().getPathString();
-        }
+        String fileKey = getFileAttributeAsLabel(rule).toString();
         try {
           markerData.put("FILE:" + fileKey, RepositoryFunction.fileValueToMarkerValue(fileValue));
         } catch (IOException e) {
@@ -167,75 +157,37 @@ public void finishFile(Rule rule, Path outputDirectory, Map<String, String> mark
       }
     }
 
-    private String getFileAttributeValue(Rule rule) throws RepositoryFunctionException {
-      WorkspaceAttributeMapper mapper = WorkspaceAttributeMapper.of(rule);
-      String fileAttribute;
+    private Label getFileAttributeAsLabel(Rule rule) throws RepositoryFunctionException {
       try {
-        fileAttribute = mapper.get(getFileAttrName(), Type.STRING);
+        return WorkspaceAttributeMapper.of(rule).get(getFileAttrName(), BuildType.NODEP_LABEL);
       } catch (EvalException e) {
         throw new RepositoryFunctionException(e, Transience.PERSISTENT);
       }
-      return fileAttribute;
-    }
-
-    private Label getFileAttributeAsLabel(Rule rule) throws RepositoryFunctionException {
-      Label label;
-      try {
-        // Parse a label
-        label = Label.parseCanonical(getFileAttributeValue(rule));
-      } catch (LabelSyntaxException ex) {
-        throw new RepositoryFunctionException(
-            Starlark.errorf(
-                "the '%s' attribute does not specify a valid label: %s",
-                getFileAttrName(), ex.getMessage()),
-            Transience.PERSISTENT);
-      }
-      return label;
     }
 
     @Nullable
     private FileValue getFileValue(Rule rule, Environment env)
         throws RepositoryFunctionException, InterruptedException {
-      String fileAttribute = getFileAttributeValue(rule);
-      RootedPath rootedFile;
-
-      if (LabelValidator.isAbsolute(fileAttribute)) {
-        Label label = getFileAttributeAsLabel(rule);
-        SkyKey pkgSkyKey = PackageLookupValue.key(label.getPackageIdentifier());
-        PackageLookupValue pkgLookupValue = (PackageLookupValue) env.getValue(pkgSkyKey);
-        if (pkgLookupValue == null) {
-          return null;
-        }
-        if (!pkgLookupValue.packageExists()) {
-          throw new RepositoryFunctionException(
-              Starlark.errorf("Unable to load package for %s: not found.", fileAttribute),
-              Transience.PERSISTENT);
-        }
-
-        // And now for the file
-        Root packageRoot = pkgLookupValue.getRoot();
-        rootedFile = RootedPath.toRootedPath(packageRoot, label.toPathFragment());
-      } else {
-        // TODO(dmarting): deprecate using a path for the workspace_file attribute.
-        PathFragment file = PathFragment.create(fileAttribute);
-        Path fileTarget = workspacePath.getRelative(file);
-        if (!fileTarget.exists()) {
-          throw new RepositoryFunctionException(
-              Starlark.errorf(
-                  "the '%s' attribute does not specify an existing file (%s does not exist)",
-                  getFileAttrName(), fileTarget),
-              Transience.PERSISTENT);
-        }
-
-        if (file.isAbsolute()) {
-          rootedFile =
-              RootedPath.toRootedPath(
-                  Root.fromPath(fileTarget.getParentDirectory()),
-                  PathFragment.create(fileTarget.getBaseName()));
-        } else {
-          rootedFile = RootedPath.toRootedPath(Root.fromPath(workspacePath), file);
+      Label label = getFileAttributeAsLabel(rule);
+      SkyKey pkgSkyKey = PackageLookupValue.key(label.getPackageIdentifier());
+      PackageLookupValue pkgLookupValue = (PackageLookupValue) env.getValue(pkgSkyKey);
+      if (pkgLookupValue == null) {
+        return null;
+      }
+      if (!pkgLookupValue.packageExists()) {
+        String message = pkgLookupValue.getErrorMsg();
+        if (pkgLookupValue == PackageLookupValue.NO_BUILD_FILE_VALUE) {
+          message =
+              PackageLookupFunction.explainNoBuildFileValue(label.getPackageIdentifier(), env);
         }
+        throw new RepositoryFunctionException(
+            Starlark.errorf("Unable to load package for %s: %s", label, message),
+            Transience.PERSISTENT);
       }
+
+      // And now for the file
+      Root packageRoot = pkgLookupValue.getRoot();
+      RootedPath rootedFile = RootedPath.toRootedPath(packageRoot, label.toPathFragment());
       SkyKey fileKey = FileValue.key(rootedFile);
       FileValue fileValue;
       try {
@@ -251,13 +203,17 @@ private FileValue getFileValue(Rule rule, Environment env)
         }
       } catch (IOException e) {
         throw new RepositoryFunctionException(
-            new IOException("Cannot lookup " + fileAttribute + ": " + e.getMessage()),
+            new IOException("Cannot lookup " + label + ": " + e.getMessage()),
             Transience.TRANSIENT);
       }
 
       if (!fileValue.isFile() || fileValue.isSpecialFile()) {
         throw new RepositoryFunctionException(
-            Starlark.errorf("%s is not a regular file", rootedFile.asPath()),
+            Starlark.errorf(
+                "%s is not a regular file; if you're using a relative or absolute path for "
+                    + "`build_file` in your `new_local_repository` rule, please switch to using a "
+                    + "label instead",
+                rootedFile.asPath()),
             Transience.PERSISTENT);
       }
 
@@ -284,7 +240,7 @@ private static void symlinkFile(FileValue fileValue, String filename, Path outpu
   public static class NewRepositoryWorkspaceFileHandler extends BaseFileHandler {
 
     public NewRepositoryWorkspaceFileHandler(Path workspacePath) {
-      super(workspacePath, "WORKSPACE");
+      super("WORKSPACE");
     }
 
     @Override
@@ -310,7 +266,7 @@ protected String getDefaultContent(Rule rule) {
   public static class NewRepositoryBuildFileHandler extends BaseFileHandler {
 
     public NewRepositoryBuildFileHandler(Path workspacePath) {
-      super(workspacePath, "BUILD.bazel");
+      super("BUILD.bazel");
     }
 
     @Override

src/test/shell/bazel/local_repository_test.sh

@@ -192,10 +192,6 @@ function test_new_local_repository_with_build_file() {
   do_new_local_repository_test "build_file"
 }
 
-function test_new_local_repository_with_labeled_build_file() {
-  do_new_local_repository_test "build_file+label"
-}
-
 function test_new_local_repository_with_build_file_content() {
   do_new_local_repository_test "build_file_content"
 }
@@ -224,22 +220,17 @@ public class Mongoose {
 }
 EOF
 
-  if [ "$1" == "build_file" -o "$1" == "build_file+label" ] ; then
-    build_file=BUILD.carnivore
-    build_file_str="${build_file}"
-    if [ "$1" == "build_file+label" ]; then
-      build_file_str="//:${build_file}"
-      cat > BUILD
-    fi
+  if [ "$1" == "build_file" ] ; then
+    touch BUILD
     cat >> $(create_workspace_with_default_repos WORKSPACE) <<EOF
 new_local_repository(
     name = 'endangered',
     path = '$project_dir',
-    build_file = '$build_file',
+    build_file = '//:BUILD.carnivore',
 )
 EOF
 
-    cat > $build_file <<EOF
+    cat > BUILD.carnivore <<EOF
 java_library(
     name = "mongoose",
     srcs = ["carnivore/Mongoose.java"],
@@ -462,7 +453,7 @@ EOF
 new_local_repository(
     name = "bar",
     path = "$bar",
-    build_file = "BUILD",
+    build_file = "//:BUILD",
 )
 EOF
   touch BUILD
@@ -567,14 +558,15 @@ function test_overlaid_build_file() {
 new_local_repository(
     name = "mutant",
     path = "$mutant",
-    build_file = "mutant.BUILD"
+    build_file = "//:mutant.BUILD"
 )
 
 bind(
     name = "best-turtle",
     actual = "@mutant//:turtle",
 )
 EOF
+  touch BUILD
   cat > mutant.BUILD <<EOF
 genrule(
     name = "turtle",
@@ -1088,10 +1080,11 @@ EOF
 new_local_repository(
     name="r",
     path="$r",
-    build_file="BUILD.r"
+    build_file="//:BUILD.r"
 )
 EOF
 
+  touch BUILD
   cat > BUILD.r <<EOF
 cc_library(name = "a", srcs = ["a.cc"])
 EOF