Skip to content

Commit 44339f7

Browse files
nagilsonnagilson
committed
Don't write to script file if its not fresh content, hash file paths in logs
1 parent 4d31fd2 commit 44339f7

File tree

4 files changed

+68
-20
lines changed

4 files changed

+68
-20
lines changed

‎vscode-dotnet-runtime-library/src/Acquisition/AcquisitionInvoker.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import { IAcquisitionInvoker } from './IAcquisitionInvoker';
2121
import { IDotnetInstallationContext } from './IDotnetInstallationContext';
2222
import { IInstallScriptAcquisitionWorker } from './IInstallScriptAcquisitionWorker';
2323
import { InstallScriptAcquisitionWorker } from './InstallScriptAcquisitionWorker';
24+
import { TelemetryUtilities } from '../EventStream/TelemetryUtilities';
2425

2526
export class AcquisitionInvoker extends IAcquisitionInvoker {
2627
private readonly scriptWorker: IInstallScriptAcquisitionWorker;
@@ -49,10 +50,10 @@ You will need to restart VS Code after these changes. If PowerShell is still not
4950
async (error, stdout, stderr) => {
5051
if (error) {
5152
if (stdout) {
52-
this.eventStream.post(new DotnetAcquisitionScriptOuput(installContext.version, stdout));
53+
this.eventStream.post(new DotnetAcquisitionScriptOuput(installContext.version, TelemetryUtilities.HashAllPaths(stdout)));
5354
}
5455
if (stderr) {
55-
this.eventStream.post(new DotnetAcquisitionScriptOuput(installContext.version, `STDERR: ${stderr}`));
56+
this.eventStream.post(new DotnetAcquisitionScriptOuput(installContext.version, `STDERR: ${TelemetryUtilities.HashAllPaths(stderr)}`));
5657
}
5758

5859
const online = await isOnline();
@@ -69,7 +70,7 @@ You will need to restart VS Code after these changes. If PowerShell is still not
6970
reject(error);
7071
}
7172
} else if (stderr && stderr.length > 0) {
72-
this.eventStream.post(new DotnetAcquisitionScriptError(new Error(stderr), installContext.version));
73+
this.eventStream.post(new DotnetAcquisitionScriptError(new Error(TelemetryUtilities.HashAllPaths(stderr)), installContext.version));
7374
reject(stderr);
7475
} else {
7576
this.eventStream.post(new DotnetAcquisitionCompleted(installContext.version, installContext.dotnetPath));

‎vscode-dotnet-runtime-library/src/Acquisition/InstallScriptAcquisitionWorker.ts

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ export class InstallScriptAcquisitionWorker implements IInstallScriptAcquisition
7272
// Protected for testing purposes
7373
protected async writeScriptAsFile(scriptContent: string, filePath: string)
7474
{
75-
this.eventStream.post(new DotnetFileWriteRequestEvent(`Request to write: ${filePath}`, new Date().toISOString()));
75+
this.eventStream.post(new DotnetFileWriteRequestEvent(`Request to write`, new Date().toISOString(), filePath));
7676

7777
if (!fs.existsSync(path.dirname(filePath))) {
7878
fs.mkdirSync(path.dirname(filePath), { recursive: true });
@@ -88,14 +88,15 @@ export class InstallScriptAcquisitionWorker implements IInstallScriptAcquisition
8888
if(!fs.existsSync(filePath))
8989
{
9090
// Create an empty file, as proper-lockfile fails to lock a file if file dne
91+
this.eventStream.post(new DotnetFileWriteRequestEvent(`File did not exist upon write request.`, new Date().toISOString(), filePath));
9192
fs.writeFileSync(filePath, '');
9293
}
9394

94-
this.eventStream.post(new DotnetLockAttemptingAcquireEvent(`Lock Acqusition request to begin: ${directoryLockPath} for ${filePath}`, new Date().toISOString()));
95+
this.eventStream.post(new DotnetLockAttemptingAcquireEvent(`Lock Acqusition request to begin.`, new Date().toISOString(), directoryLockPath, filePath));
9596
await lockfile.lock(filePath, { lockfilePath: directoryLockPath, retries: { retries: 10, maxTimeout: 1000 } } )
9697
.then(async (release) =>
9798
{
98-
this.eventStream.post(new DotnetLockAcquiredEvent(`Lock Acquired: ${directoryLockPath} for ${filePath}`, new Date().toISOString()));
99+
this.eventStream.post(new DotnetLockAcquiredEvent(`Lock Acquired.`, new Date().toISOString(), directoryLockPath, filePath));
99100

100101
// We would like to unlock the directory, but we can't grab a lock on the file if the directory is locked.
101102
// Theoretically you could: add a new filewriter lock as a 3rd party lock ...
@@ -104,19 +105,27 @@ export class InstallScriptAcquisitionWorker implements IInstallScriptAcquisition
104105
// For now, keep the entire directory locked.
105106

106107
scriptContent = eol.auto(scriptContent);
107-
108+
const existingScriptContent = fs.readFileSync(filePath).toString();
108109
// fs.writeFile will replace the file if it exists.
109110
// https://nodejs.org/api/fs.html#fswritefilefile-data-options-callback
110-
fs.writeFileSync(filePath, scriptContent);
111-
fs.chmodSync(filePath, 0o744);
111+
if(scriptContent != existingScriptContent)
112+
{
113+
fs.writeFileSync(filePath, scriptContent);
114+
fs.chmodSync(filePath, 0o744);
115+
this.eventStream.post(new DotnetFileWriteRequestEvent(`File content needed to be updated.`, new Date().toISOString(), filePath));
116+
}
117+
else
118+
{
119+
this.eventStream.post(new DotnetFileWriteRequestEvent(`File content is an exact match, not writing file.`, new Date().toISOString(), filePath));
120+
}
112121

113-
this.eventStream.post(new DotnetLockReleasedEvent(`Lock about to be released: ${directoryLockPath} for ${filePath}`, new Date().toISOString()));
122+
this.eventStream.post(new DotnetLockReleasedEvent(`Lock about to be released.`, new Date().toISOString(), directoryLockPath, filePath));
114123
return release();
115124
})
116-
.catch((e) =>
125+
.catch((e : Error) =>
117126
{
118127
// Either the lock could not be acquired or releasing it failed
119-
this.eventStream.post(new DotnetLockErrorEvent(e));
128+
this.eventStream.post(new DotnetLockErrorEvent(e, e.message, new Date().toISOString(), directoryLockPath, filePath));
120129
});
121130
// End Critical Section
122131
}

‎vscode-dotnet-runtime-library/src/EventStream/EventStreamEvents.ts

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import * as path from 'path';
77
import { IDotnetInstallationContext } from '../Acquisition/IDotnetInstallationContext';
88
import { EventType } from './EventType';
99
import { IEvent } from './IEvent';
10-
import { TelemetryUtilities } from './FileUtilities';
10+
import { TelemetryUtilities } from './TelemetryUtilities';
1111

1212
// tslint:disable max-classes-per-file
1313

@@ -271,26 +271,42 @@ export class DotnetFallbackInstallScriptUsed extends DotnetAcquisitionMessage {
271271

272272
export abstract class DotnetFileEvent extends DotnetAcquisitionMessage
273273
{
274-
constructor(public readonly eventMessage: string, public readonly time: string) { super(); }
274+
constructor(public readonly eventMessage: string, public readonly time: string, public readonly file: string) { super(); }
275275

276276
public getProperties() {
277-
return {Message: this.eventMessage, Time: this.time};
277+
return {Message: this.eventMessage, Time: this.time, File: TelemetryUtilities.HashData(this.file)};
278278
}
279279
}
280280

281-
export class DotnetLockAcquiredEvent extends DotnetFileEvent {
281+
export abstract class DotnetLockEvent extends DotnetFileEvent
282+
{
283+
constructor(public readonly eventMessage: string, public readonly time: string, public readonly lock: string, public readonly file: string) { super(eventMessage, time, file); }
284+
285+
public getProperties() {
286+
return {Message: this.eventMessage, Time: this.time, Lock: TelemetryUtilities.HashData(this.lock), File: TelemetryUtilities.HashData(this.file)};
287+
}
288+
}
289+
290+
export class DotnetLockAcquiredEvent extends DotnetLockEvent {
282291
public readonly eventName = 'DotnetLockAcquiredEvent';
283292
}
284293

285-
export class DotnetLockReleasedEvent extends DotnetFileEvent {
294+
export class DotnetLockReleasedEvent extends DotnetLockEvent {
286295
public readonly eventName = 'DotnetLockReleasedEvent';
287296
}
288297

289-
export class DotnetLockErrorEvent extends DotnetAcquisitionError {
298+
export class DotnetLockErrorEvent extends DotnetLockEvent {
290299
public readonly eventName = 'DotnetLockErrorEvent';
300+
constructor(public readonly error : Error,
301+
public readonly eventMessage: string, public readonly time: string, public readonly lock: string, public readonly file: string) { super(eventMessage, time, lock, file); }
302+
303+
public getProperties() {
304+
return {Error: this.error.toString(), Message: this.eventMessage, Time: this.time, Lock: TelemetryUtilities.HashData(this.lock), File: TelemetryUtilities.HashData(this.file)};
305+
}
306+
291307
}
292308

293-
export class DotnetLockAttemptingAcquireEvent extends DotnetFileEvent {
309+
export class DotnetLockAttemptingAcquireEvent extends DotnetLockEvent {
294310
public readonly eventName = 'DotnetLockAttemptingAcquireEvent';
295311
}
296312

‎vscode-dotnet-runtime-library/src/EventStream/TelemetryUtilities.ts

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22
* Copyright (c) Microsoft Corporation. All rights reserved.
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
* ------------------------------------------------------------------------------------------ */
5-
import * as crypto from 'crypto';
5+
import * as crypto from 'crypto';
6+
import * as fs from 'fs';
7+
import * as path from 'path';
68
import { TextEncoder } from 'util';
79

810
export class TelemetryUtilities
@@ -14,4 +16,24 @@ export class TelemetryUtilities
1416
const hashedData = hasher.update(utf8Bytes).digest('hex').toLowerCase();
1517
return hashedData;
1618
}
19+
20+
/**
21+
*
22+
* @param stringWithPaths The string that may contain paths to hash.
23+
* @returns The same string but with all paths in it hashed.
24+
* @remarks Will not hash a filename as it is a leaf file system object. It needs to be a path with at least one directory.
25+
* That's what we'd like to hash. (E.g. dotnet-install.ps1 is not needed to hash.)
26+
*/
27+
public static HashAllPaths(stringWithPaths : string) : string
28+
{
29+
let hashedPathsString = ``;
30+
stringWithPaths.split(' ').forEach(word =>
31+
{
32+
const convertedLine = word !== path.basename(word) && fs.existsSync(word) && (fs.lstatSync(word).isFile() || fs.lstatSync(word).isDirectory())
33+
? TelemetryUtilities.HashData(word) : word;
34+
hashedPathsString = `${hashedPathsString} ${convertedLine}`;
35+
return hashedPathsString;
36+
});
37+
return hashedPathsString;
38+
}
1739
}

0 commit comments

Comments
 (0)