Merge pull request #1367 from joaomoreno/fix-1111

Fix: Upgrading Extensions Blows Up

src/vs/base/common/arrays.ts

@@ -220,3 +220,7 @@ export function commonPrefixLength<T>(one: T[], other: T[], equals: (a: T, b: T)
 
 	return result;
 }
+
+export function flatten<T>(arr: T[][]): T[] {
+	return arr.reduce((r, v) => r.concat(v), []);
+}
\ No newline at end of file

src/vs/platform/plugins/common/plugins.ts

@@ -11,6 +11,7 @@ import Severity from 'vs/base/common/severity';
 export interface IPluginDescription {
 	id: string;
 	name: string;
+	version: string;
 	publisher: string;
 	isBuiltin: boolean;
 	extensionFolderPath: string;

src/vs/platform/plugins/common/pluginsRegistry.ts

@@ -150,6 +150,10 @@ export function isValidPluginDescription(extensionFolderPath: string, pluginDesc
 		notices.push(nls.localize('pluginDescription.name', "property `{0}` is mandatory and must be of type `string`", 'name'));
 		return false;
 	}
+	if (typeof pluginDescription.version !== 'string') {
+		notices.push(nls.localize('pluginDescription.version', "property `{0}` is mandatory and must be of type `string`", 'version'));
+		return false;
+	}
 	if (!pluginDescription.engines) {
 		notices.push(nls.localize('pluginDescription.engines', "property `{0}` is mandatory and must be of type `object`", 'engines'));
 		return false;

src/vs/platform/plugins/node/pluginVersionValidator.ts

@@ -7,8 +7,7 @@
 import nls = require('vs/nls');
 import {IPluginDescription, IPointListener, IActivationEventListener, IMessage} from 'vs/platform/plugins/common/plugins';
 import {isValidPluginDescription as baseIsValidPluginDescription} from 'vs/platform/plugins/common/pluginsRegistry';
-
-import {satisfies} from 'semver';
+import * as semver from 'semver';
 
 export interface IParsedVersion {
 	hasCaret: boolean;
@@ -210,5 +209,10 @@ export function isValidPluginDescription(version: string, extensionFolderPath: s
 		return false;
 	}
 
+	if (!semver.valid(pluginDescription.version)) {
+		notices.push(nls.localize('notSemver', "Extension version is not semver compatible."));
+		return false;
+	}
+
 	return isValidExtensionVersion(version, pluginDescription, notices);
 }
\ No newline at end of file

src/vs/workbench/electron-main/sharedProcessMain.ts

@@ -61,7 +61,11 @@ function main(server: Server, initData: IInitData): void {
 	instantiationService.addSingleton(IRequestService, requestService);
 
 	instantiationService.addSingleton(IExtensionsService, new SyncDescriptor(ExtensionsService));
-	server.registerService('ExtensionService', instantiationService.getInstance(IExtensionsService));
+	const extensionService = <ExtensionsService> instantiationService.getInstance(IExtensionsService);
+	server.registerService('ExtensionService', extensionService);
+
+	// eventually clean up old extensions
+	setTimeout(() => extensionService.removeDeprecatedExtensions(), 5000);
 }
 
 function setupIPC(hook: string): TPromise<Server> {

src/vs/workbench/node/extensionPoints.ts

@@ -5,19 +5,15 @@
 
 'use strict';
 
-import nls = require('vs/nls');
-
-import fs = require('fs');
 import pfs = require('vs/base/node/pfs');
-
 import {IPluginDescription} from 'vs/platform/plugins/common/plugins';
 import {TPromise} from 'vs/base/common/winjs.base';
+import {groupBy, values} from 'vs/base/common/collections';
 import paths = require('vs/base/common/paths');
 import json = require('vs/base/common/json');
-import strings = require('vs/base/common/strings');
-import {ILanguageExtensionPoint} from 'vs/editor/common/modes/languageExtensionPoint';
-import {PluginsRegistry, IPluginsMessageCollector} from 'vs/platform/plugins/common/pluginsRegistry';
+import {IPluginsMessageCollector} from 'vs/platform/plugins/common/pluginsRegistry';
 import {isValidPluginDescription} from 'vs/platform/plugins/node/pluginVersionValidator';
+import * as semver from 'semver';
 
 const MANIFEST_FILE = 'package.json';
 
@@ -26,7 +22,13 @@ export class PluginScanner {
 	/**
 	 * Scan the plugin defined in `absoluteFolderPath`
 	 */
-	public static scanPlugin(version: string, collector: IPluginsMessageCollector, absoluteFolderPath:string, isBuiltin:boolean): TPromise<IPluginDescription> {
+	public static scanPlugin(
+		version: string,
+		collector: IPluginsMessageCollector,
+		absoluteFolderPath:string,
+		isBuiltin:boolean
+	) : TPromise<IPluginDescription>
+	{
 		absoluteFolderPath = paths.normalize(absoluteFolderPath);
 		let builder = collector.scopeTo(absoluteFolderPath);
 		let absoluteManifestPath = paths.join(absoluteFolderPath, MANIFEST_FILE);
@@ -81,22 +83,37 @@ export class PluginScanner {
 	/**
 	 * Scan a list of extensions defined in `absoluteFolderPath`
 	 */
-	public static scanPlugins(version: string, collector: IPluginsMessageCollector, absoluteFolderPath:string, isBuiltin:boolean): TPromise<IPluginDescription[]> {
-		return pfs.readDirsInDir(absoluteFolderPath).then((folders) => {
-			return TPromise.join(
-				folders.map((folder) => this.scanPlugin(version, collector, paths.join(absoluteFolderPath, folder), isBuiltin))
-			);
-		}, (err) => {
-			collector.error(absoluteFolderPath, err);
-			return [];
-		}).then((results) => results.filter(item => (item !== null)));
+	public static scanPlugins(
+		version: string,
+		collector: IPluginsMessageCollector,
+		absoluteFolderPath:string,
+		isBuiltin:boolean
+	) : TPromise<IPluginDescription[]>
+	{
+		return pfs.readDirsInDir(absoluteFolderPath)
+			.then(folders => TPromise.join(folders.map(f => this.scanPlugin(version, collector, paths.join(absoluteFolderPath, f), isBuiltin))))
+			.then(plugins => plugins.filter(item => item !== null))
+			.then(plugins => {
+				const pluginsById = values(groupBy(plugins, p => p.id));
+				return pluginsById.map(p => p.sort((a, b) => semver.rcompare(a.version, b.version))[0]);
+			})
+			.then(null, err => {
+				collector.error(absoluteFolderPath, err);
+				return [];
+			});
 	}
 
 	/**
 	 * Combination of scanPlugin and scanPlugins: If a plugin manifest is found at root, we load just this plugin, otherwise we assume
 	 * the folder contains multiple extensions.
 	 */
-	public static scanOneOrMultiplePlugins(version: string, collector: IPluginsMessageCollector, absoluteFolderPath:string, isBuiltin:boolean): TPromise<IPluginDescription[]> {
+	public static scanOneOrMultiplePlugins(
+		version: string,
+		collector: IPluginsMessageCollector,
+		absoluteFolderPath:string,
+		isBuiltin:boolean
+	) : TPromise<IPluginDescription[]>
+	{
 		return pfs.fileExists(paths.join(absoluteFolderPath, MANIFEST_FILE)).then((exists) => {
 			if (exists) {
 				return this.scanPlugin(version, collector, absoluteFolderPath, isBuiltin).then((pluginDescription) => {

src/vs/workbench/node/pluginHostMain.ts

@@ -162,10 +162,9 @@ export class PluginHostMain {
 	}
 
 	private static scanPlugins(collector: IPluginsMessageCollector, builtinPluginsPath: string, userInstallPath: string, pluginDevelopmentPath: string, version: string): TPromise<IPluginDescription[]> {
-
-		let builtinPlugins: TPromise<IPluginDescription[]> = PluginScanner.scanPlugins(version, collector, builtinPluginsPath, true);
-		let userPlugins: TPromise<IPluginDescription[]> = (userInstallPath ? PluginScanner.scanPlugins(version, collector, userInstallPath, false) : TPromise.as([]));
-		let developedPlugins: TPromise<IPluginDescription[]> = (pluginDevelopmentPath ? PluginScanner.scanOneOrMultiplePlugins(version, collector, pluginDevelopmentPath, false) : TPromise.as([]));
+		const builtinPlugins = PluginScanner.scanPlugins(version, collector, builtinPluginsPath, true);
+		const userPlugins = !userInstallPath ? TPromise.as([]) : PluginScanner.scanPlugins(version, collector, userInstallPath, false);
+		const developedPlugins = !pluginDevelopmentPath ? TPromise.as([]) : PluginScanner.scanOneOrMultiplePlugins(version, collector, pluginDevelopmentPath, false);
 
 		return TPromise.join([builtinPlugins, userPlugins, developedPlugins]).then((_: IPluginDescription[][]) => {
 			let builtinPlugins = _[0];

src/vs/workbench/parts/extensions/common/extensions.ts

@@ -50,5 +50,5 @@ export interface IExtensionsService {
 	install(extension: IExtension): TPromise<IExtension>;
 	install(zipPath: string): TPromise<IExtension>;
 	uninstall(extension: IExtension): TPromise<void>;
-	getInstalled(): TPromise<IExtension[]>;
+	getInstalled(includeDuplicateVersions?: boolean): TPromise<IExtension[]>;
 }

src/vs/workbench/parts/extensions/electron-browser/extensionsQuickOpen.ts

@@ -244,10 +244,10 @@ class DataSource implements IDataSource<IExtensionEntry> {
 		const extension = entry.extension;
 
 		if (extension.galleryInformation) {
-			return extension.galleryInformation.id;
+			return `${ extension.galleryInformation.id }-${ extension.version }`;
 		}
 
-		return `local@${ extension.publisher }.${extension.name}@${ extension.path || '' }`;
+		return `local@${ extension.publisher }.${ extension.name }-${ extension.version }@${ extension.path || '' }`;
 	}
 
 	getLabel(entry: IExtensionEntry): string {

src/vs/workbench/parts/extensions/node/extensionsService.ts

@@ -13,6 +13,7 @@ import { ServiceEvent } from 'vs/base/common/service';
 import errors = require('vs/base/common/errors');
 import * as pfs from 'vs/base/node/pfs';
 import { assign } from 'vs/base/common/objects';
+import { flatten } from 'vs/base/common/arrays';
 import { extract, buffer } from 'vs/base/node/zip';
 import { Promise, TPromise } from 'vs/base/common/winjs.base';
 import { IExtensionsService, IExtension, IExtensionManifest, IGalleryInformation } from 'vs/workbench/parts/extensions/common/extensions';
@@ -22,6 +23,8 @@ import { IWorkspaceContextService } from 'vs/workbench/services/workspace/common
 import { Limiter } from 'vs/base/common/async';
 import Event, { Emitter } from 'vs/base/common/event';
 import { UserSettings } from 'vs/workbench/node/userSettings';
+import * as semver from 'semver';
+import {groupBy, values} from 'vs/base/common/collections';
 
 function parseManifest(raw: string): TPromise<IExtensionManifest> {
 	return new Promise((c, e) => {
@@ -119,7 +122,7 @@ export class ExtensionsService implements IExtensionsService {
 
 		const url = galleryInformation.downloadUrl;
 		const zipPath = path.join(tmpdir(), galleryInformation.id);
-		const extensionPath = path.join(this.extensionsPath, `${ extension.publisher }.${ extension.name }`);
+		const extensionPath = path.join(this.extensionsPath, `${ extension.publisher }.${ extension.name }-${ extension.version }`);
 		const manifestPath = path.join(extensionPath, 'package.json');
 
 		const settings = TPromise.join([
@@ -143,7 +146,7 @@ export class ExtensionsService implements IExtensionsService {
 
 	private installFromZip(zipPath: string): TPromise<IExtension> {
 		return validate(zipPath).then(manifest => {
-			const extensionPath = path.join(this.extensionsPath, `${ manifest.publisher }.${ manifest.name }`);
+			const extensionPath = path.join(this.extensionsPath, `${ manifest.publisher }.${ manifest.name }-${ manifest.version }`);
 			this._onInstallExtension.fire(manifest);
 
 			return extract(zipPath, extensionPath, { sourcePath: 'extension', overwrite: true })
@@ -162,11 +165,31 @@ export class ExtensionsService implements IExtensionsService {
 			.then(() => this._onDidUninstallExtension.fire(extension));
 	}
 
-	public getInstalled(): TPromise<IExtension[]> {
+	public getInstalled(includeDuplicateVersions: boolean = false): TPromise<IExtension[]> {
+		const all = this.getAllInstalled();
+
+		if (includeDuplicateVersions) {
+			return all;
+		}
+
+		return all.then(plugins => {
+			const byId = values(groupBy(plugins, p => `${ p.publisher }.${ p.name }`));
+			return byId.map(p => p.sort((a, b) => semver.rcompare(a.version, b.version))[0]);
+		});
+	}
+
+	private getDeprecated(): TPromise<IExtension[]> {
+		return this.getAllInstalled().then(plugins => {
+			const byId = values(groupBy(plugins, p => `${ p.publisher }.${ p.name }`));
+			return flatten(byId.map(p => p.sort((a, b) => semver.rcompare(a.version, b.version)).slice(1)));
+		});
+	}
+
+	private getAllInstalled(): TPromise<IExtension[]> {
 		const limiter = new Limiter(10);
 
 		return pfs.readdir(this.extensionsPath)
-			.then<IExtensionManifest[]>(extensions => Promise.join(extensions.map(e => {
+			.then<IExtension[]>(extensions => Promise.join(extensions.map(e => {
 				const extensionPath = path.join(this.extensionsPath, e);
 
 				return limiter.queue(
@@ -180,6 +203,11 @@ export class ExtensionsService implements IExtensionsService {
 	}
 
 	private getInstallationPath(extension: IExtension): string {
-		return extension.path ||  path.join(this.extensionsPath, `${ extension.publisher }.${ extension.name }`);
+		return extension.path || path.join(this.extensionsPath, `${ extension.publisher }.${ extension.name }-${ extension.version }`);
 	}
+
+	public removeDeprecatedExtensions(): TPromise<void> {
+		return this.getDeprecated()
+			.then<void>(extensions => TPromise.join(extensions.filter(e => !!e.path).map(e => pfs.rimraf(e.path))));
+		}
 }