Skip to content

Commit e8a64ac

Browse files
austin3dickeyaustin3dickey
authored
stop installing incompatible recursive extension dependencies (#10716)
<!-- Thank you for submitting a pull request. If this is your first pull request you can find information about contributing here: * https://github.com/posit-dev/positron/blob/main/CONTRIBUTING.md We recommend synchronizing your branch with the latest changes in the main branch by either pulling or rebasing. --> <!-- Describe briefly what problem this pull request resolves, or what new feature it introduces. Include screenshots of any new or altered UI. Link to any GitHub issues but avoid "magic" keywords that will automatically close the issue. If there are any details about your approach that are unintuitive or you want to draw attention to, please describe them here. --> Addresses #10423. Adds the `ms-python.vscode-python-envs` extension to the list of incompatible Positron extensions. I also noticed that it was still getting installed as a recursive dependency of other extensions like Pyrefly. So this PR also adds some code that checks for compatibility of recursive dependencies before installing them. In order to do that, I had to move some stuff from the workbench level to the platform level. ### Release Notes <!-- Optionally, replace `N/A` with text to be included in the next release notes. The `N/A` bullets are ignored. If you refer to one or more Positron issues, these issues are used to collect information about the feature or bugfix, such as the relevant language pack as determined by Github labels of type `lang: `. The note will automatically be tagged with the language. These notes are typically filled by the Positron team. If you are an external contributor, you may ignore this section. --> #### New Features - N/A #### Bug Fixes - N/A ### QA Notes <!-- Positron team members: please add relevant e2e test tags, so the tests can be run when you open this pull request. - Instructions: https://github.com/posit-dev/positron/blob/main/test/e2e/README.md#pull-requests-and-test-tags - Available tags: https://github.com/posit-dev/positron/blob/main/test/e2e/infra/test-runner/test-tags.ts --> <!-- Add additional information for QA on how to validate the change, paying special attention to the level of risk, adjacent areas that could be affected by the change, and any important contextual information not present in the linked issues. --> The python environments extension shouldn't be installable directly or as a dependency of e.g. pyrefly or ty. If you had it before, it won't delete it.
1 parent ee9f160 commit e8a64ac

File tree

2 files changed

+107
-53
lines changed

2 files changed

+107
-53
lines changed

‎src/vs/platform/extensionManagement/common/abstractExtensionManagementService.ts‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,69 @@ import { IUriIdentityService } from '../../uriIdentity/common/uriIdentity.js';
3434
import { IUserDataProfilesService } from '../../userDataProfile/common/userDataProfile.js';
3535
import { IMarkdownString, MarkdownString } from '../../../base/common/htmlContent.js';
3636

37+
// --- Start Positron ---
38+
/**
39+
* List of extension IDs that conflict with Positron built-in features.
40+
* Please use lower case for everything in here.
41+
*/
42+
const kPositronDuplicativeExtensions = [
43+
'ikuyadeu.r',
44+
'reditorsupport.r-lsp',
45+
'reditorsupport.r',
46+
'rdebugger.r-debugger',
47+
'mikhail-arkhipov.r',
48+
'vscode.r',
49+
'jeanp413.open-remote-ssh',
50+
'ms-python.python',
51+
'ms-python.vscode-python-envs',
52+
'github.copilot-chat'
53+
];
54+
55+
export interface PositronExtensionCompatibility {
56+
compatible: boolean;
57+
reason?: string;
58+
}
59+
60+
/**
61+
* Check if an extension is compatible with Positron (simple boolean check)
62+
* @param extension Extension to check
63+
* @returns true if compatible, false if it conflicts with Positron features
64+
*/
65+
function isPositronExtensionCompatible(extension: { name: string; publisher: string }): boolean {
66+
const id = `${extension.publisher}.${extension.name}`.toLowerCase();
67+
return !kPositronDuplicativeExtensions.includes(id);
68+
}
69+
70+
/**
71+
* Check if an extension is compatible with Positron
72+
* @param extension Extension to check
73+
* @returns Compatibility result with optional reason if incompatible
74+
*/
75+
export function positronExtensionCompatibility(extension: { name: string; publisher: string; displayName?: string }): PositronExtensionCompatibility {
76+
if (!isPositronExtensionCompatible(extension)) {
77+
return {
78+
compatible: false,
79+
reason: nls.localize(
80+
'positronExtensionConflicts',
81+
"Cannot install the '{0}' extension because it conflicts with Positron built-in features.", extension.displayName || extension.name
82+
)
83+
};
84+
}
85+
return { compatible: true };
86+
}
87+
88+
/**
89+
* Create an error for Positron extension incompatibility
90+
* @param reason Optional reason for incompatibility
91+
* @returns Error with ExtensionManagementErrorCode.Incompatible
92+
*/
93+
export function positronExtensionCompatibilityError(reason?: string): Error {
94+
const error = new Error(reason || nls.localize('positronExtensionIncompatible', "Cannot install the extension because it is incompatible with Positron"));
95+
error.name = ExtensionManagementErrorCode.Incompatible;
96+
return error;
97+
}
98+
// --- End Positron ---
99+
37100
export type InstallableExtension = { readonly manifest: IExtensionManifest; extension: IGalleryExtension | URI; options: InstallOptions };
38101

39102
export type InstallExtensionTaskOptions = InstallOptions & { readonly profileLocation: URI; readonly productVersion: IProductVersion };
@@ -362,6 +425,13 @@ export abstract class AbstractExtensionManagementService extends CommontExtensio
362425
if (existing && existing.isApplicationScoped === !!options.isApplicationScoped) {
363426
continue;
364427
}
428+
// --- Start Positron ---
429+
// Skip Positron-incompatible dependencies and pack extensions
430+
if (!isPositronExtensionCompatible(gallery)) {
431+
this.logService.info(`Skipping dependency/packed extension '${gallery.identifier.id}' because it conflicts with Positron built-in features`);
432+
continue;
433+
}
434+
// --- End Positron ---
365435
createInstallExtensionTask(manifest, gallery, options, task);
366436
}
367437
} catch (error) {

‎src/vs/workbench/services/extensionManagement/common/extensionManagementService.ts‎

Lines changed: 37 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ import { createCommandUri, IMarkdownString, MarkdownString } from '../../../../b
4848
import { verifiedPublisherIcon } from './extensionsIcons.js';
4949
import { Codicon } from '../../../../base/common/codicons.js';
5050
import { IStringDictionary } from '../../../../base/common/collections.js';
51-
import { CommontExtensionManagementService } from '../../../../platform/extensionManagement/common/abstractExtensionManagementService.js';
51+
import { CommontExtensionManagementService, positronExtensionCompatibility, positronExtensionCompatibilityError } from '../../../../platform/extensionManagement/common/abstractExtensionManagementService.js';
5252

5353
const TrustedPublishersStorageKey = 'extensions.trustedPublishers';
5454

@@ -461,11 +461,35 @@ export class ExtensionManagementService extends CommontExtensionManagementServic
461461
return manifest;
462462
}));
463463

