Fixed install script, named the required openssl libraries in the README, and fixed a small bug regarding systems without ALSA installed. #216
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I realize now there are problems with the CI, likely to do with the ALSA library issues, and wayland support. Could also be the sudo addition to cmake --install . |
… and added the names of necessary libraries in the README for OpenSSL Modified install script to either use a package manager, or compile with cmake, due to the broken links. Added 'has_alsa' check, and calls cmake with -DNO_ALSA=true if alsa does not exist
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I separated the bug fix and README edit from the install script changes because they are more minor changes. I just added two lines around #include<alsa/asoundlib.h> that check for USE_ALSA, just like the function below it. The README just contains the specific packages needed, because the openssl package does not include the -dev package needed for includes.
As for the install.sh script, I broke it into two parts, install checks if it can be installed with a package manager, and if not, it just calls the compile script rather than using the downloads. I put the second part of the script in download.sh, and commented out calling sh ./download.sh, so if the links are fixed, the install script would work the same way as before.
The compile() function also did not work on my machine without
sudo cmake --install ., so I just moved that part below where it checks for OpenBSD, and addedelse sudo cmake --install . fi, so it doesn't attempt to use install without sudo and fail the build before the check.The changes could definitely be improved, (like checking if sudo is needed in that last example), but it makes the install script functional again for distributions which use the APT package manager, which is a lot of distributions.