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

Add cargo-binstall support to wasm-bindgen #3544

Merged
merged 4 commits into from
Aug 5, 2023
Merged

Add cargo-binstall support to wasm-bindgen #3544

merged 4 commits into from
Aug 5, 2023

Conversation

NobodyXu
Copy link
Contributor

@NobodyXu NobodyXu commented Aug 3, 2023

This would enable cargo-binstall to install the pre-built binaries from release artifacts.

How to test this:

cargo binstall --manifest-path crates/cli wasm-bindgen-cli

This would enable `cargo-binstall` to install the pre-built binaries from release artifacts.

How to test this:

```
cargo binstall --manifest-path crates/cli wasm-bindgen-cli
```
@daxpedda
Copy link
Collaborator

daxpedda commented Aug 4, 2023

Is this still requires as of cargo-bins/cargo-binstall#1245?

@daxpedda daxpedda added the waiting for author Waiting for author to respond label Aug 4, 2023
@NobodyXu
Copy link
Contributor Author

NobodyXu commented Aug 5, 2023

Is this still requires as of cargo-bins/cargo-binstall#1245?

Yes, since the artifact name used in release is wasm-bindgen instead of wasm-bindgen-cli which is the crate-name, so the default pkg-url and bin-dir template cannot find the artifacts.

And the subcrate extracts from the url is cli and no release is prefixed with it, so the subcrate pkg-url also cannot find the artifact.

@daxpedda
Copy link
Collaborator

daxpedda commented Aug 5, 2023

I'm not familiar with cargo-binstall, would you mind confirming that this works?
Small nit before we merge this: could you add a note in the changelog?

@NobodyXu
Copy link
Contributor Author

NobodyXu commented Aug 5, 2023

I'm not familiar with cargo-binstall, would you mind confirming that this works?

I've tested locally before pushing this

Small nit before we merge this: could you add a note in the changelog?

Sure!

Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
@NobodyXu
Copy link
Contributor Author

NobodyXu commented Aug 5, 2023

@daxpedda I've updated changelog and also README on how to install wasm-bindgen-cli.

README.md Outdated Show resolved Hide resolved
Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
@NobodyXu NobodyXu requested a review from daxpedda August 5, 2023 13:28
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
Co-authored-by: daxpedda <daxpedda@gmail.com>
@NobodyXu NobodyXu requested a review from daxpedda August 5, 2023 13:33
Copy link
Collaborator

@daxpedda daxpedda left a comment

Choose a reason for hiding this comment

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

Thank you!

@daxpedda daxpedda merged commit bb2f424 into rustwasm:main Aug 5, 2023
23 checks passed
@NobodyXu NobodyXu deleted the main-1 branch August 5, 2023 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting for author Waiting for author to respond
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants