Skip to content

Commit 636e579

Browse files
Revert all the FS-related changes. (#8970)
(for #8890) The plan is to un-revert this stuff bit-by-bit. * Revert "Fixes to FileSystem code to use bitwise operations (#8966)" This reverts commit acb59d3. * Revert "Make sure IFileSystem is registered in DI during tests. (#8902)" This reverts commit dabd87f. * Revert "Remove IFileSystemUtils from IOC container (#8898)" This reverts commit 8ab93d7. * Revert "Skip a few more tests. (#8830)" This reverts commit d197557. * Revert "Skip problematic tests. (#8810)" This reverts commit c5f7766. * Revert "Use new VS Code filesystem API. (#8307)" This reverts commit 7cbd1c2. * Revert "Update the code base to use IFileSystem instead of fs and fs-extra. (#7915)" This reverts commit a23761d.
1 parent 7e70cb2 commit 636e579

File tree

54 files changed

+665
-3722
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

54 files changed

+665
-3722
lines changed

.gitignore

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,3 @@ tmp/**
3939
test-results.xml
4040
uitests/out/**
4141
!build/
42-
43-
# TODO (GH-8925) This is temporary. We will remove it when we adjust
44-
# tests to not leave the file behind.
45-
src/test/get-pip.py

news/2 Fixes/8890.md

Lines changed: 0 additions & 1 deletion
This file was deleted.

news/3 Code Health/6911.md

Lines changed: 0 additions & 1 deletion
This file was deleted.

src/client/common/editor.ts

Lines changed: 22 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
import { Diff, diff_match_patch } from 'diff-match-patch';
2+
import * as fs from 'fs-extra';
23
import { injectable } from 'inversify';
34
import * as md5 from 'md5';
45
import { EOL } from 'os';
56
import * as path from 'path';
67
import { Position, Range, TextDocument, TextEdit, Uri, WorkspaceEdit } from 'vscode';
7-
import { IFileSystem } from './platform/types';
88
import { IEditorUtils } from './types';
99

1010
// Code borrowed from goFormat.ts (Go Extension for VS Code)
@@ -80,11 +80,7 @@ export function getTextEditsFromPatch(before: string, patch: string): TextEdit[]
8080

8181
return textEdits;
8282
}
83-
export function getWorkspaceEditsFromPatch(
84-
filePatches: string[],
85-
fs: IFileSystem,
86-
workspaceRoot?: string
87-
): WorkspaceEdit {
83+
export function getWorkspaceEditsFromPatch(filePatches: string[], workspaceRoot?: string): WorkspaceEdit {
8884
const workspaceEdit = new WorkspaceEdit();
8985
filePatches.forEach(patch => {
9086
const indexOfAtAt = patch.indexOf('@@');
@@ -111,7 +107,7 @@ export function getWorkspaceEditsFromPatch(
111107

112108
let fileName = fileNameLines[0].substring(fileNameLines[0].indexOf(' a') + 3).trim();
113109
fileName = workspaceRoot && !path.isAbsolute(fileName) ? path.resolve(workspaceRoot, fileName) : fileName;
114-
if (!fs.fileExistsSync(fileName)) {
110+
if (!fs.existsSync(fileName)) {
115111
return;
116112
}
117113

@@ -127,7 +123,7 @@ export function getWorkspaceEditsFromPatch(
127123
throw new Error('Unable to parse Patch string');
128124
}
129125

130-
const fileSource = fs.readFileSync(fileName);
126+
const fileSource = fs.readFileSync(fileName).toString('utf8');
131127
const fileUri = Uri.file(fileName);
132128

133129
// Add line feeds and build the text edits
@@ -230,25 +226,24 @@ function getTextEditsInternal(before: string, diffs: [number, string][], startLi
230226
return edits;
231227
}
232228

233-
export async function getTempFileWithDocumentContents(
234-
document: TextDocument,
235-
fs: IFileSystem
236-
): Promise<string> {
237-
// Don't create file in temp folder since external utilities
238-
// look into configuration files in the workspace and are not able
239-
// to find custom rules if file is saved in a random disk location.
240-
// This means temp file has to be created in the same folder
241-
// as the original one and then removed.
242-
243-
const ext = path.extname(document.uri.fsPath);
244-
const filename = `${document.uri.fsPath}.${md5(document.uri.fsPath)}${ext}`;
245-
await (
246-
fs.writeFile(filename, document.getText())
247-
.catch(err => {
248-
throw Error(`Failed to create a temporary file, ${err.message}`);
249-
})
250-
);
251-
return filename;
229+
export function getTempFileWithDocumentContents(document: TextDocument): Promise<string> {
230+
return new Promise<string>((resolve, reject) => {
231+
const ext = path.extname(document.uri.fsPath);
232+
// Don't create file in temp folder since external utilities
233+
// look into configuration files in the workspace and are not able
234+
// to find custom rules if file is saved in a random disk location.
235+
// This means temp file has to be created in the same folder
236+
// as the original one and then removed.
237+
238+
// tslint:disable-next-line:no-require-imports
239+
const fileName = `${document.uri.fsPath}.${md5(document.uri.fsPath)}${ext}`;
240+
fs.writeFile(fileName, document.getText(), ex => {
241+
if (ex) {
242+
reject(`Failed to create a temporary file, ${ex.message}`);
243+
}
244+
resolve(fileName);
245+
});
246+
});
252247
}
253248

254249
/**

src/client/common/envFileParser.ts

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
import * as fs from 'fs-extra';
2+
import { IS_WINDOWS } from './platform/constants';
3+
import { PathUtils } from './platform/pathUtils';
4+
import { EnvironmentVariablesService } from './variables/environment';
5+
import { EnvironmentVariables } from './variables/types';
6+
function parseEnvironmentVariables(contents: string): EnvironmentVariables | undefined {
7+
if (typeof contents !== 'string' || contents.length === 0) {
8+
return undefined;
9+
}
10+
11+
const env: EnvironmentVariables = {};
12+
contents.split('\n').forEach(line => {
13+
const match = line.match(/^\s*([\w\.\-]+)\s*=\s*(.*)?\s*$/);
14+
if (match !== null) {
15+
let value = typeof match[2] === 'string' ? match[2] : '';
16+
if (value.length > 0 && value.charAt(0) === '"' && value.charAt(value.length - 1) === '"') {
17+
value = value.replace(/\\n/gm, '\n');
18+
}
19+
env[match[1]] = value.replace(/(^['"]|['"]$)/g, '');
20+
}
21+
});
22+
return env;
23+
}
24+
25+
export function parseEnvFile(envFile: string, mergeWithProcessEnvVars: boolean = true): EnvironmentVariables {
26+
const buffer = fs.readFileSync(envFile, 'utf8');
27+
const env = parseEnvironmentVariables(buffer)!;
28+
return mergeWithProcessEnvVars ? mergeEnvVariables(env, process.env) : mergePythonPath(env, process.env.PYTHONPATH as string);
29+
}
30+
31+
/**
32+
* Merge the target environment variables into the source.
33+
* Note: The source variables are modified and returned (i.e. it modifies value passed in).
34+
* @export
35+
* @param {EnvironmentVariables} targetEnvVars target environment variables.
36+
* @param {EnvironmentVariables} [sourceEnvVars=process.env] source environment variables (defaults to current process variables).
37+
* @returns {EnvironmentVariables}
38+
*/
39+
export function mergeEnvVariables(targetEnvVars: EnvironmentVariables, sourceEnvVars: EnvironmentVariables = process.env): EnvironmentVariables {
40+
const service = new EnvironmentVariablesService(new PathUtils(IS_WINDOWS));
41+
service.mergeVariables(sourceEnvVars, targetEnvVars);
42+
if (sourceEnvVars.PYTHONPATH) {
43+
service.appendPythonPath(targetEnvVars, sourceEnvVars.PYTHONPATH);
44+
}
45+
return targetEnvVars;
46+
}
47+
48+
/**
49+
* Merge the target PYTHONPATH value into the env variables passed.
50+
* Note: The env variables passed in are modified and returned (i.e. it modifies value passed in).
51+
* @export
52+
* @param {EnvironmentVariables} env target environment variables.
53+
* @param {string | undefined} [currentPythonPath] PYTHONPATH value.
54+
* @returns {EnvironmentVariables}
55+
*/
56+
export function mergePythonPath(env: EnvironmentVariables, currentPythonPath: string | undefined): EnvironmentVariables {
57+
if (typeof currentPythonPath !== 'string' || currentPythonPath.length === 0) {
58+
return env;
59+
}
60+
const service = new EnvironmentVariablesService(new PathUtils(IS_WINDOWS));
61+
service.appendPythonPath(env, currentPythonPath);
62+
return env;
63+
}

src/client/common/installer/moduleInstaller.ts

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License.
33

4+
import * as fs from 'fs';
45
import { injectable } from 'inversify';
56
import * as path from 'path';
67
import { OutputChannel, window } from 'vscode';
@@ -9,10 +10,9 @@ import { IServiceContainer } from '../../ioc/types';
910
import { sendTelemetryEvent } from '../../telemetry';
1011
import { EventName } from '../../telemetry/constants';
1112
import { STANDARD_OUTPUT_CHANNEL } from '../constants';
12-
import { IFileSystem } from '../platform/types';
1313
import { ITerminalServiceFactory } from '../terminal/types';
1414
import { ExecutionInfo, IConfigurationService, IOutputChannel } from '../types';
15-
import { isResource } from '../utils/misc';
15+
import { isResource, noop } from '../utils/misc';
1616
import { InterpreterUri } from './types';
1717

1818
@injectable()
@@ -38,10 +38,7 @@ export abstract class ModuleInstaller {
3838
if (!interpreter || interpreter.type !== InterpreterType.Unknown) {
3939
await terminalService.sendCommand(pythonPath, args);
4040
} else if (settings.globalModuleInstallation) {
41-
const dirname = path.dirname(pythonPath);
42-
const fs = this.serviceContainer.get<IFileSystem>(IFileSystem);
43-
const isWritable = ! await fs.isDirReadonly(dirname);
44-
if (isWritable) {
41+
if (await this.isPathWritableAsync(path.dirname(pythonPath))) {
4542
await terminalService.sendCommand(pythonPath, args);
4643
} else {
4744
this.elevatedInstall(pythonPath, args);
@@ -71,6 +68,19 @@ export abstract class ModuleInstaller {
7168
}
7269
return args;
7370
}
71+
private async isPathWritableAsync(directoryPath: string): Promise<boolean> {
72+
const filePath = `${directoryPath}${path.sep}___vscpTest___`;
73+
return new Promise<boolean>(resolve => {
74+
fs.open(filePath, fs.constants.O_CREAT | fs.constants.O_RDWR, (error, fd) => {
75+
if (!error) {
76+
fs.close(fd, () => {
77+
fs.unlink(filePath, noop);
78+
});
79+
}
80+
return resolve(!error);
81+
});
82+
});
83+
}
7484

7585
private elevatedInstall(execPath: string, args: string[]) {
7686
const options = {

src/client/common/net/fileDownloader.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,12 @@
33

44
'use strict';
55

6+
import { WriteStream } from 'fs';
67
import { inject, injectable } from 'inversify';
78
import * as requestTypes from 'request';
89
import { Progress, ProgressLocation } from 'vscode';
910
import { IApplicationShell } from '../application/types';
10-
import { IFileSystem, WriteStream } from '../platform/types';
11+
import { IFileSystem } from '../platform/types';
1112
import { DownloadOptions, IFileDownloader, IHttpClient } from '../types';
1213
import { Http } from '../utils/localize';
1314
import { noop } from '../utils/misc';

0 commit comments

Comments
 (0)