zyedidia / eget

Easily install prebuilt binaries from GitHub.
MIT License
954 stars 39 forks source link

cli/cli: too many candidates found #38

Closed ngirard closed 2 years ago

ngirard commented 2 years ago

Using latest version of Eget on Ubuntu 21.10.

❯ eget --asset=deb cli/cli
4 candidates found for asset `deb`: please select manually
(1) gh_2.11.3_linux_386.deb
(2) gh_2.11.3_linux_amd64.deb
(3) gh_2.11.3_linux_arm64.deb
(4) gh_2.11.3_linux_armv6.deb
Enter selection number: ^C

❯ eget --asset=deb --system=amd64 cli/cli
4 candidates found for asset `deb`: please select manually
(1) gh_2.11.3_linux_386.deb
(2) gh_2.11.3_linux_amd64.deb
(3) gh_2.11.3_linux_arm64.deb
(4) gh_2.11.3_linux_armv6.deb
Enter selection number: ^C

❯ eget --asset=deb --system=linux/amd64 cli/cli
4 candidates found for asset `deb`: please select manually
(1) gh_2.11.3_linux_386.deb
(2) gh_2.11.3_linux_amd64.deb
(3) gh_2.11.3_linux_arm64.deb
(4) gh_2.11.3_linux_armv6.deb
Enter selection number: ^C

I'd expect Eget to detect which package to download in all of the above cases.

Cheers, and thanks for your work !

dufferzafar commented 2 years ago

The --asset tag is specified multiple times in cases like these to get the release that you want. So you'd do: eget cli/cli -a deb -a amd64.

I think --asset conflicts with --system, and takes precedence.

ngirard commented 2 years ago

Thanks for your feedback.

In retrospect, what I meant to express is that I wish to prefer the deb package format over tarballs, when available. If there was a way of specifying this, either from the command line, or as a configuration option, then it might help resolve this issue. Maybe I should create a a dedicated issue for this feature request ?

dufferzafar commented 2 years ago

There's no way to specifiy a "preference" right now. There's -a deb but it won't prefer debs, it'll just try to find a deb, and if one doesn't exist it won't pick tarballs etc. I think you could close this issue, and create a separate one for the preference thing.

zyedidia commented 2 years ago

I think it’s reasonable that if the system argument is provided it should additionally filter assets using the auto detector. Let’s leave this open until that is implemented.

ngirard commented 2 years ago

Well sure, thanks for your dedication, @zyedidia !

dufferzafar commented 2 years ago

@zyedidia The filtering can be done in any order right? We could first filter by the SystemDetector and then by a chain of SingleAssetDetectors ?

Also, I looked at the code and there's already a priority parameter in the system detector, so for Linux it actually prefers AppImage assets over other ones. We could implement the pref feature @ngirard asked for by perhaps overriding this priority regex from a CLI argument?

There's also an anti param in the code, which avoids matching something & could perhaps be used to implement #40

https://github.com/zyedidia/eget/blob/1283b3ba3f3a748aa33cbf57fcab21398ae0d72f/detect.go#L46-L70

gesquive commented 2 years ago

I think being able to just pass in a regex to match would work, regex syntax can be used to ignore entries.

zyedidia commented 2 years ago

The system detector will now be applied even when using --asset if using it does not remove all candidates, fixing this issue. In other words, if the selected assets look like they could be filtered by the system detector, they are. If you want to consider all candidates even in such a case, you can use --system all.

ngirard commented 2 years ago

Thank you very much @zyedidia !

I'll be testing this new release later today !

ngirard commented 2 years ago

Hey @zyedidia, just a heads up: the new release is working just fine ! Thanks for your efforts !