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

Fix build without dlfcn.h #4351

Closed
wants to merge 1 commit into from
Closed

Conversation

ffontaine
Copy link
Contributor

@ffontaine ffontaine commented May 1, 2021

Fix the following build failures when dlfcn.h is not available on the toolchain (e.g. uclibc toolchain configured without dynamic library support):

sdl2-2.0.14/src/dynapi/SDL_dynapi.c:234:10: fatal error: dlfcn.h: No such file or directory
  234 | #include <dlfcn.h>
      |          ^~~~~~~~~

sdl2-2.0.14/src/thread/pthread/SDL_systhread.c:45:10: fatal error: dlfcn.h: No such file or directory
   45 | #include <dlfcn.h>
      |          ^~~~~~~~~

Signed-off-by: Fabrice Fontaine fontaine.fabrice@gmail.com

@sezero
Copy link
Contributor

sezero commented May 1, 2021

I can't reproduce the issue: I hand-edited SDL_dynapi.h to unconditionally define SDL_DYNAMIC_API
as 0, built usually like ../configure && make, no errors.

@sezero sezero added the waiting Waiting on user response label May 1, 2021
@ffontaine
Copy link
Contributor Author

To get the error, you have to build sdl with a toolchain that don't have dlfcn.h (e.g. an uclibc toolchain configured without dynamic library support). You could reproduce it by renaming/removing dlfcn.h on any "standard" distribution.

@sezero
Copy link
Contributor

sezero commented May 1, 2021

Huh. Then the issue has nothing at all to do with SDL_DYNAMIC_API

With the current configuration scripts, the closest thing would be SDL_LOADSO_DLOPEN
but autotools and cmake must be updated to define HAVE_DLFCN_H and/or HAVE_DLOPEN

@slouken, @icculus?

@ffontaine
Copy link
Contributor Author

Thanks for your feedback, I updated the PR to use SDL_LOADSO_DLOPEN. I'll update it if you have further comments. For example, perhaps it would be better to set SDL_DYNAMIC_API to 0 if SDL_LOADSO_DLOPEN is not defined.

@ffontaine ffontaine changed the title src/thread/pthread/SDL_systhread.c: fix build with SDL_DYNAMIC_API=0 src/thread/pthread/SDL_systhread.c: fix build with dlfcn.h May 1, 2021
@sezero
Copy link
Contributor

sezero commented May 1, 2021

Thanks for your feedback, I updated the PR to use SDL_LOADSO_DLOPEN.

Looks a bit saner, but our configury and cmake'ry must really be updated to
define (or not define) HAVE_DLFCN_H and/or HAVE_DLOPEN, and then
you should use them instead in pthread/SDL_systhread.c. Waiting for comments
from @slouken and/or @icculus for it.

For example, perhaps it would be better to set SDL_DYNAMIC_API to 0 if SDL_LOADSO_DLOPEN is not defined.

That makes sense to me.

@ffontaine ffontaine changed the title src/thread/pthread/SDL_systhread.c: fix build with dlfcn.h src/thread/pthread/SDL_systhread.c: fix build without dlfcn.h May 1, 2021
src/dynapi/SDL_dynapi.h Outdated Show resolved Hide resolved
Fix the following build failures when dlfcn.h is not available on the
toolchain (e.g. uclibc toolchain configured without dynamic library
support):

sdl2-2.0.14/src/dynapi/SDL_dynapi.c:234:10: fatal error: dlfcn.h: No such file or directory
  234 | #include <dlfcn.h>
      |          ^~~~~~~~~

sdl2-2.0.14/src/thread/pthread/SDL_systhread.c:45:10: fatal error: dlfcn.h: No such file or directory
   45 | #include <dlfcn.h>
      |          ^~~~~~~~~

Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
@ffontaine ffontaine changed the title src/thread/pthread/SDL_systhread.c: fix build without dlfcn.h Fix build without dlfcn.h May 1, 2021
@sezero
Copy link
Contributor

sezero commented May 1, 2021

Here is something more intrusive, but should do the job properly -I hope-
See attached patch:
dlopen_patch.txt

Comments? @slouken, @icculus: please review.

@sezero
Copy link
Contributor

sezero commented May 3, 2021

Here is something more intrusive, but should do the job properly -I hope-

OK, I created #4356 for this

@orbea
Copy link
Contributor

orbea commented May 6, 2021

I think this is at least part of the problem... #4367

@slouken
Copy link
Collaborator

slouken commented Jul 8, 2021

Is this pull request taken care of by #4356?

@sezero
Copy link
Contributor

sezero commented Jul 8, 2021

Is this pull request taken care of by #4356?

That was my intention, but #4356 has been taking more time than it should
(because I felt lazy with it.)

Maybe you can apply this one as a temporary band-aid for @ffontaine's issue?

slouken pushed a commit that referenced this pull request Aug 10, 2021
- cmake, configure (CheckDLOPEN): --enable-sdl-dlopen is now history..
  detach the dl api discovery from SDL_LOADSO_DLOPEN functionality.
  define HAVE_DLOPEN. also define DYNAPI_NEEDS_DLOPEN (CheckDLOPEN is
  called only for relevant platforms.)
- update SDL_config.in and SDL_config.cmake accordingly.
- SDL_dynapi.h: set SDL_DYNAMIC_API to 0 if DYNAPI_NEEDS_DLOPEN is
  defined, but HAVE_DLOPEN is not.
- pthread/SDL_systhread.c: conditionalize dl api use to HAVE_DLOPEN
- SDL_x11opengl.c, SDL_DirectFB_opengl.c, SDL_naclopengles.c: rely
  on HAVE_DLOPEN, not SDL_LOADSO_DLOPEN.
- SDL_config_android.h, SDL_config_iphoneos.h, SDL_config_macosx.h,
  SDL_config_pandora.h, and SDL_config_wiz.h: define HAVE_DLOPEN.

Closes: #4351
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting Waiting on user response
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants