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

1 Add Linux Installer script #219

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

Fr-Dae
Copy link

@Fr-Dae Fr-Dae commented Oct 11, 2023

Unitystation-fork#1
CL: [Add] Add Linux-install-Script

on gilles' request, and to avoid giving external links to people for the unix installer I created with peull, I share it in the stationhub documentation
and will keep this one up to date

@Fr-Dae @Peulleieoyukino

on gilles request, and to avoid giving external links to people for the unix installer I created with peull, I share it in the stationhub documentation
and will keep this one up to date
@Fr-Dae
Copy link
Author

Fr-Dae commented Oct 11, 2023

@MaxIsJoe
Copy link
Contributor

It's not safe to use submodules that aren't tied to a trusted organization.
You guys should work out a way to implement this script without linking to external repositories.

@Fr-Dae
Copy link
Author

Fr-Dae commented Oct 11, 2023

if you want the script to be regularly updated, you'll have to use
moreover it goes with gilles' wish that we share our work with you

@MaxIsJoe
Copy link
Contributor

Away from the security implications this entails, there are several issues with this script that makes this not viable to be merged in its current state.

  1. This script only focuses on Ubuntu users only. Leaving Arch users out of the picture and causing some issues with custom configs that do indeed have apt but is locked behind sudo.
  2. apt full upgrade should not be run on the user's behalf. You could brick someone's system or change something about it that they didn't want changed.
  3. why Wget? Can't you use curl directly since it comes with 99% of all linux systems already?
  4. I've talked to Bod about this before, but we should use github's releases to link to updates; instead of relying on updating a link in the script every time we need to push an update. having https://github.com/unitystation/stationhub/releases/download/933/lin933.zip in the script will always download that specific version only, not the latest one.

@Fr-Dae
Copy link
Author

Fr-Dae commented Oct 11, 2023

1 - the users who need this script are end-users, i.e. simple users, which excludes users of arch, which is not a general public distribution.
if you don't have sudo apt, that's your problem.

2 - it's part of the basic commands that are run every time the pc is launched, except that here there's no interface. and very VERY often ubuntu users don't update because they forget or they lazy
apt is precisely based on the fact that doing these updates doesn't break the system.
either their version is too old, and they don't have any updates,
or it's recent, in which case it should be updated.

3 - we've already tried curl several times, especially for macOS, and it wasn't conclusive.
wget has been installed by default on all distributions for several years now, so we use it.

4 - the "LastRelease" function doesn't work, if you know how to improve this, I'd love to hear from you.

@Fr-Dae
Copy link
Author

Fr-Dae commented Oct 11, 2023

and if you're not happy, why don't you offer something better?
Currently there are no simple solutions for users who don't have an IT university degree, you know, the simple people who use their computer without necessarily knowing how it works.

in addition, this script adds a shortcut to the start menu, "game".
Capture d’écran du 2023-10-11 10-45-14

@MaxIsJoe
Copy link
Contributor

  1. Arch is regularly used everywhere and is the main distro that the Steam Deck uses. It is very much our problem as well when most of our players are arch users. We're no longer living in 2005, stop acting like some parts of Linux is for tech wizards only.
  2. People avoid updating because some packages drop support for specific hardware or a specific package requires a dependency or change that bricks their system. I don't care what reason you have, I'm going to protest merging a change that can brick someone's computer without their consent.
  3. ok you need to be more clear, if it's installed on "all" distros; why are you re-installing it in this script?
  4. I don't know why It's difficult to do a quick google search but here https://api.github.com/repos/unitystation/stationhub/releases/latest this includes all the json data used for latest releases and all you have to do is grab the url

also

and if you're not happy, why don't you offer something better?

I've warned you on discord that I do not respond well to that tone. If you're bothered by me reviewing my PR then grow up.

@Fr-Dae
Copy link
Author

Fr-Dae commented Oct 11, 2023

steamdeck it's based on ubuntu

@MaxIsJoe
Copy link
Contributor

steamdeck it's based on ubuntu

SteamOS 3 is based on Arch and is what the deck uses.
You're thinking about SteamOS 2 which is obsolete and unsecure

@Fr-Dae
Copy link
Author

Fr-Dae commented Oct 11, 2023

  • I tackled this problem because nobody considers it important.
    now that it's done, you're upset that it was me who did it, i'm simply replying that if it bothers you, show me how you would have done it. don't be suseptible, not everything i say is necessarily an aggression towards you =)

  • if apt full-ugrde bothers you, we can remove the -y, but don't come crying to me because some people won't have done the basic system updates for a century or two, and the installer doesn't work.

  • and for your api.xxx url, i invite you to test it with curl or wget, you'll see that it doesn't work properly.

@Fr-Dae
Copy link
Author

Fr-Dae commented Oct 11, 2023

