zdharma-continuum / zinit

🌻 Flexible and fast ZSH plugin manager
MIT License
2.77k stars 123 forks source link

[bug]: gh-r installs linux binary on a may when there is no mac binary #295

Open jankatins opened 2 years ago

jankatins commented 2 years ago

Describe the bug

Running the following on a mac will download the wrong (=linux) binary, as there is only a linux build available (git-absorb release page). I would have expected an error upon installation as the binary is clearly a linux one:

[11:21:41] λ  zinit  as'null' from"gh-r" nocompile nocompletions lbin"git-absorb-* -> git-absorb" for @tummychow/git-absorb

Downloading tummychow/git-absorb…
(Requesting `git-absorb-linux-x86_64'…)
#################################################################################################################################################################################################################### 100.0%
ziextract: Successfully extracted and assigned +x chmod to the file: `git-absorb-linux-x86_64'.
linkbin annex: Created the git-absorb hard link and set +x on the git-absorb-linux-x86_64 binary

[11:21:52] λ  git absorb
/Users/jankatins/.zinit/polaris/bin/git-absorb: /Users/jankatins/.zinit/polaris/bin/git-absorb: cannot execute binary file

[11:22:14] [126] ✗  uname -a
Darwin jankatins.fritz.box 21.5.0 Darwin Kernel Version 21.5.0: Tue Apr 26 21:08:22 PDT 2022; root:xnu-8020.121.3~4/RELEASE_X86_64 x86_64

(zinit is a recent version after the xtrace changes -> my exclamation mark branch on top of main/bcedc9ff947319a5a979e5d9dfc61e7d9f39cfbd)

Steps to reproduce

  1. Be on a mac, not a linux computer
  2. enable lbin annex
  3. run zinit as'null' from"gh-r" nocompile nocompletions lbin"git-absorb-* -> git-absorb" for @tummychow/git-absorb on mac
  4. run git absorb and observe that it fails

Expected behavior

An error during installation as there is no mac binary

Screenshots and recordings

No response

Operating System & Version

darwin21.3.0 | apple | x86_64 | x86_64 | x86_64 i386

Zsh version

zsh 5.9 (x86_64-apple-darwin21.3.0)

Terminal emulator

xterm-256color (wezterm)

If using WSL on Windows, which version of WSL

No response

Additional context

No response

vladdoster commented 2 years ago

I think that knowing whether or not a project has a release asset for a respective system falls squarely on the user. I can't think of a situation where you'd add a program and not know if they support your OS.

vladdoster commented 2 years ago

At which point use the if'' ice, rust annex, or build it from the source.

vladdoster commented 2 years ago

It opens a huge can of worms in terms of complexity...

Such as whats the logic for:

jankatins commented 2 years ago

I was thinking more of an exclusion pattern: if it contains linux, don't install on Mac/windows and similar for the other OS. At least I would never download anything with linux in the filename if i want to run it on a Mac. So first remove known bad/wrong ones, then pick the best of the rest for download and install.

An ice to verify something might also be nice: better an error in install than later when you want to use something...

jankatins commented 2 years ago

FYI: git absorb 0.6.7 has now a linux + mac + windows binary (release workflow similar to ripgrep ones), so the current release cannot be used to run tests against it... 0.6.6 is the last one with an linux only binary.

vladdoster commented 2 years ago

@jankatins It can be still tested against using ver"0.6.6" ice, right?

vladdoster commented 1 year ago

@jankatins,

I've actually come to think this would be a very nice QoL improvement. Thanks for suggesting it, and I apologize for writing it off so quickly.

E.g., exa is unavailable for arm64 but currently selects armv7. Causing you to comment it out or add dumb workarounds.

So... were you volunteering to implement it ;).

I joke.

In all seriousness, what would you expect the behavior to be?

  1. Absolutely no possible release asset match (i.e., wrong architecture)
  2. Possible release asset (i.e., no architecture in release asset name)
Screenshot 2023-01-15 at 12 27 11
jankatins commented 1 year ago

My idea would be to write something like this (in pseudo-python code...)

os = ... whatever I am ...
filenames ["...mac...","...linux...", "...unknown..."]
possible_filenames = []

# first remove any filename where we are sure it's a different OS/Arch/...
for filename in all_filenames:
    if os != "linux" and linux_regex.match(filename):
        # We assume this is a linux, but we are not linux, so remove this filename
        continue
    ... more ifs which exclude filenames, e.g. based on OS or arch or ...
    possible_filenames.append(filename)

# Error out if nothing survives...
if possible_filenames = []: 
    print("Couldn't find a release for this platform, stopping...")
    sys.exit(1)

# ... and here the old logic which chooses the best filename, just now from possible_filenames...

E.g. in the volta example above and on macos, it would at least remove the filenames with windows and linux and probably end up with the two macosx files. And on x86, it would also remove the aarch64 one. On aarch64 it would result in two files and now the code has to choose the better one, which hopefully would result in the aarch64 one, as it is the more specific one.

jankatins commented 10 months ago

Stumbled over it again: https://github.com/zdharma-continuum/zinit/issues/575 (this time for eza, a replacement for the unmaintained exa...)