bazelbuild / vscode-bazel

Bazel support for Visual Studio Code
https://marketplace.visualstudio.com/items?itemName=BazelBuild.vscode-bazel
Apache License 2.0
231 stars 76 forks source link

ci: Build the VSCode extension using GitHub Actions #364

Closed vogelsgesang closed 2 months ago

vogelsgesang commented 3 months ago

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 commented 3 months ago

See https://github.com/vogelsgesang/vscode-bazel/actions/runs/8530517980 for an example run

vogelsgesang commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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:

What are the benefits of using Bazel CI?

cameron-martin commented 3 months ago

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

vogelsgesang commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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

meteorcloudy commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 https://github.com/bazelbuild/vscode-bazel/pull/360) past the CLA check?

I have no idea, but it's definitely possible. The @dependabot seems to pass the CLA in https://github.com/bazelbuild/continuous-integration/pull/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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

🤔 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 commented 2 months ago

@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 commented 2 months ago

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

vogelsgesang commented 2 months ago

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

vogelsgesang commented 2 months ago

@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:

Pro GitHub:

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

cameron-martin commented 2 months ago

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 commented 2 months ago

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

vogelsgesang commented 2 months ago

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

vogelsgesang commented 2 months ago

See https://github.com/vogelsgesang/vscode-bazel/actions/runs/8742637975 for an up-to-date run

vogelsgesang commented 2 months ago

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 commented 2 months ago

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

vogelsgesang commented 2 months ago

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