Distinguish between I/O exceptions caused by user decisions versus en…

…vironmental issues: symlink cycles are the user's fault, and should have a different exit code from generic I/O exceptions.

PiperOrigin-RevId: 381351065

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

@@ -37,6 +37,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/collect/nestedset",
         "//src/main/java/com/google/devtools/build/lib/concurrent",
         "//src/main/java/com/google/devtools/build/lib/events",
+        "//src/main/java/com/google/devtools/build/lib/io:file_symlink_exception",
         "//src/main/java/com/google/devtools/build/lib/packages/semantics",
         "//src/main/java/com/google/devtools/build/lib/profiler",
         "//src/main/java/com/google/devtools/build/lib/skyframe:detailed_exceptions",

src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java

@@ -25,6 +25,7 @@
 import com.google.devtools.build.lib.cmdline.LabelValidator;
 import com.google.devtools.build.lib.cmdline.PackageIdentifier;
 import com.google.devtools.build.lib.events.Event;
+import com.google.devtools.build.lib.io.FileSymlinkException;
 import com.google.devtools.build.lib.packages.Globber.BadGlobException;
 import com.google.devtools.build.lib.packages.PackageFactory.PackageContext;
 import com.google.devtools.build.lib.packages.RuleClass.Builder.ThirdPartyLicenseExistencePolicy;
@@ -114,7 +115,16 @@ public Sequence<?> glob(
               excludes.isEmpty() ? "" : " - [" + Joiner.on(", ").join(excludes) + "]",
               e.getMessage());
       Location loc = thread.getCallerLocation();
-      Event error = Package.error(loc, errorMessage, Code.GLOB_IO_EXCEPTION);
+      Event error =
+          Package.error(
+              loc,
+              errorMessage,
+              // If there are other IOExceptions that can result from user error, they should be
+              // tested for here. Currently FileNotFoundException is not one of those, because globs
+              // only encounter that error in the presence of an inconsistent filesystem.
+              e instanceof FileSymlinkException
+                  ? Code.EVAL_GLOBS_SYMLINK_ERROR
+                  : Code.GLOB_IO_EXCEPTION);
       context.eventHandler.handle(error);
       context.pkgBuilder.setIOException(e, errorMessage, error.getProperty(DetailedExitCode.class));
       matches = ImmutableList.of();

src/main/java/com/google/devtools/build/lib/unix/InvalidArgumentIOException.java

@@ -0,0 +1,27 @@
+// Copyright 2021 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.unix;
+
+import java.io.IOException;
+
+/**
+ * {@link IOException} indicating that the argument to an operation was invalid, like trying to read
+ * a normal file as a symbolic link. Instantiated only by JNI.
+ */
+public class InvalidArgumentIOException extends IOException {
+  public InvalidArgumentIOException(String message) {
+    super(message);
+  }
+}

src/main/java/com/google/devtools/build/lib/unix/UnixFileSystem.java

