Njs: refactoring of the fs module.

[drsm]

1. Handle arguments containing path strings in one place:
   https://github.com/nginx/njs/commit/1e9f7f506472cb7f1ae218681b319bdd279bb1ec

[drsm]

1. Fixed callback invocations in "fs" module. Fixes #200.
   https://gist.github.com/drsm/3f4567cacbc0f088862b04a0b5d5d7f2 (Ready)

[drsm]

TODO: (in no particular order)

• TypedArrays & ArrayBuffer
• better testsuite
• benchmark
• add more encodings

[xeioex]

Fixed callback invocations in "fs" module. Fixes #200.
https://gist.github.com/drsm/3f4567cacbc0f088862b04a0b5d5d7f2 (Ready)

Thanks, committed with some improvements: https://github.com/nginx/njs/commit/cd10a4b329a6612782335acd5a954a408897df25.

[drsm]

1. Introduced fs.promises API. / Introduced fs.access and friends.
   https://gist.github.com/drsm/d950402101765c7395c36cb4051576ba (Ready)

@xeioex please take a look.
isn't it broken by design?

[xeioex]

@drsm

isn't it broken by design?

Looks promising. Have no issues with the design.

[drsm]

I've updated the patch above.
The solution is compatible with nodejs except the cases like fs.promises.readFile() (our version will throw).

[xeioex]

@drsm I am on large (Array objects) refactoring now. I plan to merge the patch this or next week.

[drsm]

1. Added fs.symlink(), fs.unlink(), fs.mkdir(), fs.rmdir() and friends.
   https://gist.github.com/drsm/d0b44c1d6252a9c2a0627c15aed97c31 (outdated)

[drsm]

JFF

var fs = require('fs'), fsp = fs.promises;
var current;

Promise.resolve('100k fs.access(/dev/null)')
.then((name) => {
    current = name;
    console.log('start', current);
    console.time(current);
})
.then(() => {
    console.time(current + ' sync');
    for (var i = 0; i < 100000; ++i) {
        fs.accessSync('/dev/null');
    }
    console.timeEnd(current + ' sync');
})
.then(() => {
    console.time(current + ' promise');
    var n = 100000;
    var f = () => {
        if (n--) {
            return fsp.access('/dev/null').then(() => f());
        }
        console.timeEnd(current + ' promise');
        return Promise.resolve();
    };

    return f();
})
/*.then(() => {
    var f = async () => {
        console.time(current + ' async');
        for (var i = 0; i < 100000; ++i) {
            await fsp.access('/dev/null');
        }
        console.timeEnd(current + ' async');
    };

    return f();
})*/
.then(() => {
    return new Promise((resolve, reject) => {
        console.time(current + ' callback');
        var n = 100000;
        var f = (err) => {
            if (err) {
                reject(err);
                return;
            }
            if (n--) {
                fs.access('/dev/null', f);
                return;
            }
            console.timeEnd(current + ' callback');
            resolve();
        };

        f();
    })
})
.then(() => {
    console.timeEnd(current);
    console.log('stop', current);
    current = void 0;
})
.catch((e) => {
    console.log(current, 'failed', e)
});

$ node fs_bench.js 
start 100k fs.access(/dev/null)
100k fs.access(/dev/null) sync: 131.249ms
100k fs.access(/dev/null) promise: 1226.996ms
100k fs.access(/dev/null) callback: 965.285ms
100k fs.access(/dev/null): 2326.356ms
stop 100k fs.access(/dev/null)
$ build/njs fs_bench.js 
start 100k fs.access(/dev/null)
100k fs.access(/dev/null) sync: 123.637338ms
100k fs.access(/dev/null) promise: 426.900264ms
100k fs.access(/dev/null) callback: 151.360074ms
100k fs.access(/dev/null): 781.825911ms
stop 100k fs.access(/dev/null)

[xeioex]

@drsm

Introduced fs.promises API. / Introduced fs.access and friends.
https://gist.github.com/drsm/d950402101765c7395c36cb4051576ba (Ready)

