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

Export dynamic and static library names in sdl2_image-config cmake input file #390

Closed
wants to merge 2 commits into from

Conversation

bcarmo-caio
Copy link

@bcarmo-caio bcarmo-caio commented Oct 4, 2023

I am opening this PR to export SDL2_image library names (whether it is dynamic or static) so end user may call them just using a variable as it done in SDL2

Currently, a CMakeFile.txt has to be like

...
target_link_libraries(${PROJECT_NAME} ${SDL2_LIBRARIES} SDL2_image::SDL2_image)
...

And with this fix, we can call it using

target_link_libraries(${PROJECT_NAME} ${SDL2_LIBRARIES} ${SDL2_IMAGE_LIBRARIES})
or
target_link_libraries(${PROJECT_NAME} ${SDL2_LIBRARIES} ${SDL2_IMAGE_STATIC_LIBRARIES})

I kept the same format as SDL2 so users may expect similar naming


P.S.: I don't why SDL2 exports the variables but does not use them inside sdl2-config.cmake itself. I mean, they

set(SDL2_LIBRARIES SDL2::SDL2)                                                   
set(SDL2_STATIC_LIBRARIES SDL2::SDL2-static)

but then

if(TARGET SDL2::SDL2-static AND NOT TARGET SDL2::SDL2)

instead of

if(TARGET ${SDL2_STATIC_LIBRARIES} AND NOT TARGET ${SDL2_LIBRARIES})

I did test both cases (dynamic and static) here and I could remove the hardcoded SDL2_image::SDL2_image and SDL2_image::SDL2_image-static from the entire file.


When using dynamic (target_link_libraries(${PROJECT_NAME} ${SDL2_LIBRARIES} ${SDL2_IMAGE_LIBRARIES})):

$readelf -d test
Dynamic section at offset 0x52d88 contains 32 entries:
  Tag        Type                         Name/Value
 0x0000000000000001 (NEEDED)             Shared library: [libSDL2-2.0.so.0]
 0x0000000000000001 (NEEDED)             Shared library: [libSDL2_image-2.0.so.0] <-- IMG_Load will be loaded from here
...
 0x000000000000001d (RUNPATH)            Library runpath: [/opt/SDL2-2.28.2/lib:/home/user/SDL2_image-2.6.3_fixed/lib]



$readelf -s test
Symbol table '.dynsym' contains 122 entries:
...
101: 0000000000000000     0 FUNC    GLOBAL DEFAULT  UND IMG_Load  <-- IMG_Load call is present in .dynsym, so code
                                                                      is not in this binary

When using static (target_link_libraries(${PROJECT_NAME} ${SDL2_LIBRARIES} ${SDL2_IMAGE_STATIC_LIBRARIES})):

$readelf -d test
Dynamic section at offset 0x78d88 contains 31 entries:
  Tag        Type                         Name/Value
 0x0000000000000001 (NEEDED)             Shared library: [libSDL2-2.0.so.0]
...
 0x000000000000001d (RUNPATH)            Library runpath: [/opt/SDL2-2.28.2/lib] <-- No SDL2_image will be searched



readelf -s test
Symbol table '.symtab' contains 3595 entries:
3262: 0000000000035fa0    89 FUNC    GLOBAL DEFAULT   14 IMG_Load <--- because IMG_Load code is here in binary =)

Please do let me know if I am doing something wrong in removing the hardcoded names

Best regards

@madebr
Copy link
Contributor

madebr commented Oct 4, 2023

I intentionally did not add cmake variables to the satellite libraries as those are "the old way", and easy to corrupt.
SDL2 has these variables for backwards compatibility.

I think it's nicer to use SDL2_image::SDL2_image then ${SDL2_IMAGE_LIBRARIES}. CMake variables don't add value.

@bcarmo-caio
Copy link
Author

Oh! I see. Thanks for letting me know that it was an old fashioned way and that SDL2 has kept it just for backwards compatibility.
I'm changing my project CMakeLists.txt to
target_link_libraries(${PROJECT_NAME} SDL2::SDL2 SDL2_image::SDL2_image)
right now =)

Also closing this PR as it seems to make no sense then.

Regards

@bcarmo-caio bcarmo-caio closed this Oct 5, 2023
@madebr
Copy link
Contributor

madebr commented Oct 5, 2023

SDL3 will not provide any CMake variables, so now you're prepared 😉

@bcarmo-caio
Copy link
Author

Nice! Thanks for the heads up 👍

@madebr
Copy link
Contributor

madebr commented Oct 5, 2023

You might want to add SDL2_image::SDL2_image first.

target_link_libraries(${PROJECT_NAME} PRIVATE SDL2_image::SDL2_image SDL2::SDL2)

When SDL2_image is a static library, SDL2 symbols are required at link time.
SDL2_image::SDL2image does not automatically add SDL2::SDL2 so the user can choose between a shared/static SDL2 library, and also we don't know whether the SDL2::SDL2 target actually exists.

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.

2 participants