Skip to content

Commit

Permalink
Fix atomic createWriteStream (#8194)
Browse files Browse the repository at this point in the history
  • Loading branch information
mischnic committed Jun 13, 2022
1 parent 58bd6d7 commit 1006807
Show file tree
Hide file tree
Showing 21 changed files with 64 additions and 784 deletions.
1 change: 0 additions & 1 deletion .eslintignore
Expand Up @@ -10,7 +10,6 @@ packages/*/*/test/integration/**
packages/*/*/test/mochareporters.json
packages/core/integration-tests/test/input/**
packages/core/utils/test/input/**
packages/utils/fs-write-stream-atomic/**
packages/utils/create-react-app/templates
packages/examples

Expand Down
2 changes: 1 addition & 1 deletion .mocharc.json
@@ -1,5 +1,5 @@
{
"spec": "packages/*/!(integration-tests|fs-write-stream-atomic)/test/*.js",
"spec": "packages/*/!(integration-tests)/test/*.js",
"require": ["@parcel/babel-register", "@parcel/test-utils/src/mochaSetup.js"],
// TODO: Remove this when https://github.com/nodejs/node/pull/28788 is resolved
"exit": true
Expand Down
24 changes: 0 additions & 24 deletions flow-libs/fs-write-stream-atomic.js.flow

This file was deleted.

1 change: 0 additions & 1 deletion gulpfile.js
Expand Up @@ -20,7 +20,6 @@ const IGNORED_PACKAGES = [
'!packages/core/utils/**',
'!packages/reporters/cli/**',
'!packages/reporters/dev-server/**',
'!packages/utils/fs-write-stream-atomic/**',
];

const paths = {
Expand Down
1 change: 0 additions & 1 deletion packages/core/fs/package.json
Expand Up @@ -55,7 +55,6 @@
"@parcel/workers": "2.6.0"
},
"devDependencies": {
"@parcel/fs-write-stream-atomic": "2.6.0",
"graceful-fs": "^4.2.4",
"ncp": "^2.0.0",
"nullthrows": "^1.1.1",
Expand Down
68 changes: 60 additions & 8 deletions packages/core/fs/src/NodeFS.js
Expand Up @@ -14,7 +14,7 @@ import nativeFS from 'fs';
import ncp from 'ncp';
import {promisify} from 'util';
import {registerSerializableClass} from '@parcel/core';
import fsWriteStreamAtomic from '@parcel/fs-write-stream-atomic';
import {hashStream} from '@parcel/utils';
import watcher from '@parcel/watcher';
import packageJSON from '../package.json';

Expand Down Expand Up @@ -58,7 +58,64 @@ export class NodeFS implements FileSystem {
: searchNative.findFirstFile;

createWriteStream(filePath: string, options: any): Writable {
return fsWriteStreamAtomic(filePath, options);
// Make createWriteStream atomic
let tmpFilePath = getTempFilePath(filePath);
let failed = false;

const move = async () => {
if (!failed) {
try {
await fs.promises.rename(tmpFilePath, filePath);
} catch (e) {
// This is adapted from fs-write-stream-atomic. Apparently
// Windows doesn't like renaming when the target already exists.
if (
process.platform === 'win32' &&
e.syscall &&
e.syscall === 'rename' &&
e.code &&
e.code === 'EPERM'
) {
let [hashTmp, hashTarget] = await Promise.all([
hashStream(writeStream.__atomicTmp),
hashStream(writeStream.__atomicTarget),
]);

await this.unlink(writeStream.__atomicTmp);

if (hashTmp != hashTarget) {
throw e;
}
}
}
}
};

let writeStream = fs.createWriteStream(tmpFilePath, {
...options,
fs: {
...fs,
close: (fd, cb) => {
fs.close(fd, err => {
if (err) {
cb(err);
} else {
move().then(
() => cb(),
err => cb(err),
);
}
});
},
},
});

writeStream.once('error', () => {
failed = true;
fs.unlinkSync(tmpFilePath);
});

return writeStream;
}

async writeFile(
Expand All @@ -67,12 +124,7 @@ export class NodeFS implements FileSystem {
options: ?FileOptions,
): Promise<void> {
let tmpFilePath = getTempFilePath(filePath);
await fs.promises.writeFile(
tmpFilePath,
contents,
// $FlowFixMe
options,
);
await fs.promises.writeFile(tmpFilePath, contents, options);
await fs.promises.rename(tmpFilePath, filePath);
}

Expand Down
9 changes: 0 additions & 9 deletions packages/utils/fs-write-stream-atomic/.travis.yml

This file was deleted.

15 changes: 0 additions & 15 deletions packages/utils/fs-write-stream-atomic/LICENSE

This file was deleted.

34 changes: 0 additions & 34 deletions packages/utils/fs-write-stream-atomic/README.md

This file was deleted.

This file was deleted.

0 comments on commit 1006807

Please sign in to comment.