-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
[nodefs] Remove compatibility code for ancient node versions #23316
base: main
Are you sure you want to change the base?
Changes from 5 commits
ab2cc9c
6f92152
cafbbba
7092bdc
559f485
fb334a3
e93545c
6177f50
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,11 +19,7 @@ addToLibrary({ | |
isWindows: false, | ||
staticInit() { | ||
NODEFS.isWindows = !!process.platform.match(/^win/); | ||
var flags = process.binding("constants"); | ||
// Node.js 4 compatibility: it has no namespaces for constants | ||
if (flags["fs"]) { | ||
flags = flags["fs"]; | ||
} | ||
var flags = process.binding("constants")["fs"]; | ||
NODEFS.flagsForNodeMap = { | ||
"{{{ cDefs.O_APPEND }}}": flags["O_APPEND"], | ||
"{{{ cDefs.O_CREAT }}}": flags["O_CREAT"], | ||
|
@@ -123,15 +119,6 @@ addToLibrary({ | |
var stat; | ||
NODEFS.tryFSOperation(() => stat = fs.lstatSync(path)); | ||
if (NODEFS.isWindows) { | ||
// node.js v0.10.20 doesn't report blksize and blocks on Windows. Fake | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We do still support MIN_NODE_VERSION=101900 (10.19.0) so if we want to drop this I guess we would need to bump that minimum too? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh wait, thats is 0.10.20! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you add some stat tests to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How's 6f92152 look? |
||
// them with default blksize of 4096. | ||
// See http://support.microsoft.com/kb/140365 | ||
if (!stat.blksize) { | ||
stat.blksize = 4096; | ||
} | ||
if (!stat.blocks) { | ||
stat.blocks = (stat.size+stat.blksize-1)/stat.blksize|0; | ||
} | ||
// Windows does not report the 'x' permission bit, so propagate read | ||
// bits to execute bits. | ||
stat.mode |= (stat.mode & {{{ cDefs.S_IRUGO }}}) >> 2; | ||
|
@@ -252,8 +239,6 @@ addToLibrary({ | |
stream.shared.refcount++; | ||
}, | ||
read(stream, buffer, offset, length, position) { | ||
// Node.js < 6 compatibility: node errors on 0 length reads | ||
if (length === 0) return 0; | ||
return NODEFS.tryFSOperation(() => | ||
fs.readSync(stream.nfd, new Int8Array(buffer.buffer, offset, length), 0, length, position) | ||
); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was going to suggest single quotes here but it looks like this file users double-quote a lot, so maybe we leave as is.