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

gitpod support #4700

Closed
wants to merge 6 commits into from
Closed

gitpod support #4700

wants to merge 6 commits into from

Conversation

mockersf
Copy link
Member

@mockersf mockersf commented May 8, 2022

Objective

  • Being able to use Bevy in gitpod

Solution

  • Add a gitpod config file. As I want to use it for wasm, it install everything for wasm and run a first build. The environment gets ready in less than a minute, and the first wasm build in 4 minutes.
  • If the gitpod integration is fully enabled, it would be much faster, and a button would be added to every PR to open that PR in gitpod

You can try this PR in gitpod at https://gitpod.io/#https://github.com/bevyengine/bevy/pull/4700

Comment on lines +7 to +9
addCheck: false
addComment: false
addBadge: true
Copy link
Member

Choose a reason for hiding this comment

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

I feel like the check version would be less intrusive here, if it correctly links to the same point. Looking at the docs, the badge doesn't look too bad though.

Copy link
Member Author

Choose a reason for hiding this comment

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

addCheck would make CI fail if pre-building for gitpod fails
addComment would add a comment to every PR
addBadge adds a button on every PR, which doesn't send notifications

@@ -0,0 +1,30 @@
github:
Copy link
Member

Choose a reason for hiding this comment

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

I don't like adding this new file to the root. In theory, gitpod should behave like bors and allow this file to live in the .github folder.

I would assume it's an advertising feature for gitpod to not allow it to live in a non-root folder, but that's a scummy practise. See also gitpod-io/gitpod#729, which still doesn't suggest to use .github.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks @cart for putting a comment through.

I don't expect much movement though, unfortunately.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks @cart for putting a comment through.

I don't expect much movement though, unfortunately.

cargo quickinstall wasm-bindgen-cli
cargo quickinstall cargo-watch
init: |
cargo run -p build-wasm-example -- lighting
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to pre-suppose which example is relevant. It would be very cool, for example, to select a relevant example which has been updated by the given PR. Or at least to have a dropdown with all the examples.

OTOH, this might be the best possible option within the technical constraints of the gitpod platform.

Copy link
Member Author

Choose a reason for hiding this comment

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

The lighting example is one where a lot of things could go wrong, so it's a good first pick. My goal was mostly to cache everything for a wasm build, so why not go all the way to build an example

Copy link

@loujaybee loujaybee Jun 29, 2022

Choose a reason for hiding this comment

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

select a relevant example which has been updated by the given PR. Or at least to have a dropdown with all the examples.

Thought: You could do this via a regular HTML form (hosted wherever), or a series of gitpod links hardcoded with an environment variable that triggers if/else logic in your configuration

See: https://www.gitpod.io/docs/environment-variables#providing-one-time-environment-variables-via-url

An example of a community doing this is drupal / drupalpod: https://github.com/shaal/DrupalPod


- name: Install linux dependencies
before: |
sudo apt-get install --no-install-recommends -yq libasound2-dev libudev-dev
Copy link
Member

Choose a reason for hiding this comment

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

Since we're building for wasm, why is this needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

for rust-analyzer. I guess I could add config to have it work in wasm by default, but I'm not sure where that would live

Copy link
Contributor

Choose a reason for hiding this comment

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

That is rust-analyzer.cargo.target at wherever gitpod allow configuring rust-analyzer. For vscode that would be .vscode/settings.json.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I added the file correctly, but still keeping the linux dependencies because they add almost nothing to the build time and they are still helpful

@alice-i-cecile alice-i-cecile added A-Build-System Related to build systems or continuous integration A-Meta About the project itself labels May 9, 2022
@DJMcNab DJMcNab added the X-Controversial There is active debate or serious implications around merging this PR label May 9, 2022
@alice-i-cecile
Copy link
Member

Testing this out I'm generally happy with the ease of use.

Graphics and windowing aren't working though unfortunately, which limits the utility as both a reviewing and teaching tool.

@IceSentry
Copy link
Contributor

@alice-i-cecile you can run any examples that can be ran in wasm, you just need to build it with the new tool this PR adds. So you can also test graphics, with all the limitations wasm implies though.

following command.

