Fix move, mkdir and tweak dir listing

Makes sure mount point is a directory.

Fixes #8110 by making it explicit that progress is per-file.

Author: tannewt

docs/workflows.md

@@ -209,7 +209,7 @@ time resolution) used for the directories modification time. The RTC time will u
 
 Returns:
 
-* `204 No Content` - Directory exists
+* `204 No Content` - Directory or file exists
 * `201 Created` - Directory created
 * `401 Unauthorized` - Incorrect password
 * `403 Forbidden` - No `CIRCUITPY_WEB_API_PASSWORD` set

extmod/vfs_blockdev.c

@@ -77,6 +77,8 @@ void mp_vfs_blockdev_init(mp_vfs_blockdev_t *self, mp_obj_t bdev) {
 
 int mp_vfs_blockdev_read(mp_vfs_blockdev_t *self, size_t block_num, size_t num_blocks, uint8_t *buf) {
     if (self->flags & MP_BLOCKDEV_FLAG_NATIVE) {
+        // CIRCUITPY-CHANGE: Pass the blockdev object into native readblocks so
+        // it has the corresponding state.
         mp_uint_t (*f)(mp_obj_t self, uint8_t *, uint32_t, uint32_t) = (void *)(uintptr_t)self->readblocks[2];
         return f(self->readblocks[1], buf, block_num, num_blocks);
     } else {
@@ -109,6 +111,8 @@ int mp_vfs_blockdev_write(mp_vfs_blockdev_t *self, size_t block_num, size_t num_
     }
 
     if (self->flags & MP_BLOCKDEV_FLAG_NATIVE) {
+        // CIRCUITPY-CHANGE: Pass the blockdev object into native readblocks so
+        // it has the corresponding state.
         mp_uint_t (*f)(mp_obj_t self, const uint8_t *, uint32_t, uint32_t) = (void *)(uintptr_t)self->writeblocks[2];
         return f(self->writeblocks[1], buf, block_num, num_blocks);
     } else {
@@ -141,6 +145,7 @@ int mp_vfs_blockdev_write_ext(mp_vfs_blockdev_t *self, size_t block_num, size_t
 
 mp_obj_t mp_vfs_blockdev_ioctl(mp_vfs_blockdev_t *self, uintptr_t cmd, uintptr_t arg) {
     if (self->flags & MP_BLOCKDEV_FLAG_HAVE_IOCTL) {
+        // CIRCUITPY-CHANGE: Support native IOCTL so it can run outside of the VM.
         if (self->flags & MP_BLOCKDEV_FLAG_NATIVE) {
             size_t out_value;
             bool (*f)(mp_obj_t self, uint32_t, uint32_t, size_t *) = (void *)(uintptr_t)self->u.ioctl[2];

extmod/vfs_fat.h

@@ -34,6 +34,9 @@ typedef struct _fs_user_mount_t {
     mp_obj_base_t base;
     mp_vfs_blockdev_t blockdev;
     FATFS fatfs;
+
+    // CIRCUITPY-CHANGE: Count the users that are manipulating the blockdev via
+    // native fatfs so we can lock and unlock the blockdev.
     int8_t lock_count;
 } fs_user_mount_t;
 

locale/circuitpython.pot

@@ -896,10 +896,6 @@ msgstr ""
 msgid "Deep sleep pins must use a rising edge with pulldown"
 msgstr ""
 
-#: shared-module/jpegio/JpegDecoder.c
-msgid "Destination bitmap too small to contain image"
-msgstr ""
-
 #: shared-bindings/audiobusio/PDMIn.c
 msgid "Destination capacity is smaller than destination_length."
 msgstr ""
@@ -1379,7 +1375,11 @@ msgid "Missing jmp_pin. %q[%u] jumps on pin"
 msgstr ""
 
 #: shared-module/storage/__init__.c
-msgid "Mount point missing. Create first (maybe via USB)"
+msgid "Mount point directory missing. Create first (maybe via USB)"
+msgstr ""
+
+#: shared-module/storage/__init__.c
+msgid "Mount point must be a directory"
 msgstr ""
 
 #: shared-bindings/busio/UART.c shared-bindings/displayio/Group.c
@@ -3883,10 +3883,6 @@ msgstr ""
 msgid "parameters must be registers in sequence r0 to r3"
 msgstr ""
 
-#: shared-bindings/bitmaptools/__init__.c
-msgid "pixel coordinates out of bounds"
-msgstr ""
-
 #: extmod/vfs_posix_file.c
 msgid "poll on file not available on win32"
 msgstr ""

shared-module/storage/__init__.c

@@ -177,16 +177,20 @@ void common_hal_storage_mount(mp_obj_t vfs_obj, const char *mount_path, bool rea
     args[0] = readonly ? mp_const_true : mp_const_false;
     args[1] = mp_const_false; // Don't make the file system automatically when mounting.
 
-    // Check that there's file or directory with the same name as the mount point.
+    // Check that there is a directory with the same name as the mount point.
     // But it's ok to mount '/' in any case.
     if (strcmp(vfs->str, "/") != 0) {
         nlr_buf_t nlr;
         if (nlr_push(&nlr) == 0) {
-            common_hal_os_stat(mount_path);
+            mp_obj_t mount_point_stat = common_hal_os_stat(mount_path);
             nlr_pop();
+            mp_obj_tuple_t *t = MP_OBJ_TO_PTR(mount_point_stat);
+            if ((MP_OBJ_SMALL_INT_VALUE(t->items[0]) & MP_S_IFDIR) == 0) {
+                mp_raise_RuntimeError(MP_ERROR_TEXT("Mount point must be a directory"));
+            }
         } else {
-            // Something with the same name doesn't exists.
-            mp_raise_RuntimeError(MP_ERROR_TEXT("Mount point missing. Create first (maybe via USB)"));
+            // Something with the same name doesn't exist.
+            mp_raise_RuntimeError(MP_ERROR_TEXT("Mount point directory missing. Create first (maybe via USB)"));
         }
     }
 

supervisor/shared/web_workflow/static/directory.html

@@ -17,7 +17,7 @@ <h1><a href="/"><img src="/favicon.ico"/></a>&nbsp;<span id="path"></span></h1>
     <hr>
     <label>📄 <input type="file" id="files" multiple></label>
     <label for="dirs">📁 <input type="file" id="dirs" multiple webkitdirectory></label>
-    <label>Upload progress:<progress value="0"></progress></label>
+    <span id="progress"></span>
     <hr>
     +📁&nbsp;<input type="text" id="name"><button type="submit" id="mkdir">Create Directory</button>
     <label>Disk usage:<span id="usage"></span></label>

supervisor/shared/web_workflow/static/directory.js

@@ -72,9 +72,11 @@ async function refresh_list() {
     let editable = data.writable;
     new_directory_name.disabled = !editable;
     set_upload_enabled(editable);
+    let usbwarning = document.querySelector("#usbwarning");
     if (!editable) {
-        let usbwarning = document.querySelector("#usbwarning");
         usbwarning.style.display = "block";
+    } else {
+        usbwarning.style.display = "none";
     }
 
     if (current_path != "/") {
@@ -191,10 +193,13 @@ async function mkdir(e) {
 
 async function upload(e) {
     set_upload_enabled(false);
-    let progress = document.querySelector("progress");
+    let progress = document.querySelector("#progress");
     let made_dirs = new Set();
-    progress.max = files.files.length + dirs.files.length;
-    progress.value = 0;
+    let total = files.files.length + dirs.files.length;
+
+    var uploaded = 0;
+    var failed = 0;
+    progress.textContent = `Uploaded ${uploaded} out of ${total} files`;
     for (const file of [...files.files, ...dirs.files]) {
         let file_name = file.name;
         if (file.webkitRelativePath) {
@@ -221,12 +226,21 @@ async function upload(e) {
         )
         if (response.ok) {
             refresh_list();
+            uploaded += 1;
+        } else {
+            failed += 1;
+        }
+        progress.textContent = `Uploaded ${uploaded} out of ${total} files`;
+    }
+    var s = "";
+    if (failed > 0) {
+        if (failed > 1) {
+            s = "s";
         }
-        progress.value += 1;
+        progress.textContent = `${failed} upload${s} failed`;
     }
     files.value = "";
     dirs.value = "";
-    progress.value = 0;
     set_upload_enabled(true);
 }
 

supervisor/shared/web_workflow/web_workflow.c

@@ -1189,28 +1189,7 @@ static bool _reply(socketpool_socket_obj_t *socket, _request *request) {
             _decode_percents(request->path);
 
             char *path = request->path + 3;
-            const char *path_out = NULL;
             size_t pathlen = strlen(path);
-
-            mp_vfs_mount_t *vfs = mp_vfs_lookup_path(path, &path_out);
-            if (vfs == MP_VFS_NONE) {
-                _reply_missing(socket, request);
-                return false;
-            }
-            fs_user_mount_t *fs_mount;
-            if (vfs == MP_VFS_ROOT) {
-                fs_mount = filesystem_circuitpy();
-            } else {
-                fs_mount = MP_OBJ_TO_PTR(vfs->obj);
-                // Skip non-fat and non-native block file systems.
-                if (!filesystem_native_fatfs(fs_mount)) {
-                    _reply_missing(socket, request);
-                    return false;
-                }
-                path += strlen(vfs->str);
-                pathlen = strlen(path);
-            }
-            FATFS *fs = &fs_mount->fatfs;
             // Trailing / is a directory.
             bool directory = false;
             if (path[pathlen - 1] == '/') {
@@ -1220,18 +1199,24 @@ static bool _reply(socketpool_socket_obj_t *socket, _request *request) {
                 }
                 directory = true;
             }
+
+            // These manipulations work on the full path so do them first.
+
             // Delete is almost identical for files and directories so share the
             // implementation.
             if (strcasecmp(request->method, "DELETE") == 0) {
                 FRESULT result = supervisor_workflow_delete_recursive(path);
-                if (result == FR_NO_PATH || result == FR_NO_FILE) {
+                if (result == FR_WRITE_PROTECTED) {
+                    _reply_conflict(socket, request);
+                } else if (result == FR_NO_PATH || result == FR_NO_FILE) {
                     _reply_missing(socket, request);
                 } else if (result != FR_OK) {
                     _reply_server_error(socket, request);
                 } else {
                     _reply_no_content(socket, request);
                     return true;
                 }
+                return false;
             } else if (strcasecmp(request->method, "MOVE") == 0) {
                 _decode_percents(request->destination);
                 char *destination = request->destination + 3;
@@ -1253,7 +1238,51 @@ static bool _reply(socketpool_socket_obj_t *socket, _request *request) {
                     _reply_created(socket, request);
                     return true;
                 }
-            } else if (directory) {
+                return false;
+            } else if (directory && strcasecmp(request->method, "PUT") == 0) {
+                DWORD fattime = 0;
+                if (request->timestamp_ms > 0) {
+                    truncate_time(request->timestamp_ms * 1000000, &fattime);
+                }
+                FRESULT result = supervisor_workflow_mkdir_parents(fattime, path);
+                if (result == FR_WRITE_PROTECTED) {
+                    _reply_conflict(socket, request);
+                } else if (result == FR_EXIST) {
+                    _reply_no_content(socket, request);
+                } else if (result == FR_NO_PATH) {
+                    _reply_missing(socket, request);
+                } else if (result != FR_OK) {
+                    _reply_server_error(socket, request);
+                } else {
+                    _reply_created(socket, request);
+                    return true;
+                }
+                return false;
+            }
+
+            // These responses don't use helpers because they stream data in and
+            // out. So, share the mount lookup code.
+            const char *path_out = NULL;
+            mp_vfs_mount_t *vfs = mp_vfs_lookup_path(path, &path_out);
+            if (vfs == MP_VFS_NONE) {
+                _reply_missing(socket, request);
+                return false;
+            }
+            fs_user_mount_t *fs_mount;
+            if (vfs == MP_VFS_ROOT) {
+                fs_mount = filesystem_circuitpy();
+            } else {
+                fs_mount = MP_OBJ_TO_PTR(vfs->obj);
+                // Skip non-fat and non-native block file systems.
+                if (!filesystem_native_fatfs(fs_mount)) {
+                    _reply_missing(socket, request);
+                    return false;
+                }
+                path += strlen(vfs->str);
+                pathlen = strlen(path);
+            }
+            FATFS *fs = &fs_mount->fatfs;
+            if (directory) {
                 if (strcasecmp(request->method, "GET") == 0) {
                     FF_DIR dir;
                     FRESULT res = f_opendir(fs, &dir, path);
@@ -1274,24 +1303,6 @@ static bool _reply(socketpool_socket_obj_t *socket, _request *request) {
                     }
 
                     f_closedir(&dir);
-                } else if (strcasecmp(request->method, "PUT") == 0) {
-                    DWORD fattime = 0;
-                    if (request->timestamp_ms > 0) {
-                        truncate_time(request->timestamp_ms * 1000000, &fattime);
-                    }
-                    FRESULT result = supervisor_workflow_mkdir_parents(fattime, path);
-                    if (result == FR_WRITE_PROTECTED) {
-                        _reply_conflict(socket, request);
-                    } else if (result == FR_EXIST) {
-                        _reply_no_content(socket, request);
-                    } else if (result == FR_NO_PATH) {
-                        _reply_missing(socket, request);
-                    } else if (result != FR_OK) {
-                        _reply_server_error(socket, request);
-                    } else {
-                        _reply_created(socket, request);
-                        return true;
-                    }
                 }
             } else { // Dealing with a file.
                 if (strcasecmp(request->method, "GET") == 0) {

supervisor/shared/workflow.c

@@ -133,7 +133,7 @@ FRESULT supervisor_workflow_move(const char *old_path, const char *new_path) {
     }
     FATFS *fs = &active_mount->fatfs;
 
-    FRESULT result = f_rename(fs, old_path, new_path);
+    FRESULT result = f_rename(fs, old_mount_path, new_mount_path);
     filesystem_unlock(active_mount);
     return result;
 }
@@ -144,13 +144,27 @@ FRESULT supervisor_workflow_mkdir(DWORD fattime, const char *full_path) {
     if (active_mount == NULL || !filesystem_native_fatfs(active_mount)) {
         return FR_NO_PATH;
     }
+
+    // If there is a mount on the directory, then the mount_path will be empty.
+    if (strlen(mount_path) == 0) {
+        return FR_EXIST;
+    }
+
+    // Check to see if the directory exists already. We don't care about writing
+    // it if it already exists.
+    FATFS *fs = &active_mount->fatfs;
+    FILINFO file;
+    FRESULT result = f_stat(fs, mount_path, &file);
+    if (result == FR_OK) {
+        return FR_EXIST;
+    }
+
     if (!filesystem_lock(active_mount)) {
         return FR_WRITE_PROTECTED;
     }
-    FATFS *fs = &active_mount->fatfs;
 
     override_fattime(fattime);
-    FRESULT result = f_mkdir(fs, mount_path);
+    result = f_mkdir(fs, mount_path);
     override_fattime(0);
     filesystem_unlock(active_mount);
     return result;
@@ -171,8 +185,10 @@ FRESULT supervisor_workflow_mkdir_parents(DWORD fattime, char *path) {
         }
     }
     // Make the target directory.
-    if (result != FR_OK && result != FR_EXIST) {
+    if (result == FR_OK || result == FR_EXIST) {
         result = supervisor_workflow_mkdir(fattime, path);
+        // This may return FR_EXIST when a file with the same name already exists.
+        // FATFS does the same thing.
     }
     override_fattime(0);
     return result;
@@ -229,13 +245,13 @@ FRESULT supervisor_workflow_delete_recursive(const char *full_path) {
     }
     FATFS *fs = &active_mount->fatfs;
     FILINFO file;
-    FRESULT result = f_stat(fs, full_path, &file);
+    FRESULT result = f_stat(fs, mount_path, &file);
     if (result == FR_OK) {
         if ((file.fattrib & AM_DIR) != 0) {
-            result = supervisor_workflow_delete_directory_contents(fs, full_path);
+            result = supervisor_workflow_delete_directory_contents(fs, mount_path);
         }
         if (result == FR_OK) {
-            result = f_unlink(fs, full_path);
+            result = f_unlink(fs, mount_path);
         }
     }
     filesystem_unlock(active_mount);