devops-works / binenv

One binary to rule them all. Manage all those pesky binaries (kubectl, helm, terraform, ...) easily.
MIT License
375 stars 44 forks source link

bottom: Same problem as with bat #260

Open axgkl opened 3 months ago

axgkl commented 3 months ago

Sry for the noise and no stress - but after trying bat, tried bottom - and they ship now also with completion file

i.e. you need also here the "^btm$" instead the -btm.

manual download:

root@ax-proxy:~# tar xfvz bottom_x86_64-unknown-linux-gnu.tar.gz
btm
completion/
completion/btm.elv
completion/_btm
completion/btm.fish
completion/_btm.ps1
completion/btm.bash

root@ax-proxy:~# ./btm --version
bottom 0.9.6

root@ax-proxy:~# bottom
2024-07-26T03:52:10+02:00 FTL unable to execute bottom error="exec format error"

root@ax-proxy:~# head .binenv/binaries/bottom/0.9.6
_btm() {
    local i cur prev opts cmd
    COMPREPLY=()

Hmm, that's not so good, when authors add completion files to their releases.

Maybe(?), when trying to identifying the binary and >1 match, a few options which come to my mind:

  1. do a file check (but file is not always there)
  2. take the biggest, when >1 match?
  3. check for the executable flag
  4. Add the "^..$" everywhere
  5. Ignore all directories with 'complet' in them
  6. Ask the user when in doubt
  7. Prefer an exact match over any other match

Guess 7, 2 and/or 3 would elimate most of such probs, no?

Cheers, I love this tool, fastest installer of them all :heart_decoration:

axgkl commented 3 months ago

Next, no joke, tried btop - similar problem with the choosing of the binary. Here you took the license file, which does not even contain the word btop, strange. Guess because the archive unpacks to 'btop' (?)

root@ax-proxy:~/foo# curl -sL  https://github.com/aristocratos/btop/releases/download/v1.3.2/btop-x86_64-linux-musl.tbz > xxx.tbz

root@ax-proxy:~/foo# tar -xjf xxx.tbz
root@ax-proxy:~/foo# btop/bin/btop --version
btop version: 1.3.2
root@ax-proxy:~/foo# btop
2024-07-26T04:36:38+02:00 FTL unable to execute btop error="exec format error"
root@ax-proxy:~/foo# head ../.binenv/binaries/btop/1.3.2

                                 Apache License
                           Version 2.0, January 2004
                        http://www.apache.org/licenses/
 root@ax-proxy:~/foo# tree
.
├── btop
│   ├── bin
│   │   └── btop
│   ├── btop.desktop
│   ├── CHANGES.md
│   ├── Img
│   │   ├── icon.png
│   │   └── icon.svg
│   ├── install.sh
│   ├── LICENSE
│   ├── Makefile
│   ├── README.md
│   ├── setuid.sh
│   ├── themes
│   │   ├── adapta.theme
│   │   ├── adwaita.theme
│   │   ├── ayu.theme
│   │   ├── dracula.theme
│   │   ├── dusklight.theme
│   │   ├── elementarish.theme
│   │   ├── everforest-dark-hard.theme
│   │   ├── everforest-dark-medium.theme
│   │   ├── flat-remix-light.theme
│   │   ├── flat-remix.theme
│   │   ├── greyscale.theme
│   │   ├── gruvbox_dark.theme
│   │   ├── gruvbox_dark_v2.theme
│   │   ├── gruvbox_material_dark.theme
│   │   ├── horizon.theme
│   │   ├── HotPurpleTrafficLight.theme
│   │   ├── kyli0x.theme
│   │   ├── matcha-dark-sea.theme
│   │   ├── monokai.theme
│   │   ├── night-owl.theme
│   │   ├── nord.theme
│   │   ├── onedark.theme
│   │   ├── paper.theme
│   │   ├── solarized_dark.theme
│   │   ├── solarized_light.theme
│   │   ├── tokyo-night.theme
│   │   ├── tokyo-storm.theme
│   │   ├── tomorrow-night.theme
│   │   └── whiteout.theme
│   └── uninstall.sh
└── xxx.tbz

Guess I'd apply suggestion number 7, exact match when in doubt. If > 1 match exact, check for exec flag. If still > 1: biggest one is it :-)

leucos commented 3 months ago

You're a gold digger mate :joy: Another (rather easy to implement) option is to have binenv itself check if the file is an executable before installing it (e.g. like solution 1, but not depending on file being present).

btop issue is weird though, will have a look at it.

axgkl commented 3 months ago

You're a gold digger mate 😂

"Everything I touch breaks" as my wife would put it :-/

Another (rather easy to implement) option is to have binenv itself check if the file is an executable before installing it (e.g. like solution 1, but not depending on file being present).

Imho "btop" shows, that there must be a problematic behaviour in the matcher - you seem to not just search the configured string on the filename but on the whole relative filepath.

And then, the simple checking for exe perms are not enough, e.g. ./bat/install.sh is also executable.

I'd remove that and match only the filename.

Can provide you with a line or 2 of bash which does that but I guess you want 100% go.

axgkl commented 3 months ago

you seem to not just search the configured string on the filename but on the whole relative filepath.

just see that this is not a bug but a documented feature.

Image 1 Image 2

Sorry for the lame joke, its friday :-)

So path is documented and can't be changed for backwards compat (Btw: why? Do these crazy devs change the names of their apps all the time?)

While I can think of many many reasons, why that name begins to shows up in dirnames, while devs progress/reorganize their stuff into documented, licensed, whatever structure.

So yeah, I'd stick to the flow above then, even more needed than, such a reducer, no?