Skip to content

internal: migrate to vscode.FileSystem API #8951

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

Merged
merged 3 commits into from
Jun 14, 2021
Merged

internal: migrate to vscode.FileSystem API #8951

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3ca7f61
internal: use vscode.FileSystem API in main.ts
wxb1ank May 23, 2021
0448b73
migrate from `fs` to `vscode.FileSystem` API
wxb1ank May 24, 2021
388a194
Lint toolchain.ts
wxb1ank Jun 2, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions editors/code/src/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@ export class Config {
enableProposedApi: boolean | undefined;
} = vscode.extensions.getExtension(this.extensionId)!.packageJSON;

readonly globalStoragePath: string;
readonly globalStorageUri: vscode.Uri;

constructor(ctx: vscode.ExtensionContext) {
this.globalStoragePath = ctx.globalStorageUri.fsPath;
this.globalStorageUri = ctx.globalStorageUri;
vscode.workspace.onDidChangeConfiguration(this.onDidChangeConfiguration, this, ctx.subscriptions);
this.refreshLogging();
}
Expand Down
32 changes: 15 additions & 17 deletions editors/code/src/main.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import * as vscode from 'vscode';
import * as path from "path";
import * as os from "os";
import { promises as fs, PathLike } from "fs";

import * as commands from './commands';
import { activateInlayHints } from './inlay_hints';
Expand Down Expand Up @@ -160,7 +158,7 @@ export async function deactivate() {
}

