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: implement lutimes #33399

Closed
wants to merge 1 commit 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 51 additions & 0 deletions doc/api/fs.md
Original file line number Diff line number Diff line change
Expand Up @@ -2396,6 +2396,38 @@ changes:

Synchronous lchown(2). Returns `undefined`.

## `fs.lutimes(path, atime, mtime, callback)`
<!-- YAML
addded: REPLACEME
-->

* `path` {string|Buffer|URL}
* `atime` {number|string|Date}
* `mtime` {number|string|Date}
* `callback` {Function}
* `err` {Error}

Changes the access and modification times of a file in the same way as
[`fs.utimes()`][], with the difference that if the path refers to a symbolic
link, then the link is not dereferenced: instead, the timestamps of the
symbolic link itself are changed.

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

## `fs.lutimesSync(path, atime, mtime)`
<!-- YAML
added: REPLACEME
-->

* `path` {string|Buffer|URL}
* `atime` {number|string|Date}
* `mtime` {number|string|Date}

Change the file system timestamps of the symbolic link referenced by `path`.
Returns `undefined`, or throws an exception when parameters are incorrect or
the operation fails. This is the synchronous version of [`fs.lutimes()`][].

## `fs.link(existingPath, newPath, callback)`
<!-- YAML
added: v0.1.31
Expand Down Expand Up @@ -5035,6 +5067,23 @@ changes:
Changes the ownership on a symbolic link then resolves the `Promise` with
no arguments upon success.

### `fsPromises.lutimes(path, atime, mtime)`
<!-- YAML
added: REPLACEME
-->

* `path` {string|Buffer|URL}
* `atime` {number|string|Date}
* `mtime` {number|string|Date}
* Returns: {Promise}

Changes the access and modification times of a file in the same way as
[`fsPromises.utimes()`][], with the difference that if the path refers to a
symbolic link, then the link is not dereferenced: instead, the timestamps of
the symbolic link itself are changed.

Upon success, the `Promise` is resolved without arguments.

### `fsPromises.link(existingPath, newPath)`
<!-- YAML
added: v10.0.0
Expand Down Expand Up @@ -5853,6 +5902,7 @@ the file contents.
[`fs.ftruncate()`]: #fs_fs_ftruncate_fd_len_callback
[`fs.futimes()`]: #fs_fs_futimes_fd_atime_mtime_callback
[`fs.lstat()`]: #fs_fs_lstat_path_options_callback
[`fs.lutimes()`]: #fs_fs_lutimes_path_atime_mtime_callback
[`fs.mkdir()`]: #fs_fs_mkdir_path_options_callback
[`fs.mkdtemp()`]: #fs_fs_mkdtemp_prefix_options_callback
[`fs.open()`]: #fs_fs_open_path_flags_mode_callback
Expand All @@ -5876,6 +5926,7 @@ the file contents.
[`fs.writev()`]: #fs_fs_writev_fd_buffers_position_callback
[`fsPromises.open()`]: #fs_fspromises_open_path_flags_mode
[`fsPromises.opendir()`]: #fs_fspromises_opendir_path_options
[`fsPromises.utimes()`]: #fs_fspromises_utimes_path_atime_mtime
[`inotify(7)`]: http://man7.org/linux/man-pages/man7/inotify.7.html
[`kqueue(2)`]: https://www.freebsd.org/cgi/man.cgi?query=kqueue&sektion=2
[`net.Socket`]: net.html#net_class_net_socket
Expand Down
24 changes: 24 additions & 0 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -1325,6 +1325,28 @@ function futimesSync(fd, atime, mtime) {
handleErrorFromBinding(ctx);
}

function lutimes(path, atime, mtime, callback) {
callback = makeCallback(callback);
path = getValidatedPath(path);

const req = new FSReqCallback();
req.oncomplete = callback;
binding.lutimes(pathModule.toNamespacedPath(path),
toUnixTimestamp(atime),
toUnixTimestamp(mtime),
req);
}

function lutimesSync(path, atime, mtime) {
path = getValidatedPath(path);
const ctx = { path };
binding.lutimes(pathModule.toNamespacedPath(path),
toUnixTimestamp(atime),
toUnixTimestamp(mtime),
undefined, ctx);
handleErrorFromBinding(ctx);
}

