rework the getenv test again

 * use a virtual fat filesystem during the test
 * this makes the file I/O part more closely patch runtime which is nice
 * side-steps the need to add a special function for testing
   * but test still can't be run on a device, because the vfs calls
     are incompatible, and you intentionally can't remount "/" anyway
 * and side-steps problems with storing 'bad' toml files

Author: jepler

.pre-commit-config.yaml

@@ -8,9 +8,9 @@ repos:
     hooks:
     -   id: check-yaml
     -   id: end-of-file-fixer
-        exclude: '^(tests/.*\.toml|tests/.*\.exp|tests/cmdline/.*|tests/.*/data/.*|ports/espressif/esp-idf-config/.*|ports/espressif/boards/.*/sdkconfig)'
+        exclude: '^(tests/.*\.exp|tests/cmdline/.*|tests/.*/data/.*|ports/espressif/esp-idf-config/.*|ports/espressif/boards/.*/sdkconfig)'
     -   id: trailing-whitespace
-        exclude: '^(tests/.*\.toml|tests/.*\.exp|tests/cmdline/.*|tests/.*/data/.*|lib/mbedtls_errors/.*)'
+        exclude: '^(tests/.*\.exp|tests/cmdline/.*|tests/.*/data/.*|lib/mbedtls_errors/.*)'
 -   repo: local
     hooks:
     -   id: translations

ports/unix/main.c

