Add a recursive option to mkdir (#729)

* Add a recursive option to mkdir

I really will need a second pair of eyes here.

* Update bridge and defaults in JS API

* Fix options default assignment

* Add a test

* Add callback test

* Remove default

* Detect windows separator

* Update recursive algorithm

* Bug fixes

* Keep

* Revert

* Debug

* Fixes

* Revert "Revert"

This reverts commit 24627b6.

* Fix spacing on jsdoc

* Remove log

* Update JSDoc

* Remove unused headers

* Add add missing recursive option to test

api/README.md

@@ -1148,21 +1148,23 @@ Creates a link to `dest` from `dest`.
 | :---         | :--- | :---        |
 | Not specified | Promise |  |
 
-## [`mkdir(path, options)`](https://github.com/socketsupply/socket/blob/master/api/fs/promises.js#L225)
+## [`mkdir(path, options)`](https://github.com/socketsupply/socket/blob/master/api/fs/promises.js#L226)
 
 Asynchronously creates a directory.
 
 
 | Argument | Type | Default | Optional | Description |
 | :---     | :--- | :---:   | :---:    | :---        |
-| path | String |  | false | The path to create |
-| options | Object |  | false | The optional options argument can be an integer specifying mode (permission and sticky bits), or an object with a mode property and a recursive property indicating whether parent directories should be created. Calling fs.mkdir() when path is a directory that exists results in an error only when recursive is false. |
+| path | string |  | false | The path to create |
+| options | object |  | true | The optional options argument can be an integer specifying mode (permission and sticky bits), or an object with a mode property and a recursive property indicating whether parent directories should be created. Calling fs.mkdir() when path is a directory that exists results in an error only when recursive is false. |
+| options.recursive | boolean | false | true | Recursively create missing path segments. |
+| options.mode | number | 0o777 | true | Set the mode of directory, or missing path segments when recursive is true. |
 
 | Return Value | Type | Description |
 | :---         | :--- | :---        |
 | Not specified | Promise<any> | Upon success, fulfills with undefined if recursive is false, or the first directory path created if recursive is true. |
 
-## [`open(path, flags, mode)`](https://github.com/socketsupply/socket/blob/master/api/fs/promises.js#L253)
+## [`open(path, flags, mode)`](https://github.com/socketsupply/socket/blob/master/api/fs/promises.js#L254)
 
 External docs: https://nodejs.org/api/fs.html#fspromisesopenpath-flags-mode
 Asynchronously open a file.
@@ -1178,7 +1180,7 @@ Asynchronously open a file.
 | :---         | :--- | :---        |
 | Not specified | Promise<FileHandle> |  |
 
-## [`opendir(path, options)`](https://github.com/socketsupply/socket/blob/master/api/fs/promises.js#L265)
+## [`opendir(path, options)`](https://github.com/socketsupply/socket/blob/master/api/fs/promises.js#L266)
 
 External docs: https://nodejs.org/api/fs.html#fspromisesopendirpath-options
 
@@ -1194,7 +1196,7 @@ External docs: https://nodejs.org/api/fs.html#fspromisesopendirpath-options
 | :---         | :--- | :---        |
 | Not specified | Promise<Dir> |  |
 
-## [`readdir(path, options)`](https://github.com/socketsupply/socket/blob/master/api/fs/promises.js#L277)
+## [`readdir(path, options)`](https://github.com/socketsupply/socket/blob/master/api/fs/promises.js#L278)
 
 External docs: https://nodejs.org/dist/latest-v20.x/docs/api/fs.html#fspromisesreaddirpath-options
 
@@ -1206,7 +1208,7 @@ External docs: https://nodejs.org/dist/latest-v20.x/docs/api/fs.html#fspromisesr
 | options.encoding | string? | utf8 | true |  |
 | options.withFileTypes | boolean? | false | true |  |
 
-## [`readFile(path, options)`](https://github.com/socketsupply/socket/blob/master/api/fs/promises.js#L310)
+## [`readFile(path, options)`](https://github.com/socketsupply/socket/blob/master/api/fs/promises.js#L311)
 
 External docs: https://nodejs.org/dist/latest-v20.x/docs/api/fs.html#fspromisesreadfilepath-options
 
@@ -1223,7 +1225,7 @@ External docs: https://nodejs.org/dist/latest-v20.x/docs/api/fs.html#fspromisesr
 | :---         | :--- | :---        |
 | Not specified | Promise<Buffer \| string> |  |
 
-## [`readlink(path)`](https://github.com/socketsupply/socket/blob/master/api/fs/promises.js#L327)
+## [`readlink(path)`](https://github.com/socketsupply/socket/blob/master/api/fs/promises.js#L328)
 
 Reads link at `path`
 
@@ -1235,7 +1237,7 @@ Reads link at `path`
 | :---         | :--- | :---        |
 | Not specified | Promise<string> |  |
 
-## [`realpath(path)`](https://github.com/socketsupply/socket/blob/master/api/fs/promises.js#L346)
+## [`realpath(path)`](https://github.com/socketsupply/socket/blob/master/api/fs/promises.js#L347)
 
 Computes real path for `path`
 
@@ -1247,7 +1249,7 @@ Computes real path for `path`
 | :---         | :--- | :---        |
 | Not specified | Promise<string> |  |
 
-## [`rename(src, dest)`](https://github.com/socketsupply/socket/blob/master/api/fs/promises.js#L366)
+## [`rename(src, dest)`](https://github.com/socketsupply/socket/blob/master/api/fs/promises.js#L367)
 
 Renames file or directory at `src` to `dest`.
 
@@ -1260,7 +1262,7 @@ Renames file or directory at `src` to `dest`.
 | :---         | :--- | :---        |
 | Not specified | Promise |  |
 
-## [`rmdir(path)`](https://github.com/socketsupply/socket/blob/master/api/fs/promises.js#L387)
+## [`rmdir(path)`](https://github.com/socketsupply/socket/blob/master/api/fs/promises.js#L388)
 
 Removes directory at `path`.
 
@@ -1272,7 +1274,7 @@ Removes directory at `path`.
 | :---         | :--- | :---        |
 | Not specified | Promise |  |
 
-## [`stat(path, options)`](https://github.com/socketsupply/socket/blob/master/api/fs/promises.js#L406)
+## [`stat(path, options)`](https://github.com/socketsupply/socket/blob/master/api/fs/promises.js#L407)
 
 External docs: https://nodejs.org/api/fs.html#fspromisesstatpath-options
 
@@ -1287,7 +1289,7 @@ External docs: https://nodejs.org/api/fs.html#fspromisesstatpath-options
 | :---         | :--- | :---        |
 | Not specified | Promise<Stats> |  |
 
-## [`symlink(src, dest)`](https://github.com/socketsupply/socket/blob/master/api/fs/promises.js#L418)
+## [`symlink(src, dest)`](https://github.com/socketsupply/socket/blob/master/api/fs/promises.js#L419)
 
 Creates a symlink of `src` at `dest`.
 
@@ -1300,7 +1302,7 @@ Creates a symlink of `src` at `dest`.
 | :---         | :--- | :---        |
 | Not specified | Promise |  |
 
-## [`unlink(path)`](https://github.com/socketsupply/socket/blob/master/api/fs/promises.js#L453)
+## [`unlink(path)`](https://github.com/socketsupply/socket/blob/master/api/fs/promises.js#L454)
 
 Unlinks (removes) file at `path`.
 
@@ -1312,7 +1314,7 @@ Unlinks (removes) file at `path`.
 | :---         | :--- | :---        |
 | Not specified | Promise |  |
 
-## [`writeFile(path, data, options)`](https://github.com/socketsupply/socket/blob/master/api/fs/promises.js#L477)
+## [`writeFile(path, data, options)`](https://github.com/socketsupply/socket/blob/master/api/fs/promises.js#L478)
 
 External docs: https://nodejs.org/dist/latest-v20.x/docs/api/fs.html#fspromiseswritefilefile-data-options
 
@@ -1331,7 +1333,7 @@ External docs: https://nodejs.org/dist/latest-v20.x/docs/api/fs.html#fspromisesw
 | :---         | :--- | :---        |
 | Not specified | Promise<void> |  |
 
-## [`watch(, options)`](https://github.com/socketsupply/socket/blob/master/api/fs/promises.js#L497)
+## [`watch(, options)`](https://github.com/socketsupply/socket/blob/master/api/fs/promises.js#L498)
 
 Watch for changes at `path` calling `callback`
 

api/fs/index.js

@@ -396,7 +396,7 @@ export function mkdir (path, options, callback) {
   }
 
   const mode = options.mode || 0o777
-  const recursive = options.recursive === true
+  const recursive = Boolean(options.recursive) // default to false
 
   if (typeof mode !== 'number') {
     throw new TypeError('mode must be a number.')

api/fs/promises.js

@@ -216,15 +216,16 @@ export async function link (src, dest) {
 
 /**
  * Asynchronously creates a directory.
- * @todo recursive option is not implemented yet.
  *
- * @param {String} path - The path to create
- * @param {Object} options - The optional options argument can be an integer specifying mode (permission and sticky bits), or an object with a mode property and a recursive property indicating whether parent directories should be created. Calling fs.mkdir() when path is a directory that exists results in an error only when recursive is false.
+ * @param {string} path - The path to create
+ * @param {object} [options] - The optional options argument can be an integer specifying mode (permission and sticky bits), or an object with a mode property and a recursive property indicating whether parent directories should be created. Calling fs.mkdir() when path is a directory that exists results in an error only when recursive is false.
+ * @param {boolean} [options.recursive=false] - Recursively create missing path segments.
+ * @param {number} [options.mode=0o777] - Set the mode of directory, or missing path segments when recursive is true.
  * @return {Promise<any>} - Upon success, fulfills with undefined if recursive is false, or the first directory path created if recursive is true.
  */
 export async function mkdir (path, options = {}) {
   const mode = options.mode ?? 0o777
-  const recursive = options.recurisve === true
+  const recursive = Boolean(options.recursive)
 
   if (typeof mode !== 'number') {
     throw new TypeError('mode must be a number.')

api/index.d.ts

@@ -2742,13 +2742,17 @@ declare module "socket:fs/promises" {
     export function link(src: string, dest: string): Promise<any>;
     /**
      * Asynchronously creates a directory.
-     * @todo recursive option is not implemented yet.
      *
-     * @param {String} path - The path to create
-     * @param {Object} options - The optional options argument can be an integer specifying mode (permission and sticky bits), or an object with a mode property and a recursive property indicating whether parent directories should be created. Calling fs.mkdir() when path is a directory that exists results in an error only when recursive is false.
+     * @param {string} path - The path to create
+     * @param {object} [options] - The optional options argument can be an integer specifying mode (permission and sticky bits), or an object with a mode property and a recursive property indicating whether parent directories should be created. Calling fs.mkdir() when path is a directory that exists results in an error only when recursive is false.
+     * @param {boolean} [options.recursive=false] - Recursively create missing path segments.
+     * @param {number} [options.mode=0o777] - Set the mode of directory, or missing path segments when recursive is true.
      * @return {Promise<any>} - Upon success, fulfills with undefined if recursive is false, or the first directory path created if recursive is true.
      */
-    export function mkdir(path: string, options?: any): Promise<any>;
+    export function mkdir(path: string, options?: {
+        recursive?: boolean;
+        mode?: number;
+    }): Promise<any>;
     /**
      * Asynchronously open a file.
      * @see {@link https://nodejs.org/api/fs.html#fspromisesopenpath-flags-mode }

src/core/core.hh

@@ -510,6 +510,7 @@ namespace SSC {
             const String seq,
             const String path,
             int mode,
+            bool recursive,
             Module::Callback cb
           );
           void readlink (

src/core/fs.cc

@@ -1845,14 +1845,16 @@ namespace SSC {
     const String seq,
     const String path,
     int mode,
+    bool recursive,
     Module::Callback cb
   ) {
     this->core->dispatchEventLoop([=, this]() {
+      int err = 0;
       auto filename = path.c_str();
       auto loop = &this->core->eventLoop;
       auto ctx = new RequestContext(seq, cb);
       auto req = &ctx->req;
-      auto err = uv_fs_mkdir(loop, req, filename, mode, [](uv_fs_t* req) {
+      const auto callback = [](uv_fs_t* req) {
         auto ctx = (RequestContext *) req->data;
         auto json = JSON::Object {};
 
@@ -1875,7 +1877,35 @@ namespace SSC {
 
         ctx->cb(ctx->seq, json, Post{});
         delete ctx;
-      });
+      };
+
+      if (!recursive) {
+        err = uv_fs_mkdir(loop, req, filename, mode, callback);
+      } else {
+        const auto sep = String(1, std::filesystem::path::preferred_separator);
+        const auto components = split(path, sep);
+        auto queue = std::queue(std::deque(components.begin(), components.end()));
+        auto currentComponents = Vector<String>();
+        while (queue.size() > 0) {
+          uv_fs_t req;
+          const auto currentComponent = queue.front();
+          queue.pop();
+          currentComponents.push_back(currentComponent);
+          const auto joinedComponents = join(currentComponents, sep);
+          const auto currentPath = joinedComponents.empty() ? sep : joinedComponents;
+          if (queue.size() == 0) {
+            err = uv_fs_mkdir(loop, &ctx->req, currentPath.c_str(), mode, callback);
+          } else {
+            err = uv_fs_mkdir(loop, &req, currentPath.c_str(), mode, nullptr);
+          }
+          if (err == 0 || err == -EEXIST) {
+            err = 0;
+            continue;
+          } else {
+            break;
+          }
+        }
+      }
 
       if (err < 0) {
         auto json = JSON::Object::Entries {

src/ipc/bridge.cc

@@ -737,9 +737,10 @@ static void initRouterTable (Router *router) {
   });
 
   /**
-   * Creates a directory at `path` with an optional mode.
+   * Creates a directory at `path` with an optional mode and an optional recursive flag.
    * @param path
    * @param mode
+   * @param recursive
    * @see mkdir(2)
    */
   router->map("fs.mkdir", [](auto message, auto router, auto reply) {
@@ -756,10 +757,12 @@ static void initRouterTable (Router *router) {
       message.seq,
       message.get("path"),
       mode,
+      message.get("recursive") == "true",
       RESULT_CALLBACK_FROM_CORE_CALLBACK(message, reply)
     );
   });
 
+
   /**
    * Opens a file descriptor at `path` for `id` with `flags` and `mode`
    * @param id

test/src/fs/index.js

@@ -181,6 +181,21 @@ if (process.platform !== 'ios') {
         })
       })
     })
+
+    test('fs.mkdir recursive', async (t) => {
+      const randomDirSegment = () => Math.random().toString(16).slice(2)
+      const dirname = path.join(FIXTURES, randomDirSegment(), randomDirSegment(), randomDirSegment())
+      await new Promise((resolve, reject) => {
+        fs.mkdir(dirname, { recursive: true }, (err) => {
+          if (err) reject(err)
+
+          fs.stat(dirname, (err) => {
+            if (err) reject(err)
+            resolve()
+          })
+        })
+      })
+    })
   }
   test('fs.open', async (t) => {})
   test('fs.opendir', async (t) => {})

test/src/fs/promises.js

@@ -40,6 +40,16 @@ if (process.platform !== 'ios') {
     test('fs.promises.mkdir', async (t) => {
       const dirname = FIXTURES + Math.random().toString(16).slice(2)
       await fs.mkdir(dirname, {})
+      await fs.stat(dirname)
+      t.pass('fs.promises.mkdir made a directory and stat\'d it')
+    })
+
+    test('fs.promises.mkdir recursive', async (t) => {
+      const randomDirName = () => Math.random().toString(16).slice(2)
+      const dirname = path.join(FIXTURES, randomDirName(), randomDirName(), randomDirName())
+      await fs.mkdir(dirname, { recursive: true })
+      await fs.stat(dirname)
+      t.pass('fs.promises.mkdir recursive made a few directories and stat\'d the last one')
     })
   }