vsx: update compatibility check for builtins #9486
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vince-fugnitto
added
builtins
The changeset updates the logic when getting the latest compatible version of a builtin extension (individual or pack) by fetching the version which is defined by api version supported by the framework. Since the `engines.vscode` property is generally not maintained by vscode builtin extensions (since they are bundled with vscode at a specific version) we cannot rely on the `engines.vscode` version to fetch a compatible extension. Instead, we now fetch a version which corresponds to the api level the framework defines. Signed-off-by: vince-fugnitto <[email protected]>
vince-fugnitto
force-pushed
the
vf/builtin-compatibility
branch
from
1379f80
to
d110811
Compare
1 task
alvsan09
reviewed
May 18, 2021
There was a problem hiding this comment.
I have tested the behavior is as expected,
In addition to the suggested test, I did:
- bring the
VSCODE_DEFAULT_API_VERSIONdown to as early as 1.44.2 works to select the proper versions from UI. - Installing a
builtin-extension-packfrom the registry at runtime works
Just a minor optimization as inline comment is pending.
1 task
|
You may want to clean this folder as well, specially if you tested installing the pack with the example app: |
marcdumais-work
approved these changes
May 20, 2021
There was a problem hiding this comment.
Tested locally: we pull appropriate versions of built-ins, individually at runtime and when installing the pack too. Thanks @vince-fugnitto !
|
I'll merge early next week if there are no objections, thank you for the reviews! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
The framework requires special logic when handling
builtinextensions which are maintained by the project:vscode-builtin-extension: builtin extensions which are bundled with vscodevscode-builtin-extension-packs: builtin extension pack containing bundled vscode extensions referenced by id.The issue is that these such extensions do not necessarily maintain their
engines.vscodefield so instead we should be smarter and fetch compatible versions of these extensions by their actualversion(which corresponds to the actual vscode version). This way, we are more likely to fetch a valid version of an extension which satisfies our API level.What it does
The changes include updating the logic to determine compatible versions of extensions:
project maintained builtins(vscode): we check compatibility based onversion.builtin(non-vscode bundled extensions): we check compatibility based onengines.vscodeversion.engines.vscodeversion.The changes determine builtin compatibility by using
semver.lte(less than or equal) in order to get a version which matches the API version, or a previous version to ensure that extensions are compatible.The changes also include the
VSXBuiltinNamespacesnamespace which contains information on reserved builtin namespaces:vscode: reserved for individual vscode builtin extensions.eclipse-theia: reserved for builtin extension-packs of individual vscode builtin extensions.How to test
rm -rf plugins/,rm -rf /tmp/vscode-unpacked,rm -rf ~/theia/extensions).json-language-features- the extension should be at1.50.0(current API level).1.50.0if it exists, else a previous version if it exists.Review checklist
Reminder for reviewers
Signed-off-by: vince-fugnitto [email protected]