Skip to content

Commit 88a0494

Browse files
authored
Merge pull request #1766 from Microsoft/master
Merge for 0.16.1 release
2 parents 3bbfed6 + b32debd commit 88a0494

File tree

5 files changed

+137
-78
lines changed

5 files changed

+137
-78
lines changed

Extension/CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
# C/C++ for Visual Studio Code Change Log
22

3+
## Version 0.16.1: March 30, 2018
4+
* Fix random deadlock caused by logging code on Linux/Mac. [#1759](https://github.com/Microsoft/vscode-cpptools/issues/1759)
5+
* Fix compiler from `compileCommands` not being queried for includes/defines if `compilerPath` isn't set on Windows. [#1754](https://github.com/Microsoft/vscode-cpptools/issues/1754)
6+
* Fix OSX `UseShellExecute` I/O bug. [#1756](https://github.com/Microsoft/vscode-cpptools/issues/1756)
7+
* Invalidate partially unzipped files from package manager. [#1757](https://github.com/Microsoft/vscode-cpptools/issues/1757)
8+
39
## Version 0.16.0: March 28, 2018
410
* Enable autocomplete for local and global scopes. [#13](https://github.com/Microsoft/vscode-cpptools/issues/13)
511
* Add a setting to define multiline comment patterns: `C_Cpp.commentContinuationPatterns`. [#1100](https://github.com/Microsoft/vscode-cpptools/issues/1100), [#1539](https://github.com/Microsoft/vscode-cpptools/issues/1539)

Extension/package-lock.json

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Extension/package.json

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
"name": "cpptools",
33
"displayName": "C/C++",
44
"description": "C/C++ IntelliSense, debugging, and code browsing.",
5-
"version": "0.16.0",
5+
"version": "0.16.1",
66
"publisher": "ms-vscode",
77
"preview": true,
88
"icon": "LanguageCCPP_color_128x.png",
@@ -1145,7 +1145,7 @@
11451145
"runtimeDependencies": [
11461146
{
11471147
"description": "C/C++ language components (Linux / x86_64)",
1148-
"url": "https://go.microsoft.com/fwlink/?linkid=867977",
1148+
"url": "https://go.microsoft.com/fwlink/?linkid=871264",
11491149
"platforms": [
11501150
"linux"
11511151
],
@@ -1159,7 +1159,7 @@
11591159
},
11601160
{
11611161
"description": "C/C++ language components (Linux / x86)",
1162-
"url": "https://go.microsoft.com/fwlink/?linkid=867978",
1162+
"url": "https://go.microsoft.com/fwlink/?linkid=871265",
11631163
"platforms": [
11641164
"linux"
11651165
],
@@ -1175,7 +1175,7 @@
11751175
},
11761176
{
11771177
"description": "C/C++ language components (OS X)",
1178-
"url": "https://go.microsoft.com/fwlink/?linkid=867979",
1178+
"url": "https://go.microsoft.com/fwlink/?linkid=871266",
11791179
"platforms": [
11801180
"darwin"
11811181
],
@@ -1186,7 +1186,7 @@
11861186
},
11871187
{
11881188
"description": "C/C++ language components (Windows)",
1189-
"url": "https://go.microsoft.com/fwlink/?linkid=867980",
1189+
"url": "https://go.microsoft.com/fwlink/?linkid=871267",
11901190
"platforms": [
11911191
"win32"
11921192
],

Extension/src/common.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -408,4 +408,26 @@ export function checkDistro(platformInfo: PlatformInformation): void {
408408
// or SunOS (the other platforms supported by node)
409409
getOutputChannelLogger().appendLine(`Warning: Debugging has not been tested for this platform. ${getReadmeMessage()}`);
410410
}
411+
}
412+
413+
export async function unlinkPromise(fileName: string): Promise<void> {
414+
return new Promise<void>((resolve, reject) => {
415+
fs.unlink(fileName, err => {
416+
if (err) {
417+
return reject(err);
418+
}
419+
return resolve();
420+
});
421+
});
422+
}
423+
424+
export async function renamePromise(oldName: string, newName: string): Promise<void> {
425+
return new Promise<void>((resolve, reject) => {
426+
fs.rename(oldName, newName, err => {
427+
if (err) {
428+
return reject(err);
429+
}
430+
return resolve();
431+
});
432+
});
411433
}

Extension/src/packageManager.ts

Lines changed: 103 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -137,69 +137,74 @@ export class PackageManager {
137137
});
138138
}
139139

140-
private DownloadPackage(pkg: IPackage): Promise<void> {
140+
private async DownloadPackage(pkg: IPackage): Promise<void> {
141141
this.AppendChannel(`Downloading package '${pkg.description}' `);
142142

143143
this.SetStatusText("$(cloud-download) Downloading packages...");
144144
this.SetStatusTooltip(`Downloading package '${pkg.description}'...`);
145145

146+
const tmpResult: tmp.SyncResult = await this.CreateTempFile(pkg);
147+
await this.DownloadPackageWithRetries(pkg, tmpResult);
148+
}
149+
150+
private async CreateTempFile(pkg: IPackage): Promise<tmp.SyncResult> {
146151
return new Promise<tmp.SyncResult>((resolve, reject) => {
147152
tmp.file({ prefix: "package-" }, (err, path, fd, cleanupCallback) => {
148153
if (err) {
149154
return reject(new PackageManagerError('Error from temp.file', 'DownloadPackage', pkg, err));
150155
}
151156

152-
resolve(<tmp.SyncResult>{ name: path, fd: fd, removeCallback: cleanupCallback });
157+
return resolve(<tmp.SyncResult>{ name: path, fd: fd, removeCallback: cleanupCallback });
153158
});
154-
})
155-
.then((tmpResult) => {
156-
pkg.tmpFile = tmpResult;
157-
158-
let lastError: any = null;
159-
let retryCount: number = 0;
160-
let handleDownloadFailure: (num: any, error: any) => void = (num, error) => {
161-
retryCount = num;
162-
lastError = error;
159+
});
160+
}
161+
162+
private async DownloadPackageWithRetries(pkg: IPackage, tmpResult: tmp.SyncResult): Promise<void> {
163+
pkg.tmpFile = tmpResult;
164+
165+
let success: boolean = false;
166+
let lastError: any = null;
167+
let retryCount: number = 0;
168+
const MAX_RETRIES: number = 5;
169+
170+
// Retry the download at most MAX_RETRIES times with 2-32 seconds delay.
171+
do {
172+
try {
173+
await this.DownloadFile(pkg.url, pkg, retryCount);
174+
success = true;
175+
} catch (error) {
176+
retryCount += 1;
177+
lastError = error;
178+
if (retryCount >= MAX_RETRIES) {
179+
this.AppendChannel(` Failed to download ` + pkg.url);
180+
throw error;
181+
} else {
182+
// This will skip the success = true.
163183
this.AppendChannel(` Failed. Retrying...`);
164-
};
165-
// Retry the download at most 5 times with 2-32 seconds delay.
166-
return this.DownloadFile(pkg.url, pkg, 0).catch((error) => {
167-
handleDownloadFailure(1, error);
168-
return this.DownloadFile(pkg.url, pkg, 1).catch((error) => {
169-
handleDownloadFailure(2, error);
170-
return this.DownloadFile(pkg.url, pkg, 2).catch((error) => {
171-
handleDownloadFailure(3, error);
172-
return this.DownloadFile(pkg.url, pkg, 3).catch((error) => {
173-
handleDownloadFailure(4, error);
174-
return this.DownloadFile(pkg.url, pkg, 4).catch((error) => {
175-
handleDownloadFailure(5, error);
176-
return this.DownloadFile(pkg.url, pkg, 5); // Last try, don't catch the error.
177-
});
178-
});
179-
});
180-
});
181-
}).then(() => {
182-
this.AppendLineChannel(" Done!");
183-
if (retryCount !== 0) {
184-
// Log telemetry to see if retrying helps.
185-
let telemetryProperties: { [key: string]: string } = {};
186-
telemetryProperties["success"] = `OnRetry${retryCount}`;
187-
if (lastError instanceof PackageManagerError) {
188-
let packageError: PackageManagerError = lastError;
189-
telemetryProperties['error.methodName'] = packageError.methodName;
190-
telemetryProperties['error.message'] = packageError.message;
191-
if (packageError.pkg) {
192-
telemetryProperties['error.packageName'] = packageError.pkg.description;
193-
telemetryProperties['error.packageUrl'] = packageError.pkg.url;
194-
}
195-
if (packageError.errorCode) {
196-
telemetryProperties['error.errorCode'] = packageError.errorCode;
197-
}
198-
}
199-
Telemetry.logDebuggerEvent("acquisition", telemetryProperties);
200-
}
201-
});
202-
});
184+
continue;
185+
}
186+
}
187+
} while (!success && retryCount < MAX_RETRIES);
188+
189+
this.AppendLineChannel(" Done!");
190+
if (retryCount !== 0) {
191+
// Log telemetry to see if retrying helps.
192+
let telemetryProperties: { [key: string]: string } = {};
193+
telemetryProperties["success"] = `OnRetry${retryCount}`;
194+
if (lastError instanceof PackageManagerError) {
195+
let packageError: PackageManagerError = lastError;
196+
telemetryProperties['error.methodName'] = packageError.methodName;
197+
telemetryProperties['error.message'] = packageError.message;
198+
if (packageError.pkg) {
199+
telemetryProperties['error.packageName'] = packageError.pkg.description;
200+
telemetryProperties['error.packageUrl'] = packageError.pkg.url;
201+
}
202+
if (packageError.errorCode) {
203+
telemetryProperties['error.errorCode'] = packageError.errorCode;
204+
}
205+
}
206+
Telemetry.logDebuggerEvent("acquisition", telemetryProperties);
207+
}
203208
}
204209

205210
// reloadCpptoolsJson in main.ts uses ~25% of this function.
@@ -276,11 +281,11 @@ export class PackageManager {
276281
});
277282

278283
response.on('end', () => {
279-
resolve();
284+
return resolve();
280285
});
281286

282287
response.on('error', (error) => {
283-
reject(new PackageManagerWebResponseError(response.socket, 'HTTP/HTTPS Response error', 'DownloadFile', pkg, error.stack, error.name));
288+
return reject(new PackageManagerWebResponseError(response.socket, 'HTTP/HTTPS Response error', 'DownloadFile', pkg, error.stack, error.name));
284289
});
285290

286291
// Begin piping data from the response to the package file
@@ -291,7 +296,7 @@ export class PackageManager {
291296
let request: ClientRequest = https.request(options, handleHttpResponse);
292297

293298
request.on('error', (error) => {
294-
reject(new PackageManagerError('HTTP/HTTPS Request error' + (urlString.includes("fwlink") ? ": fwlink" : ""), 'DownloadFile', pkg, error.stack, error.message));
299+
return reject(new PackageManagerError('HTTP/HTTPS Request error' + (urlString.includes("fwlink") ? ": fwlink" : ""), 'DownloadFile', pkg, error.stack, error.message));
295300
});
296301

297302
// Execute the request
@@ -316,6 +321,15 @@ export class PackageManager {
316321
return reject(new PackageManagerError('Zip file error', 'InstallPackage', pkg, err));
317322
}
318323

324+
// setup zip file events
325+
zipfile.on('end', () => {
326+
return resolve();
327+
});
328+
329+
zipfile.on('error', err => {
330+
return reject(new PackageManagerError('Zip File Error', 'InstallPackage', pkg, err, err.code));
331+
});
332+
319333
zipfile.readEntry();
320334

321335
zipfile.on('entry', (entry: yauzl.Entry) => {
@@ -334,26 +348,52 @@ export class PackageManager {
334348
util.checkFileExists(absoluteEntryPath).then((exists: boolean) => {
335349
if (!exists) {
336350
// File - extract it
337-
zipfile.openReadStream(entry, (err, readStream) => {
351+
zipfile.openReadStream(entry, (err, readStream: fs.ReadStream) => {
338352
if (err) {
339353
return reject(new PackageManagerError('Error reading zip stream', 'InstallPackage', pkg, err));
340354
}
341355

342-
mkdirp.mkdirp(path.dirname(absoluteEntryPath), { mode: 0o775 }, (err) => {
356+
readStream.on('error', (err) => {
357+
return reject(new PackageManagerError('Error in readStream', 'InstallPackage', pkg, err));
358+
});
359+
360+
mkdirp.mkdirp(path.dirname(absoluteEntryPath), { mode: 0o775 }, async (err) => {
343361
if (err) {
344362
return reject(new PackageManagerError('Error creating directory', 'InstallPackage', pkg, err, err.code));
345363
}
346364

365+
// Create as a .tmp file to avoid partially unzipped files
366+
// counting as completed files.
367+
let absoluteEntryTempFile: string = absoluteEntryPath + ".tmp";
368+
if (fs.existsSync(absoluteEntryTempFile)) {
369+
try {
370+
await util.unlinkPromise(absoluteEntryTempFile);
371+
} catch (err) {
372+
return reject(new PackageManagerError(`Error unlinking file ${absoluteEntryTempFile}`, 'InstallPackage', pkg, err));
373+
}
374+
}
375+
347376
// Make sure executable files have correct permissions when extracted
348377
let fileMode: number = (pkg.binaries && pkg.binaries.indexOf(absoluteEntryPath) !== -1) ? 0o755 : 0o664;
349-
350-
let writeStream: fs.WriteStream = fs.createWriteStream(absoluteEntryPath, { mode: fileMode });
351-
readStream.pipe(writeStream);
352-
writeStream.on('close', () => {
378+
let writeStream: fs.WriteStream = fs.createWriteStream(absoluteEntryTempFile, { mode: fileMode });
379+
380+
writeStream.on('close', async () => {
381+
try {
382+
// Remove .tmp extension from the file.
383+
await util.renamePromise(absoluteEntryTempFile, absoluteEntryPath);
384+
} catch (err) {
385+
return reject(new PackageManagerError(`Error renaming file ${absoluteEntryTempFile}`, 'InstallPackage', pkg, err));
386+
}
353387
// Wait till output is done writing before reading the next zip entry.
354388
// Otherwise, it's possible to try to launch the .exe before it is done being created.
355389
zipfile.readEntry();
356390
});
391+
392+
writeStream.on('error', (err) => {
393+
return reject(new PackageManagerError('Error in writeStream', 'InstallPackage', pkg, err));
394+
});
395+
396+
readStream.pipe(writeStream);
357397
});
358398
});
359399
} else {
@@ -366,20 +406,11 @@ export class PackageManager {
366406
});
367407
}
368408
});
369-
370-
zipfile.on('end', () => {
371-
resolve();
372-
});
373-
374-
zipfile.on('error', err => {
375-
reject(new PackageManagerError('Zip File Error', 'InstallPackage', pkg, err, err.code));
376-
});
377-
});
378-
})
379-
.then(() => {
380-
// Clean up temp file
381-
pkg.tmpFile.removeCallback();
382409
});
410+
}).then(() => {
411+
// Clean up temp file
412+
pkg.tmpFile.removeCallback();
413+
});
383414
}
384415

385416
private AppendChannel(text: string): void {

0 commit comments

Comments
 (0)