Openal changes #2026
Openal changes #2026
Conversation
I know this is already merged, but I thought I would leave some notes on this
| - name: Generate package | ||
| run: cmake --install build --prefix ./build/artifacts ${{ matrix.build.strip }} | ||
| - uses: actions/upload-artifact@v2 | ||
| with: | ||
| name: '86Box${{ matrix.build.slug }}-Linux-x86_64-gha${{ github.run_number }}' | ||
| path: build/artifacts/** |
There was a problem hiding this comment.
Nitpick: I would avoid creating any Linux artifacts that are not self-contained and statically linked with all dependencies, otherwise the binary is useless on pretty much anything that is not the host distro.
There was a problem hiding this comment.
Yeah, I'm just cleaning stuff up, I recall the conversation about static linking and fully agree with you too. But that's for another day, unless you want to beat me to it.
| include_directories(${OPENAL_INCLUDE_DIR}) | ||
| target_link_libraries(86Box ${OPENAL_LIBRARY}) | ||
| if(OPENAL) | ||
| find_package(OpenAL REQUIRED) |
There was a problem hiding this comment.
Perhaps the find_package call could be not REQUIRED and just have OPENAL be forced off if it's not found?
There was a problem hiding this comment.
I'm really not all that good with cmake build scripts, so if you wanna, go right ahead, it'd be very welcome. (:
Summary
Allows openAL support to be disabled
A couple of consistency improvements to the github actions build
Checklist
References
Provide links to datasheets or other documentation that helped you implement this pull request.