steamdeck it's based on ubuntu

SteamOS 3 is based on Arch and is what the deck uses. You're thinking about SteamOS 2 which is obsolete and unsecure

Okay, that's good news, I guess.

@MaxIsJoe
Copy link
Contributor

image

i don't know what to tell you
I've told you that I'm not interested in pursuing highschool level drama here, and you still have not changed your attitude with me and keep using every interaction I have with you as some kind of aggression

I was interested in giving you guys a solution to use the releases tags since it seems that you don't know how to use JSON files or google, but I'm no longer interested in continuing this conversation.
I'm not interested in working with people like you.

When you grow up, I'll maybe help you later; but from now on I'm only going to give you reviews, and you need to follow it with my request changes.
If you have an issue with my review, take it to another maintainer; because I'm officially no longer speaking to you in any capacity except via PR and issue reviews.

Also, stop using github for arguing.

@Fr-Dae
Copy link
Author

Fr-Dae commented Oct 11, 2023

image

i don't know what to tell you I've told you that I'm not interested in pursuing highschool level drama here, and you still have not changed your attitude with me and keep using every interaction I have with you as some kind of aggression

I was interested in giving you guys a solution to use the releases tags since it seems that you don't know how to use JSON files or google, but I'm no longer interested in continuing this conversation. I'm not interested in working with people like you.

When you grow up, I'll maybe help you later; but from now on I'm only going to give you reviews, and you need to follow it with my request changes. If you have an issue with my review, take it to another maintainer; because I'm officially no longer speaking to you in any capacity except via PR and issue reviews.

Also, stop using github for arguing.

Once again, don't see aggression where there isn't any.
and you're not working with me or for me, but for the Unitystation project.

and i take you as an example, i work on what i want, how i want, without reporting to anyone, except maybe gilles bod, and atner who don't take personally every thing i say and stay constructive.

@Fr-Dae
Copy link
Author

Fr-Dae commented Oct 11, 2023

also, I've just checked the script, there's no -y on full-upgrade
so the terminal lists what should be updated and proposes to do it.
so the user is free to do Y/N, the script just highlights that the user should do an update, not that he has to.

@corp-0
Copy link
Member

corp-0 commented Oct 11, 2023

We offer the flatpak, which is compatible with most distros. I dont think this script is needed or wanted by anyone but you, Dae.

@Fr-Dae
Copy link
Author

Fr-Dae commented Oct 11, 2023

We offer the flatpak, which is compatible with most distros. I dont think this script is needed or wanted by anyone but you, Dae.

https://github.com/Unitystation-fork/Unitystation-Others/blob/main/Installation-Script/UnityStationInstaller.sh
as you can see, I've updated the script to handle several distros, 6 to be exact.

in addition, flatpak doesn't create desktop shortcuts

@corp-0
Copy link
Member

corp-0 commented Oct 11, 2023

Okay, great. But we can't accept an script from outside our ecosystem. One day you guys can be mad at us and change it to do something malicious and we would have no way to know it is happening until it is too late. So accepting this PR as it is, is impossible for us.

Second issue is the code is in French, so it is harder for us to maintain. Any script we officially distribute to the players must be maintainable by us.

Third issue is I still don't see the need for this. Does this even handle updating version of StationHub? By far, flatpak is the best option if you want to have the launcher integrated into your desktop environment.

@Fr-Dae
Copy link
Author

Fr-Dae commented Oct 11, 2023

Okay, great. But we can't accept an script out of our ecosystem. One day you guys can be mad at us and change it to do something malicious and we would have no way to know it is happening until it is too late. So accepting this PR as it is, is impossible for us.
stationhub updates itself normally since your last update.

but i didn't see it that way, i only assumed that nobody would update this script except me. and that i didn't want to bother with fork/branch/pr for a simple number change.

Second issue is the code is in French, so it is harder for us to maintain. Any script we officially distribute to the players must be maintainable by us.
Only #commentary but i can modify it if you want

Third issue is I still don't see the need for this. Does this even handle updating version of StationHub? By far, flatpak is the best option if you want to have the launcher integrated into your desktop environment.
flatpak's performance is really mediocre, and creating a desktop shortcut to a flatpak application is even more difficult.

once again, I'm not asking you to remove flatpak, I'm proposing an alternative that works (or worked until 20 minutes ago).

@Fr-Dae
Copy link
Author

Fr-Dae commented Oct 13, 2023

