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.

Sign up for GitHub

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

Jump to bottom

Fix build #176

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Fix build #176

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b0cb9a3
Fix build
Barabas5532 Jun 24, 2023
9a4636a
Run cmake formatter
Barabas5532 Jun 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions example/app/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,17 @@
add_library (juce_modules STATIC)
add_library (focusrite-e2e::juce_modules ALIAS juce_modules)

target_link_libraries (juce_modules PRIVATE juce::juce_gui_extra)

# We're linking the modules privately, but we need to export their compile flags
target_compile_definitions (
juce_modules PUBLIC JUCE_WEB_BROWSER=0 JUCE_USE_CURL=0
INTERFACE $<TARGET_PROPERTY:juce_modules,COMPILE_DEFINITIONS>)

# We also need to export the include directories for the modules
target_include_directories (
juce_modules INTERFACE $<TARGET_PROPERTY:juce_modules,INCLUDE_DIRECTORIES>)

juce_add_gui_app (e2e-example-app VERSION 0.0.0)

target_sources (
Expand All @@ -12,14 +26,12 @@ target_compile_definitions (
e2e-example-app
PRIVATE
DONT_SET_USING_JUCE_NAMESPACE=1
JUCE_WEB_BROWSER=0
JUCE_USE_CURL=0
JUCE_APPLICATION_NAME_STRING="$<TARGET_PROPERTY:e2e-example-app,JUCE_PRODUCT_NAME>"
JUCE_APPLICATION_VERSION_STRING="$<TARGET_PROPERTY:e2e-example-app,JUCE_VERSION>"
)

target_link_libraries (
e2e-example-app PRIVATE juce::juce_gui_extra
e2e-example-app INTERFACE juce_modules
PUBLIC juce::juce_recommended_config_flags juce::juce_recommended_lto_flags
juce::juce_recommended_warning_flags)

Expand Down
28 changes: 2 additions & 26 deletions source/cpp/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,4 @@
add_library (
focusrite-e2e
include/focusrite/e2e/ClickableComponent.h
include/focusrite/e2e/Command.h
include/focusrite/e2e/CommandHandler.h
include/focusrite/e2e/ComponentSearch.h
include/focusrite/e2e/Event.h
include/focusrite/e2e/Response.h
include/focusrite/e2e/TestCentre.h
add_library (focusrite-e2e STATIC
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I read somewhere (I'll try to dig it out) that it's best practice not to include STATIC if you don't know who is consuming your library. This means that the consumer can change libraries globally with BUILD_SHARED_LIBS.

I'm not sure where I read this, so I'm happy to include STATIC if there's a good reason.

source/Command.cpp
source/ComponentSearch.cpp
source/Connection.cpp
Expand All @@ -23,23 +15,7 @@ add_library (focusrite-e2e::focusrite-e2e ALIAS focusrite-e2e)

target_include_directories (focusrite-e2e PUBLIC include)

set (JUCE_MODULES juce_core juce_events)

foreach (JUCE_MODULE ${JUCE_MODULES})

if (NOT TARGET ${JUCE_MODULE})
message(FATAL_ERROR "Missing JUCE module: ${JUCE_MODULE}, enable FOCUSRITE_E2E_FETCH_JUCE to fetch JUCE")
endif()

get_target_property (MODULE_INCLUDES ${JUCE_MODULE}
INTERFACE_INCLUDE_DIRECTORIES)
target_include_directories (focusrite-e2e PUBLIC ${MODULE_INCLUDES})
endforeach ()

target_include_directories (focusrite-e2e PRIVATE ${juce_SOURCE_DIR}/modules)

target_compile_definitions (focusrite-e2e
PRIVATE JUCE_GLOBAL_MODULE_SETTINGS_INCLUDED=1)
target_link_libraries(focusrite-e2e PUBLIC focusrite-e2e::juce_modules)

set_common_target_properties (focusrite-e2e)

Expand Down