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.

Sign up for GitHub

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

Jump to bottom

Update externals stubs atomically #1149

Merged
merged 1 commit into from Mar 4, 2022
Merged

Update externals stubs atomically #1149

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
27 changes: 23 additions & 4 deletions packages/core/src/babel-plugin-adjust-imports.ts
View file
Open in desktop
Expand Up @@ -4,11 +4,12 @@ import type { NodePath } from '@babel/traverse';
import type * as Babel from '@babel/core';
import type { types as t } from '@babel/core';
import { PackageCache, Package, V2Package, explicitRelative } from '@embroider/shared-internals';
import { outputFileSync } from 'fs-extra';
import { Memoize } from 'typescript-memoize';
import { compile } from './js-handlebars';
import { handleImportDeclaration } from './mini-modules-polyfill';
import { ImportUtil } from 'babel-import-util';
import { randomBytes } from 'crypto';
import { outputFileSync, pathExistsSync, renameSync } from 'fs-extra';

interface State {
adjustFile: AdjustFile;
Expand Down Expand Up @@ -332,8 +333,8 @@ function handleExternal(specifier: string, sourceFile: AdjustFile, opts: Options
}

function makeMissingModule(specifier: string, sourceFile: AdjustFile, opts: Options): string {
let target = join(opts.externalsDir, specifier + '.js');
outputFileSync(
let target = join(opts.externalsDir, 'missing', specifier + '.js');
atomicWrite(
target,
dynamicMissingModule({
moduleName: specifier,
Expand All @@ -344,7 +345,7 @@ function makeMissingModule(specifier: string, sourceFile: AdjustFile, opts: Opti

function makeExternal(specifier: string, sourceFile: AdjustFile, opts: Options): string {
let target = join(opts.externalsDir, specifier + '.js');
outputFileSync(
atomicWrite(
target,
externalTemplate({
runtimeName: specifier,
Expand All @@ -353,6 +354,24 @@ function makeExternal(specifier: string, sourceFile: AdjustFile, opts: Options):
return explicitRelative(dirname(sourceFile.name), target.slice(0, -3));
}

function atomicWrite(path: string, content: string) {
if (pathExistsSync(path)) {
return;
}
let suffix = randomBytes(8).toString('hex');
outputFileSync(path + suffix, content);
try {
renameSync(path + suffix, path);
} catch (err: any) {
// windows throws EPERM for concurrent access. For us it's not an error
// condition because the other thread is writing the exact same value we
// would have.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thinking about async func here with few retry attempts. I think it’s important to do job, instead of fail

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we hit EPERM here it's only because another process is already doing the exact same job we were trying to do, so we can move on and let that process take care of it.

As far as I can tell, that's the only way to get EPERM here. Since this is inside our own temp dir, you shouldn't ever have unrelated permission failures in the usual Unix EPERM sense.

if (err.code !== 'EPERM') {
throw err;
}
}
}

export default function main(babel: typeof Babel) {
let t = babel.types;
return {
Expand Down