@corp-0
As requested, here is an edited version including

  • a script offering to download the latest version of Stationhub, adapts to the distribution of the person.

  • a script proposing to remove station hub and / or unitystation

  • No submodules (no longer useful as I don't normally need to modify the path)

I wouldn't say no to a volunteer to test it under arch and debian,
I've carried out my own tests, but a test carried out on two different machines doesn't represent sufficient statistical data.

Copy link
Member

@corp-0 corp-0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some suggestions for me to accept this PR:

  1. Change all your files to another folder, I suggest creating a Misc at the root of the repo and inside that another folder for your installation scripts.
  2. Remove the dependencies from the script. Try to make this work without installing any other software.
  3. Are you sure the .desktop file works in all of the other distros mentioned in the script?
  4. Change all displayed text and comments from French to English.
  5. Consider adding something to update, which would just uninstall and install again.


Open terminal
```shell
wget https://raw.githubusercontent.com/Unitystation-fork/Unitystation-Others/main/Installation-Script/UnityStationInstaller.sh -O ~/UnityStationInstaller.sh; sudo chmod 750 ~/UnityStationInstaller.sh; sudo ~/UnityStationInstaller.sh
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are still pointing to the other repo here

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you please make a branch for me to write in?
I could then have the right links (adding hand)

that's why i wanted to make a submodule, easier to upload without modifying the documentation (and i'm glad i learned that)

### Uninstall
please copy past this on your terminal Open terminal
```shell
wget -O ~/UnitystationUnInstaller.sh https://raw.githubusercontent.com/Unitystation-fork/Unitystation-Others/main/Installation-Script/UnitystationUnInstaller.sh ; sudo chmod 750 ~/UnityStationUnInstaller.sh; sudo ~/UnityStationUnInstaller.sh
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are also giving 750 perms to the wrong script

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no it's the Un-installation script
but I need to edit it to add the folder ~/.config/unity3d/Unitystation

to make the uninstall command as complete as possible.

1)
# Commandes pour Ubuntu
apt update
apt full-upgrade
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do not upgrade the user's system.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as I said earlier, there is no -y
so the user is free to do Y/N if the suggested updates don't suit, it doesn't block the script.

it's more for "the sake of ", to remind the user to do his updates
don't forget that most users are lazy

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's more for "the sake of ", to remind the user to do his updates

this is an awful justification. This script doesn't need the user to upgrade their system, do not upgrade someone's computer because you think it is best for them. Respect the user.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keeping your system up to date is precisely the first recommendation you should make to a user, especially if they're too lazy to do it.

and i've created this script for people who want everything to work "all-in-one", the flemmes are a parameter to take into account.

you don't seem convinced?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The whole point of a script like this is to do one thing and one thing only, install stationhub.

We don't install firefox automatically because some people are too lazy or force them to install Arch Linux because they're too stupid, because it is not our concern what the user does with their PC and we shouldn't be changing anything about it just because we think its for the best.

# Commandes pour Ubuntu
apt update
apt full-upgrade
apt install -y wget p7zip || handle_error
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

try to get rid of these dependencies.

All linux distros should already have installed unzip so you can use that. Same with curl instead of wget

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

simple tools for simple use
curl is not installed by default on some distribution,
plus wget is perfectly suited to this use, download a script,

Copy link
Member

@corp-0 corp-0 Oct 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

# use curl or wget, depending on which one we find
curl_or_wget=$(if hash curl 2>/dev/null; then echo "curl -s"; elif hash wget 2>/dev/null; then echo "wget -qO-"; fi);

if [ -z "$curl_or_wget" ]; then
        echo "Neither curl nor wget found. Installing wget." >&2
        //install wget here
fi

x=$($curl_or_wget "$url")

see if either is installed in the system before installing the one you prefer.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I see, a bit of a twisted approach but why not.

and why do you insist on using curl?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had the impression that curl is the most common found in most linux systems. I read now that both are equally common, so I think the best option is to check if the system has either before installing wget

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't basing this reflection on that.

curl being a very powerful and versatile command, it often frightens people, and rightly so.

wget is all about downloading a file.
the most powerful thing you can do with it is resume an interrupted download.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know where you got that "it frightens people" thing, but I managed to get curl working pretty quick in my own installer script and the best part is that we didn't need any external dependencies.

wget is not available on all platforms or distros, and like I've mentioned; this shell script literally doesn't need an entirely new library for it to work, especially one that can affect user security.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wget, affected the security of user :D best joke ever ...

wget -O /usr/share/applications/Stationhub.desktop https://raw.githubusercontent.com/Unitystation-fork/Unitystation-Others/main/Installation-Script/Unitystation.desktop || handle_error

# Extraction du fichier zip
unzip /usr/share/Unitystation/lin-latest.zip || handle_error
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like you are using unzip here, so what do you need p7zip for?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

of memory when you want to use archive extration via a script (for example sh or py) you need p7zip

but I could be mistaken

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

7zip and unzip are both valid, some distros for some reason dont' come with 7zip

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in that case, the script would also have to find if the system has either and use that, like I suggested for curl and wget

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also you can do this to check for the existence of 7zip:

