eclipse-theia / theia

Eclipse Theia is a cloud & desktop IDE framework implemented in TypeScript.
http://theia-ide.org
Eclipse Public License 2.0
19.87k stars 2.48k forks source link

Odd and unexpected optimization for user-installed VS Code extensions #12848

Closed jcortell68 closed 1 year ago

jcortell68 commented 1 year ago

Bug Description:

If a vsix file is dropped into the system plugin directory (what's specified via --plugins), it's decompressed every time you launch the Theia app. However, if the vsix is installed for the user only (i.e., dropped into c:\users\j.doe\.theia\extensions), then the vsix isn't extracted if it was previously extracted. (Note that extraction occurs to the temp directory in the user's data folder in all cases. More on this shortly.) What this means is that if I make a change to the vsix contents (without changing the filename) and relaunch the Theia app, that change may have no effect depending on whether I installed it in the user's extensions folder or the app's plugins folder. That just doesn't make much sense to me and I could see someone losing a lot of time trying to understand why the vsix change isn't taking effect for him, but someone else (who happens to put the vsix in the system plugins folder rather than the user one) has a different experience.

So my ask is this: why are we being inconsistent between user and system plugins? You can see this is deliberate in the code, but there is no explanation as to why.

If there is in fact a good explanation, can someone please put it in the code at least? This seems like a serious tripping hazard.

Steps to Reproduce:

git clone -b master https://github.com/jcortell68/theiasandbox
cd theiasandbox
yarn
  1. Download the two files attached to this ticket: myext-0.0.1.vsix.version1 and myext-0.0.1.vsix.version2. The two are identical except for the title of the command they contribute. In version 1, it's Hello World. In version 2, it's Hellooo World
  2. Copy the two files to c:\users\<username>\.theia\extensions
  3. Remove the .version1 extension from the first file and launch the app with yarn start:electron
  4. See that there's a Hello World command in the Command Palette
  5. Terminate the Theia app
  6. Return the .version1 extension to the first file and remove the .version2 from the second one.
  7. Launch the app with yarn start:electron
  8. There should be a Hellooo World in the command palette but there's a Hello World
  9. Looking at stdout, you'll find
    <timestamp> root INFO [myext-0.0.1.vsix]: already found

    Looking at the code referenced above, you'll see that means that the vsix wasn't extracted. The reason is that Theia found

    C:\Users\<username>\AppData\Local\Temp\vscode-unpacked\myext-0.0.1.vsix

    from the first launch and left it alone.

This is presumably an optimization, but why it the behavior exclusive to user-specific plugins? If you repeat the steps above but putting the files in the plugins directory at the root of the monorepo, you'll see that Hellooo World appears when the version2 vsix is in place. If you try this, make sure to first delete the vsix files from c:\users\<username>\.theia\extensions

Again, I could see someone pulling their hair because they get one behavior but someone else gets a different behavior. The only difference between the two individuals is where they put the vsix file.

Additional Information

jcortell68 commented 1 year ago

extensions.zip

jcortell68 commented 1 year ago

I think I can now answer my own question after looking at more plugin logic. I wish the explanation was simple, but here goes...

When the user of a Theia app installs a VS Code extension, it is installed as a user extension. The formal installation mechanism downloads and puts the vsix file (a zip file) into c:\users\j.doe\.theia\extensions as a first step, then decompresses it to the user’s temp folder, specifically C:\Users\j.doe\AppData\Local\Temp\vscode-unpacked. Keep in mind some of these vsix files can be MBs in size (10x that decompressed) and have tens of thousands of files.

Now, imagine the user has 20 vscode extensions installed. Consider the cost of having to extract all 20 vsix files in c:\users\j.doe\.theia\extensions every time you launch the Theia app. We could be talking about adding minutes to the app launch. Obviously, that’s a no-go.

Thus, on launch, for each vsix file Theia finds in c:\users\j.doe\.theia\extensions, it looks to see if there’s a directory in C:\Users\j.doe\AppData\Local\Temp\vscode-unpacked with the same name and uses it instead of unzipping the vsix anew.

