fs: add two new modes "as" and "as+" #18801

SirR4T · 2018-02-15T18:31:49Z

Two new modes 'as' and 'as+' are added, for opening files:

'as': append in synchronous mode,
'as+': read write in synchronous mode.

Fixes #18635 .

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

Affected core subsystem(s)

fs

vsemozhetbyt · 2018-02-15T19:09:11Z

Just a reminder that fs.promises.open() maybe also need to be updated (maybe in a separate PR, I am not sure).

SirR4T · 2018-02-16T15:39:00Z

@vsemozhetbyt could you elaborate? I couldn't find any references to flags or couldn't discern any thing special in node_file.cc that needs to be done to add this support. Also, don't know if this is a good enough test, but test-fs-filehandle.js seems to pass even when using as+ instead of r.

vsemozhetbyt · 2018-02-16T20:48:34Z

@SirR4T Sorry, I just see the same flags here:

https://github.com/nodejs/node/blob/master/doc/api/fs.md#fspromisesopenpath-flags-mode

But I do not know the implementation details. Maybe @jasnell can clarify if any additions are needed.

SirR4T · 2018-02-17T03:54:17Z

Thanks for pointing out! updated the docs for now, Will take up if there's any implementation issue.

joyeecheung · 2018-02-17T09:32:55Z

doc/api/fs.md

 * `'a+'` - Open file for reading and appending.
 The file is created if it does not exist.

 * `'ax+'` - Like `'a+'` but fails if `path` exists.

+* `'as+'` - Open file for reading and writing in synchronous mode.


s/reading and writing/reading and appending/

joyeecheung · 2018-02-17T09:33:01Z

doc/api/fs.md

 * `'a+'` - Open file for reading and appending.
 The file is created if it does not exist.

 * `'ax+'` - Like `'a+'` but fails if `path` exists.

+* `'as+'` - Open file for reading and writing in synchronous mode.


s/reading and writing/reading and appending/

addaleax · 2018-02-17T13:26:05Z

doc/api/fs.md

@@ -2019,11 +2019,17 @@ The file is created if it does not exist.

 * `'ax'` - Like `'a'` but fails if `path` exists.

+* `'as'` - Open file for appending in synchronous mode.


Regarding the phrasing here … I’m not sure, but I think this might lead people to believe that stream.write('foo'); writes the data synchronously, when in reality it still passes it off to the libuv threadpool.

How about something like Open a file for appending with the `O_SYNC` flag set, see open(2) for more details.?

I think the meaning of synchronous mode is addressed in the 'rs+' mode above. Would it suffice If i can reference to that? How do i do that in markdown?

