fix plugins are not loaded when staring theia without internet access #6056
Conversation
fix eclipse-theia#6055 If all the vscode entensions dependencies(listed in the extensionDependencies field of vscode extension) are already downloaded to local directory before staring theia, then there is no need to download them from the remote(vscode marketplace) during the theia starting process. Signed-off-by: tom-shan <swt0008411@163.com>
| @@ -131,8 +131,8 @@ export class PluginDeployerImpl implements PluginDeployer { | |||
|
|
|||
| for (const deployerEntry of pluginDeployerEntries) { | |||
| const metadata = await this.pluginDeployerHandler.getPluginMetadata(deployerEntry); | |||
| if (metadata && !pluginsToDeploy.has(metadata.model.id)) { | |||
| pluginsToDeploy.set(metadata.model.id, deployerEntry); | |||
| if (metadata && !pluginsToDeploy.has(metadata.model.uniqueId)) { | |||
There was a problem hiding this comment.
metadata.model.id is already unique, the check is already in place
Why it is not enough?
There was a problem hiding this comment.
for metadata.model.extensionDependencies will add a prefix of vscode:extension/
see https://github.com/theia-ide/theia/blob/85b75768f702656db4ee8ff6eaad303be8102643/packages/plugin-ext-vscode/src/node/scanner-vscode.ts#L66
There was a problem hiding this comment.
But at the end the extension dependency will be resolved to publisher.name and should be filtered out by pluginsToDeploy.has(metadata.model.id), since there is already an extension with such publisher and name installed from local vsix.
metadata.model.id has to be unique identifier among all extensions, there should not be another
There was a problem hiding this comment.
Sorry I misunderstood your question.
metadata.model.idis already unique.
That's right.
The reason I use metadata.model.uniqueId is that:
if using pluginsToDeploy.set(metadata.model.id, deployerEntry), considering queue.push(...metadata.model.extensionDependencies),
then queue will have some extensionDependencies elements which have the 'vscode:extension/' suffix,
and if (visited.has(current) || pluginsToDeploy.has(current)) { will not be true.
Because the keys of pluginsToDeploy and the elements of queue will never be equal even for the extensionDependencies.
There was a problem hiding this comment.
But we should not continue to resolve the extensionDependencies when pluginsToDeploy contains them.
There was a problem hiding this comment.
Because it will resolve twice for the same plugin.
|
I thought the fix should be to wrap a loop body into try catch block, so failure to download one extension does not break the deployment flow. |
| @@ -115,7 +115,7 @@ export class PluginDeployerImpl implements PluginDeployer { | |||
| const queue = [...pluginEntries]; | |||
| while (queue.length) { | |||
| const current = queue.shift()!; | |||
| if (visited.has(current)) { | |||
| if (visited.has(current) || pluginsToDeploy.has(current)) { | |||
There was a problem hiding this comment.
current is deployment entry, it is not a plugin id
One can only check by a plugin id after resolution without it we don't know anything what is behind current.
|
Sorry I'm out now. I will try about an hour later. |
What it does
fix #6055
If all the vscode entensions dependencies(listed in the extensionDependencies field of vscode extension)
are already downloaded to local directory before staring theia, then there is no need to download them
from the remote(vscode marketplace) during the theia starting process.
How to test
1 download two vscode plugins from vscode marketplac
2 copy these two plugins to
pluginsdirectory3 then start theia(
cd examples/browser; yarn start), and check if all of the vscode extensins can be loaded successfullyReview checklist
Reminder for reviewers