Thanks, committed (https://github.com/nginx/njs/commit/096f5aaee1897bdfc8551f92e56df8e871cd37ff, https://github.com/nginx/njs/commit/d71a881ce5bf16254d0f5a5638719e0db546d2be) with some refactoring included.

[drsm]

1. Added fs.symlink(), fs.unlink(), fs.realpath() and friends.
   https://gist.github.com/drsm/3d0402f41dc332b3c87bbe5dfcb7892e

[xeioex]

@drsm

Added fs.symlink(), fs.unlink(), fs.realpath() and friends.
https://gist.github.com/drsm/3d0402f41dc332b3c87bbe5dfcb7892e

Thanks, committed with the following patch over .

[drsm]

@xeioex

please take a look:
https://gist.github.com/drsm/23a3167637465b48ddb0b82fc5481516

some code was taken from here

[drsm]

Added fs.mkdir(), fs.rmdir() and friends.

https://gist.github.com/drsm/4785ae96a8b1ccee27a56c2919d6ba18

[xeioex]

@drsm

Thanks Artem. Will take a look at the end of the week.

[xeioex]

@drsm

please take a look:
https://gist.github.com/drsm/23a3167637465b48ddb0b82fc5481516

It seems to me that the concept is very straightforward to put any copyrights here.

[xeioex]

@drsm

Added fs.mkdir(), fs.rmdir() and friends.

BTW, why not support recursive mode also?

[xeioex]

@drsm

Added fs.mkdir(), fs.rmdir() and friends.
https://gist.github.com/drsm/4785ae96a8b1ccee27a56c2919d6ba18

Committed, thanks.

Also added missing parts for rename() method in https://github.com/nginx/njs/commit/0f28a80e9a80b9c7429090e767bf57f19003aed2.

[drsm]

@xeioex

BTW, why not support recursive mode also?

I'll to do it.

[drsm]

@xeioex

Please take a look:

Improved fs.mkdir() to support recursive directory creation.
https://gist.github.com/drsm/342c4300b17b90cf749f3e316fc3b403

[xeioex]

@drsm

Please take a look:

Improved fs.mkdir() to support recursive directory creation.
https://gist.github.com/drsm/342c4300b17b90cf749f3e316fc3b403

Thanks, committed with some changes. Most notably avoiding any changes to const char * memory.

[drsm]

@xeioex

Improved fs.rmdir() to support recursive directory removal.
https://gist.github.com/drsm/38714fecb523293d70296741cb62eb02

The upstream solution is still buggy

[xeioex]

@drsm

Take a look, https://gist.github.com/xeioex/cf5d4d451642510777552614ca1396af

Coverity (static analyser) found the following issue with the previous mkdir patch.

[drsm]

@xeioex

Take a look, https://gist.github.com/xeioex/cf5d4d451642510777552614ca1396af

Coverity (static analyser) found the following issue with the previous mkdir patch.

Yes, there is a race condition.
I was unsure about error codes, so decided to stat it first :).
https://github.com/freebsd/freebsd/blob/master/bin/mkdir/mkdir.c#L182

The fix looks fine to me.

[drsm]

@xeioex

The fix looks fine to me.

BTW, we may lose an original mkdir() errno after stat:
https://github.com/openbsd/src/blob/master/bin/mkdir/mkdir.c#L150
but i don't think it's a serous issue.

[xeioex]

@drsm

Improved fs.mkdir() to support recursive directory creation.
https://gist.github.com/drsm/342c4300b17b90cf749f3e316fc3b403

https://gist.github.com/144da2743e6c6230bcb6cddb22ea951d

I had to implement njs_file_tree_walk() from scratch because ftw.h is a total mess, even though it is a POSIX interface.
Alpine (libc musl) vs Ubuntu (glibc) vs Freebsd all they respond differently to _XOPEN_SOURCE feature macros.

[drsm]

@xeioex
Great Job!

[xeioex]

@drsm

Committed in https://github.com/nginx/njs/commit/f8c8fd850d39915871b2a04bebc9af051766869f, thanks.

[xeioex]

@drsm

I added support for Buffer object in fs module, feel free to catch any issues.

[drsm]

@xeioex

Thanks!

This place is questionable:

>> fs.realpath(Buffer.from('/none'), (e) => void console.log(e.path))
undefined
Buffer [47,110,111,110,101]

vs.

> fs.realpath(Buffer.from('/none'), (e) => void console.log(e.path))
undefined
> /none

[xeioex]

@drsm

This place is questionable:

Agree, take a look
https://gist.github.com/xeioex/8d3fa0a42c04042693c9db5196f3bd52

it also eliminates dynamic allocation for path arguments.