@@ -420,13 +420,6 @@ STATIC void set_sys_argv(char *argv[], int argc, int start_arg) {
 #define PATHLIST_SEP_CHAR ':'
 #endif
 
-mp_obj_t common_hal_os_getenv_path(const char *path, const char *key, mp_obj_t default_);
-STATIC mp_obj_t getenv_from_file(mp_obj_t path_in, mp_obj_t key_to_get_in) {
-    return common_hal_os_getenv_path(mp_obj_str_get_str(path_in),
-        mp_obj_str_get_str(key_to_get_in), mp_const_none);
-}
-MP_DEFINE_CONST_FUN_OBJ_2(getenv_from_file_obj, getenv_from_file);
-
 MP_NOINLINE int main_(int argc, char **argv);
 
 int main(int argc, char **argv) {
@@ -547,7 +540,6 @@ MP_NOINLINE int main_(int argc, char **argv) {
         MP_DECLARE_CONST_FUN_OBJ_0(extra_cpp_coverage_obj);
         mp_store_global(MP_QSTR_extra_coverage, MP_OBJ_FROM_PTR(&extra_coverage_obj));
         mp_store_global(MP_QSTR_extra_cpp_coverage, MP_OBJ_FROM_PTR(&extra_cpp_coverage_obj));
-        mp_store_global(MP_QSTR_getenv_from_file, MP_OBJ_FROM_PTR(&getenv_from_file_obj));
     }
     #endif
 

ports/unix/modos.c

@@ -193,7 +193,12 @@ STATIC mp_obj_t mod_os_system(mp_obj_t cmd_in) {
 }
 MP_DEFINE_CONST_FUN_OBJ_1(mod_os_system_obj, mod_os_system);
 
+mp_obj_t common_hal_os_getenv(const char *key, mp_obj_t default_);
 STATIC mp_obj_t mod_os_getenv(mp_obj_t var_in) {
+    mp_obj_t result = common_hal_os_getenv(mp_obj_str_get_str(var_in), mp_const_none);
+    if (result != mp_const_none) {
+        return result;
+    }
     const char *s = getenv(mp_obj_str_get_str(var_in));
     if (s == NULL) {
         return mp_const_none;

shared-module/os/getenv.c

@@ -39,43 +39,29 @@
 #include "supervisor/filesystem.h"
 #include "supervisor/memory.h"
 
-#define GETENV_PATH "settings.toml"
+#define GETENV_PATH "/settings.toml"
 
-#if defined(UNIX)
-typedef FILE *file_arg;
-STATIC bool open_file(const char *name, file_arg *active_file) {
-    FILE *result = fopen(name, "r");
-    if (result) {
-        *active_file = result;
-    }
-    return result != NULL;
-}
-STATIC void close_file(file_arg *active_file) {
-    fclose(*active_file);
-}
-STATIC bool is_eof(file_arg *active_file) {
-    return feof(*active_file);
-}
-STATIC uint8_t get_next_byte(file_arg *active_file) {
-    int value = fgetc(*active_file);
-    if (value == EOF) {
-        return 0;
-    }
-    return value;
-}
-__attribute__((unused))
-STATIC void seek_eof(file_arg *active_file) {
-    fseek(*active_file, 0, SEEK_END);
-    (void)fgetc(*active_file);
-}
-#else
 #include "extmod/vfs.h"
 #include "extmod/vfs_fat.h"
 typedef FIL file_arg;
 STATIC bool open_file(const char *name, file_arg *active_file) {
+    #if defined(UNIX)
+    nlr_buf_t nlr;
+    if (nlr_push(&nlr) == 0) {
+        mp_obj_t file_obj = mp_call_function_2(MP_OBJ_FROM_PTR(&mp_builtin_open_obj), mp_obj_new_str(name, strlen(name)), MP_ROM_QSTR(MP_QSTR_rb));
+        mp_arg_validate_type(file_obj, &mp_type_vfs_fat_fileio, MP_QSTR_file);
+        pyb_file_obj_t *file = MP_OBJ_TO_PTR(file_obj);
+        *active_file = file->fp;
+        nlr_pop();
+        return true;
+    } else {
+        return false;
+    }
+    #else
     FATFS *fs = filesystem_circuitpy();
     FRESULT result = f_open(fs, active_file, name, FA_READ);
     return result == FR_OK;
+    #endif
 }
 STATIC void close_file(file_arg *active_file) {
     // nothing
@@ -95,7 +81,6 @@ STATIC uint8_t get_next_byte(FIL *active_file) {
 STATIC void seek_eof(file_arg *active_file) {
     f_lseek(active_file, f_size(active_file));
 }
-#endif
 
 // For a fixed buffer, record the required size rather than throwing
 STATIC void vstr_add_byte_nonstd(vstr_t *vstr, byte b) {
@@ -363,7 +348,7 @@ mp_obj_t common_hal_os_getenv_path(const char *path, const char *key, mp_obj_t d
 
     vstr_init(&buf, 64);
     os_getenv_err_t result = os_getenv_vstr(path, key, &buf, &quoted);
-    if (result == GETENV_ERR_NOT_FOUND) {
+    if (result == GETENV_ERR_NOT_FOUND || result == GETENV_ERR_OPEN) {
         return default_;
     }
     throw_getenv_error(result);

tests/circuitpython/bad1.toml

@@ -1,20 +1,78 @@
-def run_test(f, k=None):
+import uos
+
+uos.umount("/")
+
+
+class RAMBlockDevice:
+    ERASE_BLOCK_SIZE = 512
+
+    def __init__(self, blocks):
+        self.data = bytearray(blocks * self.ERASE_BLOCK_SIZE)
+
+    def readblocks(self, block, buf, off=0):
+        addr = block * self.ERASE_BLOCK_SIZE + off
+        for i in range(len(buf)):
+            buf[i] = self.data[addr + i]
+
+    def writeblocks(self, block, buf, off=None):
+        if off is None:
+            # erase, then write
+            off = 0
+        addr = block * self.ERASE_BLOCK_SIZE + off
+        for i in range(len(buf)):
+            self.data[addr + i] = buf[i]
+
+    def ioctl(self, op, arg):
+        if op == 4:  # block count
+            return len(self.data) // self.ERASE_BLOCK_SIZE
+        if op == 5:  # block size
+            return self.ERASE_BLOCK_SIZE
+        if op == 6:  # erase block
+            return 0
+
+
+bdev = RAMBlockDevice(64)
+uos.VfsFat.mkfs(bdev)
+uos.mount(uos.VfsFat(bdev), "/")
+
+content_good = """
+# comment
+key0 = "hello world"
+key1 = 7
+ cstring = "hello comment" # comment
+  cnumber = 0x7f # comment
+key2= "\n"
+key3 ="\u00c1x"
+key4 = "\U000000c1x"
+key5 = "\f\"\\"
+key6 = "\t\r\b"
+key7 = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
+key8 = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
+[section]
+subvalue = "hi"
+"""
+
+content_bad = [
+    'key = "\n',
+    'key = """\n',
+    "key =\n",
+    'key="',
+]
+
+
+def run_test(key, content):
+    with open("/settings.toml", "w") as f:
+        f.write(content)
+
     try:
-        v = getenv_from_file(f"{BASE}/{f}.toml", k or f)
-        print(f, k, repr(v))
+        v = uos.getenv(key)
+        print(key, repr(v))
     except Exception as e:
-        print(f, k, str(e))
+        print(key, str(e))
 
 
-BASE = __file__.rpartition("/")[0] or "."
+for i in range(9):
+    run_test(f"key{i}", content_good)
 
-run_test("good", "notpresent")
-run_test("good", "string")
-run_test("good", "number")
-run_test("good", "cstring")
-run_test("good", "cnumber")
-run_test("good", "subvalue")
-for i in range(8):
-    run_test("good", f"string{i}")
-for i in range(1, 5):
-    run_test(f"bad{i}", f"string")
+for content in content_bad:
+    run_test("key", content)

tests/circuitpython/bad2.toml

@@ -1,18 +1,13 @@
-good.toml notpresent None
-good.toml string 'hello world'
-good.toml number 7
-good.toml cstring 'hello comment'
-good.toml cnumber 127
-good.toml subvalue None
-good.toml string0 None
-good.toml string1 '\n'
-good.toml string2 'Áx'
-good.toml string3 'Áx'
-good.toml string4 '\x0c"\\'
-good.toml string5 '\t\r\x08'
-good.toml string6 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'
-good.toml string7 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'
-bad1.toml.bin string Invalid byte '\n'
-bad2.toml.bin string Invalid byte '"'
-bad3.toml.bin string invalid syntax for integer with base 10: ''
-bad4.toml.bin string Invalid byte 'EOF'
+key0 'hello world'
+key1 7
+key2 Invalid byte '\n'
+key3 'Áx'
+key4 'Áx'
+key5 Invalid byte '\\'
+key6 '\t\r\x08'
+key7 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'
+key8 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'
+key Invalid byte '\n'
+key Invalid byte '"'
+key invalid syntax for integer with base 10: ''
+key Invalid byte 'EOF'