Add libpq version discovery #323
Conversation
| @@ -899,6 +914,7 @@ INIT_MODULE(_psycopg)(void) | |||
| /* set some module's parameters */ | |||
| PyModule_AddStringConstant(module, "__version__", PSYCOPG_VERSION); | |||
| PyModule_AddStringConstant(module, "__doc__", "psycopg PostgreSQL driver"); | |||
| PyModule_AddIntConstant(module, "__libpq_version__", PG_VERSION_NUM); | |||
There was a problem hiding this comment.
I think we may just use strtol(PG_VERSION_HEX, NULL, 0) here and avoid introducing PG_VERSION_NUM?
There was a problem hiding this comment.
On Mon, Jun 1, 2015 at 6:41 PM, Daniele Varrazzo notifications@github.com
wrote:
In psycopg/psycopgmodule.c
#323 (comment):@@ -899,6 +914,7 @@ INIT_MODULE(_psycopg)(void)
/* set some module's parameters */
PyModule_AddStringConstant(module, "version", PSYCOPG_VERSION);
PyModule_AddStringConstant(module, "doc", "psycopg PostgreSQL driver");
- PyModule_AddIntConstant(module, "libpq_version", PG_VERSION_NUM);
I think we may just use strtol(PG_VERSION_HEX, NULL, 0) here and avoid
introducing PG_VERSION_NUM?I think that is going to be confusing, since both PQserverVersion() and
PQlibVersion() are specified to return the number constructed in decimal,
and we already have a check in skip_before/after_postgres that uses this
exact format.
Moreover, PG_VERSION_HEX is not a string, but also a number like
PG_VERSION_NUM, just constructed differently, so that strtol() call won't
even compile.
Now that I look at how PG_VERSION_HEX is constructed, I think it's going to
be a lot more confusing when PG version number wraps around 10: it's going
to look like, e.g. 0x0a0102, while PG_VERSION_NUM for the same version
would be 100102. Since we do not even expose the hex version in python, I
would see more sense in moving away from it in favor of decimal one.
Alex
There was a problem hiding this comment.
It could be a good idea actually. I actually never noticed that PQserverVersion is decimal while ours is hex :)
There was a problem hiding this comment.
And I think we don't really need to supply the version through setup.py, we can just include pg_config.h, where PG_VERSION_NUM is defined.
There was a problem hiding this comment.
I'm not 100% sure we have pg_config.h available if only libpq-dev is installed. I'm fine having PG_VERSION_NUM only for now. I'll drop _HEX on top of your patches and merge it, thank you.
|
I would love to see a test here, just to verify that the constant exists and is a number and the function exists and returns a number or, if it raises the exception, that the constant reports < 9.1. Thank you very much for the patch! |
|
Good point, test added. |
|
I did some adjustments to the docs, dropped the _HEX constant and merged. Now you may use it for the test in #321. Thank you very much! |
Patch for issue #35