464+
// --- Start Positron ---
465+
// Check Positron compatibility for all extensions
466+
for (let i = 0; i < extensions.length; i++) {
467+
const { extension, options } = extensions[i];
468+
const compat = positronExtensionCompatibility(extension);
469+
if (!compat.compatible) {
470+
results.set(extension.identifier.id.toLowerCase(), {
471+
identifier: extension.identifier,
472+
source: extension,
473+
error: positronExtensionCompatibilityError(compat.reason),
474+
operation: InstallOperation.Install,
475+
profileLocation: options.profileLocation ?? this.userDataProfileService.currentProfile.extensionsResource
476+
});
477+
}
478+
}
479+
// --- End Positron ---
480+
464481
if (extensions.some(e => e.options?.context?.[EXTENSION_INSTALL_SKIP_PUBLISHER_TRUST_CONTEXT] !== true)) {
465482
await this.checkForTrustedPublishers(extensions.map((e, index) => ({ extension: e.extension, manifest: manifests[index], checkForPackAndDependencies: !e.options?.donotIncludePackAndDependencies })));
466483
}
467484

468485
await Promise.all(extensions.map(async ({ extension, options }) => {
486+
// --- Start Positron ---
487+
// Skip extensions that already failed Positron compatibility check
488+
if (results.has(extension.identifier.id.toLowerCase())) {
489+
return;
490+
}
491+
// --- End Positron ---
492+
469493
try {
470494
const manifest = await this.extensionGalleryService.getManifest(extension, CancellationToken.None);
471495
if (!manifest) {
@@ -523,22 +547,22 @@ export class ExtensionManagementService extends CommontExtensionManagementServic
523547
}
524548
// --- End Positron ---
525549
const manifest = await this.extensionGalleryService.getManifest(gallery, CancellationToken.None);
526-
if (!manifest) {
527-
throw new Error(localize('Manifest is not found', "Installing Extension {0} failed: Manifest is not found.", gallery.displayName || gallery.name));
528-
}
550+
if (!manifest) {
551+
throw new Error(localize('Manifest is not found', "Installing Extension {0} failed: Manifest is not found.", gallery.displayName || gallery.name));
552+
}
529553

530-
if (installOptions?.context?.[EXTENSION_INSTALL_SKIP_PUBLISHER_TRUST_CONTEXT] !== true) {
531-
await this.checkForTrustedPublishers([{ extension: gallery, manifest, checkForPackAndDependencies: !installOptions?.donotIncludePackAndDependencies }],);
532-
}
554+
if (installOptions?.context?.[EXTENSION_INSTALL_SKIP_PUBLISHER_TRUST_CONTEXT] !== true) {
555+
await this.checkForTrustedPublishers([{ extension: gallery, manifest, checkForPackAndDependencies: !installOptions?.donotIncludePackAndDependencies }],);
556+
}
533557

534-
if (installOptions?.context?.[EXTENSION_INSTALL_SOURCE_CONTEXT] !== ExtensionInstallSource.SETTINGS_SYNC) {
558+
if (installOptions?.context?.[EXTENSION_INSTALL_SOURCE_CONTEXT] !== ExtensionInstallSource.SETTINGS_SYNC) {
535559

536-
await this.checkForWorkspaceTrust(manifest, false);
560+
await this.checkForWorkspaceTrust(manifest, false);
537561

538-
if (!installOptions?.donotIncludePackAndDependencies) {
539-
await this.checkInstallingExtensionOnWeb(gallery, manifest);
540-
}
541-
}
562+
if (!installOptions?.donotIncludePackAndDependencies) {
563+
await this.checkInstallingExtensionOnWeb(gallery, manifest);
564+
}
565+
}
542566

543567
servers = servers?.length ? this.validServers(gallery, manifest, servers) : await this.getExtensionManagementServersToInstall(gallery, manifest);
544568
if (!installOptions || isUndefined(installOptions.isMachineScoped)) {
@@ -1201,46 +1225,6 @@ export class ExtensionManagementService extends CommontExtensionManagementServic
12011225
}
12021226
}
12031227

1204-
// --- Start Positron ---
1205-
const kPositronDuplicativeExtensions = [
1206-
'ikuyadeu.r',
1207-
'reditorsupport.r-lsp',
1208-
'reditorsupport.r',
1209-
'rdebugger.r-debugger',
1210-
'mikhail-arkhipov.r',
1211-
'vscode.r',
1212-
'jeanp413.open-remote-ssh',
1213-
'ms-python.python',
1214-
'GitHub.copilot-chat'
1215-
];
1216-
1217-
interface PositronExtensionCompatibilty {
1218-
compatible: boolean;
1219-
reason?: string;
1220-
}
1221-
1222-
function positronExtensionCompatibility(extension: { name: string; publisher: string; displayName?: string }): PositronExtensionCompatibilty {
1223-
const id = `${extension.publisher}.${extension.name}`.toLowerCase();
1224-
if (kPositronDuplicativeExtensions.includes(id)) {
1225-
return {
1226-
compatible: false,
1227-
reason: localize(
1228-
'positronExtensionConflicts',
1229-
"Cannot install the '{0}' extension because it conflicts with Positron built-in features.", extension.displayName || extension.name
1230-
)
1231-
};
1232-
} else {
1233-
return {
1234-
compatible: true
1235-
};
1236-
}
1237-
}
1238-
function positronExtensionCompatibilityError(reason?: string) {
1239-
const error = new Error(reason || localize('positronExtensionIncompatible', "Cannot install the extension because it is incompatible with Positron"));
1240-
error.name = ExtensionManagementErrorCode.Incompatible;
1241-
return error;
1242-
}
1243-
// --- End Positron ---
12441228

12451229
class WorkspaceExtensionsManagementService extends Disposable {
12461230

0 commit comments

Comments
 (0)