jaemk / self_update

Self updates for rust executables
MIT License
771 stars 65 forks source link

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

Open LordGaav opened 2 weeks ago

LordGaav commented 2 weeks ago

The change in 8bd32c2c714e56fc48b05d883886b1e6a94bb2c7 introduces a regression where (at least on Gitlab) the right target cannot be find, and the first one that matches OS / ARCH is returned instead.

This introduces an issue when a release contains assets for both x86_64-unknown-linux-musl and x86_64-unknown-linux-gnu. Both contain linux and x86_64, so even though the first part of the if statement does not match, the second one always does.

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.

Personally I don't think the logic introduced in 8bd32c2c714e56fc48b05d883886b1e6a94bb2c7 is positive in any way, because this also allows accidental upgrades to another target. For instance, if I'm self updating on an OS that only has musl (like Alpine), and the first entry found is x86_64-unknown-linux-gnu, the current logic will happily update to a binary that requires libc that cannot be run. This is bad, because the right target might be later in the list, or might be omitted intentionally for a specific version.

LordGaav commented 2 weeks ago

For anyone else running into this issue, downgrading to self_update@0.39.0 works in the mean time as that was the version before the logic above was introduced.