Skip to content

Conversation

@GideonBear
Copy link

Closes #136

Reverts #123
In its current state, this is a breaking change.


There are two ways to solve #136:

1. Revert #123

This is the current approach this PR takes. In my opinion, the full target should just be in the filename. This is not cargo-binstall; it's totally valid to have a requirement for the filename format when using self_update.

2. Fix logic, prioritize target

Quoting from #136:

If we really want a fallback to a less specific asset target when no specific one can be found, we should first check all assets to determine if one can be found. As it stands now, the check is short circuited because .find stops after the first match, which will most likely be the OS / ARCH check.

This is also fine, and probably what @DervexDev intended to do in #123 in the first place, given the way the changelog is written. If you want to take this approach instead, let me know and I'll edit the PR.

@GideonBear
Copy link
Author

CI failure is a spurious network error, lol

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 this pull request may close these issues.

asset_for does not find the right target because of check on OS / ARCH

1 participant