Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

plugin-ext-vscode: ignore vsix files in local-plugins dir #13435

Merged
merged 1 commit into from
Apr 24, 2024

Conversation

xai
Copy link
Contributor

@xai xai commented Feb 29, 2024

What it does

In the local-plugins directory (i.e., the one passed to theia via the --plugins argument), we expect to find and deploy unpacked extensions.

This patch fixes an unexpected behavior where theia would also pick up and deploy .vsix files from the local-plugins directory into the deployedPlugins directory, where they will be treated as user-installed extensions on the next start of theia.

Instead, we now ignore .vsix files in the local-plugins directory and print a warning to the console.

Fixes #13222

Contributed on behalf of STMicroelectronics

How to test

  1. Add any vsix file to the plugins folder (choose one that is not already in ~/.theia/deployedPlugins/)
  2. Start Theia
  3. Verify that the extension is NOT loaded in Theia (neither as installed nor as builtin)
  4. Verify that the extension was NOT unpacked into ~/.theia/deployedPlugins and that it is NOT loaded in Theia.
  5. Unpack the extension in the plugins folder
  6. Start Theia
  7. Verify that the extension is loaded as builtin in Theia
  8. Verify that the extension was NOT unpacked into ~/.theia/deployedPlugins

Follow-ups

Review checklist

Reminder for reviewers

@tsmaeder
Copy link
Contributor

I'm not sure about the approach here: what this PR does is exclude exactly files of type *.vsix from exactly the location given via the --plugins CLI parameter. I'm not really sure what we consider "built-ins" if I'm looking at the related issue. Looking at the code, I would think that any PluginDeployerEntry that is of type System should be considered a built-in plugin. Wouldn't it make more sense in that case to simply not apply the file handlers in PluginDeployerImpl.applyFileHandlers?

@tsmaeder
Copy link
Contributor

tsmaeder commented Mar 21, 2024

@xai ping?

@xai
Copy link
Contributor Author

xai commented Mar 21, 2024

Yes, you are right, this would be the better approach instead of a very specific exception. I'll adjust the PR.

@xai
Copy link
Contributor Author

xai commented Mar 21, 2024

@tsmaeder pong

This patch fixes an unexpected behavior where Theia would also pick up
and deploy .vsix files from the local-plugins directory into the
deployedPlugins directory, where they will be treated as user-installed
extensions on the next start of theia.

Instead, we now only apply the file handlers for .vsix files if they are
user extensions. For system plugins, we print a warning message indicating
that the plugin has to be unpacked manually.

Fixes eclipse-theia#13222

Contributed on behalf of STMicroelectronics

Signed-off-by: Olaf Lessenich <olessenich@eclipsesource.com>
@tsmaeder
Copy link
Contributor

While testing, I found #13638, but it happens also in master, so unrelated.

@tsmaeder tsmaeder merged commit 1578d2d into eclipse-theia:master Apr 24, 2024
12 of 14 checks passed
@jfaltermeier jfaltermeier added this to the 1.49.0 milestone Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Builtin VSIX plugin is added to the installed list instead of the builtin list
3 participants