self_update icon indicating copy to clipboard operation
self_update copied to clipboard

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

Open LordGaav opened this issue 1 year ago • 3 comments

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 avatar Aug 26 '24 14:08 LordGaav

For anyone else running into this issue, downgrading to [email protected] works in the mean time as that was the version before the logic above was introduced.

LordGaav avatar Aug 27 '24 13:08 LordGaav

I've run into this same issue. My project (https://github.com/amtep/ck3-tiger) publishes three binaries in separate archives, with versions for both linux and windows, and the OS logic makes it pick the wrong one when updating. This is with the github backend.

amtep avatar Mar 23 '25 04:03 amtep

Sorry, false alarm. It turns out 0.42 doesn't cause a problem for me, I was just testing wrong. My problem was only with 0.40.

amtep avatar Mar 23 '25 08:03 amtep