bpo-11063: Add a configure check for uuid_generate_time_safe#4287
bpo-11063: Add a configure check for uuid_generate_time_safe#4287berkerpeksag merged 5 commits intopython:masterfrom
Conversation
pyconfig.h.in
Outdated
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
There was a problem hiding this comment.
Is this comment copy-pasted from somewhere else? It doesn't seem to apply here...
configure
Outdated
There was a problem hiding this comment.
The actual signature here is int uuid_generate_time_safe(uuid_t out).
configure
Outdated
There was a problem hiding this comment.
This call should match the function's signature.
configure
Outdated
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
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
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:
|
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 ;-)
vstinner
left a comment
There was a problem hiding this comment.
LGTM.
Thank you Berker, you are more brave than me :-)
https://bugs.python.org/issue11063