builtin cd: print error about broken symlinks

When cd is passed a broken symlink, this changes the error message from
"no such directory" to "broken symbolic link".  This scenario probably
won't happen very often since completion won't suggest broken symlinks
but it can't hurt to give a good error.

Fish used to do this until 7ac5932.  This logic used to be in
path_get_cdpath, however, that is only used for highlighting, so we
don't need error messages there. Changing cd is enough.

Reword from "rotten" to "broken" since that's what file(1) uses.
Clean-up leftovers from old "rotten" code (nomen est omen).

See #8264

CHANGELOG.rst

@@ -66,6 +66,7 @@ Scripting improvements
 - ``help`` now knows which section is in which document again (:issue:`8245`).
 - Some error messages occuring after fork, like "text file busy" have been replaced by bespoke error messages for fish. This also restores error messages with current glibc versions that removed sys_errlist (:issue:`8234`, :issue:`4183`).
 - The ``realpath`` builtin now also squashes leading slashes with the ``--no-symlinks`` option (:issue:`8281`).
+- When trying to ``cd`` to a dangling (broken) symbolic link, fish will print an error noting that the target is a broken link (:issue:`8264`).
 
 Interactive improvements
 ------------------------

src/builtin_cd.cpp

@@ -68,6 +68,7 @@ maybe_t<int> builtin_cd(parser_t &parser, io_streams_t &streams, const wchar_t *
 
     errno = 0;
     auto best_errno = errno;
+    wcstring broken_symlink, broken_symlink_target;
 
     for (const auto &dir : dirs) {
         wcstring norm_dir = normalize_path(dir);
@@ -83,7 +84,12 @@ maybe_t<int> builtin_cd(parser_t &parser, io_streams_t &streams, const wchar_t *
             // - if in another directory there was a *file* by the correct name
             // we prefer *that* error because it's more specific
             if (errno == ENOENT) {
-                if (!best_errno) best_errno = errno;
+                maybe_t<wcstring> tmp;
+                if (broken_symlink.empty() && (tmp = wreadlink(norm_dir))) {
+                    broken_symlink = norm_dir;
+                    broken_symlink_target = std::move(*tmp);
+                } else if (!best_errno)
+                    best_errno = errno;
                 continue;
             } else if (errno == ENOTDIR) {
                 best_errno = errno;
@@ -105,11 +111,12 @@ maybe_t<int> builtin_cd(parser_t &parser, io_streams_t &streams, const wchar_t *
 
     if (best_errno == ENOTDIR) {
         streams.err.append_format(_(L"%ls: '%ls' is not a directory\n"), cmd, dir_in.c_str());
+    } else if (!broken_symlink.empty()) {
+        streams.err.append_format(_(L"%ls: '%ls' is a broken symbolic link to '%ls'\n"), cmd,
+                                  broken_symlink.c_str(), broken_symlink_target.c_str());
     } else if (best_errno == ENOENT) {
         streams.err.append_format(_(L"%ls: The directory '%ls' does not exist\n"), cmd,
                                   dir_in.c_str());
-    } else if (best_errno == EROTTEN) {
-        streams.err.append_format(_(L"%ls: '%ls' is a rotten symlink\n"), cmd, dir_in.c_str());
     } else if (best_errno == EACCES) {
         streams.err.append_format(_(L"%ls: Permission denied: '%ls'\n"), cmd, dir_in.c_str());
     } else {

src/path.h

@@ -9,9 +9,6 @@
 #include "common.h"
 #include "env.h"
 
-/// Return value for path_cdpath_get when locatied a rotten symlink.
-#define EROTTEN 1
-
 /// Returns the user configuration directory for fish. If the directory or one of its parents
 /// doesn't exist, they are first created.
 ///
@@ -60,9 +57,8 @@ wcstring_list_t path_get_paths(const wcstring &cmd, const environment_t &vars);
 /// directories for relative paths.
 ///
 /// If no valid path is found, false is returned and errno is set to ENOTDIR if at least one such
-/// path was found, but it did not point to a directory, EROTTEN if a rotten symbolic link was
-/// found, or ENOENT if no file of the specified name was found. If both a rotten symlink and a file
-/// are found, it is undefined which error status will be returned.
+/// path was found, but it did not point to a directory, or ENOENT if no file of the specified
+/// name was found.
 ///
 /// \param dir The name of the directory.
 /// \param wd The working directory. The working directory must end with a slash.

src/wutil.cpp

@@ -203,6 +203,29 @@ int make_fd_blocking(int fd) {
     return err == -1 ? errno : 0;
 }
 
+maybe_t<wcstring> wreadlink(const wcstring &file_name) {
+    struct stat buf;
+    if (lwstat(file_name, &buf) == -1) {
+        return none();
+    }
+    ssize_t bufsize = buf.st_size + 1;
+    char target_buf[bufsize];
+    const std::string tmp = wcs2string(file_name);
+    ssize_t nbytes = readlink(tmp.c_str(), target_buf, bufsize);
+    if (nbytes == -1) {
+        wperror(L"readlink");
+        return none();
+    }
+    // The link might have been modified after our call to lstat.  If the link now points to a path
+    // that's longer than the original one, we can't read everything in our buffer.  Simply give
+    // up. We don't need to report an error since our only caller will already fall back to ENOENT.
+    if (nbytes == bufsize) {
+        return none();
+    }
+
+    return str2wcstring(target_buf, nbytes);
+}
+
 /// Wide character realpath. The last path component does not need to be valid. If an error occurs,
 /// wrealpath() returns none() and errno is likely set.
 maybe_t<wcstring> wrealpath(const wcstring &pathname) {

src/wutil.h

@@ -43,6 +43,9 @@ void wperror(const wchar_t *s);
 /// Wide character version of getcwd().
 wcstring wgetcwd();
 
+/// Wide character version of readlink().
+maybe_t<wcstring> wreadlink(const wcstring &file_name);
+
 /// Wide character version of realpath function.
 /// \returns the canonicalized path, or none if the path is invalid.
 maybe_t<wcstring> wrealpath(const wcstring &pathname);

tests/checks/cd.fish

@@ -233,3 +233,27 @@ cd ""
 # CHECKERR: called on line {{\d+}} of file {{.*}}/cd.fish
 echo $status
 # CHECK: 1
+
+ln -s no/such/directory broken-symbolic-link
+begin
+    set -lx CDPATH
+    cd broken-symbolic-link
+end
+# CHECKERR: cd: '{{.*}}/broken-symbolic-link' is a broken symbolic link to 'no/such/directory'
+# CHECKERR: {{.*}}/cd.fish (line {{\d+}}):
+# CHECKERR: builtin cd $argv
+# CHECKERR: ^
+# CHECKERR: in function 'cd' with arguments 'broken-symbolic-link'
+# CHECKERR: called on line {{\d+}} of file {{.*}}/cd.fish
+
+# Make sure that "broken symlink" is reported over "no such file or directory".
+begin
+    set -lx CDPATH other
+    cd broken-symbolic-link
+end
+# CHECKERR: cd: '{{.*}}/broken-symbolic-link' is a broken symbolic link to 'no/such/directory'
+# CHECKERR: {{.*}}/cd.fish (line {{\d+}}):
+# CHECKERR: builtin cd $argv
+# CHECKERR: ^
+# CHECKERR: in function 'cd' with arguments 'broken-symbolic-link'
+# CHECKERR: called on line {{\d+}} of file {{.*}}/cd.fish