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 some redundancy for no internet connection check #743

Merged
merged 1 commit into from
Feb 13, 2023

Conversation

trentondyck
Copy link
Contributor

Was going through the install process for someone and they kept getting this No internet connection error message. Maybe the archlinux.org site was blocking ping? Nice to have some redundancy with another domain here since it's not actually dependent on that website. I tested and the fix works. Here's the error we saw (multiple tries same result):

  inflating: /home/deck/horizon-xi/stl/sonic2kk-steamtinkerlaunch-e7c5ada/misc/vortexgames.txt  
  inflating: /home/deck/horizon-xi/stl/sonic2kk-steamtinkerlaunch-e7c5ada/misc/winedebugchannels.txt  
  inflating: /home/deck/horizon-xi/stl/sonic2kk-steamtinkerlaunch-e7c5ada/misc/x64dbg.reg  
  inflating: /home/deck/horizon-xi/stl/sonic2kk-steamtinkerlaunch-e7c5ada/steamtinkerlaunch  
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 12113  100 12113    0     0  52463      0 --:--:-- --:--:-- --:--:-- 52665
Preparing to install SteamTinkerLaunch on Steam Deck
WARNING: No Internet Connection - SteamTinkerLaunch will attempt to install offline
No existing STL installation found, so copying installation files to '/home/deck/stl/prefix' for offline installation...

Dependency 'wget' already installed, nothing to do.
Installing dependency 'innoextract'
WARNING: No Internet Connection, cannot download dependency 'innoextract-1.9-7-x86_64.pkg.tar.zst'. For offline installation, manually place the file in '/home/deck/stl/deps'.
Could not find any archive in '/home/deck/stl/deps' for 'innoextract-1.9-7-x86_64.pkg.tar.zst', aborting install...

Failed to install SteamTinkerLaunch, check command line or log file at '/dev/shm/steamtinkerlaunch/steamtinkerlaunch.log' for errors

@sonic2kk
Copy link
Owner

sonic2kk commented Feb 13, 2023

Very nice! I didn't know the fix would be this straightforward. This issue has been brought up by various users, it's to do with the ISP blocking the call and not archlinux blocking it. I can't find the issue now but apparently sometimes on some connections, ping won't actually return a valid response. One user tested on their home(?) connection and at their college, and the college worked on the same machine.

While this fix works, I am wondering if we should do this in other places. If I recall there are checks for pinging GitHub specifically, but perhaps we could include this check in other places as well, like on line 22816. I don't want to change the GitHub pings in case GitHub is actually down, though I'd welcome your thoughts on that if you think having redundancy there is a good idea. Outside of that, having the extra ping to Google everywhere else is a good idea.

Thank you very much for your contribution! :-)

@trentondyck
Copy link
Contributor Author

yeah I dont know about the other places, at first glance it seemed like those needed the website they were pinging whereas this one definitely didn't

@sonic2kk
Copy link
Owner

No worries, we can leave this check just for the Steam Deck internet connection check.

Merging now, thanks!

@sonic2kk sonic2kk merged commit 7e59326 into sonic2kk:master Feb 13, 2023
@trentondyck
Copy link
Contributor Author

up to you though, as you said its a simple addition as long as you intend for the result to be "success if I can ping either website"

sonic2kk added a commit that referenced this pull request Feb 18, 2023
@sonic2kk
Copy link
Owner

There was a slight regression introduced as part of this change (SC1105). I fixed it in #743 by changing the syntax to the following and confirming that it still worked as expected on my shell: ! ping -q -c1 archlinux.org &>/dev/null || ping -q -c1 google.com &>/dev/null.

Just a small thing :-) I highly recommend using ShellCheck in general to vet Bash scripts. Unfortunately 0.9.0 seems to have introduced a regression (koalaman/shellcheck#2652), so 0.8.0 has to be used for some projects including SteamTinkerLaunch. It's a really nice tool though!

@trentondyck
Copy link
Contributor Author

trentondyck commented Feb 18, 2023

@sonic2kk that changes the logic. instead it should be

! (ping -q -c1 archlinux.org &>/dev/null || ping -q -c1 google.com &>/dev/null)

we need to but the "not" symbol outside the "or" block to ensure we are logically requiring (logically equivalent to) both archlinux and google not being able to be pinged. See this simple test as a demonstration:

(1)(deck@steamdeck horizon_scripts)$ ! false || true
(deck@steamdeck horizon_scripts)$ echo $?
0
(deck@steamdeck horizon_scripts)$ ! true || false
(1)(deck@steamdeck horizon_scripts)$ echo $?
1
(deck@steamdeck horizon_scripts)$ ! (false || true)
(1)(deck@steamdeck horizon_scripts)$ echo $?
1
(deck@steamdeck horizon_scripts)$ ! (true || false)
(1)(deck@steamdeck horizon_scripts)$ echo $?
1

(deck@steamdeck horizon_scripts)$ 

This option passes shellcheck.

@trentondyck trentondyck mentioned this pull request Feb 18, 2023
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