async function bootstrap(config: Config, state: PersistentState): Promise<string> {
await fs.mkdir(config.globalStoragePath, { recursive: true });
await vscode.workspace.fs.createDirectory(config.globalStorageUri);

if (!config.currentExtensionIsNightly) {
await state.updateNightlyReleaseId(undefined);
Expand Down Expand Up @@ -222,7 +220,7 @@ async function bootstrapExtension(config: Config, state: PersistentState): Promi
const artifact = latestNightlyRelease.assets.find(artifact => artifact.name === "rust-analyzer.vsix");
assert(!!artifact, `Bad release: ${JSON.stringify(latestNightlyRelease)}`);

const dest = path.join(config.globalStoragePath, "rust-analyzer.vsix");
const dest = vscode.Uri.joinPath(config.globalStorageUri, "rust-analyzer.vsix");

await downloadWithRetryDialog(state, async () => {
await download({
Expand All @@ -233,8 +231,8 @@ async function bootstrapExtension(config: Config, state: PersistentState): Promi
});
});

await vscode.commands.executeCommand("workbench.extensions.installExtension", vscode.Uri.file(dest));
await fs.unlink(dest);
await vscode.commands.executeCommand("workbench.extensions.installExtension", dest);
await vscode.workspace.fs.delete(dest);

await state.updateNightlyReleaseId(latestNightlyRelease.id);
await state.updateLastCheck(now);
Expand All @@ -259,7 +257,7 @@ async function bootstrapServer(config: Config, state: PersistentState): Promise<
return path;
}

async function patchelf(dest: PathLike): Promise<void> {
async function patchelf(dest: vscode.Uri): Promise<void> {
await vscode.window.withProgress(
{
location: vscode.ProgressLocation.Notification,
Expand All @@ -279,11 +277,11 @@ async function patchelf(dest: PathLike): Promise<void> {
'';
}
`;
const origFile = dest + "-orig";
await fs.rename(dest, origFile);
const origFile = vscode.Uri.file(dest.path + "-orig");
await vscode.workspace.fs.rename(dest, origFile);
progress.report({ message: "Patching executable", increment: 20 });
await new Promise((resolve, reject) => {
const handle = exec(`nix-build -E - --argstr srcStr '${origFile}' -o '${dest}'`,
const handle = exec(`nix-build -E - --argstr srcStr '${origFile.path}' -o '${dest.path}'`,
(err, stdout, stderr) => {
if (err != null) {
reject(Error(stderr));
Expand All @@ -294,7 +292,7 @@ async function patchelf(dest: PathLike): Promise<void> {
handle.stdin?.write(expression);
handle.stdin?.end();
});
await fs.unlink(origFile);
await vscode.workspace.fs.delete(origFile);
}
);
}
Expand Down Expand Up @@ -334,20 +332,20 @@ async function getServer(config: Config, state: PersistentState): Promise<string
platform = "x86_64-unknown-linux-musl";
}
const ext = platform.indexOf("-windows-") !== -1 ? ".exe" : "";
const dest = path.join(config.globalStoragePath, `rust-analyzer-${platform}${ext}`);
const exists = await fs.stat(dest).then(() => true, () => false);
const dest = vscode.Uri.joinPath(config.globalStorageUri, `rust-analyzer-${platform}${ext}`);
const exists = await vscode.workspace.fs.stat(dest).then(() => true, () => false);
if (!exists) {
await state.updateServerVersion(undefined);
}

if (state.serverVersion === config.package.version) return dest;
if (state.serverVersion === config.package.version) return dest.path;

if (config.askBeforeDownload) {
const userResponse = await vscode.window.showInformationMessage(
`Language server version ${config.package.version} for rust-analyzer is not installed.`,
"Download now"
);
if (userResponse !== "Download now") return dest;
if (userResponse !== "Download now") return dest.path;
}

const releaseTag = config.package.releaseTag;
Expand All @@ -374,7 +372,7 @@ async function getServer(config: Config, state: PersistentState): Promise<string
}

await state.updateServerVersion(config.package.version);
return dest;
return dest.path;
}

function serverPath(config: Config): string | null {
Expand All @@ -383,7 +381,7 @@ function serverPath(config: Config): string | null {

async function isNixOs(): Promise<boolean> {
try {
const contents = await fs.readFile("/etc/os-release");
const contents = (await vscode.workspace.fs.readFile(vscode.Uri.file("/etc/os-release"))).toString();
return contents.indexOf("ID=nixos") !== -1;
} catch (e) {
return false;
Expand Down
30 changes: 15 additions & 15 deletions editors/code/src/net.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,14 +73,14 @@ export interface GithubRelease {
assets: Array<{
name: string;
// eslint-disable-next-line camelcase
browser_download_url: string;
browser_download_url: vscode.Uri;
}>;
}

interface DownloadOpts {
progressTitle: string;
url: string;
dest: string;
url: vscode.Uri;
dest: vscode.Uri;
mode?: number;
gunzip?: boolean;
httpProxy?: string;
Expand All @@ -90,9 +90,9 @@ export async function download(opts: DownloadOpts) {
// Put artifact into a temporary file (in the same dir for simplicity)
// to prevent partially downloaded files when user kills vscode
// This also avoids overwriting running executables
const dest = path.parse(opts.dest);
const randomHex = crypto.randomBytes(5).toString("hex");
const tempFile = path.join(dest.dir, `${dest.name}${randomHex}`);
const rawDest = path.parse(opts.dest.path);
const tempFilePath = vscode.Uri.joinPath(vscode.Uri.file(rawDest.dir), `${rawDest.name}${randomHex}`);

await vscode.window.withProgress(
{
Expand All @@ -102,7 +102,7 @@ export async function download(opts: DownloadOpts) {
},
async (progress, _cancellationToken) => {
let lastPercentage = 0;
await downloadFile(opts.url, tempFile, opts.mode, !!opts.gunzip, opts.httpProxy, (readBytes, totalBytes) => {
await downloadFile(opts.url, tempFilePath, opts.mode, !!opts.gunzip, opts.httpProxy, (readBytes, totalBytes) => {
const newPercentage = Math.round((readBytes / totalBytes) * 100);
if (newPercentage !== lastPercentage) {
progress.report({
Expand All @@ -116,28 +116,28 @@ export async function download(opts: DownloadOpts) {
}
);

await fs.promises.rename(tempFile, opts.dest);
await vscode.workspace.fs.rename(tempFilePath, opts.dest);
}

async function downloadFile(
url: string,
destFilePath: fs.PathLike,
url: vscode.Uri,
destFilePath: vscode.Uri,
mode: number | undefined,
gunzip: boolean,
httpProxy: string | null | undefined,
onProgress: (readBytes: number, totalBytes: number) => void
): Promise<void> {
const res = await (() => {
if (httpProxy) {
log.debug(`Downloading ${url} via proxy: ${httpProxy}`);
return fetch(url, { agent: new HttpsProxyAgent(httpProxy) });
log.debug(`Downloading ${url.path} via proxy: ${httpProxy}`);
return fetch(url.path, { agent: new HttpsProxyAgent(httpProxy) });
}

return fetch(url);
return fetch(url.path);
})();

if (!res.ok) {
log.error("Error", res.status, "while downloading file from", url);
log.error("Error", res.status, "while downloading file from", url.path);
log.error({ body: await res.text(), headers: res.headers });

throw new Error(`Got response ${res.status} when trying to download a file.`);
Expand All @@ -146,15 +146,15 @@ async function downloadFile(
const totalBytes = Number(res.headers.get('content-length'));
assert(!Number.isNaN(totalBytes), "Sanity check of content-length protocol");

log.debug("Downloading file of", totalBytes, "bytes size from", url, "to", destFilePath);
log.debug("Downloading file of", totalBytes, "bytes size from", url.path, "to", destFilePath.path);

let readBytes = 0;
res.body.on("data", (chunk: Buffer) => {
readBytes += chunk.length;
onProgress(readBytes, totalBytes);
});

const destFileStream = fs.createWriteStream(destFilePath, { mode });
const destFileStream = fs.createWriteStream(destFilePath.path, { mode });
const srcStream = gunzip ? res.body.pipe(zlib.createGunzip()) : res.body;

await pipeline(srcStream, destFileStream);
Expand Down
19 changes: 6 additions & 13 deletions editors/code/src/toolchain.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import * as cp from 'child_process';
import * as os from 'os';
import * as path from 'path';
import * as fs from 'fs';
import * as readline from 'readline';
import { OutputChannel } from 'vscode';
import * as vscode from 'vscode';
import { execute, log, memoize } from './util';

interface CompilationArtifact {
Expand All @@ -19,7 +18,7 @@ export interface ArtifactSpec {
}

export class Cargo {
constructor(readonly rootFolder: string, readonly output: OutputChannel) { }
constructor(readonly rootFolder: string, readonly output: vscode.OutputChannel) { }

// Made public for testing purposes
static artifactSpec(args: readonly string[]): ArtifactSpec {
Expand Down Expand Up @@ -158,9 +157,9 @@ export const getPathForExecutable = memoize(
try {
// hmm, `os.homedir()` seems to be infallible
// it is not mentioned in docs and cannot be infered by the type signature...
const standardPath = path.join(os.homedir(), ".cargo", "bin", executableName);
const standardPath = vscode.Uri.joinPath(vscode.Uri.file(os.homedir()), ".cargo", "bin", executableName);

if (isFile(standardPath)) return standardPath;
if (isFile(standardPath.path)) return standardPath.path;
Copy link
Member

Choose a reason for hiding this comment

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

We convert an Uri to a string here, then convert it back to an Uri. And I'm not sure what happens if the path doesn't exist -- is there a way to require("vscode") in the Developer Tools?

Copy link
Member

Choose a reason for hiding this comment

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

is there a way to require("vscode") in the Developer Tools?

I would expect not. The developer tools run inside the chromium based part of electron. The extensions run in the node.js based part.

Copy link
Member

Choose a reason for hiding this comment

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

@wxb1ank I haven't looked at the code outside of the diff, but do you think this can be simplified?

} catch (err) {
log.error("Failed to read the fs info", err);
}
Expand All @@ -181,12 +180,6 @@ function lookupInPath(exec: string): boolean {
return candidates.some(isFile);
}

function isFile(suspectPath: string): boolean {
// It is not mentionned in docs, but `statSync()` throws an error when
// the path doesn't exist
try {
return fs.statSync(suspectPath).isFile();
} catch {
return false;
}
async function isFile(path: string): Promise<boolean> {
return ((await vscode.workspace.fs.stat(vscode.Uri.file(path))).type & vscode.FileType.File) !== 0;
Copy link
Member

Choose a reason for hiding this comment

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

What happens here if the file doesn't exist?

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 I had to guess... it throws. (Note: that's for FileSystemProvider, not FileSystem, but I'm assuming the latter is a wrapper over the former?) This should definitely be wrapped in a try/catch block ASAP.

I'm not very proud of my implementation for this function, either; I wanted to maintain the original function signature (i.e. with a string path, not Uri) so that it could be reused in lookupInPath().

Looking at it now, I think it would be best to separate isFile() into two functions: one that takes a string path, and one that takes a Uri. The former should be a simple wrapper over the latter, translating the string into a Uri with Uri.file(path).

This PR has already been merged, so tomorrow (later today?) I can try and clean this up in a new one.

}