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

web: Reduce docker build IO #19053

Merged
merged 1 commit into from
Dec 26, 2024
Merged

web: Reduce docker build IO #19053

merged 1 commit into from
Dec 26, 2024

Conversation

divinity76
Copy link
Contributor

  • binaryen xz is no longer stored on disk, instead being piped directly to tar
  • only extract wasm-opt (the only file we're interested in)
  • locate wasm-opt using wildcard, instead of keeping path in sync with version number.

we're doing the same with sh.rustup.rs below.

@torokati44
Copy link
Member

Then I assume --quiet would make more sense instead of --progress=:giga, like below.

Also please tag the commit message (and PR title) according to https://github.com/ruffle-rs/ruffle/blob/master/CONTRIBUTING.md#commit-message-guidelines, and add a T-* and A-* label.

@divinity76 divinity76 changed the title reduce docker build IO web: reduce docker build IO Dec 24, 2024
@divinity76 divinity76 changed the title web: reduce docker build IO web: Reduce docker build IO Dec 24, 2024
@divinity76
Copy link
Contributor Author

Then I assume --quiet would make more sense instead of --progress=:giga, like below.

progress=:giga is there for a reason. The download can take a long time, it's 100MB compressed and sometimes github's download servers are randomly slow. Without the progress indicator, the docker build may appear to hang. Progress indicator is printed to stderr and does not corrupt the data stream for tar. Also it wasn't introduced by me, it appears to be introduced by you in a66ba15.

Also please tag the commit message (and PR title) according to https://github.com/ruffle-rs/ruffle/blob/master/CONTRIBUTING.md#commit-message-guidelines

done

, and add a T-* and A-* label.

I don't understand, please elaborate

@danielhjacobs
Copy link
Contributor

Regarding the labels those can only be added if you have at least triage access to the repo which I don't think you have. I'll add them.

@danielhjacobs danielhjacobs added A-web Area: Web & Extensions T-refactor Type: Refactor / Cleanup labels Dec 24, 2024
@torokati44
Copy link
Member

Without the progress indicator, the docker build may appear to hang. Progress indicator is printed to stderr and does not corrupt the data stream for tar.

Regarding the labels those can only be added if you have at least triage access to the repo which I don't think you have. I'll add them.

Both good points, and thank you!

- binaryen xz is no longer stored on disk, instead being piped directly to tar
- only extract wasm-opt (the only file we're interested in)
- locate wasm-opt using wildcard, instead of keeping path in sync with version number.

we're doing the same with sh.rustup.rs below.
@torokati44 torokati44 enabled auto-merge (rebase) December 26, 2024 14:44
@torokati44 torokati44 merged commit e03daf4 into ruffle-rs:master Dec 26, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-web Area: Web & Extensions T-refactor Type: Refactor / Cleanup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants