Replace wget with curl for Steam Deck support #59

Merged
Skitals merged 5 commits from patch-1 into main 2022-04-17 22:13:25 +02:00
Skitals commented 2022-04-12 14:57:43 +02:00 (Migrated from github.com)

wget is not included in SteamOS and not easy to install because the file system is read-only by default and any changes will get wiped by the next system update. This is the only instance where wget is used, curl is used extensively in appimage.sh.

wget is not included in SteamOS and not easy to install because the file system is read-only by default and any changes will get wiped by the next system update. This is the only instance where wget is used, curl is used extensively in appimage.sh.
edisionnano commented 2022-04-13 11:48:54 +02:00 (Migrated from github.com)

How does our ci script affect the steamdeck?

How does our ci script affect the steamdeck?
Skitals commented 2022-04-13 14:30:35 +02:00 (Migrated from github.com)

How does our ci script affect the steamdeck?

I'm not sure what the ci script is, but in regards to this pr steam deck does not have wget so $GITVER is always null and the update script will never run. Steam deck does have curl, which is already used elsewhere in this project.

> How does our ci script affect the steamdeck? I'm not sure what the ci script is, but in regards to this pr steam deck does not have wget so $GITVER is always null and the update script will never run. Steam deck does have curl, which is already used elsewhere in this project.
qurious-pixel commented 2022-04-13 18:59:08 +02:00 (Migrated from github.com)

CURL is not installed by default on many distros, and so if using it, there should be a fallback to wget

CURL is not installed by default on many distros, and so if using it, there should be a fallback to `wget`
Skitals commented 2022-04-13 19:04:19 +02:00 (Migrated from github.com)

Curl is already used 9 other times in the project. This change removes 1 dependency and adds 0.

> Curl is already used 9 other times in the project. This change removes 1 dependency and adds 0.
MGThePro commented 2022-04-14 12:45:39 +02:00 (Migrated from github.com)

Curl is used in .github/workflows/appimage.sh, and .github/workflows/docker.sh, but those don't run through the appimage, those are just for the CI where we know curl is installed.

I also tested the auto updater appimage after uninstalling curl just to check, and it doesn't seem to have any issues or output any errors, so I'm pretty sure it's not used anywhere yet. And curl isn't part of the base package on Arch, so it's possible it isn't installed on some Arch systems (and possibly other distros as well). Using both curl and wget with one as a fallback would maximise compatibility with as many distros and systems as possible.

Here's one way how this could be implemented, but I'm sure there's probably a better and more maintainable solution using a variable for which program to use.

if [ -x "$(command -v curl)" ]; then
    GITVER=`curl -sL https://api.github.com/repos/pineappleEA/pineapple-src/releases/latest | grep "EA-" | grep -o 'EA-[[:digit:]]*'`
elif [ -x "$(command -v wget)" ]; then
    GITVER=`wget -qO- https://api.github.com/repos/pineappleEA/pineapple-src/releases/latest | grep "EA-" | grep -o 'EA-[[:digit:]]*'`
else
    printf "Neither curl nor wget found, exiting...\n"
    exit
fi

EDIT: heh, ignore the not preinstalled on arch part, turns out uninstalling it broke pacman and bricked my install (was able to recover it via chrooting from an arch boot usb though so all good) 😄

Curl is used in `.github/workflows/appimage.sh`, and `.github/workflows/docker.sh`, but those don't run through the appimage, those are just for the CI where we know curl is installed. I also tested the auto updater appimage after uninstalling curl just to check, and it doesn't seem to have any issues or output any errors, so I'm pretty sure it's not used anywhere yet. And curl isn't part of the base package on Arch, so it's possible it isn't installed on some Arch systems (and possibly other distros as well). Using both curl and wget with one as a fallback would maximise compatibility with as many distros and systems as possible. Here's one way how this could be implemented, but I'm sure there's probably a better and more maintainable solution using a variable for which program to use. ``` if [ -x "$(command -v curl)" ]; then GITVER=`curl -sL https://api.github.com/repos/pineappleEA/pineapple-src/releases/latest | grep "EA-" | grep -o 'EA-[[:digit:]]*'` elif [ -x "$(command -v wget)" ]; then GITVER=`wget -qO- https://api.github.com/repos/pineappleEA/pineapple-src/releases/latest | grep "EA-" | grep -o 'EA-[[:digit:]]*'` else printf "Neither curl nor wget found, exiting...\n" exit fi ``` EDIT: heh, ignore the not preinstalled on arch part, turns out uninstalling it broke pacman and bricked my install (was able to recover it via chrooting from an arch boot usb though so all good) :smile:
Skitals commented 2022-04-14 20:53:22 +02:00 (Migrated from github.com)

What about including wget or curl in the AppImage?

What about including wget or curl in the AppImage?
MGThePro commented 2022-04-15 22:56:51 +02:00 (Migrated from github.com)

That would work, but it would also be fair to assume that 99% of systems have either wget or curl, unless there is a popular distro that doesn't have one installed by default

That would work, but it would also be fair to assume that 99% of systems have either wget or curl, unless there is a popular distro that doesn't have one installed by default
Skitals (Migrated from github.com) reviewed 2022-04-16 00:40:42 +02:00
Skitals (Migrated from github.com) left a comment

Pushed a new commit. One liner, reverts to wget but tries curl is wget fails. This code should work if you have either wget or curl or both.

Pushed a new commit. One liner, reverts to wget but tries curl is wget fails. This code should work if you have either wget or curl or both.
MGThePro commented 2022-04-16 12:40:55 +02:00 (Migrated from github.com)

LGTM 👍.
@edisionnano @qurious-pixel do you have anything to add or change?

LGTM :+1:. @edisionnano @qurious-pixel do you have anything to add or change?
edisionnano (Migrated from github.com) approved these changes 2022-04-17 22:12:35 +02:00
Sign in to join this conversation.
No description provided.