Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fs: return first folder made by mkdir recursive #31530

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 9 additions & 5 deletions doc/api/fs.md
Expand Up @@ -2452,8 +2452,10 @@ changes:
* `callback` {Function}
* `err` {Error}

Asynchronously creates a directory. No arguments other than a possible exception
are given to the completion callback.
Asynchronously creates a directory.

The callback is given a possible exception and, if `recursive` is `true`, the
first folder path created, `(err, [path])`.

The optional `options` argument can be an integer specifying `mode` (permission
and sticky bits), or an object with a `mode` property and a `recursive`
Expand Down Expand Up @@ -2497,8 +2499,10 @@ changes:
* `options` {Object|integer}
* `recursive` {boolean} **Default:** `false`
* `mode` {string|integer} Not supported on Windows. **Default:** `0o777`.
* Returns: {string|undefined}

Synchronously creates a directory. Returns `undefined`.
Synchronously creates a directory. Returns `undefined`, or if `recursive` is
`true`, the first folder path created.
This is the synchronous version of [`fs.mkdir()`][].

See also: mkdir(2).
Expand Down Expand Up @@ -4798,8 +4802,8 @@ added: v10.0.0
* `mode` {string|integer} Not supported on Windows. **Default:** `0o777`.
* Returns: {Promise}

Asynchronously creates a directory then resolves the `Promise` with no
arguments upon success.
Asynchronously creates a directory then resolves the `Promise` with either no
arguments, or the first folder path created if `recursive` is `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`
Expand Down
9 changes: 6 additions & 3 deletions lib/fs.js
Expand Up @@ -856,10 +856,13 @@ function mkdirSync(path, options) {
throw new ERR_INVALID_ARG_TYPE('recursive', 'boolean', recursive);

const ctx = { path };
binding.mkdir(pathModule.toNamespacedPath(path),
parseFileMode(mode, 'mode'), recursive, undefined,
ctx);
const result = binding.mkdir(pathModule.toNamespacedPath(path),
parseFileMode(mode, 'mode'), recursive,
undefined, ctx);
handleErrorFromBinding(ctx);
if (recursive) {
return result;
}
}