@@ -344,9 +344,8 @@ protected PathFragment readSymbolicLink(PathFragment path) throws IOException {
     long startTime = Profiler.nanoTimeMaybe();
     try {
       return PathFragment.create(NativePosixFiles.readlink(name));
-    } catch (IOException e) {
-      // EINVAL => not a symbolic link.  Anything else is a real error.
-      throw e.getMessage().endsWith("(Invalid argument)") ? new NotASymlinkException(path) : e;
+    } catch (InvalidArgumentIOException e) {
+      throw new NotASymlinkException(path, e);
     } finally {
       profiler.logSimpleTask(startTime, ProfilerTask.VFS_READLINK, name);
     }

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

@@ -51,6 +51,7 @@ java_library(
         ":pathfragment",
         "//src/main/java/com/google/devtools/build/lib/clock",
         "//src/main/java/com/google/devtools/build/lib/concurrent",
+        "//src/main/java/com/google/devtools/build/lib/io:file_symlink_exception",
         "//src/main/java/com/google/devtools/build/lib/profiler",
         "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
         "//src/main/java/com/google/devtools/build/lib/util:filetype",

src/main/java/com/google/devtools/build/lib/vfs/FileSymlinkLoopException.java

@@ -0,0 +1,32 @@
+// Copyright 2021 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.vfs;
+
+import com.google.devtools.build.lib.io.FileSymlinkException;
+
+final class FileSymlinkLoopException extends FileSymlinkException {
+  FileSymlinkLoopException(String message) {
+    super(message);
+  }
+
+  FileSymlinkLoopException(PathFragment pathFragment) {
+    this(pathFragment.getPathString() + " (Too many levels of symbolic links)");
+  }
+
+  @Override
+  public String getUserFriendlyMessage() {
+    return getMessage();
+  }
+}

src/main/java/com/google/devtools/build/lib/vfs/FileSystem.java

@@ -52,7 +52,11 @@ public DigestHashFunction getDigestFunction() {
    */
   protected static final class NotASymlinkException extends IOException {
     public NotASymlinkException(PathFragment path) {
-      super(path + " is not a symlink");
+      super(path.getPathString() + " is not a symlink");
+    }
+
+    public NotASymlinkException(PathFragment path, Throwable cause) {
+      super(path.getPathString() + " is not a symlink", cause);
     }
   }
 
@@ -354,7 +358,7 @@ protected final PathFragment appendSegment(PathFragment dir, String child, int m
     }
 
     if (maxLinks-- == 0) {
-      throw new IOException(naive + " (Too many levels of symbolic links)");
+      throw new FileSymlinkLoopException(naive);
     }
     if (linkTarget.isAbsolute()) {
       dir = PathFragment.createAlreadyNormalized(linkTarget.getDriveStr());
@@ -774,4 +778,5 @@ protected abstract void createFSDependentHardLink(
    * implement this in order to warm the filesystem's internal caches.
    */
   protected void prefetchPackageAsync(PathFragment path, int maxDirs) {}
+
 }

src/main/native/unix_jni.cc

@@ -81,10 +81,17 @@ void PostException(JNIEnv *env, int error_number, const std::string& message) {
                    // (aka EOPNOTSUPP)
       exception_classname = "java/lang/UnsupportedOperationException";
       break;
+    case EINVAL:  // Invalid argument
+      exception_classname =
+          "com/google/devtools/build/lib/unix/InvalidArgumentIOException";
+      break;
+    case ELOOP:  // Too many symbolic links encountered
+      exception_classname =
+          "com/google/devtools/build/lib/vfs/FileSymlinkLoopException";
+      break;
     case EBADF:         // Bad file number or descriptor already closed.
     case ENAMETOOLONG:  // File name too long
     case ENODATA:    // No data available
-    case EINVAL:     // Invalid argument
 #if defined(EMULTIHOP)
     case EMULTIHOP:  // Multihop attempted
 #endif
@@ -98,7 +105,6 @@ void PostException(JNIEnv *env, int error_number, const std::string& message) {
     case EROFS:      // Read-only file system
     case EEXIST:     // File exists
     case EMLINK:     // Too many links
-    case ELOOP:      // Too many symbolic links encountered
     case EISDIR:     // Is a directory
     case ENOTDIR:    // Not a directory
     case ENOTEMPTY:  // Directory not empty

src/main/protobuf/failure_details.proto

@@ -1186,7 +1186,7 @@ message PackageLoading {
     PACKAGE_MISSING = 11 [(metadata) = { exit_code: 1 }];
     TARGET_MISSING = 12 [(metadata) = { exit_code: 1 }];
     NO_SUCH_THING = 13 [(metadata) = { exit_code: 1 }];
-    GLOB_IO_EXCEPTION = 14 [(metadata) = { exit_code: 1 }];
+    GLOB_IO_EXCEPTION = 14 [(metadata) = { exit_code: 36 }];
     DUPLICATE_LABEL = 15 [(metadata) = { exit_code: 1 }];
     INVALID_PACKAGE_SPECIFICATION = 16 [(metadata) = { exit_code: 1 }];
     SYNTAX_ERROR = 17 [(metadata) = { exit_code: 1 }];

src/test/java/com/google/devtools/build/lib/buildtool/BUILD

@@ -180,8 +180,10 @@ java_test(
     ],
     deps = [
         "//src/main/java/com/google/devtools/build/lib/actions",
+        "//src/main/java/com/google/devtools/build/lib/cmdline",
         "//src/main/java/com/google/devtools/build/lib/vfs",
         "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
+        "//src/main/protobuf:failure_details_java_proto",
         "//src/test/java/com/google/devtools/build/lib/buildtool/util",
         "//src/test/java/com/google/devtools/build/lib/packages:testutil",
         "//third_party:junit4",

src/test/java/com/google/devtools/build/lib/buildtool/CustomRealFilesystemBuildIntegrationTest.java

@@ -34,7 +34,6 @@
 import com.google.devtools.build.lib.vfs.Dirent;
 import com.google.devtools.build.lib.vfs.FileStatus;
 import com.google.devtools.build.lib.vfs.FileSystem;
-import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.lib.vfs.RootedPath;
@@ -97,15 +96,18 @@ public void testIOExceptionMidLevel() throws Exception {
   }
 
   @Test
-  public void globDanglingSymlink() throws Exception {
-    Path packageDirPath = write("foo/BUILD", "exports_files(glob(['*.txt']))").getParentDirectory();
-    write("foo/existing.txt");
-    Path badSymlink = packageDirPath.getChild("bad.txt");
-    FileSystemUtils.ensureSymbolicLink(badSymlink, "nope");
-    customFileSystem.alwaysError(badSymlink);
+  public void globIOException() throws Exception {
+    Path fooBuild = write("foo/BUILD", "exports_files(glob(['**/*.txt']))").getParentDirectory();
+    Path badDir = fooBuild.getChild("bad");
+    assertThat(badDir.createDirectory()).isTrue();
+    customFileSystem.alwaysError(badDir);
     TargetParsingException e =
         assertThrows(TargetParsingException.class, () -> buildTarget("//foo:all"));
-    assertThat(e).hasMessageThat().contains("no such package 'foo': error globbing [*.txt]: nope");
+    assertThat(e)
+        .hasMessageThat()
+        .contains("no such package 'foo': error globbing [**/*.txt]: nope");
+    assertThat(e.getDetailedExitCode().getFailureDetail().getPackageLoading().getCode())
+        .isEqualTo(FailureDetails.PackageLoading.Code.GLOB_IO_EXCEPTION);
   }
 
   /**

src/test/java/com/google/devtools/build/lib/buildtool/DanglingSymlinkTest.java

@@ -18,7 +18,10 @@
 
 import com.google.devtools.build.lib.actions.BuildFailedException;
 import com.google.devtools.build.lib.buildtool.util.GoogleBuildIntegrationTestCase;
+import com.google.devtools.build.lib.cmdline.TargetParsingException;
 import com.google.devtools.build.lib.packages.util.MockGenruleSupport;
+import com.google.devtools.build.lib.server.FailureDetails;
+import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import org.junit.Before;
@@ -112,4 +115,30 @@ public void testDanglingSymlinkMidLevel() throws Exception {
     assertThrows(BuildFailedException.class, () -> buildTarget("//foo:top"));
     events.assertContainsError("Symlinking //foo:foo failed: missing input file '//foo:foo.sh'");
   }
+
+  @Test
+  public void globDanglingSymlink() throws Exception {
+    Path packageDirPath = write("foo/BUILD", "exports_files(glob(['*.txt']))").getParentDirectory();
+    write("foo/existing.txt");
+    Path badSymlink = packageDirPath.getChild("bad.txt");
+    FileSystemUtils.ensureSymbolicLink(badSymlink, "nope");
+    // Successful build: dangling symlinks in glob are ignored.
+    buildTarget("//foo:all");
+  }
+
+  @Test
+  public void globSymlinkCycle() throws Exception {
+    Path fooBuildFile = write("foo/BUILD", "sh_library(name = 'foo', srcs = glob(['*.sh']))");
+    fooBuildFile
+        .getParentDirectory()
+        .getChild("foo.sh")
+        .createSymbolicLink(PathFragment.create("foo.sh"));
+    TargetParsingException e =
+        assertThrows(TargetParsingException.class, () -> buildTarget("//foo:foo"));
+    assertThat(e.getDetailedExitCode().getFailureDetail().getPackageLoading().getCode())
+        .isEqualTo(FailureDetails.PackageLoading.Code.EVAL_GLOBS_SYMLINK_ERROR);
+  }
+
+  @Test
+  public void globMissingFile() throws Exception {}
 }