Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add wget fallback support for minimal OS's #262

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jeff-hykin
Copy link

@jeff-hykin jeff-hykin commented Apr 30, 2023

This should also fix #256. This PR has minimal edits to use wget as a fallback of curl, which is useful for Docker instances and other minimal OS's

@MarkTiedemann
Copy link
Contributor

At the top of the installer script, it says "Keep this script simple". I'm not sure this change is simple since it's complicating the one-liners for most users.

If you don't have curl installed in your minimal system, then you probably also know how to make the necessary adjustments. So I'd prefer not to add this change.

@jeff-hykin
Copy link
Author

jeff-hykin commented May 4, 2023

In terms of script modification, do you know of a more simple way of addressing #256?

my understanding was that the simplicity was so that other people could pretty easily verify the code wasn't malicious or vulnerable, which is definitely something I'm onboard with. I didn't think the complexity of the one liner fell into that, so I optimized it for minimal characters.

would this be considered more simple?

sh <<<"$(curl -fsSL https://deno.land/install.sh || wget -qO- https://deno.land/install.sh)"

I'm working with undergrad students who install the base version of Ubuntu which doesn't have curl, so they're not exactly skilled users.

@MarkTiedemann
Copy link
Contributor

I think #256 is not really a deno_install issue, but rather a Ubuntu specific issue with some curl installations.

Then again, I'm not a maintainer, just a contributor, so it's not my call.

@mmastrac
Copy link
Contributor

mmastrac commented Feb 8, 2024

I think that we can add wget support to install.sh, but we should have separate wget instructions as a fallback rather than putting both cURL and wget into a single command.

Here's what I suggest we do:

  1. Revert the README changes and replace them with a "fallback" section (something like "if you don't have curl...")
  2. Limit the install.sh changes to just this:
curl --fail --location --progress-bar --output "$exe.zip" "$deno_uri" || \
	wget --output-document="$exe.zip" "$deno_uri" || \
	echo "When installing deno, I looked for the 'curl' and for 'wget' commands but I didn't see either of them. Please install one of them, otherwise I have no way to install Deno" 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Warn users that snap curl is unsupported.
4 participants