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.

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

feat: remove sync fs calls from backend+plugin code #12798

Merged
merged 11 commits into from
Aug 14, 2023
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
- `visual`: Display a visual preview of the tab. (The preview support was added with this PR)
- [repo] updated GitHub workflow to stop publishing `next` versions [#12699](https://github.com/eclipse-theia/theia/pull/12699)
- [workspace] split `CommonWorkspaceUtils` into `WorkspaceFileService` and `UntitledWorkspaceService` [#12420](https://github.com/eclipse-theia/theia/pull/12420)
- [plugin] Removed synchronous `fs` calls from the backend application and plugins. The plugin scanner, directory and file handlers, and the plugin deploy entry has async API now. Internal `protected` APIs have been affected. [#12798](https://github.com/eclipse-theia/theia/pull/12798)

## v1.39.0 - 06/29/2023

Expand Down
3 changes: 2 additions & 1 deletion packages/git/src/node/dugite-git.ts
Original file line number Diff line number Diff line change
Expand Up @@ -766,7 +766,8 @@ export class DugiteGit implements Git {
const out = result.stdout;
if (out && out.length !== 0) {
try {
return fs.realpathSync(out.trim());
const realpath = await fs.realpath(out.trim());
return realpath;
} catch (e) {
this.logger.error(e);
return undefined;
Expand Down
10 changes: 5 additions & 5 deletions packages/git/src/node/git-locator/git-locator-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export class GitLocatorImpl implements GitLocator {
}

protected async doLocate(basePath: string, context: GitLocateContext): Promise<string[]> {
const realBasePath = fs.realpathSync(basePath);
const realBasePath = await fs.realpath(basePath);
if (context.visited.has(realBasePath)) {
return [];
}
Expand All @@ -77,9 +77,9 @@ export class GitLocatorImpl implements GitLocator {
}
});
if (context.maxCount >= 0 && paths.length >= context.maxCount) {
return paths.slice(0, context.maxCount).map(GitLocatorImpl.map);
return Promise.all(paths.slice(0, context.maxCount).map(GitLocatorImpl.map));
msujew marked this conversation as resolved.
Show resolved Hide resolved
}
const repositoryPaths = paths.map(GitLocatorImpl.map);
const repositoryPaths = await Promise.all(paths.map(GitLocatorImpl.map));
return this.locateFrom(
newContext => this.generateNested(repositoryPaths, newContext),
context,
Expand Down Expand Up @@ -145,8 +145,8 @@ export class GitLocatorImpl implements GitLocator {
return result;
}

static map(repository: string): string {
return fs.realpathSync(path.dirname(repository));
static async map(repository: string): Promise<string> {
return fs.realpath(path.dirname(repository));
}

}
22 changes: 11 additions & 11 deletions packages/plugin-dev/src/node/hosted-instance-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import { HostedPluginSupport } from '@theia/plugin-ext/lib/hosted/node/hosted-pl
import { MetadataScanner } from '@theia/plugin-ext/lib/hosted/node/metadata-scanner';
import { PluginDebugConfiguration } from '../common/plugin-dev-protocol';
import { HostedPluginProcess } from '@theia/plugin-ext/lib/hosted/node/hosted-plugin-process';
import { isENOENT } from '@theia/plugin-ext/lib/common/errors';

const DEFAULT_HOSTED_PLUGIN_PORT = 3030;

Expand Down Expand Up @@ -84,7 +85,7 @@ export interface HostedInstanceManager {
*
* @param uri uri to the plugin source location
*/
isPluginValid(uri: URI): boolean;
isPluginValid(uri: URI): Promise<boolean>;
}

const HOSTED_INSTANCE_START_TIMEOUT_MS = 30000;
Expand Down Expand Up @@ -224,19 +225,18 @@ export abstract class AbstractHostedInstanceManager implements HostedInstanceMan
}
}

isPluginValid(uri: URI): boolean {
async isPluginValid(uri: URI): Promise<boolean> {
const pckPath = path.join(FileUri.fsPath(uri), 'package.json');
if (fs.existsSync(pckPath)) {
const pck = fs.readJSONSync(pckPath);
try {
this.metadata.getScanner(pck);
return true;
} catch (e) {
console.error(e);
return false;
try {
const pck = await fs.readJSON(pckPath);
this.metadata.getScanner(pck);
return true;
} catch (err) {
if (!isENOENT(err)) {
console.error(err);
}
return false;
}
return false;
}

protected async getStartCommand(port?: number, debugConfig?: PluginDebugConfiguration): Promise<string[]> {
Expand Down
8 changes: 4 additions & 4 deletions packages/plugin-dev/src/node/hosted-plugins-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,15 +131,15 @@ export class HostedPluginsManagerImpl implements HostedPluginsManager {
*
* @param pluginPath path to plugin's root directory
*/
protected checkWatchScript(pluginPath: string): boolean {
protected async checkWatchScript(pluginPath: string): Promise<boolean> {
const pluginPackageJsonPath = path.join(pluginPath, 'package.json');
if (fs.existsSync(pluginPackageJsonPath)) {
const packageJson = fs.readJSONSync(pluginPackageJsonPath);
try {
const packageJson = await fs.readJSON(pluginPackageJsonPath);
const scripts = packageJson['scripts'];
if (scripts && scripts['watch']) {
return true;
}
}
} catch { }
return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,33 +18,47 @@ import * as path from 'path';
import * as filenamify from 'filenamify';
import * as fs from '@theia/core/shared/fs-extra';
import { inject, injectable } from '@theia/core/shared/inversify';
import { RecursivePartial } from '@theia/core';
import type { RecursivePartial, URI } from '@theia/core';
import { Deferred } from '@theia/core/lib/common/promise-util';
import { getTempDirPathAsync } from '@theia/plugin-ext/lib/main/node/temp-dir-util';
import {
PluginDeployerDirectoryHandler, PluginDeployerEntry, PluginDeployerDirectoryHandlerContext,
PluginDeployerEntryType, PluginPackage, PluginType, PluginIdentifiers
} from '@theia/plugin-ext';
import { FileUri } from '@theia/core/lib/node';
import { getTempDir } from '@theia/plugin-ext/lib/main/node/temp-dir-util';
import { PluginCliContribution } from '@theia/plugin-ext/lib/main/node/plugin-cli-contribution';

@injectable()
export class PluginVsCodeDirectoryHandler implements PluginDeployerDirectoryHandler {

protected readonly deploymentDirectory = FileUri.create(getTempDir('vscode-copied'));
protected readonly deploymentDirectory: Deferred<URI>;

@inject(PluginCliContribution) protected readonly pluginCli: PluginCliContribution;

accept(plugin: PluginDeployerEntry): boolean {
constructor() {
this.deploymentDirectory = new Deferred();
getTempDirPathAsync('vscode-copied')
.then(deploymentDirectoryPath => this.deploymentDirectory.resolve(FileUri.create(deploymentDirectoryPath)));
}

async accept(plugin: PluginDeployerEntry): Promise<boolean> {
console.debug(`Resolving "${plugin.id()}" as a VS Code extension...`);
return this.attemptResolution(plugin);
}

protected attemptResolution(plugin: PluginDeployerEntry): boolean {
return this.resolvePackage(plugin) || this.deriveMetadata(plugin);
protected async attemptResolution(plugin: PluginDeployerEntry): Promise<boolean> {
if (this.resolvePackage(plugin)) {
return true;
}
return this.deriveMetadata(plugin);
}

protected deriveMetadata(plugin: PluginDeployerEntry): boolean {
return this.resolveFromSources(plugin) || this.resolveFromVSIX(plugin) || this.resolveFromNpmTarball(plugin);
protected async deriveMetadata(plugin: PluginDeployerEntry): Promise<boolean> {
return (
(await this.resolveFromSources(plugin)) ||
(await this.resolveFromVSIX(plugin)) ||
(this.resolveFromNpmTarball(plugin))
);
msujew marked this conversation as resolved.
Show resolved Hide resolved
}

async handle(context: PluginDeployerDirectoryHandlerContext): Promise<void> {
Expand All @@ -68,11 +82,12 @@ export class PluginVsCodeDirectoryHandler implements PluginDeployerDirectoryHand
const origin = entry.originalPath();
const targetDir = await this.getExtensionDir(context);
try {
if (fs.existsSync(targetDir) || !entry.path().startsWith(origin)) {
if (await fs.pathExists(targetDir) || !entry.path().startsWith(origin)) {
console.log(`[${id}]: already copied.`);
} else {
console.log(`[${id}]: copying to "${targetDir}"`);
await fs.mkdirp(FileUri.fsPath(this.deploymentDirectory));
const deploymentDirectory = await this.deploymentDirectory.promise;
await fs.mkdirp(FileUri.fsPath(deploymentDirectory));
await context.copy(origin, targetDir);
entry.updatePath(targetDir);
if (!this.deriveMetadata(entry)) {
Expand All @@ -86,22 +101,25 @@ export class PluginVsCodeDirectoryHandler implements PluginDeployerDirectoryHand
}
}

protected resolveFromSources(plugin: PluginDeployerEntry): boolean {
protected async resolveFromSources(plugin: PluginDeployerEntry): Promise<boolean> {
const pluginPath = plugin.path();
return this.resolvePackage(plugin, { pluginPath, pck: this.requirePackage(pluginPath) });
const pck = await this.requirePackage(pluginPath);
return this.resolvePackage(plugin, { pluginPath, pck });
}

protected resolveFromVSIX(plugin: PluginDeployerEntry): boolean {
if (!fs.existsSync(path.join(plugin.path(), 'extension.vsixmanifest'))) {
protected async resolveFromVSIX(plugin: PluginDeployerEntry): Promise<boolean> {
if (!(await fs.pathExists(path.join(plugin.path(), 'extension.vsixmanifest')))) {
return false;
}
const pluginPath = path.join(plugin.path(), 'extension');
return this.resolvePackage(plugin, { pluginPath, pck: this.requirePackage(pluginPath) });
const pck = await this.requirePackage(pluginPath);
return this.resolvePackage(plugin, { pluginPath, pck });
}

protected resolveFromNpmTarball(plugin: PluginDeployerEntry): boolean {
protected async resolveFromNpmTarball(plugin: PluginDeployerEntry): Promise<boolean> {
const pluginPath = path.join(plugin.path(), 'package');
return this.resolvePackage(plugin, { pluginPath, pck: this.requirePackage(pluginPath) });
const pck = await this.requirePackage(pluginPath);
return this.resolvePackage(plugin, { pluginPath, pck });
}

protected resolvePackage(plugin: PluginDeployerEntry, options?: {
Expand All @@ -125,9 +143,9 @@ export class PluginVsCodeDirectoryHandler implements PluginDeployerDirectoryHand
return true;
}

protected requirePackage(pluginPath: string): PluginPackage | undefined {
protected async requirePackage(pluginPath: string): Promise<PluginPackage | undefined> {
try {
const plugin = fs.readJSONSync(path.join(pluginPath, 'package.json')) as PluginPackage;
const plugin: PluginPackage = await fs.readJSON(path.join(pluginPath, 'package.json'));
plugin.publisher ??= PluginIdentifiers.UNPUBLISHED;
return plugin;
} catch {
Expand All @@ -136,6 +154,7 @@ export class PluginVsCodeDirectoryHandler implements PluginDeployerDirectoryHand
}

protected async getExtensionDir(context: PluginDeployerDirectoryHandlerContext): Promise<string> {
return FileUri.fsPath(this.deploymentDirectory.resolve(filenamify(context.pluginEntry().id(), { replacement: '_' })));
const deploymentDirectory = await this.deploymentDirectory.promise;
return FileUri.fsPath(deploymentDirectory.resolve(filenamify(context.pluginEntry().id(), { replacement: '_' })));
}
}
29 changes: 20 additions & 9 deletions packages/plugin-ext-vscode/src/node/plugin-vscode-file-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@ import { PluginDeployerFileHandler, PluginDeployerEntry, PluginDeployerFileHandl
import * as fs from '@theia/core/shared/fs-extra';
import * as path from 'path';
import * as filenamify from 'filenamify';
import { injectable, inject } from '@theia/core/shared/inversify';
import { getTempDir } from '@theia/plugin-ext/lib/main/node/temp-dir-util';
import type { URI } from '@theia/core';
import { inject, injectable } from '@theia/core/shared/inversify';
import { Deferred } from '@theia/core/lib/common/promise-util';
import { getTempDirPathAsync } from '@theia/plugin-ext/lib/main/node/temp-dir-util';
import { PluginVSCodeEnvironment } from '../common/plugin-vscode-environment';
import { FileUri } from '@theia/core/lib/node/file-uri';

Expand All @@ -31,13 +33,21 @@ export class PluginVsCodeFileHandler implements PluginDeployerFileHandler {
@inject(PluginVSCodeEnvironment)
protected readonly environment: PluginVSCodeEnvironment;

private readonly systemExtensionsDirUri = FileUri.create(getTempDir('vscode-unpacked'));
private readonly systemExtensionsDirUri: Deferred<URI>;

accept(resolvedPlugin: PluginDeployerEntry): boolean {
if (!resolvedPlugin.isFile()) {
return false;
}
return isVSCodePluginFile(resolvedPlugin.path());
constructor() {
this.systemExtensionsDirUri = new Deferred();
getTempDirPathAsync('vscode-unpacked')
.then(systemExtensionsDirPath => this.systemExtensionsDirUri.resolve(FileUri.create(systemExtensionsDirPath)));
}

async accept(resolvedPlugin: PluginDeployerEntry): Promise<boolean> {
return resolvedPlugin.isFile().then(file => {
if (!file) {
return false;
}
return isVSCodePluginFile(resolvedPlugin.path());
});
}

async handle(context: PluginDeployerFileHandlerContext): Promise<void> {
Expand All @@ -55,7 +65,8 @@ export class PluginVsCodeFileHandler implements PluginDeployerFileHandler {
}

protected async getExtensionDir(context: PluginDeployerFileHandlerContext): Promise<string> {
return FileUri.fsPath(this.systemExtensionsDirUri.resolve(filenamify(context.pluginEntry().id(), { replacement: '_' })));
const systemExtensionsDirUri = await this.systemExtensionsDirUri.promise;
return FileUri.fsPath(systemExtensionsDirUri.resolve(filenamify(context.pluginEntry().id(), { replacement: '_' })));
}

protected async decompress(extensionDir: string, context: PluginDeployerFileHandlerContext): Promise<void> {
Expand Down
26 changes: 26 additions & 0 deletions packages/plugin-ext/src/common/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
// SPDX-License-Identifier: EPL-2.0 OR GPL-2.0-only WITH Classpath-exception-2.0
// *****************************************************************************

import { isObject } from '@theia/core/lib/common/types';

export function illegalArgument(message?: string): Error {
if (message) {
return new Error(`Illegal argument: ${message}`);
Expand All @@ -35,3 +37,27 @@ export function disposed(what: string): Error {
result.name = 'DISPOSED';
return result;
}

interface Errno {
readonly code: string;
readonly errno: number
}
const ENOENT = 'ENOENT' as const;

type ErrnoException = Error & Errno;
function isErrnoException(arg: unknown): arg is ErrnoException {
return arg instanceof Error
&& isObject<Partial<Errno>>(arg)
&& typeof arg.code === 'string'
&& typeof arg.errno === 'number';
}

/**
* _(No such file or directory)_: Commonly raised by `fs` operations to indicate that a component of the specified pathname does not exist — no entity (file or directory) could be
* found by the given path.
*/
export function isENOENT(
arg: unknown
): arg is ErrnoException & Readonly<{ code: typeof ENOENT }> {
return isErrnoException(arg) && arg.code === ENOENT;
}
10 changes: 5 additions & 5 deletions packages/plugin-ext/src/common/plugin-protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ export interface PluginScanner {
*/
getLifecycle(plugin: PluginPackage): PluginLifecycle;

getContribution(plugin: PluginPackage): PluginContribution | undefined;
getContribution(plugin: PluginPackage): Promise<PluginContribution | undefined>;

/**
* A mapping between a dependency as its defined in package.json
Expand Down Expand Up @@ -376,15 +376,15 @@ export interface PluginDeployerResolver {

export const PluginDeployerDirectoryHandler = Symbol('PluginDeployerDirectoryHandler');
export interface PluginDeployerDirectoryHandler {
accept(pluginDeployerEntry: PluginDeployerEntry): boolean;
accept(pluginDeployerEntry: PluginDeployerEntry): Promise<boolean>;

handle(context: PluginDeployerDirectoryHandlerContext): Promise<void>;
}

export const PluginDeployerFileHandler = Symbol('PluginDeployerFileHandler');
export interface PluginDeployerFileHandler {

accept(pluginDeployerEntry: PluginDeployerEntry): boolean;
accept(pluginDeployerEntry: PluginDeployerEntry): Promise<boolean>;

handle(context: PluginDeployerFileHandlerContext): Promise<void>;
}
Expand Down Expand Up @@ -477,9 +477,9 @@ export interface PluginDeployerEntry {

getChanges(): string[];

isFile(): boolean;
isFile(): Promise<boolean>;

isDirectory(): boolean;
isDirectory(): Promise<boolean>;

/**
* Resolved if a resolver has handle this plugin
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ export class HostedPluginDeployerHandler implements PluginDeployerHandler {

const { type } = entry;
const deployed: DeployedPlugin = { metadata, type };
deployed.contributes = this.reader.readContribution(manifest);
deployed.contributes = await this.reader.readContribution(manifest);
await this.localizationService.deployLocalizations(deployed);
deployedPlugins.set(id, deployed);
deployPlugin.debug(`Deployed ${entryPoint} plugin "${id}" from "${metadata.model.entryPoint[entryPoint] || pluginPath}"`);
Expand Down
2 changes: 1 addition & 1 deletion packages/plugin-ext/src/hosted/node/plugin-reader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ export class HostedPluginReader implements BackendApplicationContribution {
return pluginMetadata;
}

readContribution(plugin: PluginPackage): PluginContribution | undefined {
async readContribution(plugin: PluginPackage): Promise<PluginContribution | undefined> {
const scanner = this.scanner.getScanner(plugin);
return scanner.getContribution(plugin);
}
Expand Down
Loading
Loading