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

Fix install.sh script if curl is not present #414

Merged
merged 1 commit into from
Nov 26, 2020

Conversation

bashofmann
Copy link
Contributor

@bashofmann bashofmann commented Nov 24, 2020

At least with bash 5.0.18, GNU Wget 1.20.3 and GNU Awk 5.1.0 the wget installation methods fails because the awk regex does not match the location header. In these versions, instead of /^ Location:... it must be /^Location:.... In order to retain compatibility with other versions having a different behavior, I changed it to /^\s*Location:....

bash-5.0$ wget /~https://github.com/rancher/k3d/releases/latest --server-response -O /dev/null 2>&1 | awk '/^  Location: /{DEST=$2} END{ print DEST}'

bash-5.0$ wget /~https://github.com/rancher/k3d/releases/latest --server-response -O /dev/null 2>&1 | awk '/^Location: /{DEST=$2} END{ print DEST}'
/~https://github.com/rancher/k3d/releases/tag/v3.3.0

bash-5.0$ wget /~https://github.com/rancher/k3d/releases/latest --server-response -O /dev/null 2>&1 | awk '/^\s*Location: /{DEST=$2} END{ print DEST}'
/~https://github.com/rancher/k3d/releases/tag/v3.3.0

This also adds drone steps to test this script with both curl and wget.

@bashofmann bashofmann changed the title If install.sh script if curl is not present Fix install.sh script if curl is not present Nov 24, 2020
@bashofmann bashofmann force-pushed the fix-install-script-wget branch from 3175c70 to 253280e Compare November 24, 2020 11:52
@iwilltry42
Copy link
Member

Thanks for this @bashofmann :)
I don't really understand, why this is happening, but the fix looks reasonable to me 👍
However, I wouldn't go with the extra drone steps, as they introduce unnecessary risks (network) that could prevent releases from being published.
I could imagine though putting them into GitHub Actions that run after a release has been published and then even check for the correct version being present (k3d version) after the installation 🤔

At least with bash 5.0.18, GNU Wget 1.20.3 and GNU Awk 5.1.0 the wget installation methods fails because the awk regex does not match the location header. In these versions, instead of `/^  Location:...` it must be `/^Location:...`. In order to retain compatibility with other versions having a different behavior, I changed it to `/^\s*Location:...`.

Signed-off-by: Bastian Hofmann <bashofmann@gmail.com>
@bashofmann bashofmann force-pushed the fix-install-script-wget branch from 253280e to ed0f437 Compare November 26, 2020 10:49
@bashofmann
Copy link
Contributor Author

Sounds good, I removed the drone steps and will work an a Github Actions based test in a separate PR.

@iwilltry42
Copy link
Member

Awesome, thank you!

@iwilltry42 iwilltry42 merged commit 13470c0 into k3d-io:main Nov 26, 2020
rancherio-gh-m pushed a commit that referenced this pull request Nov 26, 2020
Author: Bastian Hofmann <mail@bastianhofmann.de>
Date:   Thu Nov 26 12:25:30 2020 +0100

    Fix install.sh script if curl is not present (#414)

    At least with bash 5.0.18, GNU Wget 1.20.3 and GNU Awk 5.1.0 the wget installation methods fails because the awk regex does not match the location header. In these versions, instead of `/^  Location:...` it must be `/^Location:...`. In order to retain compatibility with other versions having a different behavior, I changed it to `/^\s*Location:...`.

    Signed-off-by: Bastian Hofmann <bashofmann@gmail.com>
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.

2 participants