@SirR4T You can add an anchor around rs+ with <a id="rs-flag"></a> and reference that as [`rs+`](#rs-flag)

jasnell · 2018-02-17T15:41:19Z

The promises functions use the same code path here so nothing additional needs to be done (except perhaps updating the docs? I cannot remember for sure tho)

SirR4T · 2018-02-17T17:27:39Z

@jasnell updated the docs for both fs.open as well as fs.promises.open, after being pointed out by @vsemozhetbyt .

BridgeAR · 2018-02-18T11:04:34Z

Light-CI https://ci.nodejs.org/job/node-test-commit-light/293/

PR-URL: nodejs#18801 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Matheus Marchini <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>

BridgeAR · 2018-02-22T11:13:03Z

Landed in b1e52fe 🎉

MylesBorins · 2018-03-06T21:22:12Z

Should this be backported to v9.x-staging? If yes please follow the guide and raise a backport PR, if not let me know or add the dont-land-on label.

Refs: nodejs#18801

PR-URL: nodejs#19585 Refs: nodejs#18801 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]>

PR-URL: #19585 Refs: #18801 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]>

PR-URL: nodejs#18801 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Matheus Marchini <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>

PR-URL: #18801 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Matheus Marchini <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>

PR-URL: #19585 Refs: #18801 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]>

PR-URL: #18801 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Matheus Marchini <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>

PR-URL: #19585 Refs: #18801 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]>

PR-URL: #18801 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Matheus Marchini <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>

PR-URL: #19585 Refs: #18801 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]>

PR-URL: #18801 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Matheus Marchini <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>

PR-URL: #19585 Refs: #18801 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]>

PR-URL: #18801 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Matheus Marchini <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>

PR-URL: #19585 Refs: #18801 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]>

Notable Changes: * async_hooks: - rename PromiseWrap.parentId (Ali Ijaz Sheikh) #18633 - remove runtime deprecation (Ali Ijaz Sheikh) #19517 - deprecate unsafe emit{Before,After} (Ali Ijaz Sheikh) #18513 * cluster: - add cwd to cluster.settings (cjihrig) #18399 - support windowsHide option for workers (Todd Wong) #17412 * crypto: - allow passing null as IV unless required (Tobias Nießen) #18644 * deps: - upgrade npm to 6.2.0 (Kat Marchán) #21592 - upgrade libuv to 1.19.2 (cjihrig) #18918 - Upgrade node-inspect to 1.11.5 (Jan Krems) #21055 * fs,net: - support as and as+ flags in stringToFlags() (Sarat Addepalli) #18801 - emit 'ready' for fs streams and sockets (Sameer Srivastava) #19408 * http, http2: - add options to http.createServer() (Peter Marton) #15752 - add 103 Early Hints status code (Yosuke Furukawa) #16644 - add http fallback options to .createServer (Peter Marton) #15752 * n-api: - take n-api out of experimental (Michael Dawson) #19262 * perf_hooks: - add warning when too many entries in the timeline (James M Snell) #18087 * src: - add public API for managing NodePlatform (Cheng Zhao) #16981 - allow --perf-(basic-)?prof in NODE\_OPTIONS (Leko) #17600 - node internals' postmortem metadata (Matheus Marchini) #14901 * tls: - expose Finished messages in TLSSocket (Anton Salikhmetov) #19102 * **trace_events**: - add file pattern cli option (Andreas Madsen) #18480 * util: - implement util.getSystemErrorName() (Joyee Cheung) #18186 PR-URL: #21593

Notable Changes: * async_hooks: - rename PromiseWrap.parentId (Ali Ijaz Sheikh) nodejs#18633 - remove runtime deprecation (Ali Ijaz Sheikh) nodejs#19517 - deprecate unsafe emit{Before,After} (Ali Ijaz Sheikh) nodejs#18513 * cluster: - add cwd to cluster.settings (cjihrig) nodejs#18399 - support windowsHide option for workers (Todd Wong) nodejs#17412 * crypto: - allow passing null as IV unless required (Tobias Nießen) nodejs#18644 * deps: - upgrade npm to 6.2.0 (Kat Marchán) nodejs#21592 - upgrade libuv to 1.19.2 (cjihrig) nodejs#18918 - Upgrade node-inspect to 1.11.5 (Jan Krems) nodejs#21055 * fs,net: - support as and as+ flags in stringToFlags() (Sarat Addepalli) nodejs#18801 - emit 'ready' for fs streams and sockets (Sameer Srivastava) nodejs#19408 * http, http2: - add options to http.createServer() (Peter Marton) nodejs#15752 - add 103 Early Hints status code (Yosuke Furukawa) nodejs#16644 - add http fallback options to .createServer (Peter Marton) nodejs#15752 * n-api: - take n-api out of experimental (Michael Dawson) nodejs#19262 * perf_hooks: - add warning when too many entries in the timeline (James M Snell) nodejs#18087 * src: - add public API for managing NodePlatform (Cheng Zhao) nodejs#16981 - allow --perf-(basic-)?prof in NODE\_OPTIONS (Leko) nodejs#17600 - node internals' postmortem metadata (Matheus Marchini) nodejs#14901 * tls: - expose Finished messages in TLSSocket (Anton Salikhmetov) nodejs#19102 * **trace_events**: - add file pattern cli option (Andreas Madsen) nodejs#18480 * util: - implement util.getSystemErrorName() (Joyee Cheung) nodejs#18186 PR-URL: nodejs#21593

Notable Changes: * async_hooks: - rename PromiseWrap.parentId (Ali Ijaz Sheikh) #18633 - remove runtime deprecation (Ali Ijaz Sheikh) #19517 - deprecate unsafe emit{Before,After} (Ali Ijaz Sheikh) #18513 * cluster: - add cwd to cluster.settings (cjihrig) #18399 - support windowsHide option for workers (Todd Wong) #17412 * crypto: - allow passing null as IV unless required (Tobias Nießen) #18644 * deps: - upgrade npm to 6.4.1 (Kat Marchán) #22591 - upgrade libuv to 1.19.2 (cjihrig) #18918 - Upgrade node-inspect to 1.11.5 (Jan Krems) #21055 * fs,net: - support as and as+ flags in stringToFlags() (Sarat Addepalli) #18801 - emit 'ready' for fs streams and sockets (Sameer Srivastava) #19408 * http, http2: - add options to http.createServer() (Peter Marton) #15752 - add 103 Early Hints status code (Yosuke Furukawa) #16644 - add http fallback options to .createServer (Peter Marton) #15752 * n-api: - take n-api out of experimental (Michael Dawson) #19262 * perf_hooks: - add warning when too many entries in the timeline (James M Snell) #18087 * src: - add public API for managing NodePlatform (Cheng Zhao) #16981 - allow --perf-(basic-)?prof in NODE\_OPTIONS (Leko) #17600 - node internals' postmortem metadata (Matheus Marchini) #14901 * tls: - expose Finished messages in TLSSocket (Anton Salikhmetov) #19102 * **trace_events**: - add file pattern cli option (Andreas Madsen) #18480 * util: - implement util.getSystemErrorName() (Joyee Cheung) #18186 PR-URL: #21593

Notable Changes: * async_hooks: - rename PromiseWrap.parentId (Ali Ijaz Sheikh) #18633 - remove runtime deprecation (Ali Ijaz Sheikh) #19517 - deprecate unsafe emit{Before,After} (Ali Ijaz Sheikh) #18513 * cluster: - add cwd to cluster.settings (cjihrig) #18399 - support windowsHide option for workers (Todd Wong) #17412 * crypto: - allow passing null as IV unless required (Tobias Nießen) #18644 * deps: - upgrade npm to 6.2.0 (Kat Marchán) #21592 - upgrade libuv to 1.19.2 (cjihrig) #18918 - Upgrade node-inspect to 1.11.5 (Jan Krems) #21055 * fs,net: - support as and as+ flags in stringToFlags() (Sarat Addepalli) #18801 - emit 'ready' for fs streams and sockets (Sameer Srivastava) #19408 * http, http2: - add options to http.createServer() (Peter Marton) #15752 - add 103 Early Hints status code (Yosuke Furukawa) #16644 - add http fallback options to .createServer (Peter Marton) #15752 * n-api: - take n-api out of experimental (Michael Dawson) #19262 * perf_hooks: - add warning when too many entries in the timeline (James M Snell) #18087 * src: - add public API for managing NodePlatform (Cheng Zhao) #16981 - allow --perf-(basic-)?prof in NODE\_OPTIONS (Leko) #17600 - node internals' postmortem metadata (Matheus Marchini) #14901 * tls: - expose Finished messages in TLSSocket (Anton Salikhmetov) #19102 * **trace_events**: - add file pattern cli option (Andreas Madsen) #18480 * util: - implement util.getSystemErrorName() (Joyee Cheung) #18186 PR-URL: #21593

nodejs-github-bot added the fs Issues and PRs related to the fs subsystem / file system. label Feb 15, 2018

BridgeAR requested a review from jasnell February 16, 2018 18:52

SirR4T force-pushed the syncWithAppend branch 2 times, most recently from abfa13f to a716f4e Compare February 17, 2018 03:53

joyeecheung approved these changes Feb 17, 2018

View reviewed changes

daynin approved these changes Feb 17, 2018

View reviewed changes

mmarchini approved these changes Feb 17, 2018

View reviewed changes

addaleax approved these changes Feb 17, 2018

View reviewed changes

jasnell approved these changes Feb 17, 2018

View reviewed changes

SirR4T added 2 commits February 17, 2018 22:30

fs: support as and as+ flags in stringToFlags()

bc65182

fs: tests and docs for additional modes as and as+

16537b9

SirR4T force-pushed the syncWithAppend branch from a716f4e to 16537b9 Compare February 17, 2018 17:04

lpinca approved these changes Feb 17, 2018

View reviewed changes

BridgeAR added the semver-minor PRs that contain new features and should be released in the next minor version. label Feb 18, 2018

BridgeAR approved these changes Feb 18, 2018

View reviewed changes

BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 18, 2018

BridgeAR closed this Feb 22, 2018

MylesBorins added the backport-requested-v9.x label Mar 6, 2018

tniessen added a commit to tniessen/node that referenced this pull request Mar 25, 2018

doc: add missing metadata for fs.open

1dad16a

Refs: nodejs#18801

tniessen mentioned this pull request Mar 25, 2018

doc: add missing metadata for fs.open #19585

Closed

2 tasks

targos pushed a commit that referenced this pull request Mar 27, 2018

doc: add missing metadata for fs.open

7e9b7a5

PR-URL: #19585 Refs: #18801 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]>

MylesBorins added the land-on-v8.x label Aug 7, 2018

MylesBorins mentioned this pull request Aug 16, 2018

v8.12.0 proposal #21593

Merged

rvagg mentioned this pull request Nov 9, 2018

doc: update fs.open() changes record for optional 'flags' #24240

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fs: add two new modes "as" and "as+" #18801

fs: add two new modes "as" and "as+" #18801

SirR4T commented Feb 15, 2018

vsemozhetbyt commented Feb 15, 2018

SirR4T commented Feb 16, 2018

vsemozhetbyt commented Feb 16, 2018

SirR4T commented Feb 17, 2018

joyeecheung Feb 17, 2018

joyeecheung Feb 17, 2018

addaleax Feb 17, 2018

SirR4T Feb 17, 2018

joyeecheung Feb 18, 2018

jasnell commented Feb 17, 2018

SirR4T commented Feb 17, 2018

BridgeAR commented Feb 18, 2018

BridgeAR commented Feb 22, 2018

MylesBorins commented Mar 6, 2018

		@@ -2019,11 +2019,17 @@ The file is created if it does not exist.

		* `'ax'` - Like `'a'` but fails if `path` exists.

		* `'as'` - Open file for appending in synchronous mode.

fs: add two new modes "as" and "as+" #18801

fs: add two new modes "as" and "as+" #18801

Conversation

SirR4T commented Feb 15, 2018

Checklist

Affected core subsystem(s)

vsemozhetbyt commented Feb 15, 2018

SirR4T commented Feb 16, 2018

vsemozhetbyt commented Feb 16, 2018

SirR4T commented Feb 17, 2018

joyeecheung Feb 17, 2018

Choose a reason for hiding this comment

joyeecheung Feb 17, 2018

Choose a reason for hiding this comment

addaleax Feb 17, 2018

Choose a reason for hiding this comment

SirR4T Feb 17, 2018

Choose a reason for hiding this comment

joyeecheung Feb 18, 2018

Choose a reason for hiding this comment

jasnell commented Feb 17, 2018

SirR4T commented Feb 17, 2018

BridgeAR commented Feb 18, 2018

BridgeAR commented Feb 22, 2018

MylesBorins commented Mar 6, 2018