search

houseabsolute / ubi

The Universal Binary Installer
Apache License 2.0
190 stars 6 forks source link

Fix binary ending in xz #27

Closed mfontani closed 1 year ago

mfontani commented 1 year ago

I previously submitted a patch for making .xz DWIM, but it wasn't enough.

On another project, I added a list of multi-arch binaries which are then xz'ed, but ubi didn't work:

$ cargo run -- --project mfontani/tstdin --tag v0.2.3
    Finished dev [unoptimized + debuginfo] target(s) in 0.11s
     Running `target/debug/ubi --project mfontani/tstdin --tag v0.2.3`
[ubi][ERROR] could not find a release for this OS and architecture from tstdin-darwin-amd64.xz, tstdin-linux-amd64.xz, tstdin.exe

The os (linux) matches; the arch (amd64) matches; it "just" chokes on the ".xz" ending, which it can totally handle.

So I'm adding ".xz" to the list of extensions it does, indeed, support.

... and the actual problem is now a test case, too.

autarch commented 1 year ago

Hi, thanks for your PR! I'm pretty finicky about my projects (see this blog post for details), so I rarely merge a PR as-is. I can move forward on your PR in one of two ways:

  1. I check it out locally, fiddle with it as needed, merge it locally, and simply close this PR. This will preserve at least one commit with your name on it, but the PR will show up as closed in your GitHub stats.
  2. If you enable me to push directly to your fork, I can do my fiddling, then force push to your fork and merge the resulting PR. Again, this will preserve at least one commit with your name on it, but you also get credit for the PR merge in your GitHub stats. The only downside is that I will be force pushing directly to your fork.

Please let me know which approach you'd prefer. If I don't hear from you before I get around to working on this PR I'll go with option #1.

Thanks again for your contribution!

(But I recently tried option #2 with someone on another project and I couldn't push to their PR branch even though they'd checked the box to do so, so I can't promise it will work.)

mfontani commented 1 year ago

Option 1 is fine, thanks!

autarch commented 1 year ago

Merged from the CLI. Thanks!