echo "Looking for 7zip.."
if ! command -v 7z &> /dev/null
then
    echo "7zip could not be found. Extract files manually."
    case $OS in
    'Linux')
        xdg-open https://7-zip.org/
        ;;
    'Windows')
        msg "%username%" 7zip could not be found. Extract files manually.
        start https://7-zip.org/
        ;;
    'Mac') 
        open https://7-zip.org/
        ;;
    'AIX') ;;
    *) ;;
    esac
    exit 1
fi

if ! command -v 7z &> /dev/null works on all platforms and distros + it checks if 7z is available, then it does things based on the user's OS; which can also be used to check if unzip is available as well and fall back to it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your script is crap and you know it.
opening a terminal and then opening a web page, did you miss the lessons on intuitiveness and the 3-click rule or what? does na notion of "user-friendly" mean anything to you?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mad lol

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mad lol

eventually,
but you did create a script because you wanted to do what I did, as if you wanted my approval and recognition, it's a shame not to have your own ideas and have to copy others because you have no talent.

In short, gilles and bod have given me recommendations as soon as I have a bit of time, and I'm going to redo it.

However, I have to thank you, so you motivated me to improve my script to prove you wrong, =)

@Fr-Dae
Copy link
Author

Fr-Dae commented Oct 13, 2023

Some suggestions for me to accept this PR:

1. Change all your files to another folder, I suggest creating a `Misc` at the root of the repo and inside that another folder for your installation scripts.

Sorry i don't understand, could you ad more details ?

2. Remove the dependencies from the script. Try to make this work without installing any other software.

is it that big a deal, to check that wget is correctly installed / up to date for the user?

3. Are you sure the `.desktop` file works in all of the other distros mentioned in the script?

normally yes, that's why with peull we chose /usr/share/ as it's the only folder that's common to all Unixds

4. Change all displayed text and comments from French to English.

ok noted

5. Consider adding something to update, which would just uninstall and install again.

I don't understand, the new version of the station hub since the 932 doesn't update itself?

@corp-0
Copy link
Member

corp-0 commented Oct 13, 2023

Sorry i don't understand, could you ad more details ?

Right now all the files are insdie a docs folder. Your scripts are not documentation. Create another folder and call it Misc as in "miscelaneous". Inside that folder, create a folder for you scripts (and remember to update the url in your readme files).

I don't understand, the new version of the station hub since the 932 doesn't update itself?

It doesn't. Users have to delete all the files and download again to update them.

@Fr-Dae
Copy link
Author

Fr-Dae commented Oct 13, 2023

Sorry i don't understand, could you ad more details ?

Right now all the files are insdie a docs folder. Your scripts are not documentation. Create another folder and call it Misc as in "miscelaneous". Inside that folder, create a folder for you scripts (and remember to update the url in your readme files).

I don't understand, the new version of the station hub since the 932 doesn't update itself?

It doesn't. Users have to delete all the files and download again to update them.

Ok thanks you

could you make a fresh branch where i can write in ?
it will make my life easier to link them and make the PR

@corp-0
Copy link
Member

corp-0 commented Oct 13, 2023

you already have a branch you own on your fork. You can do whatever you want there. Any change you make and commit will appear in this PR.

@Fr-Dae
Copy link
Author

Fr-Dae commented Oct 13, 2023

ou already have a branch you own on your fork. You can do whatever you want there. Any change you make and commit will appear in this PR.

that's exactly what's annoying me

I end up with a folder in "Unitystation-other", and
one in "Stationhub
and one in "Stationhub-fork".
given the number of changes I'd have to make to suit you, I'd rather make a separate PR in a separate branch of the Station Hub, rather than do fork-branch

if that suits you (plus it will allow me to clean up)

@corp-0
Copy link
Member

corp-0 commented Oct 13, 2023

All you have to do is right click your script folder and cut. Create another folder in the root of the repo and paste there. Commit that change.

@MaxIsJoe
Copy link
Contributor

@corp-0
Copy link
Member

corp-0 commented Oct 14, 2023

Okay, Dae. There a single thing I will say here. Stop acting like that, take the feedback and improve, no need to start dumb arguments.

Take our review and apply the changes, otherwise we will just reject your PR. Simple as.

@@ -0,0 +1,10 @@
[Desktop Entry]
Name=UnityStation
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a desktop entry for StationHub, not for Unitystation.

@@ -0,0 +1,674 @@
GNU GENERAL PUBLIC LICENSE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this need to be under a separate license? IMO if you're PR-ing it to the StationHub repo, just use the StationHub license.


3)
# Commandes pour Fedora
dnf update -y
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You said above:

as I said earlier, there is no -y
so the user is free to do Y/N if the suggested updates don't suit, it doesn't block the script.

However this DOES force a user to update their system, please do not do this.


5)
# Commandes pour CentOS
yum update -y
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

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.

4 participants