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

Fix panics in build --mode no-install #598

Merged
merged 1 commit into from
Mar 22, 2019

Conversation

alexcrichton
Copy link
Contributor

This commit fixes the wasm-pack build --mode no-install command from
unconditionally panicking as well as --mode force. These steps were
calling an unwrap() on an internal Option<T> which was supposed to
be set during step_install_wasm_bindgen, but that step wasn't run in
these modes. The mode configuration of steps has been refactored
slightly to ensure that more steps are shared between these modes to
reduce duplication.

This commit fixes the `wasm-pack build --mode no-install` command from
unconditionally panicking as well as `--mode force`. These steps were
calling an `unwrap()` on an internal `Option<T>` which was supposed to
be set during `step_install_wasm_bindgen`, but that step wasn't run in
these modes. The mode configuration of steps has been refactored
slightly to ensure that more steps are shared between these modes to
reduce duplication.
Copy link
Member

@ashleygwilliams ashleygwilliams left a comment

Choose a reason for hiding this comment

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

gonna merge this because i think it'd be a nice thing to get a point release out for. a little hesistant because i found it a little difficult to follow at first. it's a lot less clear than the code previously was but i dont think enough to block this. maybe i'll go back and see if there's a way to take this more efficient strategy and reintro some more expressivity, or maybe i'll decide that i don't care/it doesn't matter that much.

thanks for the quick fix @alexcrichton !!

@ashleygwilliams ashleygwilliams merged commit b07ec95 into rustwasm:master Mar 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants