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

ci: Build the VSCode extension using GitHub Actions #364

Merged
merged 11 commits into from
Apr 19, 2024

Conversation

vogelsgesang
Copy link
Collaborator

This commit adds a Github action which builds the VS Code extension. The .vsix file is uploaded as an artifcat such that it can be easily downloaded from GitHub (in the "Artifacts" section of the action's summary page). Those artifacts are also uploaded as part of pull requests, which makes it easier to quickly download, install and test the extension built from a pull request.

In addition, if the workflow is triggered because a new release was tagged, the .vsix is also attached as an artifact to the newly created release.

As part of this change, the vsce tool is now listed as a devDependency and thereby its exact version is now part of the package-lock.json, making the packaging step more reproducible. The new, recommend way to package the extension is now npm run package.

@vogelsgesang
Copy link
Collaborator Author

@vogelsgesang
Copy link
Collaborator Author

Note that this does not conflict with #340. Even if we run the build as part of a Github Action instead of bazelci, we could still call bazel instead of npm run package.

It brings up the question, though, if we want to keep the bazelci integration at all, or if we want to switch over to Github Actions completely.

@cameron-martin
Copy link
Collaborator

Building in both github actions and bazelci doesn't seem ideal. I wonder how easy it'd be to use the build artifacts from bazelci?

@vogelsgesang
Copy link
Collaborator Author

vogelsgesang commented Apr 4, 2024

Building in both github actions and bazelci doesn't seem ideal.

If you want, I can remove the bazelci config, then we would be 100% on Github

I wonder how easy it'd be to use the build artifacts from bazelci?

I see a couple of challenges with Bazel CI:

  • I cannot experiment with the Bazel CI workflows on their private repository. This makes it extremely hard to iterate on the CI definition. Github Actions are much easier to setup and modify / maintain since they also work for forks and changes can therefore first be tested in a personal fork.
  • We need some authentication token, otherwise we won't be able to access the Github APIs. Afaict, bazelci offers no GH_TOKEN equivalent?
  • Afaict, Bazel CI can only trigger on code changes, but not on "release" events on Github?

What are the benefits of using Bazel CI?

@cameron-martin
Copy link
Collaborator

I'm not sure personally. Maybe some of the other maintainers can give some context on why we (plus a lot of the other repos in this org) use bazelci over github actions? cc @coeuvre @jfirebaugh

This commit adds a Github action which builds the VS Code extension.
The .vsix file is uploaded as an artifcat such that it can be easily
downloaded from GitHub (in the "Artifacts" section of the action's
summary page). Those artifacts are also uploaded as part of pull
requests, which makes it easier to quickly download, install and test
the extension built from a pull request.

In addition, if the workflow is triggered because a new release was
tagged, the .vsix is also attached as an artifact to the newly created
release.

As part of this change, the `vsce` tool is now listed as a devDependency
and thereby its exact version is now part of the `package-lock.json`,
making the packaging step more reproducible. The new, recommend way to
package the extension is now `npm run package`.
@vogelsgesang vogelsgesang force-pushed the avogelsgesang-ci-build branch from 89ccf89 to 235835b Compare April 8, 2024 22:16
@vogelsgesang vogelsgesang marked this pull request as draft April 8, 2024 22:25
@vogelsgesang
Copy link
Collaborator Author

Converting to a draft, since I would still like to make some changes, in case we actually decide that we actually want to start using GitHub Actions for building

@meteorcloudy
Copy link
Contributor

Bazel team here, I think Bazel CI provides better integration with Bazel, like specifying build/test flags and targets without explicitly running the bazel command and some native support for remote caching. But I can see https://github.com/bazelbuild/vscode-bazel/blob/master/.bazelci/presubmit.yml is quite simple and requires more custom setup like installing nodejs, so I'm not against vscode-bazel moving to using GitHub Actions, it could be a better choice.

@cameron-martin
Copy link
Collaborator

I was hoping to start building this extension with bazel, so those features could be useful then. @meteorcloudy do you know if it's possible to get build artifacts out of bazelci, in this case for consuming within GitHub actions?

@meteorcloudy
Copy link
Contributor

What are those build artifacts for? I hope it's not release artifacts?

@meteorcloudy
Copy link
Contributor

meteorcloudy commented Apr 9, 2024

BuildKite does support uploading artifacts after the build, which is later publicly accessible. See https://github.com/bazelbuild/continuous-integration/blob/35fc1da116a96631f83b45217ceb84e6ccee0106/buildkite/bazelci.py#L2659-L2665, but we need to implement a way to do that for artifacts at given paths in bazelci.py

@cameron-martin
Copy link
Collaborator

What are those build artifacts for? I hope it's not release artifacts?

The intention was to use them for this. Is that not a good idea?

@meteorcloudy
Copy link
Contributor

https://buildkite.com/bazel should only be used for testing purposes, because by design it's an untrusted environment as we allow running PR presubmits without any review, so there are definitely security concerns when using it to build release artifacts. For release builds, we have a different Buildkite org: https://buildkite.com/bazel-trusted, but it's only used for google owned projects and only googlers can have access to it.

@cameron-martin
Copy link
Collaborator

Ah okay, is it okay to use GitHub actions to build release artifacts? I think rules_rust does this already if I'm not mistaken.

Also while you're here, do you know if there's any way to get automatically-created PRs (such as #360) past the CLA check? Maybe there's a way to use the @bazel-io account?

@meteorcloudy
Copy link
Contributor

Ah okay, is it okay to use GitHub actions to build release artifacts? I think rules_rust does this already if I'm not mistaken.

Yes, I believe that's fine.

Also while you're here, do you know if there's any way to get automatically-created PRs (such as #360) past the CLA check?

I have no idea, but it's definitely possible. The @dependabot seems to pass the CLA in bazelbuild/continuous-integration#1938

Maybe there's a way to use the @bazel-io account?

Using @bazel-io is probably not possible since that requires generate a access token, we probably cannot share that with non-googler. I'm working on moving projects from bazelbuild to bazel-contrib. I think vscode-bazel should probably be one of them? And hopefully, that'll make such things easier in future.

@vogelsgesang
Copy link
Collaborator Author

I think vscode-bazel should probably be one of them?

I guess this depends on how much Google will be involved in the VS Code extension in the future...
Is Google internally using this extensions? Are there any plans to invest into it? If not, I guess moving this over to bazel-contrib and giving some interested non-Googlers the necessary permissions to sustain the VS Code extension by themselves, would make sense

@meteorcloudy
Copy link
Contributor

Google isn't using this extension internally and we don't have plan or capacity to invest into it so far. I think moving this repo to bazel-contrib provides a better governance model for the repo and it won't prevent possible google contribution in future.

@vogelsgesang
Copy link
Collaborator Author

🤔 do you have any thoughts whether the Marketplace listing would also be handed over to the community? Or would that publishing still be done by Googlers?

@meteorcloudy
Copy link
Contributor

@alexeagle is going through a similar process for setup-bazel on GH marketplace. I do hope the community maintainers can take over this process, so the publisher should change from the Bazel Team to Bazel Contrib or something else. But we can still help verify domain like (contrib.bazel.build) if necessary.

@jfirebaugh
Copy link
Collaborator

Related discussion about transferring repositories to bazel-contrib: https://github.com/orgs/bazelbuild/discussions/2.

.github/workflows/build.yml Outdated Show resolved Hide resolved
RELEASE.md Outdated
2. We publish the extension to the Visual Studio Marketplace so that it can be found in search results and downloaded from Visual Studio Code's Extensions area. This requires publishing rights for the Bazel organization on the Visual Studio Marketplace. Florian Weikert <[email protected]> has handled recent versions.

You can now delete the .vsix file if you wish; it will not be used when publishing to the marketplace.
1. We create a .vsix package to upload as a GitHub release, since this is a useful archiving method and it allows users to download and roll back to a previous version of the plugin if necessary. This is automated by the "Build VS Code extension" GitHub action which will automatically run as soon as a new GitHub release gets created.
Copy link
Contributor

Choose a reason for hiding this comment

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

This paragraph seems to maybe be out of date? I don't see the workflow using a "Build VS Code extension" action in use here, and frankly don't really see why you'd need one; the workflow is simple enough as is.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I confused the terms "action" and "workflow" here. It is the workflow which is named "Build VS Code extension". I udpated the README

.github/workflows/build.yml Outdated Show resolved Hide resolved
@vogelsgesang vogelsgesang force-pushed the avogelsgesang-ci-build branch from 8dcbf11 to a6c917c Compare April 17, 2024 22:36
@vogelsgesang
Copy link
Collaborator Author

Thanks for your review, @adam-azarchs. Very good points, I incorporated them into the updated version of this pull request

@vogelsgesang
Copy link
Collaborator Author

@jfirebaugh @cameron-martin How do we want to move ahead with this?

During the last couple of weeks, I think I got a better understanding of the pros/cons of Bazel CI vs. GitHub. Below my updated understanding.

Pro Bazel CI:

  • Runs against commits to the main Bazel repository and not only for commits to the vscode-bazel repository. This allows rule authors to detect breakages earlier and allows Bazel contributors to be aware whether their changes are breaking any of the open-source rules.
  • Remote cache: Bazel CI comes with a pre-configured remote cache. I am not aware of an equivalent offering which could be used part of GitHub actions.
  • Machines provided by Google. Those machines seem to be more powerful than the default open-source Github workers (?)

Pro GitHub:

  • Proper sandboxing: In contrast to Bazel CI, GitHub actions are properly sandboxed. In Bazel CI, a malicious PR could, e.g., poison the remote cache or do something similarly nefarious to inject malware into artifacts subsequently built on the same machine. Hence, we should not
  • Changes to the CI config can be verified in a personal fork before merging. This is not possible with Bazel CI because it does not run for personal forks.
  • Deeper integration with Github: GH_TOKEN available out-of-the-box. Can also trigger flows on "release" events and not only on "pre-submit"

Maybe I am still forgetting some pros & cons - not sure...

But under the assumption that this list is complete, I would still lean towards using GitHub actions going forward. I don't think this extension benefits much from the Bazel CI benefits, because

  • it is rather small and quick to build. Hence, the remote caching and the faster build machines are not so important to this repo
  • this extension is unlikely to be broken by upstream commits. We aren't writing rules, and are primarily using the bazel command line, which seems to be more stable than the Starlark interfaces

@vogelsgesang vogelsgesang changed the title ci: Build the VSCode extension as part of a Github Action ci: Build the VSCode extension using Github Actions Apr 18, 2024
@vogelsgesang vogelsgesang changed the title ci: Build the VSCode extension using Github Actions ci: Build the VSCode extension using GitHub Actions Apr 18, 2024
@cameron-martin
Copy link
Collaborator

I'd also lean towards using GitHub actions, mainly due to it being the only reasonable way to do automated releases and the downsides aren't significant.

@jfirebaugh
Copy link
Collaborator

I agree with your assessment @vogelsgesang and also support switching to GitHub Actions.

@vogelsgesang
Copy link
Collaborator Author

I updated this commit to fix the linter and also add the prettier-checks.
It should be ready for review again

@vogelsgesang vogelsgesang marked this pull request as ready for review April 18, 2024 18:08
@vogelsgesang
Copy link
Collaborator Author

@vogelsgesang
Copy link
Collaborator Author

It seems to get this merged, we will first need to remove this project from Bazel CI and remove buildkite from the required checks. @meteorcloudy can you help with this?

@meteorcloudy meteorcloudy enabled auto-merge (squash) April 18, 2024 18:53
@meteorcloudy meteorcloudy disabled auto-merge April 18, 2024 18:53
@meteorcloudy
Copy link
Contributor

I removed buildkite and added "Build VS Code extension" for presubmit check

if: ${{ github.event_name == 'release' }}
shell: bash
run: |
upload_url=`echo '${{ github.event.release.upload_url }}' | cut -f1 -d"{"`;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we cut the upload URL at { here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

One of those instances where I am unhappy with my former self for not commenting the code properly... I just copied this over from https://github.com/vogelsgesang/hyper-ir-lsp/blob/main/.github/workflows/build.yml, but don't remember anymore why I wrote it this way in the first place.

I remember this to be some "poor man's template parsing". I think the upload_url was something like github.com/bazelbuild/vscode-bazel/releases/{some_placeholder} and I wanted to strip the placeholder.

I will reverse-engineer this later tonight and add comments to the code

Copy link
Collaborator

Choose a reason for hiding this comment

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

Using the gh command line tool is maybe clearer here, like this. You should be able to get the tag name as github.event.release.tag_name, and avoid any poor man's template parsing.

Choose a reason for hiding this comment

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

Did you consider an existing composite Action to do this? For bazel rulesets, we use this one: https://github.com/bazel-contrib/.github/blob/master/.github/workflows/release_ruleset.yaml#L69-L77

(In general I think it would be nice for this automation to be similar to that of rulesets where possible)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@alexeagle if understand correctly, the action-gh-release@v1 creates a new release. What we are trying to achieve here is slightly different, though: We want to attach artifacts to an already existing release

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@cameron-martin good point. I updated the PR to use gh release upload instead

.github/workflows/build.yml Show resolved Hide resolved
@vogelsgesang vogelsgesang force-pushed the avogelsgesang-ci-build branch from b49f8b2 to 55795f4 Compare April 19, 2024 18:27
Copy link
Collaborator

@cameron-martin cameron-martin left a comment

Choose a reason for hiding this comment

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

Do we have a way of knowing if the release workflow works? I found this very hard on bazel-lsp - I only got feedback after doing a release, so it took quite a few iterations to get right.

@vogelsgesang
Copy link
Collaborator Author

I have been testing this flow on my personal fork and created a couple of "releases" there

https://github.com/vogelsgesang/vscode-bazel/releases/tag/test4

https://github.com/vogelsgesang/vscode-bazel/actions/runs/8760198382/job/24044751663

@vogelsgesang vogelsgesang merged commit 8972b0e into bazel-contrib:master Apr 19, 2024
2 checks passed
@vogelsgesang vogelsgesang deleted the avogelsgesang-ci-build branch April 19, 2024 22:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants