bpo-11063: Add a configure check for uuid_generate_time_safe #4287
Conversation
pyconfig.h.in
Outdated
| @@ -571,6 +571,9 @@ | |||
| /* Define to 1 if you have the <libutil.h> header file. */ | |||
| #undef HAVE_LIBUTIL_H | |||
|
|
|||
| /* Define to 1 if you have the `uuid' library (-luuid). */ | |||
There was a problem hiding this comment.
It's not the uuid library, it's specifically uuid_generate_time_safe() (which exists on Linux, but not on OS X, even though OS X does have libuuid).
There was a problem hiding this comment.
That looks autogenerated. The configure check works here on Linux.
With the patch:
checking for uuid_generate_time_safe in -luuid... yes
Now use a non-existing function name:
AC_CHECK_LIB(uuid, uuid_generate_time_unsafe)
checking for uuid_generate_time_unsafe in -luuid... no
I did not look at the autogenerated configure, but it seems to work here.
configure
Outdated
| cat confdefs.h - <<_ACEOF >conftest.$ac_ext | ||
| /* end confdefs.h. */ | ||
|
|
||
| /* Override any GCC internal prototype to avoid an error. |
There was a problem hiding this comment.
Is this comment copy-pasted from somewhere else? It doesn't seem to apply here...
configure
Outdated
| #ifdef __cplusplus | ||
| extern "C" | ||
| #endif | ||
| char uuid_generate_time_safe (); |
There was a problem hiding this comment.
The actual signature here is int uuid_generate_time_safe(uuid_t out).
configure
Outdated
| int | ||
| main () | ||
| { | ||
| return uuid_generate_time_safe (); |
There was a problem hiding this comment.
This call should match the function's signature.
configure
Outdated
| $as_echo_n "(cached) " >&6 | ||
| else | ||
| ac_check_lib_save_LIBS=$LIBS | ||
| LIBS="-luuid $LIBS" |
There was a problem hiding this comment.
setup.py has a bit of code to find libuuid in the proper place (self.compiler.find_library_file(lib_dirs, 'uuid')). It sounds like you should replicate that in the configure script, but it looks like you don't?
configure
Outdated
| #define HAVE_LIBUUID 1 | ||
| _ACEOF | ||
|
|
||
| LIBS="-luuid $LIBS" |
There was a problem hiding this comment.
I'm not sure about this. -luuid is only needed for the _uuid extension module, and it's already added by setup.py.
Modules/_uuidmodule.c
Outdated
| static PyObject * | ||
| py_uuid_generate_time_safe(void) | ||
| { | ||
| #ifdef HAVE_TIME_SAFE | ||
| #if defined(HAVE_UUID_GENERATE_TIME_SAFE) && HAVE_UUID_GENERATE_TIME_SAFE |
There was a problem hiding this comment.
I don't think that it's correct to test the define value. Only check if it's defined:
#ifdef HAVE_UUID_GENERATE_TIME_SAFE
There was a problem hiding this comment.
Yeah, I was about to write about that. I couldn't make autoconf set #define HAVE_UUID_GENERATE_TIME_SAFE 0 if uuid_generate_time_safe() isn't available yet. Working on it :)
There was a problem hiding this comment.
What was broken about the original patch except that some autogenerated comments were suboptimal?
The existing AC_CHECK_LIB(dld, shl_load) should have all the same problems.
|
Ok, I think I've addressed most of Antoine's (and Victor's) comments:
|
berkerpeksag
force-pushed
the
11063-configure
branch
from
57a020a to
0f5703a
Compare
configure.ac
Outdated
| uuid_t out; | ||
| uuid_generate_time_safe(out); | ||
| ])], | ||
| [ax_cv_uuid_generate_time_safe=yes], |
There was a problem hiding this comment.
Is it possible to put directly the AC_DEFINE() here, instead of using a variable?
[ax_cv_uuid_generate_time_safe=no] would be replaced with [] in that case.
There was a problem hiding this comment.
I tried that but I got the following warning:
configure.ac:2688: warning: AC_CACHE_VAL(ax_cv_uuid_generate_time_safe, ...): suspicious presence of an AC_DEFINE in the second argument, where no actions should be taken
There was a problem hiding this comment.
Oh, I hate autotools, mysterious tools... Ok, leave it as it is ;-)
https://bugs.python.org/issue11063