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

Subcrate support for more crates #1244

Closed
NobodyXu opened this issue Aug 3, 2023 · 1 comment · Fixed by #1245
Closed

Subcrate support for more crates #1244

NobodyXu opened this issue Aug 3, 2023 · 1 comment · Fixed by #1245

Comments

@NobodyXu
Copy link
Member

NobodyXu commented Aug 3, 2023

Doesn't seem to be handling wasm-bindgen-cli:

Resolved repo_info = RepoInfo {
    repo: Url {
        scheme: "https",
        cannot_be_a_base: false,
        username: "",
        password: None,
        host: Some(
            Domain(
                "github.com",
            ),
        ),
        port: None,
        path: "/rustwasm/wasm-bindgen/tree/main/crates/cli",
        query: None,
        fragment: None,
    },
    repository_host: GitHub,
    subcrate: None,
}

and this looks like because it's expecting /[owner]/[repo]/tree/[branch]/[subcrate] but wasm-bindgen is using /[owner]/[repo]/tree/[branch]/crates/[subcrate]. This seems to be the general recommendation.

Originally posted by @CAD97 in #1231 (comment)

NobodyXu added a commit that referenced this issue Aug 3, 2023
Fixed #1244

Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
@NobodyXu NobodyXu changed the title Subcrate support for wasm-bindgen-cli Subcrate support for more crates Aug 3, 2023
NobodyXu added a commit that referenced this issue Aug 3, 2023
Fixed #1244

Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
NobodyXu added a commit that referenced this issue Aug 3, 2023
Fixed #1244

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

NobodyXu commented Aug 3, 2023

@CAD97 I've opened a PR in wasm-bindgen rustwasm/wasm-bindgen#3544 since this is the only way to support cargo-binstall there.

The repository url contains crates/url, which isn't the name used in release like others, so it would have to override package.metadata.binstall.

@CAD97 I've tested it locally with all targets supported by upstream, if you have time, can you please double-check my PR to make sure it actually works for you?

Edit:

Wrong url

github-merge-queue bot pushed a commit that referenced this issue Aug 3, 2023
Fixed #1244

Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant