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

Changed the install.sh script a lot, added the minor alsa include fix. #219

Closed
wants to merge 23 commits into from

Conversation

map588
Copy link
Contributor

@map588 map588 commented Jan 27, 2025

I changed the ordering of the install script to try the package manager, download, and cmake compile (I think) every platform. Also that tiny alsa #include that breaks every build on a platform with no alsa.

@map588
Copy link
Contributor Author

map588 commented Jan 27, 2025

The alsa library is impossible to fix without this repo adding the:

#if defined(USE_ALSA)
#include <alsa/asoundlib.h>
#endif

Because the compile step pulls the current state of the repo. @Slackadays

Copy link

codecov bot commented Jan 27, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 48.69%. Comparing base (cc00a8d) to head (7e267dd).
Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #219      +/-   ##
==========================================
+ Coverage   47.25%   48.69%   +1.44%     
==========================================
  Files          48       53       +5     
  Lines        3028     3768     +740     
==========================================
+ Hits         1431     1835     +404     
- Misses       1597     1933     +336     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@map588
Copy link
Contributor Author

map588 commented Jan 27, 2025

Also, the links are still not working (at least as they are written in the installer script) @Slackadays
Could it just need time to update, or are they at a new location?
image

@Slackadays
Copy link
Owner

I'm thinking we should just can the nightly links because it's not guaranteed that a nightly build will work, instead using the release links instead which never expire. Do you mind making that change so we don't need to worry about nightlies anymore?

@map588
Copy link
Contributor Author

map588 commented Jan 28, 2025

I made a separate pull request with only one commit from an organization called synchrono-org. @Slackadays

@Slackadays
Copy link
Owner

Thank you for helping fix this monster of an install script!

@map588
Copy link
Contributor Author

map588 commented Jan 28, 2025

Sure! I'm invested at this point. Does this CI cost you anything? I don't want to run up a bill on you. @Slackadays

@map588
Copy link
Contributor Author

map588 commented Jan 28, 2025

I can get the libs into /usr/local/lib, cb into /usr/local/bin, but they are not properly linked. I changed the CMakeLists.txt to not require ALSA, but instead if NO_ALSA then check for ALSA, and if it doesn't exist, just don't set USE_ALSA. That might have some weird CMake side effects, but the build was failing every time before because of that.

In a machine without alsa, if you try to include it, it will fail, so thats what the guards are for. All tested platforms can be "installed", but the executable fails at runtime for those platforms.

x86_64 sometimes shows up in $(uname -m) as amd64, so adding amd64 solved some problems.

Overall net zero attempt, possibly negative, but if its a problem with the CI configuration, then it might work on more systems now. Hopefully this gives some insight for what the problems may be.

@map588 map588 closed this Jan 28, 2025
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.

None yet

2 participants