[BUG] rofi filebrowser does not reliably list all directory entries on remote filesystems

[uninsane]

Rofi version (rofi -v)

Version: cda0be1b (next)

Configuration

https://gist.github.com/uninsane/146429f0dda7feba1cf4e8d8aa036bac

Theme

https://gist.github.com/uninsane/c0433dcf34d4cae7dc54a23fc333ef89

Timing report

No response

Launch command

rofi -show filebrowser

Step to reproduce

1. mount an NFS export to /mnt/nfs
2. rofi -show filebrowser
3. navigate to /mnt/nfs within rofi
4. observe that sometimes rofi shows only a subset of the files/directories within an NFS mount, as compared against ls

Expected behavior

rofi should display files/directories on any filesystem. specifically, it should not rely on readdir populating the d_type field.

Actual behavior

sometimes rofi will show every directory entry. sometimes it won't, even on the same directory where previously it did. deleting all the rofi files in ~/.cache does not impact this behavior.

Additional information

readdir notes that:

         Currently, only some filesystems (among them: Btrfs, ext2,
         ext3, and ext4) have full support for returning the file
         type in d_type.  All applications must properly handle a
         return of DT_UNKNOWN.

meanwhile, filebrowser.c silently ignores entries of type DT_UNKNOWN:

      switch (rd->d_type) {
      case DT_BLK:
      case DT_CHR:
      case DT_FIFO:
      case DT_UNKNOWN:
      case DT_SOCK:
      default:
        break;
      case DT_REG:
      case DT_DIR:
      ...

the trivial fix (which does reliably remedy this on my machine) is to move the DT_UNKNOWN case into the section after break so that it's handled as DT_DIR. that addresses the worst of the issue, but leaves some warts such as (depending on the user's config):

• DT_UNKNOWN entries which are in fact directories are sorted as if they were files, if filebrowser { directories-first: true; }.
• icons may be inaccurate.

a more sophisticated fix could perhaps address those shortcomings by falling back to a more costly stat on the entry itself in the case that readdir gives DT_UNKNOWN.

Using wayland display server protocol

• No, I don't use the wayland display server protocol

I've checked if the issue exists in the latest stable release

• Yes, I have checked the problem exists in the latest stable version

[DaveDavenport]

Aah, I never tested it on a remote file system.. having to fall back on stat will slow things down a lot.
For recursive browser this is not a big issue, for file-browser that currently is not async this is.

It it planned to make file-browser async, I would propose to fix this in it once that is done.

[DaveDavenport]

This will do a stat when unknown.

