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

Publish Platform Specific Extension #905

Merged
merged 14 commits into from
Jan 26, 2022
Merged

Publish Platform Specific Extension #905

merged 14 commits into from
Jan 26, 2022

Conversation

jpogran
Copy link
Contributor

@jpogran jpogran commented Jan 14, 2022

This converts the extension to use a bundled terraform-ls binary and packages the VSIX as a Platform Specific Extension. A matrix of all supported platforms and architectures run the commands necessary to bundle terraform-ls and the extension into a VSIX. After packaging, it publishes all stored VSIXs to the Marketplace. This results in a VSIX that is ready to run at install time, instead of having to use the network at runtime to download a terraform-ls binary.

This also adds minification as a final step to the interactive development and packaging processes. This now produces a single file extension, compared to previously shipping several thousand files. This results in a much faster installation experience and reduces the deployment size of the extension.

@jpogran jpogran added enhancement New feature or request ci Continuous integration/delivery related labels Jan 14, 2022
@jpogran jpogran requested a review from a team January 14, 2022 18:12
@jpogran jpogran self-assigned this Jan 14, 2022
@jpogran jpogran linked an issue Jan 14, 2022 that may be closed by this pull request
3 tasks
Copy link
Member

@dbanck dbanck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work! I'm amazed that esbuild doesn't need a bulky config file. It looks so lean with your package.json scripts.

I haven't looked a the GitHub workflows yet; as you mentioned, you want to test them first.

I wondered if we could avoid downloading the language server every time we run 'Launch client' in the local development workflow?

.vscode/tasks.json Outdated Show resolved Hide resolved
package.json Show resolved Hide resolved
src/extension.ts Outdated Show resolved Hide resolved
build/downloader.ts Outdated Show resolved Hide resolved
src/extension.ts Outdated Show resolved Hide resolved
@jpogran
Copy link
Contributor Author

jpogran commented Jan 18, 2022

I wondered if we could avoid downloading the language server every time we run 'Launch client' in the local development workflow?

It won't download every time for local development because it uses the npm watch task. On the second run, it's already running so it won't rerun npm watch. If the terminal that's running watch is terminated, then it will redownload. I'm fine with having it exit early if the folder is present.

@jpogran jpogran force-pushed the bundle_terraform_ls branch 3 times, most recently from 54dfdfe to 0916ea1 Compare January 19, 2022 19:50
@jpogran jpogran marked this pull request as ready for review January 20, 2022 13:31
@jpogran jpogran requested a review from a team January 20, 2022 13:32
Copy link
Member

@dbanck dbanck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking the time to update the development docs; this will be very helpful in the future!

Most of the code looks good to me! I've added mainly question to gain a better understanding.

.github/workflows/preview.yml Outdated Show resolved Hide resolved
.github/workflows/preview.yml Show resolved Hide resolved
DEVELOPMENT.md Outdated Show resolved Hide resolved
.github/workflows/preview.yml Show resolved Hide resolved
build/downloader.ts Show resolved Hide resolved
build/preview.ts Outdated Show resolved Hide resolved
build/preview.ts Outdated Show resolved Hide resolved
build/preview.ts Outdated Show resolved Hide resolved
Copy link
Member

@radeksimko radeksimko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for all the work, and esp. for the updated docs!

.github/workflows/preview.yml Outdated Show resolved Hide resolved
.github/workflows/preview.yml Outdated Show resolved Hide resolved
.github/workflows/publish.yml Outdated Show resolved Hide resolved
.github/workflows/publish.yml Outdated Show resolved Hide resolved
DEVELOPMENT.md Outdated Show resolved Hide resolved
DEVELOPMENT.md Outdated Show resolved Hide resolved
DEVELOPMENT.md Outdated Show resolved Hide resolved
build/downloader.ts Outdated Show resolved Hide resolved
jpogran and others added 11 commits January 25, 2022 11:47
This adds a typescript file that downloads a version of terraform-ls that matches the current OS and architecture of the machine running the script. The version of terraform-ls that is chosen is pulled from the `langServer` attribute in the `package.json` file.

This script can be invoked several ways:

```
npm run download:ls

npm run download:ls --target=darwin-x64
```

It can also be combined with `npm package` to download and package an alternate OS and architecture:

```
ls_target=linux-x64 npm run package -- --target=linux-x64

$env:ls_target="linux-x64"; npm run package -- --target=linux-x64
```
Organize npm scripts and link download:ls to watch and package
This converts the extensiom to use the bundled terraform-ls binary instead of one downloaded at runtime. This effectively changes the extension to not download the terraform-ls binary and instead rely on one being present in the `bin` directory inside the extension path.

It also moves @hashicorp/js-releases to a development dependency, which then allows openpgp, semver, and yauzl to be removed. These dependencies are no longer needed in the extension as we no longer download the terraform-ls bianry at runtime.

We still will use @hashicorp/js-releases to download the terraform-ls binary at development and package time.
This updates the publish and preview github actions to package platform specific extensions.

A matrix of all supported platforms and architectures run the commands necessary to bundle terraform-ls and the extension into a VSIX. After packaging, it publishes all stored VSIXs to the Marketplace.
This adds esbuild as a final step to the interactive developement and packaging processes. This produces a single file extension for deployment, compared to several thousand files.

This results in a much faster installation experience and reduces the deployment size of the extension.
Co-authored-by: Radek Simko <radek.simko@gmail.com>
build/downloader.ts Outdated Show resolved Hide resolved
jpogran and others added 2 commits January 26, 2022 08:23
Co-authored-by: Radek Simko <radek.simko@gmail.com>
@jpogran jpogran merged commit 2a38881 into main Jan 26, 2022
Terraform Editor Experience Public Roadmap automation moved this from In Progress to Done Jan 26, 2022
@jpogran jpogran deleted the bundle_terraform_ls branch January 26, 2022 15:01
@jpogran jpogran added this to the 2.20.0 milestone Feb 18, 2022
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
ci Continuous integration/delivery related enhancement New feature or request
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Bundle language server with extension release
3 participants