Skip to content

Commit 234ce7c

Browse files
tannewttannewt
committed
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.
1 parent 24359f9 commit 234ce7c

File tree

9 files changed

+118
-69
lines changed

9 files changed

+118
-69
lines changed

‎docs/workflows.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ time resolution) used for the directories modification time. The RTC time will u
209209

210210
Returns:
211211

212-
* `204 No Content` - Directory exists
212+
* `204 No Content` - Directory or file exists
213213
* `201 Created` - Directory created
214214
* `401 Unauthorized` - Incorrect password
215215
* `403 Forbidden` - No `CIRCUITPY_WEB_API_PASSWORD` set

‎extmod/vfs_blockdev.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,8 @@ void mp_vfs_blockdev_init(mp_vfs_blockdev_t *self, mp_obj_t bdev) {
7777

7878
int mp_vfs_blockdev_read(mp_vfs_blockdev_t *self, size_t block_num, size_t num_blocks, uint8_t *buf) {
7979
if (self->flags & MP_BLOCKDEV_FLAG_NATIVE) {
80+
// CIRCUITPY-CHANGE: Pass the blockdev object into native readblocks so
81+
// it has the corresponding state.
8082
mp_uint_t (*f)(mp_obj_t self, uint8_t *, uint32_t, uint32_t) = (void *)(uintptr_t)self->readblocks[2];
8183
return f(self->readblocks[1], buf, block_num, num_blocks);
8284
} else {
@@ -109,6 +111,8 @@ int mp_vfs_blockdev_write(mp_vfs_blockdev_t *self, size_t block_num, size_t num_
109111
}
110112

111113
if (self->flags & MP_BLOCKDEV_FLAG_NATIVE) {
114+
// CIRCUITPY-CHANGE: Pass the blockdev object into native readblocks so
115+
// it has the corresponding state.
112116
mp_uint_t (*f)(mp_obj_t self, const uint8_t *, uint32_t, uint32_t) = (void *)(uintptr_t)self->writeblocks[2];
113117
return f(self->writeblocks[1], buf, block_num, num_blocks);
114118
} else {
@@ -141,6 +145,7 @@ int mp_vfs_blockdev_write_ext(mp_vfs_blockdev_t *self, size_t block_num, size_t
141145

142146
mp_obj_t mp_vfs_blockdev_ioctl(mp_vfs_blockdev_t *self, uintptr_t cmd, uintptr_t arg) {
143147
if (self->flags & MP_BLOCKDEV_FLAG_HAVE_IOCTL) {
148+
// CIRCUITPY-CHANGE: Support native IOCTL so it can run outside of the VM.
144149
if (self->flags & MP_BLOCKDEV_FLAG_NATIVE) {
145150
size_t out_value;
146151
bool (*f)(mp_obj_t self, uint32_t, uint32_t, size_t *) = (void *)(uintptr_t)self->u.ioctl[2];

‎extmod/vfs_fat.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@ typedef struct _fs_user_mount_t {
3434
mp_obj_base_t base;
3535
mp_vfs_blockdev_t blockdev;
3636
FATFS fatfs;
37+
38+
// CIRCUITPY-CHANGE: Count the users that are manipulating the blockdev via
39+
// native fatfs so we can lock and unlock the blockdev.
3740
int8_t lock_count;
3841
} fs_user_mount_t;
3942

‎locale/circuitpython.pot

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -896,10 +896,6 @@ msgstr ""
896896
msgid "Deep sleep pins must use a rising edge with pulldown"
897897
msgstr ""
898898

899-
#: shared-module/jpegio/JpegDecoder.c
900-
msgid "Destination bitmap too small to contain image"
901-
msgstr ""
902-
903899
#: shared-bindings/audiobusio/PDMIn.c
904900
msgid "Destination capacity is smaller than destination_length."
905901
msgstr ""
@@ -1379,7 +1375,11 @@ msgid "Missing jmp_pin. %q[%u] jumps on pin"
13791375
msgstr ""
13801376

13811377
#: shared-module/storage/__init__.c
1382-
msgid "Mount point missing. Create first (maybe via USB)"
1378+
msgid "Mount point directory missing. Create first (maybe via USB)"
1379+
msgstr ""
1380+
1381+
#: shared-module/storage/__init__.c
1382+
msgid "Mount point must be a directory"
13831383
msgstr ""
13841384

13851385
#: shared-bindings/busio/UART.c shared-bindings/displayio/Group.c
@@ -3883,10 +3883,6 @@ msgstr ""
38833883
msgid "parameters must be registers in sequence r0 to r3"
38843884
msgstr ""
38853885

3886-
#: shared-bindings/bitmaptools/__init__.c
3887-
msgid "pixel coordinates out of bounds"
3888-
msgstr ""
3889-
38903886
#: extmod/vfs_posix_file.c
38913887
msgid "poll on file not available on win32"
38923888
msgstr ""

‎shared-module/storage/__init__.c

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -177,16 +177,20 @@ void common_hal_storage_mount(mp_obj_t vfs_obj, const char *mount_path, bool rea
177177
args[0] = readonly ? mp_const_true : mp_const_false;
178178
args[1] = mp_const_false; // Don't make the file system automatically when mounting.
179179

180-
// Check that there's file or directory with the same name as the mount point.
180+
// Check that there is a directory with the same name as the mount point.
181181
// But it's ok to mount '/' in any case.
182182
if (strcmp(vfs->str, "/") != 0) {
183183
nlr_buf_t nlr;
184184
if (nlr_push(&nlr) == 0) {
185-
common_hal_os_stat(mount_path);
185+
mp_obj_t mount_point_stat = common_hal_os_stat(mount_path);
186186
nlr_pop();
187+
mp_obj_tuple_t *t = MP_OBJ_TO_PTR(mount_point_stat);
188+
if ((MP_OBJ_SMALL_INT_VALUE(t->items[0]) & MP_S_IFDIR) == 0) {
189+
mp_raise_RuntimeError(MP_ERROR_TEXT("Mount point must be a directory"));
190+
}
187191
} else {
188-
// Something with the same name doesn't exists.
189-
mp_raise_RuntimeError(MP_ERROR_TEXT("Mount point missing. Create first (maybe via USB)"));
192+
// Something with the same name doesn't exist.
193+
mp_raise_RuntimeError(MP_ERROR_TEXT("Mount point directory missing. Create first (maybe via USB)"));
190194
}
191195
}
192196

‎supervisor/shared/web_workflow/static/directory.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ <h1><a href="/"><img src="/favicon.ico"/></a>&nbsp;<span id="path"></span></h1>
1717
<hr>
1818
<label>📄 <input type="file" id="files" multiple></label>
1919
<label for="dirs">📁 <input type="file" id="dirs" multiple webkitdirectory></label>
20-
<label>Upload progress:<progress value="0"></progress></label>
20+
<span id="progress"></span>
2121
<hr>
2222
+📁&nbsp;<input type="text" id="name"><button type="submit" id="mkdir">Create Directory</button>
2323
<label>Disk usage:<span id="usage"></span></label>

‎supervisor/shared/web_workflow/static/directory.js

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,11 @@ async function refresh_list() {
7272
let editable = data.writable;
7373
new_directory_name.disabled = !editable;
7474
set_upload_enabled(editable);
75+
let usbwarning = document.querySelector("#usbwarning");
7576
if (!editable) {
76-
let usbwarning = document.querySelector("#usbwarning");
7777
usbwarning.style.display = "block";
78+
} else {
79+
usbwarning.style.display = "none";
7880
}
7981

8082
if (current_path != "/") {
@@ -191,10 +193,13 @@ async function mkdir(e) {
191193

192194
async function upload(e) {
193195
set_upload_enabled(false);
194-
let progress = document.querySelector("progress");
196+
let progress = document.querySelector("#progress");
195197
let made_dirs = new Set();
196-
progress.max = files.files.length + dirs.files.length;
197-
progress.value = 0;
198+
let total = files.files.length + dirs.files.length;
199+
200+
var uploaded = 0;
201+
var failed = 0;
202+
progress.textContent = `Uploaded ${uploaded} out of ${total} files`;
198203
for (const file of [...files.files, ...dirs.files]) {
199204
let file_name = file.name;
200205
if (file.webkitRelativePath) {
@@ -221,12 +226,21 @@ async function upload(e) {
221226
)
222227
if (response.ok) {
223228
refresh_list();
229+
uploaded += 1;
230+
} else {
231+
failed += 1;
232+
}
233+
progress.textContent = `Uploaded ${uploaded} out of ${total} files`;
234+
}
235+
var s = "";
236+
if (failed > 0) {
237+
if (failed > 1) {
238+
s = "s";
224239
}
225-
progress.value += 1;
240+
progress.textContent = `${failed} upload${s} failed`;
226241
}
227242
files.value = "";
228243
dirs.value = "";
229-
progress.value = 0;
230244
set_upload_enabled(true);
231245
}
232246

‎supervisor/shared/web_workflow/web_workflow.c

Lines changed: 52 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1189,28 +1189,7 @@ static bool _reply(socketpool_socket_obj_t *socket, _request *request) {
11891189
_decode_percents(request->path);
11901190

11911191
char *path = request->path + 3;
1192-
const char *path_out = NULL;
11931192
size_t pathlen = strlen(path);
1194-
1195-
mp_vfs_mount_t *vfs = mp_vfs_lookup_path(path, &path_out);
1196-
if (vfs == MP_VFS_NONE) {
1197-
_reply_missing(socket, request);
1198-
return false;
1199-
}
1200-
fs_user_mount_t *fs_mount;
1201-
if (vfs == MP_VFS_ROOT) {
1202-
fs_mount = filesystem_circuitpy();
1203-
} else {
1204-
fs_mount = MP_OBJ_TO_PTR(vfs->obj);
1205-
// Skip non-fat and non-native block file systems.
1206-
if (!filesystem_native_fatfs(fs_mount)) {
1207-
_reply_missing(socket, request);
1208-
return false;
1209-
}
1210-
path += strlen(vfs->str);
1211-
pathlen = strlen(path);
1212-
}
1213-
FATFS *fs = &fs_mount->fatfs;
12141193
// Trailing / is a directory.
12151194
bool directory = false;
12161195
if (path[pathlen - 1] == '/') {
@@ -1220,18 +1199,24 @@ static bool _reply(socketpool_socket_obj_t *socket, _request *request) {
12201199
}
12211200
directory = true;
12221201
}
1202+
1203+
// These manipulations work on the full path so do them first.
1204+
12231205
// Delete is almost identical for files and directories so share the
12241206
// implementation.
12251207
if (strcasecmp(request->method, "DELETE") == 0) {
12261208
FRESULT result = supervisor_workflow_delete_recursive(path);
1227-
if (result == FR_NO_PATH || result == FR_NO_FILE) {
1209+
if (result == FR_WRITE_PROTECTED) {
1210+
_reply_conflict(socket, request);
1211+
} else if (result == FR_NO_PATH || result == FR_NO_FILE) {
12281212
_reply_missing(socket, request);
12291213
} else if (result != FR_OK) {
12301214
_reply_server_error(socket, request);
12311215
} else {
12321216
_reply_no_content(socket, request);
12331217
return true;
12341218
}
1219+
return false;
12351220
} else if (strcasecmp(request->method, "MOVE") == 0) {
12361221
_decode_percents(request->destination);
12371222
char *destination = request->destination + 3;
@@ -1253,7 +1238,51 @@ static bool _reply(socketpool_socket_obj_t *socket, _request *request) {
12531238
_reply_created(socket, request);
12541239
return true;
12551240
}
1256-
} else if (directory) {
1241+
return false;
1242+
} else if (directory && strcasecmp(request->method, "PUT") == 0) {
1243+
DWORD fattime = 0;
1244+
if (request->timestamp_ms > 0) {
1245+
truncate_time(request->timestamp_ms * 1000000, &fattime);
1246+
}
1247+
FRESULT result = supervisor_workflow_mkdir_parents(fattime, path);
1248+
if (result == FR_WRITE_PROTECTED) {
1249+
_reply_conflict(socket, request);
1250+
} else if (result == FR_EXIST) {
1251+
_reply_no_content(socket, request);
1252+
} else if (result == FR_NO_PATH) {
1253+
_reply_missing(socket, request);
1254+
} else if (result != FR_OK) {
1255+
_reply_server_error(socket, request);
1256+
} else {
1257+
_reply_created(socket, request);
1258+
return true;
1259+
}
1260+
return false;
1261+
}
1262+
1263+
// These responses don't use helpers because they stream data in and
1264+
// out. So, share the mount lookup code.
1265+
const char *path_out = NULL;
1266+
mp_vfs_mount_t *vfs = mp_vfs_lookup_path(path, &path_out);
1267+
if (vfs == MP_VFS_NONE) {
1268+
_reply_missing(socket, request);
1269+
return false;
1270+
}
1271+
fs_user_mount_t *fs_mount;
1272+
if (vfs == MP_VFS_ROOT) {
1273+
fs_mount = filesystem_circuitpy();
1274+
} else {
1275+
fs_mount = MP_OBJ_TO_PTR(vfs->obj);
1276+
// Skip non-fat and non-native block file systems.
1277+
if (!filesystem_native_fatfs(fs_mount)) {
1278+
_reply_missing(socket, request);
1279+
return false;
1280+
}
1281+
path += strlen(vfs->str);
1282+
pathlen = strlen(path);
1283+
}
1284+
FATFS *fs = &fs_mount->fatfs;
1285+
if (directory) {
12571286
if (strcasecmp(request->method, "GET") == 0) {
12581287
FF_DIR dir;
12591288
FRESULT res = f_opendir(fs, &dir, path);
@@ -1274,24 +1303,6 @@ static bool _reply(socketpool_socket_obj_t *socket, _request *request) {
12741303
}
12751304

12761305
f_closedir(&dir);
1277-
} else if (strcasecmp(request->method, "PUT") == 0) {
1278-
DWORD fattime = 0;
1279-
if (request->timestamp_ms > 0) {
1280-
truncate_time(request->timestamp_ms * 1000000, &fattime);
1281-
}
1282-
FRESULT result = supervisor_workflow_mkdir_parents(fattime, path);
1283-
if (result == FR_WRITE_PROTECTED) {
1284-
_reply_conflict(socket, request);
1285-
} else if (result == FR_EXIST) {
1286-
_reply_no_content(socket, request);
1287-
} else if (result == FR_NO_PATH) {
1288-
_reply_missing(socket, request);
1289-
} else if (result != FR_OK) {
1290-
_reply_server_error(socket, request);
1291-
} else {
1292-
_reply_created(socket, request);
1293-
return true;
1294-
}
12951306
}
12961307
} else { // Dealing with a file.
12971308
if (strcasecmp(request->method, "GET") == 0) {

‎supervisor/shared/workflow.c

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ FRESULT supervisor_workflow_move(const char *old_path, const char *new_path) {
133133
}
134134
FATFS *fs = &active_mount->fatfs;
135135

136-
FRESULT result = f_rename(fs, old_path, new_path);
136+
FRESULT result = f_rename(fs, old_mount_path, new_mount_path);
137137
filesystem_unlock(active_mount);
138138
return result;
139139
}
@@ -144,13 +144,27 @@ FRESULT supervisor_workflow_mkdir(DWORD fattime, const char *full_path) {
144144
if (active_mount == NULL || !filesystem_native_fatfs(active_mount)) {
145145
return FR_NO_PATH;
146146
}
147+
148+
// If there is a mount on the directory, then the mount_path will be empty.
149+
if (strlen(mount_path) == 0) {
150+
return FR_EXIST;
151+
}
152+
153+
// Check to see if the directory exists already. We don't care about writing
154+
// it if it already exists.
155+
FATFS *fs = &active_mount->fatfs;
156+
FILINFO file;
157+
FRESULT result = f_stat(fs, mount_path, &file);
158+
if (result == FR_OK) {
159+
return FR_EXIST;
160+
}
161+
147162
if (!filesystem_lock(active_mount)) {
148163
return FR_WRITE_PROTECTED;
149164
}
150-
FATFS *fs = &active_mount->fatfs;
151165

152166
override_fattime(fattime);
153-
FRESULT result = f_mkdir(fs, mount_path);
167+
result = f_mkdir(fs, mount_path);
154168
override_fattime(0);
155169
filesystem_unlock(active_mount);
156170
return result;
@@ -171,8 +185,10 @@ FRESULT supervisor_workflow_mkdir_parents(DWORD fattime, char *path) {
171185
}
172186
}
173187
// Make the target directory.
174-
if (result != FR_OK && result != FR_EXIST) {
188+
if (result == FR_OK || result == FR_EXIST) {
175189
result = supervisor_workflow_mkdir(fattime, path);
190+
// This may return FR_EXIST when a file with the same name already exists.
191+
// FATFS does the same thing.
176192
}
177193
override_fattime(0);
178194
return result;
@@ -229,13 +245,13 @@ FRESULT supervisor_workflow_delete_recursive(const char *full_path) {
229245
}
230246
FATFS *fs = &active_mount->fatfs;
231247
FILINFO file;
232-
FRESULT result = f_stat(fs, full_path, &file);
248+
FRESULT result = f_stat(fs, mount_path, &file);
233249
if (result == FR_OK) {
234250
if ((file.fattrib & AM_DIR) != 0) {
235-
result = supervisor_workflow_delete_directory_contents(fs, full_path);
251+
result = supervisor_workflow_delete_directory_contents(fs, mount_path);
236252
}
237253
if (result == FR_OK) {
238-
result = f_unlink(fs, full_path);
254+
result = f_unlink(fs, mount_path);
239255
}
240256
}
241257
filesystem_unlock(active_mount);

0 commit comments

Comments
 (0)