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

Migration to VS Code extensions #3956

Closed
akosyakov opened this issue Jan 6, 2019 · 42 comments
Closed

Migration to VS Code extensions #3956

akosyakov opened this issue Jan 6, 2019 · 42 comments
Assignees
Labels
vscode issues related to VSCode compatibility
Milestone

Comments

@akosyakov
Copy link
Member

akosyakov commented Jan 6, 2019

I've started looking into the migration to VS Code extensions.

I want to tackle the following clients:

  • docker images
  • Electron-based products
  • Theia language extensions under theia-ide org

I'm thinking about 2 stages.

  • Stage 1: Static Reconfiguration - IDE developers should install VS Code extensions at the build time. The plugin system should not contribute any UI.
  • Stage 2: Dynamic Management - The UI to search, install manage and develop VS Code extensions should be complete. It should be optional via a new Theia extension. There should be a contribution point to connect different marketplaces. There should be a Theia extension contributing VS Code marketplace.

I want to focus on the first stage but there should be some groundwork done first. Right now the dynamic part is somehow already presented in the plugin extension. The start is to extract it in own Theia extension. As far as I know, extension-manager plugin is not used by any image or product. We can rename it to plugin-manager, reuse UI and reimplement the backend services.

Now, about the first stage. I see the following steps:

1. Migration of theiaide/theia image

It is the most popular and requires only basic extensions:

For built-in, I'm going to create a new Theia extension which will pull them, build and bundle. The plugin system should be able to pick up such bundled extensions. We should test the plugin system again these extensions. Node debugging is already in the good shape, thanks to @tolusha.

With that step finished:
theiaide/theia image is an analog of VS Code without 3rd party extensions.

2. Electron migration

The plugin system should be able to pick up VS Code extensions from the user directory. For instance from ${user-home}/.theia/extensions. .theia directory should be configurable.

With that step finished:
Existing products can start the migration. It should be possible to start deprecating relevant Theia extensions.

3. Migration of Theia language extensions

We replace Theia language extensions with VS Code extensions for all images. We can start with theia-full image. When a Theia extension is not used by images anymore, we deprecate and archive it. We deprecate language contributions in Theia when all language extensions migrated.

With that step finished:
It should simplify Monaco maintenance and language integration. We won't need monaco-languageclient and custom vscode-languageclient anymore. Without it, only Theia needs an upgrade to a new Monaco version. We can run vscode-languageclient in proper environments via VS Code extensions. Instead of forking and tweaking it to run in the browser, as for now.

All steps

Existing extensions will remain published on npm forever. We let deprecated APIs and extensions be in Theia repo for one release. We guarantee that VS Code analogous stable. We don't contribute any incomplete or unpolished UI.

Any concerns, objections to the approach?

// cc @svenefftinge, @marcdumais-work, @benoitf, @thegecko, @caseyflynn-google

@akosyakov
Copy link
Member Author

Theia extension contributing built-in VS Code extensions is here: https://github.com/theia-ide/vscode-builtin-extensions

There seems to be an issue that if one VS Code extension fails to load then consequent extensions are not loaded at all. I'm going to look into it and then test individual VS Code extensions.

@thegecko
Copy link
Member

@akosyakov

To confirm, would the VSCode extension support outlined here be in addition to Theia extensions and Theia plugins, or is the plan to replace one or both of these?

Thanks

@akosyakov
Copy link
Member Author

@thegecko Theia plugins are complementary to Theia extensions. VS Code extensions are special kind of Theia plugins.

Theia will maintain its flexibility and control for white labeled products through its original extension architecture. The additional VS Code extension support enables an alternative, less invasive and yet more robust extension mechanism on top of that.

from https://medium.com/@sven.efftinge/theia-to-support-vs-code-extensions-a7b9100ec836

@thegecko
Copy link
Member

Thanks for the confirmation :)

@marcdumais-work
Copy link
Contributor

Heads-up: Sorry to be the "IP Grinch": we will need a CQ for each vscode extension that we download as part of a Theia extension or bundle in any way, and again whenever any of them is updated to a new version.

These are "prereq" dependencies, like our production NPM dependencies. One difference is that are downloaded and updated through a different mechanism. I think 1 standalone CQ per vscode extension works.

@benoitf
Copy link
Contributor

benoitf commented Jan 25, 2019

hello, why make vscode extensions being static ?

@benoitf
Copy link
Contributor

benoitf commented Jan 28, 2019

Hello, about stage 2: I would prefer to opt-out than opt-in. The ability to provide plug-in at runtime is the key-feature, only ppl want to disable it should provide a flag, etc to disable that rather than having to add a new extension.