function readdir(path, options, callback) {
Expand Down
6 changes: 3 additions & 3 deletions src/inspector_profiler.cc
Expand Up @@ -107,9 +107,9 @@ void V8ProfilerConnection::V8ProfilerSessionDelegate::SendMessageToFrontend(
}

static bool EnsureDirectory(const std::string& directory, const char* type) {
uv_fs_t req;
int ret = fs::MKDirpSync(nullptr, &req, directory, 0777, nullptr);
uv_fs_req_cleanup(&req);
fs::FSReqWrapSync req_wrap_sync;
int ret = fs::MKDirpSync(nullptr, &req_wrap_sync.req, directory, 0777,
nullptr);
if (ret < 0 && ret != UV_EEXIST) {
char err_buf[128];
uv_err_name_r(ret, err_buf, sizeof(err_buf));
Expand Down
6 changes: 6 additions & 0 deletions src/node_file-inl.h
Expand Up @@ -21,6 +21,12 @@ void FSContinuationData::PushPath(const std::string& path) {
paths_.push_back(path);
}

void FSContinuationData::MaybeSetFirstPath(const std::string& path) {
if (first_path_.empty()) {
first_path_ = path;
}
}

std::string FSContinuationData::PopPath() {
CHECK_GT(paths_.size(), 0);
std::string path = std::move(paths_.back());
Expand Down
105 changes: 93 additions & 12 deletions src/node_file.cc
Expand Up @@ -586,6 +586,43 @@ void AfterOpenFileHandle(uv_fs_t* req) {
}
}

// Reverse the logic applied by path.toNamespacedPath() to create a
// namespace-prefixed path.
void FromNamespacedPath(std::string* path) {
#ifdef _WIN32
if (path->compare(0, 8, "\\\\?\\UNC\\", 8) == 0) {
*path = path->substr(8);
path->insert(0, "\\\\");
} else if (path->compare(0, 4, "\\\\?\\", 4) == 0) {
*path = path->substr(4);
}
#endif
}

void AfterMkdirp(uv_fs_t* req) {
FSReqBase* req_wrap = FSReqBase::from_req(req);
FSReqAfterScope after(req_wrap, req);

MaybeLocal<Value> path;
Local<Value> error;

if (after.Proceed()) {
if (!req_wrap->continuation_data()->first_path().empty()) {
std::string first_path(req_wrap->continuation_data()->first_path());
FromNamespacedPath(&first_path);
path = StringBytes::Encode(req_wrap->env()->isolate(), first_path.c_str(),
req_wrap->encoding(),
&error);
if (path.IsEmpty())
req_wrap->Reject(error);
else
req_wrap->Resolve(path.ToLocalChecked());
} else {
req_wrap->Resolve(Undefined(req_wrap->env()->isolate()));
}
}
}

void AfterStringPath(uv_fs_t* req) {
FSReqBase* req_wrap = FSReqBase::from_req(req);
FSReqAfterScope after(req_wrap, req);
Expand Down Expand Up @@ -1218,18 +1255,25 @@ int MKDirpSync(uv_loop_t* loop,
const std::string& path,
int mode,
uv_fs_cb cb) {
FSContinuationData continuation_data(req, mode, cb);
continuation_data.PushPath(std::move(path));
FSReqWrapSync* req_wrap = ContainerOf(&FSReqWrapSync::req, req);

// on the first iteration of algorithm, stash state information.
if (req_wrap->continuation_data() == nullptr) {
req_wrap->set_continuation_data(
std::make_unique<FSContinuationData>(req, mode, cb));
req_wrap->continuation_data()->PushPath(std::move(path));
}

while (continuation_data.paths().size() > 0) {
std::string next_path = continuation_data.PopPath();
while (req_wrap->continuation_data()->paths().size() > 0) {
std::string next_path = req_wrap->continuation_data()->PopPath();
int err = uv_fs_mkdir(loop, req, next_path.c_str(), mode, nullptr);
while (true) {
switch (err) {
// Note: uv_fs_req_cleanup in terminal paths will be called by
// ~FSReqWrapSync():
case 0:
if (continuation_data.paths().size() == 0) {
req_wrap->continuation_data()->MaybeSetFirstPath(next_path);
if (req_wrap->continuation_data()->paths().size() == 0) {
return 0;
}
break;
Expand All @@ -1241,9 +1285,9 @@ int MKDirpSync(uv_loop_t* loop,
std::string dirname = next_path.substr(0,
next_path.find_last_of(kPathSeparator));
if (dirname != next_path) {
continuation_data.PushPath(std::move(next_path));
continuation_data.PushPath(std::move(dirname));
} else if (continuation_data.paths().size() == 0) {
req_wrap->continuation_data()->PushPath(std::move(next_path));
req_wrap->continuation_data()->PushPath(std::move(dirname));
} else if (req_wrap->continuation_data()->paths().size() == 0) {
err = UV_EEXIST;
continue;
}
Expand All @@ -1255,7 +1299,8 @@ int MKDirpSync(uv_loop_t* loop,
err = uv_fs_stat(loop, req, next_path.c_str(), nullptr);
if (err == 0 && !S_ISDIR(req->statbuf.st_mode)) {
uv_fs_req_cleanup(req);
if (orig_err == UV_EEXIST && continuation_data.paths().size() > 0) {
if (orig_err == UV_EEXIST &&
req_wrap->continuation_data()->paths().size() > 0) {
return UV_ENOTDIR;
}
return UV_EEXIST;
Expand Down Expand Up @@ -1300,8 +1345,10 @@ int MKDirpAsync(uv_loop_t* loop,
// FSReqAfterScope::~FSReqAfterScope()
case 0: {
if (req_wrap->continuation_data()->paths().size() == 0) {
req_wrap->continuation_data()->MaybeSetFirstPath(path);
req_wrap->continuation_data()->Done(0);
} else {
req_wrap->continuation_data()->MaybeSetFirstPath(path);
uv_fs_req_cleanup(req);
MKDirpAsync(loop, req, path.c_str(),
req_wrap->continuation_data()->mode(), nullptr);
Expand Down Expand Up @@ -1364,6 +1411,25 @@ int MKDirpAsync(uv_loop_t* loop,
return err;
}

int CallMKDirpSync(Environment* env, const FunctionCallbackInfo<Value>& args,
FSReqWrapSync* req_wrap, const char* path, int mode) {
env->PrintSyncTrace();
int err = MKDirpSync(env->event_loop(), &req_wrap->req, path, mode,
nullptr);
if (err < 0) {
v8::Local<v8::Context> context = env->context();
v8::Local<v8::Object> ctx_obj = args[4].As<v8::Object>();
v8::Isolate* isolate = env->isolate();
ctx_obj->Set(context,
env->errno_string(),
v8::Integer::New(isolate, err)).Check();
ctx_obj->Set(context,
env->syscall_string(),
OneByteString(isolate, "mkdir")).Check();
}
return err;
}

static void MKDir(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

Expand All @@ -1382,14 +1448,29 @@ static void MKDir(const FunctionCallbackInfo<Value>& args) {
FSReqBase* req_wrap_async = GetReqWrap(env, args[3]);
if (req_wrap_async != nullptr) { // mkdir(path, mode, req)
AsyncCall(env, req_wrap_async, args, "mkdir", UTF8,
AfterNoArgs, mkdirp ? MKDirpAsync : uv_fs_mkdir, *path, mode);
mkdirp ? AfterMkdirp : AfterNoArgs,
mkdirp ? MKDirpAsync : uv_fs_mkdir, *path, mode);
} else { // mkdir(path, mode, undefined, ctx)
CHECK_EQ(argc, 5);
FSReqWrapSync req_wrap_sync;
FS_SYNC_TRACE_BEGIN(mkdir);
if (mkdirp) {
SyncCall(env, args[4], &req_wrap_sync, "mkdir",
MKDirpSync, *path, mode);
int err = CallMKDirpSync(env, args, &req_wrap_sync, *path, mode);
if (err == 0 &&
!req_wrap_sync.continuation_data()->first_path().empty()) {
Local<Value> error;
std::string first_path(req_wrap_sync.continuation_data()->first_path());
FromNamespacedPath(&first_path);
MaybeLocal<Value> path = StringBytes::Encode(env->isolate(),
first_path.c_str(),
UTF8, &error);
if (path.IsEmpty()) {
Local<Object> ctx = args[4].As<Object>();
ctx->Set(env->context(), env->error_string(), error).Check();
return;
}
args.GetReturnValue().Set(path.ToLocalChecked());
}
} else {
SyncCall(env, args[4], &req_wrap_sync, "mkdir",
uv_fs_mkdir, *path, mode);
Expand Down
14 changes: 14 additions & 0 deletions src/node_file.h
Expand Up @@ -19,10 +19,13 @@ class FSContinuationData : public MemoryRetainer {
inline void PushPath(std::string&& path);
inline void PushPath(const std::string& path);
inline std::string PopPath();
// Used by mkdirp to track the first path created:
inline void MaybeSetFirstPath(const std::string& path);
inline void Done(int result);

int mode() const { return mode_; }
const std::vector<std::string>& paths() const { return paths_; }
const std::string& first_path() const { return first_path_; }

void MemoryInfo(MemoryTracker* tracker) const override;
SET_MEMORY_INFO_NAME(FSContinuationData)
Expand All @@ -33,6 +36,7 @@ class FSContinuationData : public MemoryRetainer {
uv_fs_t* req_;
int mode_;
std::vector<std::string> paths_;
std::string first_path_;
};

class FSReqBase : public ReqWrap<uv_fs_t> {
Expand Down Expand Up @@ -306,8 +310,18 @@ class FSReqWrapSync {
~FSReqWrapSync() { uv_fs_req_cleanup(&req); }
uv_fs_t req;

FSContinuationData* continuation_data() const {
return continuation_data_.get();
}
void set_continuation_data(std::unique_ptr<FSContinuationData> data) {
continuation_data_ = std::move(data);
}

FSReqWrapSync(const FSReqWrapSync&) = delete;
FSReqWrapSync& operator=(const FSReqWrapSync&) = delete;

private:
bcoe marked this conversation as resolved.
Show resolved Hide resolved
std::unique_ptr<FSContinuationData> continuation_data_;
};

// TODO(addaleax): Currently, callers check the return value and assume
Expand Down
94 changes: 94 additions & 0 deletions test/parallel/test-fs-mkdir.js
Expand Up @@ -239,6 +239,100 @@ if (common.isMainThread && (common.isLinux || common.isOSX)) {
});
}

// `mkdirp` returns first folder created, when all folders are new.
{
const dir1 = nextdir();
const dir2 = nextdir();
const firstPathCreated = path.join(tmpdir.path, dir1);
const pathname = path.join(tmpdir.path, dir1, dir2);

fs.mkdir(pathname, { recursive: true }, common.mustCall(function(err, path) {
assert.strictEqual(err, null);
assert.strictEqual(fs.existsSync(pathname), true);
assert.strictEqual(fs.statSync(pathname).isDirectory(), true);
assert.strictEqual(path, firstPathCreated);
}));
}

// `mkdirp` returns first folder created, when last folder is new.
{
const dir1 = nextdir();
const dir2 = nextdir();
const pathname = path.join(tmpdir.path, dir1, dir2);
fs.mkdirSync(path.join(tmpdir.path, dir1));
fs.mkdir(pathname, { recursive: true }, common.mustCall(function(err, path) {
assert.strictEqual(err, null);
assert.strictEqual(fs.existsSync(pathname), true);
assert.strictEqual(fs.statSync(pathname).isDirectory(), true);
assert.strictEqual(path, pathname);
}));
}

// `mkdirp` returns undefined, when no new folders are created.
{
const dir1 = nextdir();
const dir2 = nextdir();
const pathname = path.join(tmpdir.path, dir1, dir2);
fs.mkdirSync(path.join(tmpdir.path, dir1, dir2), { recursive: true });
fs.mkdir(pathname, { recursive: true }, common.mustCall(function(err, path) {
assert.strictEqual(err, null);
assert.strictEqual(fs.existsSync(pathname), true);
assert.strictEqual(fs.statSync(pathname).isDirectory(), true);
assert.strictEqual(path, undefined);
}));
}

// `mkdirp.sync` returns first folder created, when all folders are new.
{
const dir1 = nextdir();
const dir2 = nextdir();
const firstPathCreated = path.join(tmpdir.path, dir1);
const pathname = path.join(tmpdir.path, dir1, dir2);
const p = fs.mkdirSync(pathname, { recursive: true });
assert.strictEqual(fs.existsSync(pathname), true);
assert.strictEqual(fs.statSync(pathname).isDirectory(), true);
assert.strictEqual(p, firstPathCreated);
}

// `mkdirp.sync` returns first folder created, when last folder is new.
{
const dir1 = nextdir();
const dir2 = nextdir();
const pathname = path.join(tmpdir.path, dir1, dir2);
fs.mkdirSync(path.join(tmpdir.path, dir1), { recursive: true });
const p = fs.mkdirSync(pathname, { recursive: true });
assert.strictEqual(fs.existsSync(pathname), true);
assert.strictEqual(fs.statSync(pathname).isDirectory(), true);
assert.strictEqual(p, pathname);
}

// `mkdirp.sync` returns undefined, when no new folders are created.
{
const dir1 = nextdir();
const dir2 = nextdir();
const pathname = path.join(tmpdir.path, dir1, dir2);
fs.mkdirSync(path.join(tmpdir.path, dir1, dir2), { recursive: true });
const p = fs.mkdirSync(pathname, { recursive: true });
assert.strictEqual(fs.existsSync(pathname), true);
assert.strictEqual(fs.statSync(pathname).isDirectory(), true);
assert.strictEqual(p, undefined);
}

// `mkdirp.promises` returns first folder created, when all folders are new.
{
const dir1 = nextdir();
const dir2 = nextdir();
const firstPathCreated = path.join(tmpdir.path, dir1);
const pathname = path.join(tmpdir.path, dir1, dir2);
async function testCase() {
const p = await fs.promises.mkdir(pathname, { recursive: true });
assert.strictEqual(fs.existsSync(pathname), true);
assert.strictEqual(fs.statSync(pathname).isDirectory(), true);
assert.strictEqual(p, firstPathCreated);
}
testCase();
}

// Keep the event loop alive so the async mkdir() requests
// have a chance to run (since they don't ref the event loop).
process.nextTick(() => {});