Skip to content

extension: Don't build vanilla WASM module for web extensions #19316

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

extension: Don't build vanilla WASM module for web extensions #19316

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
300a9dc
extension: Don't build vanilla WASM module for web extensions
danielhjacobs Jan 24, 2025
4bb4d88
docs: Add info about a reproducible build with dual-wasm support
danielhjacobs Jan 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions .github/workflows/release_nightly.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ jobs:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

build-web:
name: Build web${{ matrix.demo && ' demo' || '' }}
name: Build web${{ matrix.demo && ' demo and selfhosted' || ' extensions' }}
needs: create-nightly-release
if: needs.create-nightly-release.outputs.is_active == 'true'
runs-on: ubuntu-24.04
Expand Down Expand Up @@ -381,9 +381,10 @@ jobs:
CARGO_FEATURES: jpegxr${{ matrix.demo && ',webgpu' || '' }}
FIREFOX_EXTENSION_ID: ${{ secrets.FIREFOX_EXTENSION_ID }} # Needed to inject into manifest.json
WASM_SOURCE: cargo_and_store
BUILD_COMMAND: ${{ matrix.demo && 'npm run build:dual-wasm-repro' || 'npm run build:repro' }}
working-directory: web
shell: bash -l {0}
run: npm run build:repro
run: $BUILD_COMMAND

- name: Produce reproducible source archive
if: ${{ !matrix.demo }}
Expand All @@ -407,7 +408,7 @@ jobs:
run: npm run docs

- name: Publish npm package
if: ${{ !matrix.demo }}
if: matrix.demo
# npm scoped packages are private by default, explicitly make public
run: npm publish --access public --provenance
continue-on-error: true
Expand All @@ -416,12 +417,12 @@ jobs:
NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}

- name: Package selfhosted
if: ${{ !matrix.demo }}
if: matrix.demo
run: zip -r "${{ needs.create-nightly-release.outputs.package_prefix }}-web-selfhosted.zip" .
working-directory: web/packages/selfhosted/dist

- name: Upload selfhosted
if: ${{ !matrix.demo }}
if: matrix.demo
Copy link
Member

Choose a reason for hiding this comment

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

Why is the negation removed here? 🤔

Copy link
Contributor Author

@danielhjacobs danielhjacobs Mar 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

So selhosted continues working on Safari 16.3-. It's also why the workflow's long name is changed to "Build web demo and selfhosted"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See the second paragraph of #19316 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

Then, since it now means "demo and selfhosted", maybe this flag should be replaced with a different flag with inverse values, for "extensions"? 🤔

Copy link
Member

@torokati44 torokati44 Mar 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Oh but it should be clear that this flag will govern "whether we're building the browser extensions, or everything else", and not "whether WASM extensions are enabled".

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, looking at it again, more closely: Since (almost) all steps of the build-web job in the Release Nightly workflow are gated by the value of matrix.demo, maybe it would make more sense to just split it, getting rid of all the conditionals? 🤔

Copy link
Member

Choose a reason for hiding this comment

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

Sure, a couple of setup steps at the beginning will have to be duplicated this way, but maybe the increased clarity is worth it...?
BTW still waiting for actions/runner#1182 or something similar...

Copy link
Member

Choose a reason for hiding this comment

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

Something like this: 3299304 (#19767)

run: |
tag_name="${{ needs.create-nightly-release.outputs.tag_name }}"
package_file="${{ needs.create-nightly-release.outputs.package_prefix }}-web-selfhosted.zip"
Expand Down
3 changes: 2 additions & 1 deletion web/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,8 @@ In this project, you may run the following commands to build all packages:
- You may also use `npm run build:debug` to disable Webpack optimizations and activate the (extremely verbose) ActionScript debugging output.
- There is `npm run build:dual-wasm` as well, to build a second WebAssembly module that disables all supported WebAssembly extensions,
potentially resulting in support for more browsers, at the expense of longer build time.
- `npm run build:repro` enables reproducible builds. Note that this also requires a `version_seal.json`, which is not provided in the normal Git repository - only specially-marked reproducible source archives. Running this without a version seal will generate one based on the current state of your environment.
- `npm run build:repro` enables reproducible builds with the default WASM module. Note that this also requires a `version_seal.json`, which is not provided in the normal Git repository - only specially-marked reproducible source archives. Running this without a version seal will generate one based on the current state of your environment.
- `npm run build:dual-wasm-repro` enables reproducible builds with both WASM modules. Note that this also requires a `version_seal.json`, which is not provided in the normal Git repository - only specially-marked reproducible source archives. Running this without a version seal will generate one based on the current state of your environment.
- You will also need to run `rustup component add rust-src` with either of the previous two commands since we rebuild std for the vanilla WASM module.

From here, you may follow the instructions to [use Ruffle on your website](packages/selfhosted/README.md),
Expand Down
3 changes: 2 additions & 1 deletion web/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@
"build": "npm run build --workspace=ruffle-core && npm run build --workspace=ruffle-demo --workspace=ruffle-extension --workspace=ruffle-selfhosted",
"build:debug": "cross-env NODE_ENV=development CARGO_FEATURES=avm_debug npm run build",
"build:dual-wasm": "cross-env BUILD_WASM_MVP=true npm run build",
"build:repro": "cross-env BUILD_WASM_MVP=true ENABLE_VERSION_SEAL=true npm run build",
"build:repro": "cross-env ENABLE_VERSION_SEAL=true npm run build",
"build:dual-wasm-repro": "cross-env BUILD_WASM_MVP=true ENABLE_VERSION_SEAL=true npm run build",
"demo": "npm run preview --workspace ruffle-demo",
"test": "npm test --workspaces --if-present",
"wdio": "npm run wdio --workspaces --if-present --",
Expand Down
Loading