WIP - package and publish built-ins individually #15

marcdumais-work · 2019-12-05T17:21:27Z

I added a script, src/package-vsix.js, that packages each built-in extension in its own .vsix.

Run it like so:

$> yarn
$> yarn package-vsix:next

The generated packages will be under ./out/

TODO:

handle publishing to GH releases or our own registry
figure-out the version we should apply to the vsix files - as a first draft I kept the same we had for the npm packages
clean-up npm publishing script and related bits, if no longer used
...

Signed-off-by: Marc Dumais [email protected]

marcdumais-work · 2019-12-10T12:10:08Z

I am having issues with packaging the Typescript LS in a way that it can start:

Relevant comment from #13 (comment):

For typescript we have to copy content of vscode/extensions/node_modules to vscode-builtin-extensions/extensions/typescript-language-features/node_modules to distribute default ts compiler properly.

I am having a couple of issues with this part, once node_modules is copied to typescript-language-features:

I can't yet have vsce include the node_modules folder in the .vsix: it's stripped even if I add it as a reverse exclusion in a .vscodeignore file. If I rename it to something else, then it's preserved, but I would need to rename it back once the extension is installed and I have not found a way to to that yet
If I manually rename-back the folder once the extension is deployed to /tmp/vscode-unpacked/... the LS still doesn't start. I think it's due to the use of ".." on this line, making it look for node_modules one folder up (scroll to the right - it's a long line:

https://github.com/microsoft/vscode/blob/master/extensions/typescript-language-features/src/utils/versionProvider.ts#L149

if I change this to a single dot, then it works.
e.g:
this.getContributedVersion(TypeScriptVersionSource.Bundled, 'vscode.typescript-language-features', ['.', 'node_modules'])
I can as well change node_modules to something else that vsce does not exclude to fix the other issue above, renaming the packaged node_modules.

Is strategic patching of the vscode build-in extensions a good way forward in cases like this? It would for sure be more fragile if the upstream code changes...

Any ideas?

marcdumais-work · 2019-12-10T12:44:01Z

About releasing on GH: I have tried a couple of npm packages that help with that. It looks like we need to tie a GH release to a tag in the repo. ATM the built-in's version (at least for next) is based on the vscode commit SHA that we packaged, and in CI we reset vscode to its latest master. This means that we could have multiple next releases of the built-ins for the same vscode-builtin-extensions repo commit/tag, which I think does not work.

One option is to let the publishing tool, as part of the CI, do a release commit, tag it and push it to our repo. This way we'll always have a distinct commit/tag that corresponds to a release of the built-ins, to publish against.

Does that sound like a reasonable way forward?

benoitf · 2019-12-11T07:16:27Z

One option is to let the publishing tool, as part of the CI, do a release commit, tag it and push it to our repo. This way we'll always have a distinct commit/tag that corresponds to a release of the built-ins, to publish against.

sounds good to me. would be good to have releases for upstream releases of VS Code as well
1.39.0, 1.39.1, etc

akosyakov · 2019-12-11T08:44:09Z

@marcdumais-work Could we split to 2 steps:

package as vsix and publish as GitHub releases
remove publishing to npmjs

I would prefer if you just work on master without PRs and make it work for vsix GitHub releases. As far as this approach can do all the same what works for npmjs, we can remove npmjs support. Till then just let it be.

How to version can be the same for now for simplicity. Let's work on it after release workflow works for vxis files with Github ok? The same for publishing to our registry, let's make it another step later.

I hope such approach is good enough to unblock you from reviews.

marcdumais-work · 2019-12-11T12:41:11Z

@marcdumais-work Could we split to 2 steps:

package as vsix and publish as GitHub releases

remove publishing to npmjs

I would prefer if you just work on master without PRs and make it work for vsix GitHub releases. As far as this approach can do all the same what works for npmjs, we can remove npmjs support. Till then just let it be.

How to version can be the same for now for simplicity. Let's work on it after release workflow works for vxis files with Github ok? The same for publishing to our registry, let's make it another step later.

I hope such approach is good enough to unblock you from reviews.

Sounds good. So I'll just push to master without review, just being careful not to break npm publishing, which we will remove later, when not needed anymore.

marcdumais-work · 2019-12-11T12:44:51Z

sounds good to me. would be good to have releases for upstream releases of VS Code as well
1.39.0, 1.39.1, etc

+1

Builtins releases based on proper vscode releases feel more like "latest" releases, rather than "next" releases, based on an arbitrary vscode commit that happens to be the latest master at the time when CI runs.

So releases of builtins based on vscode releases could be done manually, at least to start-with.

akosyakov · 2019-12-11T13:25:29Z

Builtins releases based on proper vscode releases feel more like "latest" releases, rather than "next" releases, based on an arbitrary vscode commit that happens to be the latest master at the time when CI runs.

We should not switch latest though without testing.

tolusha · 2019-12-12T07:46:31Z

@marcdumais-work
How I can generate manually vsix files for VS Code v1.39.1 ?

marcdumais-work · 2019-12-12T12:13:20Z

@marcdumais-work
How I can generate manually vsix files for VS Code v1.39.1 ?

Hi @tolusha ,

There is still a bit of manual intervention needed to obtain working typescript language support. What I can do is try for a first manual "latest" release, targeting 1.39.1, and publish the result on this repo's releases tab.

I'll try to do so today.

marcdumais-work · 2019-12-12T12:22:30Z

@tolusha

If you'd like to try for yourself, starting from the PR's code, other than the change above regarding typescript language support, I think you only need to reset the vscode git submodule to the version you target and do:

$> <clone repo, checkout this PR, initialize repo according to README>
$> cd <repo>/vscode; git checkout 1.39.1; cd ..
$> yarn
$> yarn package-vsix:latest

The .vsix will be in the ./out folder.

tolusha · 2019-12-12T15:40:48Z

@marcdumais-work publishing results implies creating a tag. Do you mind?

Signed-off-by: Marc Dumais <[email protected]>

marcdumais-work · 2019-12-12T16:37:04Z

@marcdumais-work publishing results implies creating a tag. Do you mind?

Ok with me. But please pull the latest version of this PR - I added one more work-around during vsix packaging that you'll need for Typescript to work.

edit: you should not have to manually change anything with the latest version.

src/package-vsix.js

tolusha · 2019-12-13T07:06:21Z

@marcdumais-work
I don't have permissions. :(

benoitf · 2019-12-13T07:56:40Z

should be fixed now @tolusha

tolusha · 2019-12-13T08:18:57Z

Error while starting emet extension
Node modules are missed.
https://registry.npmjs.org/@theia/vscode-builtin-emmet/-/vscode-builtin-emmet-0.3.0-next.608ac23340.tgz

Uncaught Error: Cannot find module 'vscode-emmet-helper'
Error: Cannot find module 'vscode-emmet-helper'
    at Function.Module._resolveFilename (:3000/internal/modules/cjs/loader.js:582)
    at Function.Module._load (:3000/internal/modules/cjs/loader.js:508)
    at Function.module._load (:3000/home/tolusha/projects/theia-ide/theia/packages/plugin-ext-vscode/lib/node/plugin-vscode-init.js:80)
    at Module.require (:3000/internal/modules/cjs/loader.js:637)
    at require (:3000/internal/modules/cjs/helpers.js:22)
    at Object.<anonymous> (:3000/tmp/vscode-unpacked/emmet-1.39.2.vsix/extension/dist/extension.js:1)
    at n (:3000/tmp/vscode-unpacked/emmet-1.39.2.vsix/extension/dist/extension.js:1)
    at Object.t.getEmmetHelper (:3000/tmp/vscode-unpacked/emmet-1.39.2.vsix/extension/dist/extension.js:1)
    at t.DefaultCompletionItemProvider.provideCompletionItemsInternal (:3000/tmp/vscode-unpacked/emmet-1.39.2.vsix/extension/dist/extension.js:1)
    at t.DefaultCompletionItemProvider.provideCompletionItems (:3000/tmp/vscode-unpacked/emmet-1.39.2.vsix/extension/dist/extension.js:1)
    at :3000/vs/editor/editor.main.js:2549

akosyakov · 2019-12-13T11:20:54Z

@tolusha @benoitf @marcdumais-work Is there a way to remove a release?

As I said don't publish broke stuff, publish with next against commit, then test, if it is fine publish with a proper tag. I have not seen anywhere that there was an agreement to align with VS Code versions. It was said that first everything should work like as currently working for npmjs on master, after that we can look into versioning of properly tested artifacts.

akosyakov · 2019-12-13T11:25:01Z

I've deleted all releases, you can create vsix files locally till a proper solution is developed.

marcdumais-work · 2019-12-13T12:26:48Z

It was said that first everything should work like as currently working for npmjs on master, after that we can look into versioning of properly tested artifacts.

I have not tried what @tolusha had published. But when I test locally, the currently generated vsix work as well (or as badly - there are a few that generate exceptions) as the current master. So it's looking promising.

@akosyakov I seem to remember a test suite that we can use to test vscode extensions. Would that be useful with the built-ins?

akosyakov · 2019-12-13T14:34:22Z

@akosyakov I seem to remember a test suite that we can use to test vscode extensions. Would that be useful with the built-ins?

Not really, it does not test functionality of built-in extensions. Let's work on testing afterwards. I hope to finish vscode theming this month and work on the integration testing framework in Jan, with this in place we can write tests for built-in extensions. Also some built-in extensions can have tests already and some are themes which is not really auto testable.

I would rather focus on getting the first cut and be able to publish 0.2.1 and nightly builds as GH releases.

marcdumais-work · 2019-12-17T20:07:56Z

Hi @tolusha ,

I just did a manual pre-release on GitHub:
https://github.com/theia-ide/vscode-builtin-extensions/releases/tag/v1.39.1-prel

As discussed in today's dev-meeting, I have given it a version number that points to the vscode version the built-ins are from.

Let's use this release as a base to test the built-in. When we are satisfied, I can release a "solid" v1.39.1

@akosyakov I hope I did this ok: in the tagged release commit, I committed the version in src/package-vsix.js to what I used for this release and I also committed the vscode submodule to use 1.39.1; this way the build/release should be reproducible (vscode comit is reset to latest master anyway when publishing to npm, so it should not adversely affect npm).

marcdumais-work · 2019-12-17T20:08:50Z

closing this PR since we agreed that I would push directly to master, which was done for the packaging part. Publish of next TBD.

marcdumais-work · 2019-12-17T20:15:07Z

Error while starting emet extension
Node modules are missed.
https://registry.npmjs.org/@theia/vscode-builtin-emmet/-/vscode-builtin-emmet-0.3.0-next.608ac23340.tgz

Uncaught Error: Cannot find module 'vscode-emmet-helper'
Error: Cannot find module 'vscode-emmet-helper'
    at Function.Module._resolveFilename (:3000/internal/modules/cjs/loader.js:582)
    at Function.Module._load (:3000/internal/modules/cjs/loader.js:508)
    at Function.module._load (:3000/home/tolusha/projects/theia-ide/theia/packages/plugin-ext-vscode/lib/node/plugin-vscode-init.js:80)
    at Module.require (:3000/internal/modules/cjs/loader.js:637)
    at require (:3000/internal/modules/cjs/helpers.js:22)
    at Object.<anonymous> (:3000/tmp/vscode-unpacked/emmet-1.39.2.vsix/extension/dist/extension.js:1)
    at n (:3000/tmp/vscode-unpacked/emmet-1.39.2.vsix/extension/dist/extension.js:1)
    at Object.t.getEmmetHelper (:3000/tmp/vscode-unpacked/emmet-1.39.2.vsix/extension/dist/extension.js:1)
    at t.DefaultCompletionItemProvider.provideCompletionItemsInternal (:3000/tmp/vscode-unpacked/emmet-1.39.2.vsix/extension/dist/extension.js:1)
    at t.DefaultCompletionItemProvider.provideCompletionItems (:3000/tmp/vscode-unpacked/emmet-1.39.2.vsix/extension/dist/extension.js:1)
    at :3000/vs/editor/editor.main.js:2549

I think I know the issue - vsce removes node_modules folders when packaging extensions. I had the same issue with typescript-language-features, that I fixed by patching the compiled extension. Maybe something similar can be done for emmet.

marcdumais-work · 2019-12-18T12:41:48Z

@akosyakov I have a question: ATM, if I look at this repo after build, the vscode built-ins are in <repo>/vscode-builtin-extensions/extensions/. Under there, only 2 extensions have a node_modules: emmet and (through manual intervention on our part, and renamed to deps) typescript-language-features.

However some of the other extensions do have a node_modules when built from the vscode git submodule: <repo>/vscode/extensions/:

[...]/vscode-builtin-extensions/vscode/extensions$ find . -maxdepth 2  -name node_modules 
./vscode-api-tests/node_modules
./vscode-test-resolver/node_modules
./debug-server-ready/node_modules
./css-language-features/node_modules
./php-language-features/node_modules
./typescript-language-features/node_modules
./node_modules
./json-language-features/node_modules
./emmet/node_modules
./grunt/node_modules
./jake/node_modules
./markdown-language-features/node_modules
./extension-editing/node_modules
./html-language-features/node_modules
./npm/node_modules
./vscode-colorize-tests/node_modules
./image-preview/node_modules
./configuration-editing/node_modules
./git/node_modules
./debug-auto-launch/node_modules
./gulp/node_modules
./merge-conflict/node_modules
./git-ui/node_modules

I have not checked them all, but I know at least some have runtime dependencies in these node_modules folders - are they not needed? Do we rely on them being present by chance in the Theia application where the built-ins will be used?

akosyakov · 2019-12-19T11:15:01Z

I have not checked them all, but I know at least some have runtime dependencies in these node_modules folders - are they not needed? Do we rely on them being present by chance in the Theia application where the built-ins will be used?

It can be dev dependencies and binaries?

marcdumais-work · 2019-12-19T11:52:27Z

It can be dev dependencies and binaries?

Most of them, I think so. But not all:

https://github.com/microsoft/vscode/blob/master/extensions/git/package.json#L1758-L1766
https://github.com/microsoft/vscode/blob/master/extensions/json-language-features/package.json#L116-L121

marcdumais-work · 2019-12-19T13:00:47Z

Ah, I think I understand what's happening: each built-in extension's node_modules content gets webpacked along with the extension:

https://github.com/microsoft/vscode/blob/master/build/lib/extensions.ts#L243

Edit: confirmed: looking in built extension's dist/extension.js.map, I can see some node_modules code.

marcdumais-work force-pushed the individual-vsix-packaging branch 3 times, most recently from bb9d30f to 181f523 Compare December 5, 2019 18:27

marcdumais-work changed the title ~~WIP - package and publish built-ins to GH releases~~ WIP - package and publish built-ins Dec 5, 2019

marcdumais-work force-pushed the individual-vsix-packaging branch 6 times, most recently from 8062362 to e328b10 Compare December 6, 2019 02:41

marcdumais-work changed the title ~~WIP - package and publish built-ins~~ WIP - package and publish built-ins individually Dec 6, 2019

marcdumais-work force-pushed the individual-vsix-packaging branch 3 times, most recently from 6fb5f17 to 14c4f91 Compare December 6, 2019 20:12

akosyakov mentioned this pull request Dec 9, 2019

Javascript completion for react components does not display certain completion proposals eclipse-theia/theia#6720

Open

marcdumais-work force-pushed the individual-vsix-packaging branch from 14c4f91 to 3f85b75 Compare December 10, 2019 12:29

benoitf mentioned this pull request Dec 11, 2019

Allow to use VS Code builtin extensions in che-theia eclipse-che/che-theia#572

Closed

marcdumais-work force-pushed the individual-vsix-packaging branch from 3f85b75 to dea07ff Compare December 12, 2019 12:07

WIP - package and publish built-ins individually

169967c

Signed-off-by: Marc Dumais <[email protected]>

marcdumais-work force-pushed the individual-vsix-packaging branch from dea07ff to 169967c Compare December 12, 2019 16:35

akosyakov reviewed Dec 13, 2019

View reviewed changes

src/package-vsix.js Show resolved Hide resolved

akosyakov reviewed Dec 13, 2019

View reviewed changes

src/package-vsix.js Show resolved Hide resolved

marcdumais-work closed this Dec 17, 2019

marcdumais-work deleted the individual-vsix-packaging branch December 17, 2019 20:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP - package and publish built-ins individually #15

WIP - package and publish built-ins individually #15

marcdumais-work commented Dec 5, 2019 •

edited

Loading

marcdumais-work commented Dec 10, 2019

marcdumais-work commented Dec 10, 2019

benoitf commented Dec 11, 2019

akosyakov commented Dec 11, 2019 •

edited

Loading

marcdumais-work commented Dec 11, 2019

marcdumais-work commented Dec 11, 2019 •

edited

Loading

akosyakov commented Dec 11, 2019

tolusha commented Dec 12, 2019

marcdumais-work commented Dec 12, 2019 •

edited

Loading

marcdumais-work commented Dec 12, 2019

tolusha commented Dec 12, 2019

marcdumais-work commented Dec 12, 2019 •

edited

Loading

tolusha commented Dec 13, 2019

benoitf commented Dec 13, 2019

tolusha commented Dec 13, 2019

akosyakov commented Dec 13, 2019 •

edited

Loading

akosyakov commented Dec 13, 2019 •

edited

Loading

marcdumais-work commented Dec 13, 2019

akosyakov commented Dec 13, 2019 •

edited

Loading

marcdumais-work commented Dec 17, 2019

marcdumais-work commented Dec 17, 2019

marcdumais-work commented Dec 17, 2019

marcdumais-work commented Dec 18, 2019

akosyakov commented Dec 19, 2019

marcdumais-work commented Dec 19, 2019

marcdumais-work commented Dec 19, 2019 •

edited

Loading

WIP - package and publish built-ins individually #15

WIP - package and publish built-ins individually #15

Conversation

marcdumais-work commented Dec 5, 2019 • edited Loading

marcdumais-work commented Dec 10, 2019

marcdumais-work commented Dec 10, 2019

benoitf commented Dec 11, 2019

akosyakov commented Dec 11, 2019 • edited Loading

marcdumais-work commented Dec 11, 2019

marcdumais-work commented Dec 11, 2019 • edited Loading

akosyakov commented Dec 11, 2019

tolusha commented Dec 12, 2019

marcdumais-work commented Dec 12, 2019 • edited Loading

marcdumais-work commented Dec 12, 2019

tolusha commented Dec 12, 2019

marcdumais-work commented Dec 12, 2019 • edited Loading

tolusha commented Dec 13, 2019

benoitf commented Dec 13, 2019

tolusha commented Dec 13, 2019

akosyakov commented Dec 13, 2019 • edited Loading

akosyakov commented Dec 13, 2019 • edited Loading

marcdumais-work commented Dec 13, 2019

akosyakov commented Dec 13, 2019 • edited Loading

marcdumais-work commented Dec 17, 2019

marcdumais-work commented Dec 17, 2019

marcdumais-work commented Dec 17, 2019

marcdumais-work commented Dec 18, 2019

akosyakov commented Dec 19, 2019

marcdumais-work commented Dec 19, 2019

marcdumais-work commented Dec 19, 2019 • edited Loading

marcdumais-work commented Dec 5, 2019 •

edited

Loading

akosyakov commented Dec 11, 2019 •

edited

Loading

marcdumais-work commented Dec 11, 2019 •

edited

Loading

marcdumais-work commented Dec 12, 2019 •

edited

Loading

marcdumais-work commented Dec 12, 2019 •

edited

Loading

akosyakov commented Dec 13, 2019 •

edited

Loading

akosyakov commented Dec 13, 2019 •

edited

Loading

akosyakov commented Dec 13, 2019 •

edited

Loading

marcdumais-work commented Dec 19, 2019 •

edited

Loading