Update CMakeLists.txt #2534
Update CMakeLists.txt #2534
Conversation
Don't include ${PROJECT_SOURCE_DIR}/external_libs globally as it affects Linux builds. Instead add specifically for MSCV and APPLE only
| #set(oscpack_path ${PROJECT_SOURCE_DIR}/external_libs/oscpack_1_1_0) | ||
| #add_subdirectory(${oscpack_path}) | ||
| include_directories( ${PROJECT_SOURCE_DIR}/external_libs/spdlog-0.11.0/include JuceLibraryCode JuceLibraryCode/modules ${PROJECT_SOURCE_DIR}/external_libs/cxxopts ${PROJECT_SOURCE_DIR}/external_libs) | ||
| #include_directories(${PROJECT_SOURCE_DIR}/external_libs/spdlog-0.11.0/include JuceLibraryCode JuceLibraryCode/modules ${PROJECT_SOURCE_DIR}/external_libs/cxxopts) | ||
| #don't include ${PROJECT_SOURCE_DIR}/external_libs globally as it affects linux builds where it is not wanted/ Add instead just for MSCV and APPLE builds | ||
| #include_directories( ${PROJECT_SOURCE_DIR}/external_libs/spdlog-0.11.0/include JuceLibraryCode JuceLibraryCode/modules ${PROJECT_SOURCE_DIR}/external_libs/cxxopts ${PROJECT_SOURCE_DIR}/external_libs) | ||
| include_directories(${PROJECT_SOURCE_DIR}/external_libs/spdlog-0.11.0/include JuceLibraryCode JuceLibraryCode/modules ${PROJECT_SOURCE_DIR}/external_libs/cxxopts) |
There was a problem hiding this comment.
@rbnpi - any idea whether these commented lines were needed?
(The one explaining the omission of the external libs makes sense, but not sure the others are needed?) no idea 🤷♂️
There was a problem hiding this comment.
I left the commented stuff that was there already. I guess it could be removed. (look at the diff for my PR), There were two similar lines, one including ${PROJECT_SOURCE_DIR}/external_libs the other without it. I switches these lines around, and added the inclusion of ${PROJECT_SOURCE_DIR}/external_libs in existing if(MSCV) and if(APPLE) sections, where they were originally before it was tided up making this a global inclusion. However Linux builds with librtmidi-dev package installed didn't like this. The alternative would be to treat Linux the same, but to omit the package and the -dl rtmidi which links it in at the end of the CMakeList file. With thoughts of producing a .deb file (later), it is preferable to use the pakge than include it in the SP code.
There was a problem hiding this comment.
Without ${PROJECT_SOURCE_DIR}/external_libs it can't find the rtmidi headers. (on Linux - without rtmidi installed)
There was a problem hiding this comment.
But I will add that the CMakefile needs a few more changes:
diff --git a/app/gui/qt/external/sp_midi/CMakeLists.txt b/app/gui/qt/external/sp_midi/CMakeLists.txt
index 1b1bf6d4..860ded69 100644
--- a/app/gui/qt/external/sp_midi/CMakeLists.txt
+++ b/app/gui/qt/external/sp_midi/CMakeLists.txt
@@ -46,6 +47,11 @@ if(APPLE)
add_definitions(-D__MACOSX_CORE__)
endif(APPLE)
+if(UNIX)
+ list(APPEND sp_midi_sources ${PROJECT_SOURCE_DIR}/external_libs/rtmidi/RtMidi.cpp)
+ add_definitions(-D__LINUX_ALSA__)
+endif(UNIX)
+
if(APPLE)
set(juce_sources
JuceLibraryCode/include_juce_core.mm
@@ -87,6 +93,6 @@ elseif(APPLE)
target_link_libraries(libsp_midi "-framework CoreMIDI -framework CoreAudio -framework CoreFoundation -framework Accelerate -framework QuartzCore -framework AudioToolbox -framework IOKit -framework DiscRecording -framework Cocoa")
elseif(UNIX)
add_definitions(-DLINUX=1 -DNDEBUG=1 -DJUCER_LINUX_MAKE_6D53C8B4=1 -DJUCE_APP_VERSION=1.0.0 -DJUCE_APP_VERSION_HEX=0x10000)
- target_link_libraries(libsp_midi pthread ${ALSA_LIBRARY} dl rtmidi)
+ target_link_libraries(libsp_midi pthread ${ALSA_LIBRARY} dl)
endif(MSVC)There was a problem hiding this comment.
I have submitted #2542 for that. I'm not sure why Linux should be an exception here.
| @@ -38,11 +39,13 @@ set(sp_midi_sources | |||
|
|
|||
| if(MSVC) | |||
| list(APPEND sp_midi_sources ${PROJECT_SOURCE_DIR}/external_libs/rtmidi/RtMidi.cpp) | |||
| include_directories(${PROJECT_SOURCE_DIR}/external_libs) | |||
Don't include ${PROJECT_SOURCE_DIR}/external_libs globally as it affects Linux builds. Instead add specifically for MSCV and APPLE only