```sh
cargo run -p build-wasm-example -- lighting
Copy link
Contributor

@IceSentry IceSentry May 9, 2022

Choose a reason for hiding this comment

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

I would add a note somewhere near here that you can use this to change the default example that is being built for gitpod

Copy link
Member Author

Choose a reason for hiding this comment

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

to be honest I'm not completely sure how the gitpod integration will work out and would like it to be enabled before updating the "contributing" guide with this kind of info

Copy link
Contributor

@IceSentry IceSentry left a comment

Choose a reason for hiding this comment

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

I don't have any opinions on the actual configs, but it is really nice to be able to actually run code without having to pull the pr locally.

I tested it and I didn't experience any issues other than being a bit lost as to how this all works.

@mockersf
Copy link
Member Author

mockersf commented May 9, 2022

Testing this out I'm generally happy with the ease of use.

Graphics and windowing aren't working though unfortunately, which limits the utility as both a reviewing and teaching tool.

As IceSentry said, it's set up for building examples as wasm easily, and there's already a web server running to serve them.

As for teaching, I'm working on another repo outside Bevy org for that, I think it would be nice with auto rebuild and auto reload of the wasm on every code change. Having that outside of the Bevy repo will make it a lot simpler with less code to be scared of.

@IceSentry
Copy link
Contributor

Since the gitpod part is controversial. Can you extract the wasm build tool to a separate PR since that part is not really controversial and makes building wasm simpler.

@cart
Copy link
Member

cart commented May 16, 2022

Some assorted thoughts/questions:

  1. Unless we can find a way to run the wasm example from gitpod (it seems like we're just building the code here), idk if the extra complexity here is worth the value this adds over normal CI. You can already press . on any PR to open up the code in a browser-hosted vscode instance. If you can't run a bevy app in the browser, this isn't a replacement for a "change code" -> "build" -> "test" cycle.
  2. This seems identical to github's codespaces. Are there any distinguishing features? What are the pricing models like?

@cart
Copy link
Member

cart commented May 16, 2022

ohwait it is running a webserver. Maybe it is a replacement for a build/run/test cycle 😄

@cart
Copy link
Member

cart commented May 16, 2022

Alright yeah this is pretty cool

@cart
Copy link
Member

cart commented May 16, 2022

Since the gitpod part is controversial. Can you extract the wasm build tool to a separate PR since that part is not really controversial and makes building wasm simpler.

Yeah I'd merge this asap if it was split out.

@cart
Copy link
Member

cart commented May 16, 2022

But yeah the biggest things holding me back at this point are:

  1. I'd like to know what the pricing and limitations are here. If we flood this with our developers, what happens?
  2. If this isn't free, we should compare it to github codespaces (and see if that has equivalent webhosting features).

I also agree with @DJMcNab that the extra dotfile at the root is regrettable.

@mockersf
Copy link
Member Author

mockersf commented May 16, 2022

Alright yeah this is pretty cool

I've set up this repo: https://github.com/vleue/bevy_playground with the full experience:

  • gitpod prebuild, so Bevy wasm is already built when you start
  • browser window that opens on start with the wasm running
  • cargo watch that will rebuild the project in wasm for every code change
  • integrated live.js to automatically reload the wasm in the browser when the file changes

From a change in the code to it being visible in the browser, it's around 4 seconds. I could probably reduce by a second by making cargo-watch and live.js check more often but that could be quite more heavy. I want to try writing a tutorial using that.

I'd like to know what the pricing and limitations are here. If we flood this with our developers, what happens?

It is very similar. The two advantages of gitpod are prebuild and free tier
From their marketing: https://www.gitpod.io/vs/github-codespaces

Prebuild is a way to have it already build the environment and cache it for us.
You have 50 hours per month free in gitpod. A workspace is using that time from when you open it to 3 minutes after you close the browser tab. The billing is linked to each GitHub user, not to Bevy. Worst case, you can host gitpod yourself wherever you want and it's free.

GitHub codespaces is closed, and paying from the start. Billing would be to Bevy, not split on each user. You can have similar result to the prebuild feature by using a custom docker image.

I also agree with @DJMcNab that the extra dotfile at the root is regrettable.

Yup I agree

@mockersf
Copy link
Member Author

oh and most big Bevy contributors could be eligible to the open source free plan: https://www.gitpod.io/for/opensource

@mockersf
Copy link
Member Author

Since the gitpod part is controversial. Can you extract the wasm build tool to a separate PR since that part is not really controversial and makes building wasm simpler.

Yeah I'd merge this asap if it was split out.

done in #4776

bors bot pushed a commit that referenced this pull request May 17, 2022
# Objective

- add an helper to build examples in wasm (from #4700)

## Solution

- `cargo run -p build-wasm-example -- lighting`
@alice-i-cecile
Copy link
Member

alice-i-cecile commented May 17, 2022

oh and most big Bevy contributors could be eligible to the open source free plan: https://www.gitpod.io/for/opensource

Just submitted an application; I'll report back on how it goes.

EDIT: approved; that was a painless process.

exjam pushed a commit to exjam/bevy that referenced this pull request May 22, 2022
# Objective

- add an helper to build examples in wasm (from bevyengine#4700)

## Solution

- `cargo run -p build-wasm-example -- lighting`
@cart
Copy link
Member

cart commented Jun 27, 2022

Sorry for letting this sit. I've been consolidating my thoughts over time:

This seems like a useful workflow. It enables people to share around links to bevy branches and live-edit them. That being said, we shouldn't be in the business of supporting every useful workflow. This is why we don't check in things like .vscode editor config. There are too many editors and too many ways to use them. Supporting everything fills the repo with noise and creates lots of commit churn as features and opinions change.

If we adopt a particular workflow in the bevy repo (especially one visible from the root), it should be because we think it is "the one" workflow we use for a given part of our development process.

The gitpod workflow is: "share a link to gitpod with a given repo/branch link, others open the link which logs them into their gitpod account and opens a browser hosted vscode instance backed by a vm, where changes are built for wasm and deployed to a server, which can be referenced via a link".

So question number 1 is: what is this "the" workflow for?

  • Reviewing PRs?
    • I personally think we should be encouraging people to review PRs by checking out the code and building locally. This lets you use your editor, editor configuration, and build process of choice. It lets you take advantage of stronger hardware if you have it. This is harder for newbies to set up, but learning rust development workflows is something they should be learning anyway. Frontloading that learning is worthwhile and we have plenty of people providing help in our community. Checking out PRs is non-trivial / toilsome with raw git, but "bevy developers" should be using optimized PR review workflows like VSCodes "checkout github pr number " feature or the github cli gh pr checkout 4700. The complexity of installing and learning these PR checkout tools is smaller than the complexity of learning how gitpod works, especially if you aren't used to VSCode.
  • Quickly sharing/demoing Bevy examples with specific change sets?
    • Ex: "check out this bug i reproed here", "check out this new example i added", etc
    • I think this is the most compelling use case. The alternative is distributing binaries, which has security (and cross-platform) concerns.
    • However the hosted-wasm-server link doesnt have stability. It dies reasonably quickly if you leave your workspace. Authentication might also be a concern. Realistically, everyone will need to host their own workspace from their own account.
    • One alternative could be setting up our own version of this with something like showme.bevyengine.org/#PUT_REPO_BRANCH_LINK_HERE?example=EXAMPLE_NAME, which builds the repo in a vm (if it doesn't currently exist) and hosts the wasm example. Thats a large engineering effort though and hinges on us paying for / hosting vm resources, and also opens the doors to abuse. We could probably add limitations like "only main bevy repo branches and repos/branches from bevy contributors", but yeah this feels "not worth it" at this point in our lifespan.
    • Alternatively, we could just grab wasm artifacts from github actions results and host them somewhere? Maybe we could even use direct links and just host a "template" that references the wasm files + assets directly? This would be superior to gitpod if we could make this work: embeds into our existing process / feeds off of the results of existing compute. No need for "logging into gitpod", waiting for builds / vm init, etc. And in an ideal work where we can fetch wasm (from artifacts) and assets (from the branch page) using javascript, this would be effectively "free" / serverless on our end (other than hosting the template page on bevyengine.org).
  • play.rust-lang.org-style code editing and sharing?
    • This provides similar functionality, but it doesn't provide the tools to easily make changes inside the web-editor and share those changes (to my knowledge). It would still require locally cloning code, making your changes, then pushing it to a place gitpod can read from (ex: a github repo).

Question number 2 is: do users wanting to use this workflow need us to check in a specific .gitpod.yml file at the root?

With "gitpod projects", users can manually provide a .gitpod.yml if a repo doesn't have one. This requires "invasive" access to a users' github account (for the repos you enable this for), which is ... supoptimal. It is also scoped to repos the user owns specifically, which makes it less useful. You can only use gitpod / share your changes. But it would still work for those use cases without any changes on our part. And it would allow us to dump the config in a less prominent place (such as somewhere in the .github folder).

In theory we could set up a team for the bevy org and invite people in. But then we're sharing billing, which feels suboptimal. And it puts the burden on someone to manage team membership.

Another alternative I looked into was gitpod's undocumented additionalcontent feature. This would allow us to pass in override file overrides by base64 encoding them in the url (ex: encode the json { ".gitpod.yml": CONTENTS_HERE } as base64). However it looks like they've explicitly made it not work in prod, which if so, is decidedly uncool. The link from that PR doesn't work with the prod url. I also encoded the gitpod config from this pr into a matching format, but that also failed with the exact same error message.
Here it is if you are curious:

https://gitpod.io/#/additionalcontent/eyAiLmdpdHBvZC55bWwiOiAiZ2l0aHViOlxuICBwcmVidWlsZHM6XG4gICAgbWFzdGVyOiB0cnVlXG4gICAgYnJhbmNoZXM6IHRydWVcbiAgICBwdWxsUmVxdWVzdHM6IHRydWVcbiAgICBwdWxsUmVxdWVzdHNGcm9tRm9ya3M6IHRydWVcbiAgICBhZGRDaGVjazogZmFsc2VcbiAgICBhZGRDb21tZW50OiBmYWxzZVxuICAgIGFkZEJhZGdlOiB0cnVlXG5cbnRhc2tzOlxuICAtIG5hbWU6IFN0YXJ0IHdlYiBzZXJ2ZXJcbiAgICBjb21tYW5kOiBweXRob24zIC1tIGh0dHAuc2VydmVyIC0tZGlyZWN0b3J5IGV4YW1wbGVzL3dhc21cblxuICAtIG5hbWU6IFNldHVwIHJ1c3QtYW5hbHl6ZXIgZm9yIHdhc20gdGFyZ2V0XG4gICAgYmVmb3JlOiB8XG4gICAgICBzdWRvIGFwdC1nZXQgaW5zdGFsbCAtLW5vLWluc3RhbGwtcmVjb21tZW5kcyAteXEgbGliYXNvdW5kMi1kZXYgbGlidWRldi1kZXZcbiAgICBpbml0OiB8XG4gICAgICBta2RpciAudnNjb2RlXG4gICAgICBlY2hvICJ7XCJydXN0LWFuYWx5emVyLmNhcmdvLnRhcmdldFwiOlwid2FzbTMyLXVua25vd24tdW5rbm93blwifSIgPiAudnNjb2RlL3NldHRpbmdzLmpzb25cbiAgLSBuYW1lOiBJbnN0YWxsIHdhc20gdG9vbGluZ1xuICAgIGJlZm9yZTogfFxuICAgICAgcnVzdHVwIHVwZGF0ZVxuICAgICAgcnVzdHVwIHRhcmdldCBhZGQgd2FzbTMyLXVua25vd24tdW5rbm93blxuICAgICAgY2FyZ28gaW5zdGFsbCBjYXJnby1xdWlja2luc3RhbGxcbiAgICAgIGNhcmdvIHF1aWNraW5zdGFsbCB3YXNtLWJpbmRnZW4tY2xpXG4gICAgICBjYXJnbyBxdWlja2luc3RhbGwgY2FyZ28td2F0Y2hcbiAgICBpbml0OiB8XG4gICAgICBjYXJnbyBydW4gLXAgYnVpbGQtd2FzbS1leGFtcGxlIC0tIGxpZ2h0aW5nXG52c2NvZGU6XG4gIGV4dGVuc2lvbnM6XG4gICAgLSBtYXRrbGFkLnJ1c3QtYW5hbHl6ZXIiIH0=/https://github.com/bevyengine/bevy/pull/4700

Also note that while this might have a "@cart is pushing back on this" angle, I'm just trying to direct the conversation at this point / clarify our goals + priorities. Haven't made any decisions yet.

bors bot pushed a commit that referenced this pull request Jul 20, 2022
# Objective

- In #4966, @DJMcNab noted that the changes should likely have been flagged as controversial, and blocked on a final pass from @cart.
  - I think this is generally reasonable.
  - Added as an explicit guideline.
- Changes to top-level files are also typically controversial, due to the high visible impact (see #4700 for a case of that).
  - Added as an explicit guideline.
- The licensing information of our included assets is hard to find.
   - Call out the existence of CREDITS.md
inodentry pushed a commit to IyesGames/bevy that referenced this pull request Aug 8, 2022
# Objective

- In bevyengine#4966, @DJMcNab noted that the changes should likely have been flagged as controversial, and blocked on a final pass from @cart.
  - I think this is generally reasonable.
  - Added as an explicit guideline.
- Changes to top-level files are also typically controversial, due to the high visible impact (see bevyengine#4700 for a case of that).
  - Added as an explicit guideline.
- The licensing information of our included assets is hard to find.
   - Call out the existence of CREDITS.md
james7132 pushed a commit to james7132/bevy that referenced this pull request Oct 28, 2022
# Objective

- In bevyengine#4966, @DJMcNab noted that the changes should likely have been flagged as controversial, and blocked on a final pass from @cart.
  - I think this is generally reasonable.
  - Added as an explicit guideline.
- Changes to top-level files are also typically controversial, due to the high visible impact (see bevyengine#4700 for a case of that).
  - Added as an explicit guideline.
- The licensing information of our included assets is hard to find.
   - Call out the existence of CREDITS.md
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
# Objective

- In bevyengine#4966, @DJMcNab noted that the changes should likely have been flagged as controversial, and blocked on a final pass from @cart.
  - I think this is generally reasonable.
  - Added as an explicit guideline.
- Changes to top-level files are also typically controversial, due to the high visible impact (see bevyengine#4700 for a case of that).
  - Added as an explicit guideline.
- The licensing information of our included assets is hard to find.
   - Call out the existence of CREDITS.md
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
# Objective

- add an helper to build examples in wasm (from bevyengine#4700)

## Solution

- `cargo run -p build-wasm-example -- lighting`
@mockersf mockersf closed this Apr 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Build-System Related to build systems or continuous integration A-Meta About the project itself X-Controversial There is active debate or serious implications around merging this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants