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

Fix various build issues - #172, #173, #178 #182

Merged
merged 6 commits into from
Sep 17, 2022

Conversation

mkilgore
Copy link
Contributor

This is a handful of related changes:

  1. Fixed the Linux build agent sound issue. You can see the commit description for the details, but from my testing sound tests always pass on Linux now.
  2. I add a specific set of tests for $CONSOLE:ONLY dependency combinations, since $CONSOLE:ONLY is a bit jank and these have been causing us pain lately.
  3. Since the audio issues are fixed, I was able to pull in the tests from Determine why new SOUND tests do not work in CI, and add them back. #173
  4. The build fix for _DEVICES fails to compile in $CONSOLE:ONLY #178 was added, we always link -lwinmm if we're using _DEVICES.
  5. Fixed audio.cpp should not include libqb.h, doing so breaks $Console:Only #172 by adding -DDEPENDENCY_CONSOLE_ONLY when building audio.o. This tells common.h to avoid including freeglut.h, which is what caused the error.

Fixes: #172
Fixes: #173
Fixes: #178

It's still not entirely clear what the underlying issue is, but the ALSA
device provided by pulseaudio stops working after so many tests use it.
I've tried various approaches, but simply restarting pulseaudio after
every test is a bruteforce but successful solution. In practice it also
doesn't have any noticeable performance penalty, so it seems like a file
solution.

The `CI_TESTING` environment variable is used to avoid restarting
pulseaudio if the tests aren't being run in the CI environment (we don't
want to restart your pulseaudio instance if you're running them locally!)
libz tests were missing, I added one to test the libz dependency.

Additionally $CONSOLE:ONLY creates many weird interactions and is
currently broken in some cases. This adds tests for all the dependencies
that can work with $CONSOLE:ONLY to verify that they compile correctly.
The _DEVICES command pulls in some stuff that depends on `winmm`.
Normally this isn't a problem, but if `$CONSOLE:ONLY` is used then this
dependency can actually get removed from the `CXXLIBS` and cause linking
to fail.

It's easy enough to fix this by always adding `-lwinmm` if `_DEVICES` is
in use.

Fixes: QB64-Phoenix-Edition#178
A quick fix, but providing `DEPENDENCY_CONSOLE_ONLY` when compiling
audio.cpp tells `common.h` to avoid pulling in `freeglut.h`, which fixes
the linking issue.

Fixes: QB64-Phoenix-Edition#172
These tests were intended to be added by QB64-Phoenix-Edition#171, but the Linux build was
having problems. The issues with the build have since been resolved so
the tests are getting added back in.

Fixes: QB64-Phoenix-Edition#173
@mkilgore mkilgore added the bug Something isn't working label Sep 17, 2022
@mkilgore mkilgore merged commit 500bf17 into QB64-Phoenix-Edition:main Sep 17, 2022
@mkilgore mkilgore mentioned this pull request Sep 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants