mason.nvim icon indicating copy to clipboard operation
mason.nvim copied to clipboard

Busybox wget support

Open fredrikfoss opened this issue 1 year ago • 2 comments
trafficstars

Hi, this pr adds support for Busybox's implementation of wget, which comes default on Alpine Linux among other distros. Busyboxs wget only support POST, but as far as I could tell wget won't be called with other methods here. This should also close #1601. Let me know what you think.

Busybox wget documentation reference:

BusyBox v1.37.0 (2024-11-13 17:39:29 UTC) multi-call binary.

Usage: wget [-cqS] [--spider] [-O FILE] [-o LOGFILE] [--header STR]
        [--post-data STR | --post-file FILE] [-Y on/off]
        [-P DIR] [-U AGENT] [-T SEC] URL...

Retrieve files via HTTP or FTP

        --spider        Only check URL existence: $? is 0 if exists
        --header STR    Add STR (of form 'header: value') to headers
        --post-data STR Send STR using POST method
        --post-file FILE        Send FILE using POST method
        -c              Continue retrieval of aborted transfer
        -q              Quiet
        -P DIR          Save to DIR (default .)
        -S              Show server response
        -T SEC          Network read timeout is SEC seconds
        -O FILE         Save to FILE ('-' for stdout)
        -o LOGFILE      Log messages to FILE
        -U STR          Use STR for User-Agent header
        -Y on/off       Use proxy

fredrikfoss avatar Nov 18 '24 14:11 fredrikfoss

On second thought, this really is just a quick workaround and should be done more thoroughly, if this is wanted. Initially, I was made aware of this issue simply because downloading packages didn't work when the wget binary was provided by Busybox, even though curl was available on the system and wget is marked as relaxed. Busybox wget does not support a --version argument, and returns false when given one, something relaxed doesn't seem to cover.

But wget isn't the only case. There is also gzip, unzip and tar (not tar though, the Busybox implementation actually supports --version ¯_(ツ)_/¯). There exists multiple implementations of some of these, not just by Busybox. I think a better approach might be to not check their versions or return values, and just assume they will work as indended if found in PATH. Then feed them options and flags that are common and found to work with most implementations. I can't remember seeing any actual complex or uncommon option or flag used with any of the coreutils in the source. This PR only changed a few flags to also work with Busybox wget.

Changing to draft for now. You can close it if you want, but I'm interested in hearing your or other' thoughts.

fredrikfoss avatar Feb 16 '25 14:02 fredrikfoss

Seems the issue with mason not falling back to curl with busybox wget has since been fixed, so this isn't really that necessary any longer I guess. Appreciated :)

fredrikfoss avatar Feb 22 '25 13:02 fredrikfoss

I think this would make sense. The only drawback is that the fetch function technically supports all HTTP verbs, not just GET and POST (although GET is the only method ever used).

Seems the issue with mason not falling back to curl with busybox wget has since been fixed, so this isn't really that necessary any longer I guess. Appreciated :)

Yep curl should be priority and is recommended. wget is fallback.

I added some input validation for wget specifically and also made sure headers are sorted so that tests don't sporadically fail.

williamboman avatar May 24 '25 17:05 williamboman

@fredrikfoss Would you be able to verify that the latest changes still work as expected? I've tried with GNU wget and it seems fine.

edit: Tested in a clean alpine docker container with only git and neovim installed - seems to work fine. Awaiting confirmation from fredrik!

williamboman avatar May 24 '25 17:05 williamboman

Thanks, LGTM! Tested it as well and it correctly falls back on busybox's wget.

fredrikfoss avatar May 24 '25 18:05 fredrikfoss