diff --git a/source/modes/filebrowser.c b/source/modes/filebrowser.c
index 47df4588..6d12664e 100644
--- a/source/modes/filebrowser.c
+++ b/source/modes/filebrowser.c
@@ -265,7 +265,6 @@ static void get_file_browser(Mode *sw) {
       case DT_BLK:
       case DT_CHR:
       case DT_FIFO:
-      case DT_UNKNOWN:
       case DT_SOCK:
       default:
         break;
@@ -292,6 +291,8 @@ static void get_file_browser(Mode *sw) {

         pd->array_length++;
         break;
+
+      case DT_UNKNOWN:
       case DT_LNK:
         fb_resize_array(pd);
         // Rofi expects utf-8, so lets convert the filename.
@@ -304,9 +305,13 @@ static void get_file_browser(Mode *sw) {
             g_build_filename(cdir, rd->d_name, NULL);
         pd->array[pd->array_length].icon_fetch_uid = 0;
         pd->array[pd->array_length].icon_fetch_size = 0;
-        pd->array[pd->array_length].link = TRUE;
-        // Default to file.
         pd->array[pd->array_length].type = RFILE;
+        // Default to file.
+        if (rd->d_type == DT_LNK) {
+          pd->array[pd->array_length].link = TRUE;
+        } else {
+          pd->array[pd->array_length].link = FALSE;
+        }
         {
           // If we have link, use a stat to fine out what it is, if we fail, we
           // mark it as file.

[uninsane]

clever. for the record, for an NFS mount over my LAN, and a directory with 112 entries, this adds about 300ms delay (for the first time i enter the directory -- revisiting the directory it populates instantly). just enough to be slightly jarring, but not bad enough to really be painful.

for my case, correctness outweighs snappiness, so i'll keep this applied locally for now. thanks for that ❤️

[DaveDavenport]

quick hack for the recursive browser:

diff --git a/source/modes/recursivebrowser.c b/source/modes/recursivebrowser.c
index 206fdf85..99653af2 100644
--- a/source/modes/recursivebrowser.c
+++ b/source/modes/recursivebrowser.c
@@ -197,7 +197,6 @@ static void scan_dir(FileBrowserModePrivateData *pd, GFile *path) {
       case DT_BLK:
       case DT_CHR:
       case DT_FIFO:
-      case DT_UNKNOWN:
       case DT_SOCK:
       default:
         break;
@@ -209,6 +208,9 @@ static void scan_dir(FileBrowserModePrivateData *pd, GFile *path) {
         if (f->name == NULL) {
           f->name = rofi_force_utf8(rd->d_name, -1);
         }
+        if (f->name == NULL) {
+          f->name = g_strdup("n/a");
+        }
         f->type = (rd->d_type == DT_DIR) ? DIRECTORY : RFILE;
         f->icon_fetch_uid = 0;
         f->icon_fetch_size = 0;
@@ -228,6 +230,7 @@ static void scan_dir(FileBrowserModePrivateData *pd, GFile *path) {
         g_free(d);
         break;
       }
+      case DT_UNKNOWN:
       case DT_LNK: {
         FBFile *f = g_malloc0(sizeof(FBFile));
         // Rofi expects utf-8, so lets convert the filename.
@@ -236,14 +239,59 @@ static void scan_dir(FileBrowserModePrivateData *pd, GFile *path) {
         if (f->name == NULL) {
           f->name = rofi_force_utf8(rd->d_name, -1);
         }
+        if (f->name == NULL) {
+          f->name = g_strdup("n/a");
+        }
         f->icon_fetch_uid = 0;
         f->icon_fetch_size = 0;
-        f->link = TRUE;
         // Default to file.
         f->type = RFILE;
-        g_async_queue_push(pd->async_queue, f);
-        if (g_async_queue_length(pd->async_queue) > 10000) {
-          write(pd->pipefd2[1], "r", 1);
+        if (rd->d_type == DT_LNK) {
+          f->link = TRUE;
+        } else {
+          f->link = FALSE;
+        }
+        {
+          // If we have link, use a stat to fine out what it is, if we fail, we
+          // mark it as file.
+          // TODO have a 'broken link' mode?
+          // Convert full path to right encoding.
+          // DD: Path should be in file encoding, not utf-8
+          //          char *file =
+          //          g_filename_from_utf8(pd->array[pd->array_length].path,
+          //                                            -1, NULL, NULL, NULL);
+          if (f->path) {
+            GStatBuf statbuf;
+            if (g_stat(f->path, &statbuf) == 0) {
+              if (S_ISDIR(statbuf.st_mode)) {
+                g_free(f->path);
+                g_free(f->name);
+                g_free(f);
+                f = NULL;
+                char *d = g_build_filename(cdir, rd->d_name, NULL);
+                GFile *dirp = g_file_new_for_path(d);
+                scan_dir(pd, dirp);
+                g_object_unref(dirp);
+                g_free(d);
+                printf("subdir\n");
+                break;
+              } else if (S_ISREG(statbuf.st_mode)) {
+                f->type = RFILE;
+              }
+
+            } else {
+              g_warning("Failed to stat file: %s, %s", f->path,
+                        strerror(errno));
+            }
+
+            //            g_free(file);
+          }
+        }
+        if (f != NULL) {
+          g_async_queue_push(pd->async_queue, f);
+          if (g_async_queue_length(pd->async_queue) > 10000) {
+            write(pd->pipefd2[1], "r", 1);
+          }
         }
         break;
       }
@@ -335,7 +383,8 @@ static unsigned int recursive_browser_mode_get_num_entries(const Mode *sw) {
   return pd->array_length;
 }
 
-static ModeMode recursive_browser_mode_result(Mode *sw, int mretv, G_GNUC_UNUSED char **input,
+static ModeMode recursive_browser_mode_result(Mode *sw, int mretv,
+                                              G_GNUC_UNUSED char **input,
                                               unsigned int selected_line) {
   ModeMode retv = MODE_EXIT;
   FileBrowserModePrivateData *pd =