Great, but then why is the behavior different for system plugins? Well, system plugins are a bit of an oddity. A production Theia app might not even support system plugins. When an app does, presumably the app has just one or two plugins that are deployed with the app, and they should be deployed in decompressed form, not vsix. In decompressed form, Theia doesn’t copy the directory in the systems plugin folder over to the user’s temp folder; it loads the plugin directly from the app installation layout.

But now let’s consider a vsix file that’s dropped into that system plugins location. This github ticket was created to point out that Theia is inconsistent. For a vsix in the system plugin folder, it will always decompress the vsix into the user’s temp folder, whereas that’s not the case for a vsix in the user’s extensions folder. That inconsistency is likely a desire to be consistent elsewhere–with system plugins that are deployed uncompressed. Any change to a file in those plugins takes effect the next time you launch the Theia app. So a change to a system vsix file itself should have an equal effect. And thus vsix files in the system plugins folder are decompressed every time. To state the obvious, it is a really bad idea to put a large vsix in the system plugins folder; instead, decompress the vsix there.

In the end, the system plugins folder is usually a feature used during development for a plugin. E.g., in a Theia monorepo, you typically create a symlink plugins folder at the root of the monorepo and launch the Theia app using --plugins=local-dir:plugins. For obvious reasons of efficiency, the plugin you’re developing is likely in uncompressed form (not a vsix), and so the question of why Theia decompresses system vsix files every time is mostly a moot point. But if for whatever reason there is an updated vsix file in the system plugins folder, the developer wants its contents to be honored, and not for Theia to use the cached contents of the last vsix file.

marcdumais-work commented 1 year ago

Hi @jcortell68 ,

Thanks for the thoughts. We discussed related to this in https://github.com/eclipse-theia/theia/issues/12757

I think that VSIX files added to the user's folder by the IDE should be unzipped in place, e.g. in ~/.theia/extensions/, rather than being kept as VSIX. This would remove the need to use the system folder (/tmp/vscode-unpacked) to unzip them, for that common case.

Also, I think there is a bug at the line you point-to - if this is a cache, it seems we should consider that the VSIX may have changed since last time, and not just reuse a random folder of the same name, that happens to be there.

jcortell68 commented 1 year ago

I think that VSIX files added to the user's folder by the IDE should be unzipped in place, e.g. in ~/.theia/extensions/, rather than being kept as VSIX.

I couldn't agree more :-) This had crossed my mind, but I assumed I was either missing something, or that there was a historical reason for it having to be that way. But yes...when VS Code installs a VSIX file, it does exactly that--it decompresses the VSIX into ~/.vscode/extensions. and so there is no chance for any unexpected, sticky/stale behavior. VSIXes won't end up in that folder other than through the unnatural workflow of someone dropping the file(s) there. And if they do that--user beware. They will incur the cost (a possibly heavy one) of having to decompress the vsix to the tmp folder every time they launch the Theia app. That is the same as if they drop a vsix in the --plugins dir.

if this is a cache, it seems we should consider that the VSIX may have changed since last time

Well, as per my explanation, it doesn't consider that it may have changed for performance reasons. Some of these plugins can be quite large. So, you either have to add logic to try to determine if the vsix actually changed (e.g., store the file's SHA or at least the timestamp and size in the decompressed form in the tmp folder), or you have to assume it didn't (because assuming it did would be too costly). But if we were to change the behavior of the extension installer to decompress the vsix into ~/.theia/extensions and always load the plugin from there, then that takes the "buggy" behavior out of the mix.

We do however need to consider the workflow of someone manually dropping a VSIX into ~/.theia/extensions. Today such individuals are not subjected to a decompression on every app launch. And I imagine in most cases, they are not replacing the vsix file. So we don't want to bump their launch time for the edge case where someone updates the vsix. IMO, we should make Theia smarter there. I think a reasonable compromise is to store the timestamp and size of the vsix when decompressing to the tmp folder. We then do a quick check with that info to determine if we need to decompress the vsix.