bin icon indicating copy to clipboard operation
bin copied to clipboard

Bug: bin install github.com/xo/usql installes Windows binary on Linux

Open breml opened this issue 4 years ago • 6 comments

When I install github.com/xo/usql on my Linux machine, I get the windows binary installed.

$ bin install github.com/xo/usql
   • Getting latest release for xo/usql
   • Starting download of https://github.com/xo/usql/releases/download/v0.8.2/usql-0.8.2-windows-amd64.zip
14.88 MiB / 14.88 MiB [----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------] 100.00% 9.88 MiB p/s 1s
   • Copying for [email protected] into <redacted>/bin/usql.exe
   • Done installing usql.exe v0.8.2

This raises two issues:

  1. I think bin should never install a binary, which can be ruled out by not matching the OS or the ARCH. If bin is not able to find a matching archive, it should fail instead.
  2. bin does not yet support bz2 or bzip2 archives

breml avatar Mar 21 '21 17:03 breml

2. bin does not yet support bz2 or bzip2 archives

^ true, feature request I guess.

  1. I think bin should never install a binary, which can be ruled out by not matching the OS or the ARCH. If bin is not able to find a matching archive, it should fail instead.

@sirlatrom just merged a PR which adjusts scoring based on OS / extensions. I think that's missing here is to do something similar for files in tar / zipped files. Since we're not performing that check there. Definitely something to improve, let's leave this here to fix it eventually

marcosnils avatar Mar 21 '21 17:03 marcosnils

adjusts scoring based on OS / extensions. I think that's missing here is to do something similar for files in tar / zipped files. Since we're not performing that check there.

It looks like I've only implemented that feature for .tar files (and subsequently .tar.gz files):

https://github.com/marcosnils/bin/blob/4bd5e82d842e5c47128532b5e8b93edab568361b/pkg/assets/assets.go#L308-L318

It could easily be added to the other archive formats.

sirlatrom avatar Mar 21 '21 18:03 sirlatrom

I compiled bin from master and with this version the installation of the tar.bz2 worked.

breml avatar Mar 22 '21 21:03 breml

Shall we close this @breml ?

marcosnils avatar Mar 23 '21 00:03 marcosnils

Judging from this comment we only have a "solution" for .tar and .tar.gz. So my personal view on this is:

  1. We should have a filter for non matching OS regardless of the type of archive.
  2. I would prefer not matching archives to be removed from the list of candidates completely instead of just giving them a lower score. The consequence of this is, I prefer bin install ... to fail, if no match is found instead of having a windows binary in my ~/bin folder, which is of no use. If it fails with an error, me as a user can look into the issue and take action. If it is successful with a meaning less result (Windows binary on Linux) I might not read the output carefully and therefore miss, that the fact, that a completely wrong binary has been installed.
  3. The same goes for not matching architectures. There is really no point in installing an arm binary on amd64 even if both binaries are for Linux.

To summarize the above, I would prefer to keep this issue open (even if the specific case has been resolved) until the above mentioned issues are resolved as well. Alternatively we can move this to a new issue and close this one.

breml avatar Mar 23 '21 19:03 breml

You're right.. let's leave this issue open since the original issue description and main pain point is still present.

marcosnils avatar Mar 24 '21 03:03 marcosnils