function writeAll(fd, isUserFd, buffer, offset, length, callback) {
// write(fd, buffer, offset, length, position, callback)
fs.write(fd, buffer, offset, length, null, (writeErr, written) => {
Expand Down Expand Up @@ -1971,6 +1993,8 @@ module.exports = fs = {
linkSync,
lstat,
lstatSync,
lutimes,
lutimesSync,
mkdir,
mkdirSync,
mkdtemp,
Expand Down
9 changes: 9 additions & 0 deletions lib/internal/fs/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,14 @@ async function futimes(handle, atime, mtime) {
return binding.futimes(handle.fd, atime, mtime, kUsePromises);
}

async function lutimes(path, atime, mtime) {
path = getValidatedPath(path);
return binding.lutimes(pathModule.toNamespacedPath(path),
toUnixTimestamp(atime),
toUnixTimestamp(mtime),
kUsePromises);
}

async function realpath(path, options) {
options = getOptions(options, {});
path = getValidatedPath(path);
Expand Down Expand Up @@ -582,6 +590,7 @@ module.exports = {
lchown,
chown,
utimes,
lutimes,
realpath,
mkdtemp,
writeFile,
Expand Down
30 changes: 30 additions & 0 deletions src/node_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2304,6 +2304,35 @@ static void FUTimes(const FunctionCallbackInfo<Value>& args) {
}
}

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

const int argc = args.Length();
CHECK_GE(argc, 3);

BufferValue path(env->isolate(), args[0]);
CHECK_NOT_NULL(*path);

CHECK(args[1]->IsNumber());
const double atime = args[1].As<Number>()->Value();

CHECK(args[2]->IsNumber());
const double mtime = args[2].As<Number>()->Value();

FSReqBase* req_wrap_async = GetReqWrap(args, 3);
if (req_wrap_async != nullptr) { // lutimes(path, atime, mtime, req)
AsyncCall(env, req_wrap_async, args, "lutime", UTF8, AfterNoArgs,
uv_fs_lutime, *path, atime, mtime);
} else { // lutimes(path, atime, mtime, undefined, ctx)
CHECK_EQ(argc, 5);
FSReqWrapSync req_wrap_sync;
FS_SYNC_TRACE_BEGIN(lutimes);
SyncCall(env, args[4], &req_wrap_sync, "lutime",
uv_fs_lutime, *path, atime, mtime);
FS_SYNC_TRACE_END(lutimes);
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be nicer to share this code with UTimes(). The JS bindings too, although that's not as much code.

The FS_SYNC_TRACE_BEGIN/FS_SYNC_TRACE_END labels are off, by the way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to keep roughly the same pattern as for lchown and lchmod, which are both duplicated similarly.

Will update the traces 👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but remember the rule of three: duplicating something once? Probably okay. Duplicating it twice? Time to refactor.


static void Mkdtemp(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
Isolate* isolate = env->isolate();
Expand Down Expand Up @@ -2399,6 +2428,7 @@ void Initialize(Local<Object> target,

env->SetMethod(target, "utimes", UTimes);
env->SetMethod(target, "futimes", FUTimes);
env->SetMethod(target, "lutimes", LUTimes);

env->SetMethod(target, "mkdtemp", Mkdtemp);

Expand Down
73 changes: 45 additions & 28 deletions test/parallel/test-fs-utimes.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,17 @@ const common = require('../common');
const assert = require('assert');
const util = require('util');
const fs = require('fs');
const url = require('url');

const tmpdir = require('../common/tmpdir');
tmpdir.refresh();

function stat_resource(resource) {
const lpath = `${tmpdir.path}/symlink`;
fs.symlinkSync('unoent-entry', lpath);

function stat_resource(resource, statSync = fs.statSync) {
if (typeof resource === 'string') {
return fs.statSync(resource);
return statSync(resource);
}
const stats = fs.fstatSync(resource);
// Ensure mtime has been written to disk
Expand All @@ -41,9 +45,9 @@ function stat_resource(resource) {
return fs.fstatSync(resource);
}

function check_mtime(resource, mtime) {
function check_mtime(resource, mtime, statSync) {
mtime = fs._toUnixTimestamp(mtime);
const stats = stat_resource(resource);
const stats = stat_resource(resource, statSync);
const real_mtime = fs._toUnixTimestamp(stats.mtime);
return mtime - real_mtime;
}
Expand All @@ -55,8 +59,8 @@ function expect_errno(syscall, resource, err, errno) {
);
}

function expect_ok(syscall, resource, err, atime, mtime) {
const mtime_diff = check_mtime(resource, mtime);
function expect_ok(syscall, resource, err, atime, mtime, statSync) {
const mtime_diff = check_mtime(resource, mtime, statSync);
assert(
// Check up to single-second precision.
// Sub-second precision is OS and fs dependant.
Expand All @@ -68,45 +72,55 @@ function expect_ok(syscall, resource, err, atime, mtime) {

const stats = fs.statSync(tmpdir.path);

const asPath = (path) => path;
const asUrl = (path) => url.pathToFileURL(path);

const cases = [
new Date('1982-09-10 13:37'),
new Date(),
123456.789,
stats.mtime,
['123456', -1],
new Date('2017-04-08T17:59:38.008Z')
[asPath, new Date('1982-09-10 13:37')],
[asPath, new Date()],
[asPath, 123456.789],
[asPath, stats.mtime],
[asPath, '123456', -1],
[asPath, new Date('2017-04-08T17:59:38.008Z')],
[asUrl, new Date()],
];

runTests(cases.values());

function runTests(iter) {
const { value, done } = iter.next();
if (done) return;

// Support easy setting same or different atime / mtime values.
const [atime, mtime] = Array.isArray(value) ? value : [value, value];
const [pathType, atime, mtime = atime] = value;

let fd;
//
// test async code paths
//
fs.utimes(tmpdir.path, atime, mtime, common.mustCall((err) => {
fs.utimes(pathType(tmpdir.path), atime, mtime, common.mustCall((err) => {
expect_ok('utimes', tmpdir.path, err, atime, mtime);

fs.utimes('foobarbaz', atime, mtime, common.mustCall((err) => {
expect_errno('utimes', 'foobarbaz', err, 'ENOENT');
fs.lutimes(pathType(lpath), atime, mtime, common.mustCall((err) => {
expect_ok('lutimes', lpath, err, atime, mtime, fs.lstatSync);

fs.utimes(pathType('foobarbaz'), atime, mtime, common.mustCall((err) => {
expect_errno('utimes', 'foobarbaz', err, 'ENOENT');

// don't close this fd
if (common.isWindows) {
fd = fs.openSync(tmpdir.path, 'r+');
} else {
fd = fs.openSync(tmpdir.path, 'r');
}
// don't close this fd
if (common.isWindows) {
fd = fs.openSync(tmpdir.path, 'r+');
} else {
fd = fs.openSync(tmpdir.path, 'r');
}

fs.futimes(fd, atime, mtime, common.mustCall((err) => {
expect_ok('futimes', fd, err, atime, mtime);
fs.futimes(fd, atime, mtime, common.mustCall((err) => {
expect_ok('futimes', fd, err, atime, mtime);

syncTests();
syncTests();

setImmediate(common.mustCall(runTests), iter);
setImmediate(common.mustCall(runTests), iter);
}));
}));
}));
}));
Expand All @@ -115,9 +129,12 @@ function runTests(iter) {
// test synchronized code paths, these functions throw on failure
//
function syncTests() {
fs.utimesSync(tmpdir.path, atime, mtime);
fs.utimesSync(pathType(tmpdir.path), atime, mtime);
expect_ok('utimesSync', tmpdir.path, undefined, atime, mtime);

fs.lutimesSync(pathType(lpath), atime, mtime);
expect_ok('lutimesSync', lpath, undefined, atime, mtime, fs.lstatSync);

// Some systems don't have futimes
// if there's an error, it should be ENOSYS
try {
Expand All @@ -129,7 +146,7 @@ function runTests(iter) {

let err;
try {
fs.utimesSync('foobarbaz', atime, mtime);
fs.utimesSync(pathType('foobarbaz'), atime, mtime);
} catch (ex) {
err = ex;
}
Expand Down