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

cmake: errors when using SDL_sound as submodule #81

Merged
merged 7 commits into from Feb 15, 2023

Conversation

ccvelandres
Copy link
Contributor

@ccvelandres ccvelandres commented Feb 12, 2023

Genexp doesn't work in cmake versions below 3.26 in certain usage. (latest cmake release is still 3.25)

target_include_directories(SDL2_sound PUBLIC "$<TARGET_PROPERTY:SDL2::SDL2,INTERFACE_INCLUDE_DIRECTORIES>")

See note in https://cmake.org/cmake/help/latest/manual/cmake-generator-expressions.7.html#genex:TARGET_PROPERTY for reference.

Directory looks like this:

├── library1
│   ├── CMakeLists.txt              <- library_1 links to SDL_Sound
│   └── SDL_Sound                   <- submodule
├── example
│   └── CMakeLists.txt              <- exe links to library_1 
└── CMakeLists.txt                   <- root CMakeLists
CMake Error at example/CMakeLists.txt:14 (target_link_libraries):
  Error evaluating generator expression:

    $<TARGET_PROPERTY:SDL2::SDL2,INTERFACE_INCLUDE_DIRECTORIES>

  Target "SDL2::SDL2" not found.

Signed-off-by: ccvelandres <ccvelandres@gmail.com>
@ccvelandres ccvelandres changed the title cmake: replace generator expression with get_target_property cmake: errors when using SDL_sound as submodule Feb 12, 2023
Signed-off-by: ccvelandres <ccvelandres@gmail.com>
@ccvelandres
Copy link
Contributor Author

53df0ac -- Adds include directories when using SDL_sound as submodule

@sezero
Copy link
Collaborator

sezero commented Feb 12, 2023

CC: @madebr

CMakeLists.txt Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
Signed-off-by: ccvelandres <ccvelandres@gmail.com>
Signed-off-by: ccvelandres <ccvelandres@gmail.com>
@sezero
Copy link
Collaborator

sezero commented Feb 13, 2023

@madebr: Is this good now?

@madebr
Copy link
Contributor

madebr commented Feb 13, 2023

ccvelandres#1 contains my (locally tested) suggestions

cmake: create aliases + double quote escape build interface groups
@ccvelandres
Copy link
Contributor Author

Merged ccvelandres#1 and also tested locally on my end

@ccvelandres
Copy link
Contributor Author

ccvelandres commented Feb 14, 2023

One other thing I've noticed is this: we are disabling RPATH for SDL2_sound which causes this error: cannot open shared object file: No such file or directory during debug build.

In SDL, RPATH depends on ${UNIX_SYS} --> https://github.com/libsdl-org/SDL/blob/SDL2/CMakeLists.txt#L165 and https://github.com/libsdl-org/SDL/blob/SDL2/CMakeLists.txt#L438

Should we enable RPATH for unix systems same as SDL? or enable BUILD_RPATH and skip INSTALL_RPATH?

CMakeLists.txt Outdated Show resolved Hide resolved
@madebr
Copy link
Contributor

madebr commented Feb 14, 2023

Should we enable RPATH for unix systems same as SDL? or enable BUILD_RPATH and skip INSTALL_RPATH?

Personally, I would remove these 2 lines.
The cmake scripts of SDL_mixer/SDL_image/SDL_ttf don't care about the runpath.

The default behavior of CMake is imho fine: use a RUNPATH in the build directory, but remove it when installing.

@sezero sezero merged commit ba43565 into icculus:main Feb 15, 2023
@sezero
Copy link
Collaborator

sezero commented Feb 15, 2023

Thanks.

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

3 participants