@akosyakov
Copy link
Member Author

@benoitf for products which does not want to allow users to install extensions/plugins, UI part should be optional

@benoitf
Copy link
Contributor

benoitf commented Jan 28, 2019

@akosyakov yes but I think it should be opt-out instead of opt-in
--> you select plugin-ext and you have everything. If you want to disable UI then you provide a flag (compile or runtime). But not a side extension.

@tsmaeder
Copy link
Contributor

tsmaeder commented Feb 6, 2019

@akosyakov for Che as a consumer of Theia it's important that any vscode extension that has prerequisites beyond basic Theia (php, typescript, come to mind) be separately available from some place: we'll want to put those special prerequisites inside a sidecar container, no where Theia is running.

@akosyakov
Copy link
Member Author

you select plugin-ext and you have everything. If you want to disable UI then you provide a flag (compile or runtime)

@benoitf We already have an approach to plug/unplug features: extensions. We don't want to load UI code at runtime for sure, and don't want to come up with a new approach to disable features. Separating runtime and UI to different extensions would setup clear API boundaries between them, won't allow any UI code to leak at runtime and does not need any new code. Also it would be possibility to align at least the UI part with the project org conventions.

//cc @svenefftinge

@akosyakov
Copy link
Member Author

akosyakov commented Feb 28, 2019

@tsmaeder this issues is not about Che, but general support. I am fine if you publish each VS Code extension as you want for Che.

@tsmaeder
Copy link
Contributor

@akosyakov I think it's uncontroversial that the language configuration extensions should be included: it doesn't hurt, even if a particular installation does not use them.
However, if we cannot exclude extensions like python, we won't be able to consume the Theia extension you've created and we'll have to duplicate the work, which would be a pity.

@akosyakov
Copy link
Member Author

@tsmaeder ok, we could add publishing each built-in VS Code extension as an independent npm package as well

@akosyakov
Copy link
Member Author

akosyakov commented Mar 5, 2019

All built-in VS Code extensions are republished under @theia/vscode-builtin- prefix into npmjs: eclipse-theia/vscode-builtin-extensions@9886f60#diff-1b65ea7e7a9dd6aa50436ecdd461f0aaR1

https://github.com/theia-ide/vscode-builtin-extensions is still not published since some extensions (git, markdown, npm, theming, etc.) cannot be run nicely yet.

@benoitf
Copy link
Contributor

benoitf commented Mar 5, 2019

hello @akosyakov, why https://github.com/theia-ide/vscode-builtin-extensions is not an eclipse repository ?

@akosyakov
Copy link
Member Author

@benoitf I did it to get started without any frictions with CQs. Does it somehow stop you from consuming in Eclipse Che?

I've noticed also that format of npm archives is different from vsix, so they cannot be loaded out of the box. I will open a PR to support VS Code extensions out of npm archive too.

@tsmaeder
Copy link
Contributor

tsmaeder commented Apr 1, 2019

@akosyakov I'm not sure that "workaround" will work once the theia repo moves to the eclipse org. The examples will very likely consume the extensions from npm, which will require a CQ. So this might actually delay the move to the eclipse org.

@akosyakov
Copy link
Member Author

akosyakov commented Apr 1, 2019

@tsmaeder it won't delay. These extensions are not used in this repo, theia-apps is not going to be moved under eclipse org. But I agree that we should file CQs for them (not for vscode-builtin-extensions, it does not contain them) to allow Eclipse products to use them. Probably one CQ for this folder should be enough. cc @marcdumais-work ?

@tsmaeder
Copy link
Contributor

tsmaeder commented Apr 1, 2019

I know this is not first priority for the rest of the community, but I'm fear that we might have a problem with this in Che.
@akosyakov I don't quite understand the comment about "vscode-builtin-extensions". Don't we want to reuse those?

@akosyakov
Copy link
Member Author

vscode-builtin-extensions does not contain code, it uses VS Code as git submodule, pull it, rebuilds and publish to npm. Extension code still belong to VS Code repo, so i thought a CQ should be opened against VS Code. Maybe i'm wrong. I also will be completely fine if someone else opens a CQ.

@marcdumais-work
Copy link
Contributor

@akosyakov @tsmaeder @benoitf
Maybe we can discuss this during tomorrow's dev meeting?

@jgbradley1
Copy link
Contributor

One small request as this is under development: imagine a scenario where Theia is run offline (no internet access). Consider adding a preference setting that allows users to connect to a private extension marketplace/gallery. Do not hardcode in Microsoft's marketplace. Otherwise Theia will only work online.

