alesbe / sorting-visualizer

Sorting visualizer made with SFML and C++ 📊
MIT License
17 stars 11 forks source link

Adding OS detection and install dependencies with install.sh #8

Closed alesbe closed 2 years ago

alesbe commented 2 years ago

Describe you problem / suggestion:

Installing automatically the required dependencies (sfml), with install.sh would be easier that installing the dependencies manually.

Steps to reproduce (optional):

none

Possible fix (optional):

OS

bazuin-32 commented 2 years ago

I'm thinking rather than trying to detect the distribution it would be better to check which package manager exists on the system (e.g. with command -v pacman or command -v apt, etc.). If this doesn't make sense or I'm missing something, let me know.

Would I be able to implement this and make a pull request? Sorry for my lack of knowledge here, I'm new to open source and have never made a PR before, but is this a good guideline for making a PR?

I also think that option 2 of compiling SFML with the rest of the code could be used as a fallback option if none of the package managers were found, but I don't have much experience with Makefiles so I wouldn't be able to implement that part at this time.

alesbe commented 2 years ago

I'm thinking rather than trying to detect the distribution it would be better to check which package manager exists on the system (e.g. with command -v pacman or command -v apt, etc.). If this doesn't make sense or I'm missing something, let me know.

Detecting the packet manager rather than the distribution seems like a better approach, also SFML is available in the most used distros!

Would I be able to implement this and make a pull request? Sorry for my lack of knowledge here, I'm new to open source and have never made a PR before, but is this a good guideline for making a PR?

Sure, don't worry, go for it! There are some guidelines for your first PR, but can be summarized in forking this repository, commiting the changes to that fork, and then, create the PR, you can do that directly from Github.

I also think that option 2 of compiling SFML with the rest of the code could be used as a fallback option if none of the package managers were found, but I don't have much experience with Makefiles so I wouldn't be able to implement that part at this time.

Yes, that approach seems the best! At this moment the Makefile is not correctly configured, but add it if you want as an option too if the packet manager or the sfml package can't be found.

Also, after downloading the libraries from the package manager, you can use this to build the executable instead of the Makefile: g++ *.cpp -o sorting-visualizer -lsfml-graphics -lsfml-window -lsfml-system

Comment this issue if you need something else!

bazuin-32 commented 2 years ago

So far I have this function which is called from compile if sfml libraries were not found on the system already (checked through ldconfig):

install_sfml() {
        # detect the package manager and install sfml
        # accordingly

        if command -v pacman &>/dev/null; then
                pacman -S sfml
        elif command -v apt-get &>/dev/null; then
                apt-get install libsfml-dev
        elif command -v dnf &>/dev/null; then
                dnf install SFML
        else
                # package manager was not found
                # maybe compile sfml ourselves?
                echo "Could not install SFML using the system package manager"
                exit
        fi
}

Let me know if I should add any other package managers as well, these were just the ones that I knew of and could easily look up the name of their sfml packages.

This works on my Arch system as expected when running the script as root. As installing packages requires root permissions, I'm curious what you think should be done in the case that the script is run as a normal user. Should it just error, or run the command with sudo and let the user enter their password, or something else?

Also, as of right now using the Makefile compiles correctly regardless of how sfml was installed. Is there a reason I should switch to just the command above or should I just keep it as it is?

alesbe commented 2 years ago

The function seems great! Don't worry about adding more distros if it's a bit tedious, right now having at least ubuntu/debian and arch works for now.

Related to the makefile, was intended at first to build the executable using the libraries included with the project, but I couldn't figure out how to move the libraries to the path and compile them with the source code using g++, that's why I switched to the idea of just installing the libraries from the packet manager, SFML it's a popular library so the majority of packet managers should have it. Maybe using directly the g++ command is better than running make, because the makefile right now is not working properly, at least for now.

And related to the root, I'm not sure what is the usual, exiting the install script and asking the user to run it as root, or run the commands with sudo and let the user enter the password, maybe we should run the install script directly as root for convinience?

The steps can be something like:

0.- If the script wasn't run as root, exit

1.- Install sfml package, if found install through the packet manager, else makefile with the libraries included (not working for now)

2.- Compile the executable using the g++ command

3.- Ask to move the binary to /usr/bin, if yes move the executable, else exit the program

If you have any suggestions let me know!

bazuin-32 commented 2 years ago

After giving it some more thought, I decided your original logic of asking to be run as root if moving to /usr/bin was best, so I used that idea and made install_sfml tell the user to run the script as root and then exit.

I didn't touch the makefile for now since as I said before I don't have much experience with them.

I went ahead and submitted a PR, but if you think I should change anything let me know and I'll gladly do that.

Edit: just wanted to mention CMake, not sure if you've tried using it before. It is what I normally use for my projects, and it would be really easy to add sfml libraries to the project and compile it with your program in that case. You should check it out. Tomorrow I might be able to write up a CMakeLists.txt file if you'd be interested in using that.

alesbe commented 2 years ago

Sure, I'll open another issue related to that if you want to contribute, thanks!

alesbe commented 2 years ago

9 solves this installing the libraries directly from the distro packet manager, closing issue