This would be useful for privately hosted instances of Theia inside company networks. Someone else has already made a similar request to VS Code.

@marcdumais-work
Copy link
Contributor

Here's what I understand about the IP clearing (CQs or new proposed process) of the built-in VS Code extensions:

  • ATM these extensions are not used in the main Theia repo.
  • They will be crucial for any Theia-based application that wants to permit its users to use VS Code extensions
  • if not "IP cleared" by us, any Eclipse project that wants to use them will need to do so, before being allowed to use them (from their PoV, these will be external production dependencies).

As a platform, I think we should aim to facilitate the lives of our extenders / adopters, specially those who also are also part of the Eclipse Foundation eco-system and/or directly involved in our project.

Way forward?

Even though not used yet in the main Theia repo, I think we could go forward and register CQs (or follow the new CQ-less process when sufficiently clarified) so that we cover these built-in VS Code extensions, for our own eventual use and also other Eclipse project's.

I am clarifying how the new proposed process should work, that might replace CQs for production dependencies. It looks-like covering these extensions could be quick, with that process. So in the end this may not be a big deal.

@marcdumais-work
Copy link
Contributor

Hi,

Sharon from the Foundation is suggesting that we give the new "self-scan" IP process for 3rd party dependencies a try with some "live" cases and then discuss it, in a meeting on Friday. I propose we use all the VS Code built-in extensions we publish from theia-ide/vscode-builtin-extensions, as a trial case.

Since I want to learn and document how to do this by ourselves and this may be a bit more complicated than the usual "stepping the version of one of our existing prod dep" case, I volunteer to handle this in the next couple of days.

With the repo where we publish these VS Code built-in extensions being outside the Foundation, I think we need to handle the extensions as external/3rd party NPM production dependencies, for our Eclipse Theia project. They happen not to be referenced from our main repo yet, but I think that's ok.

@akosyakov I see that (at least some of) the individual extensions (@theia/vscode-builtin-*) were published to NPM about a month ago, version 0.1.0. Did you plan to re-publish soon? If so it would be good to do it first, so that we can analyse the latest published version of each; like other 3rd party production dependencies, we will need to re-scan when new versions are published.

Also, is it still the plan to publish all extensions in one single package, for those who do not need the granularity? I have not found that package, published on NPM, but maybe I missed it.

@akosyakov
Copy link
Member Author

I see that (at least some of) the individual extensions (@theia/vscode-builtin-*) were published to NPM about a month ago, version 0.1.0. Did you plan to re-publish soon?

Right now i don't have plans to republish them again. If only something is wrong with current.

Also, is it still the plan to publish all extensions in one single package, for those who do not need the granularity?

Yes, but it won't happen till all built-in extensions running nicely, 1-2 months more.

@marcdumais-work
Copy link
Contributor

@akosyakov ok, so then I will proceed with the individual @theia/vscode-builtin-* extensions that are currently published.

@marcdumais-work
Copy link
Contributor

Update: I am meeting with Sharon later today. I found nothing suspicious, license-wise, with the content of the various build-in VS Code extensions we publish. I will share what I did with Sharon and see what she thinks.

@akosyakov
Copy link
Member Author

@marcdumais-work i doubt there is anything suspicious, MS cares a lot of not relying on external dependencies and be compatible with MIT already

@marcdumais-work
Copy link
Contributor

I just had the meeting with Sharon and Wayne. We did not have time to talk specifically about this case (IP clearing for built-in VS Code extensions), but according to general discussions, this case goes a bit outside what we can handle on our own, at this time, since the suggested analysis tool will not work in this case.

So I'll do the following; I'll open one CQ for all extensions and provide the analysis that has been done, and we'll see what feedback we get. If the Foundation IP team is happy with the analysis, that's all we'll need to do until we update to new versions of these extensions, and we may have permission to do similar cases, on our own, in the future.

(I'll give more details about the meeting separately)

@marcdumais-work
Copy link
Contributor

@marcdumais-work
Copy link
Contributor

Update: the CQ is license_certified!

So we're good-to-go, w-r to built-in VS Code extensions based on VS Code v1.30.1.

@marcdumais-work
Copy link
Contributor

Something new (*) has come-up that will make the built-in VS Code extensions not license_certified anymore, until resolved.

Sharon pointed-out a VS Code NPM dependency, that has a problematic license, found when investigating a previous CQ (for another project). It turns-out that this dependency is pulled by the built-in VS Code git extension and used in production (i.e. not just for tests and/or build).

We distribute this extension as @theia/vscode-builtin-git and soon plan to use it, whenever a Theia application needs VS Code extension compatibility, as well as an eventual replacement to our own @theia/git.

The dependency is this one: jschardet@1.6, licensed under the LGPL.

See more details:
https://dev.eclipse.org/ipzilla/show_bug.cgi?id=19512#c12

Like when we found something similar for dugite and electron, we have a few options:

  • not publish/use @theia/vscode-builtin-git anymore (not what we want, but could be a short-term solution while we implement one of the following options)
  • get Foundation approval for an exception to let us keep it as a dependency (like happened recently for Electron)
  • or maybe we could have our own fork of this package, in which we replace jschardet with something equivalent, but that has an acceptable license (similar to what we did for dugite -> dugite-no-gpl, except in that case we just removed the feature instead of replacing)

I already had a quick talk with @akosyakov about this and he/we favor option 2 - ask the Board for an exception. Unlike when we did this for electron, there is no precedent at the Foundation, for jscharset, that could be used to speed-up the approval for the exception. However, unlike electron that is an all-or-nothing proposition, we can perhaps exclude this one single extension and continue with the rest in the meantime, until this is resolved.

Thoughts?

cc: @tsmaeder


(*): contrary to my initial expectations, the built-in VS Code extensions we publish are not self-contained, dependency-wise. When I looked at them it was in the form they are published; the dependencies were not in place then. However when added as dependencies to a NPM project, then their own dependencies are fetched.

Still a bit of good news: to confirm there were no other similar cases, I created an NPM project in which I added all these VS Code built-in extensions - I found no other dependency with a license issue.

@thegecko
Copy link
Member

Is this shipped with VS Code? If so, what do they do?
Can we ask the maintainer for a different license?

@marcdumais-work
Copy link
Contributor

Hi @thegecko

Is this shipped with VS Code?

Yes, I think so, though I was not able to prove it conclusively when I quickly attempted using the VS Code installed on my machine. When I grep under where VS Code is installed on my machine (/usr/share/code), I do see it referenced from .js files.

If so, what do they do?

Nothing special, as far as I can tell. There is no copyright issue as such with distributing LGPL content, as long as the license is respected. My interpretation is that the Eclipse Foundation is just being extra careful with LGPL-licensed content.

IIUC, one potential pitfall with that license: the only way one is allowed to use an LGPL library from a program, that is not itself LGPL, is when the library is dynamically linked; else the program code has to become LGPL or stop using the library. In the JS/TS world, it's believed that calling a library that was pulled from NPM as a dependency is the equivalent of dynamic linking, but I am not sure this is settled copyright law. I am also not sure if this still holds in all cases. E.g. if the program and library end-up built together in one bundle, like it's done for browser applications; this superficially looks to me more similar to static linking. So whether it's used in a Node or Browser app could be a factor.

The bit of uncertainty about all this may be enough to scare the Foundation but not Microsoft?

Can we ask the maintainer for a different license?

Good thought, but in this case it looks like it's not an option; this has been discussed on the project's GitHub, but the maintainers do not own the original copyright of this implementation or of the original (Python) implementation, on which it's based. So they have no liberty to re-license it.

@tsmaeder
Copy link
Contributor

@marcdumais I have no special thoughts about the git extension (not my immediate responsiblity). I'll call attention to the issue in the appropriate che forums.

@vince-fugnitto
Copy link
Member

I assume the following can be potentially removed in the future?:

  • @theia/java
  • @theia/java-debug
  • @theia/debug-nodejs
  • @theia/typescript?
  • @theia/tslint
  • @theia/python

@tolusha
Copy link
Contributor

tolusha commented Dec 2, 2019

@vince-fugnitto
correct

@akosyakov
Copy link
Member Author

We can also replace @theia/merge-conflicts with a VS Code counterpart after theming PR is in.

@vince-fugnitto
Copy link
Member

I assume we can also replace our custom implementation of @theia/editorconfig with that of VS Code? https://marketplace.visualstudio.com/items?itemName=EditorConfig.EditorConfig

@marcdumais-work
Copy link
Contributor

Re-reading this thread, I see I forgot to post an update about [email protected]: we obtained Eclipse Board approval for an exception to use this production dependency even though it's licensed under LGPL. So there is no legal reason not to use the builtin git extension.

@vince-fugnitto
Copy link
Member

Closed by #6933.
Please re-open if there are still items left to complete before we can officially close the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
vscode issues related to VSCode compatibility
Projects
None yet
Development

No branches or pull requests

8 participants