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

@skip_after_libpq(16) uses compile-time libpq version #1619

Closed
df7cb opened this issue Sep 10, 2023 · 6 comments
Closed

@skip_after_libpq(16) uses compile-time libpq version #1619

df7cb opened this issue Sep 10, 2023 · 6 comments

Comments

@df7cb
Copy link
Contributor

df7cb commented Sep 10, 2023

While compatibility with PG16 has been fixed in #1582, it's not entirely done yet: In Debian, when testing if upgrading libpq5 from 15 to 16 is safe, the problems fixed in #1582 reappear:

 46s test_ssl_attribute (tests.test_connection.TestConnectionInfo.test_ssl_attribute) ... FAIL
 46s test_ssl_in_use (tests.test_connection.TestConnectionInfo.test_ssl_in_use) ... ok
 46s test_ssl_not_supported (tests.test_connection.TestConnectionInfo.test_ssl_not_supported) ... skipped 'skipped because libpq 150003'

https://ci.debian.net/data/autopkgtest/unstable/amd64/p/psycopg2/37551454/log.gz

This happens when the psycopg2 compiled against libpq5 15 is run with libpq5 16. @skip_after_libpq(16) does not trigger because it uses psycopg2.__libpq_version__ which is statically initialized:

    if (0 > PyModule_AddIntConstant(module,
        "__libpq_version__", PG_VERSION_NUM))
    { return -1; }

In short, this constant shouldn't be a constant, but be loaded at run time.

I tried replacing PG_VERSION_NUM with PQlibVersion(), but that does not seem to be enough, there are other places with more version numbers, and setup.py even redefines its own PG_VERSION _NUM at which point I gave up.

Could someone with more insight into which version number is supposed to mean what check what needs to be done to fix @skip_after_libpq(16) to look at the run-time libpq version? Thanks.

@df7cb
Copy link
Contributor Author

df7cb commented Sep 10, 2023

Fwiw, switching that test to use @skip_after_postgres won't help since the server version in that test is still 15, only libpq5 is used from 16.

@df7cb
Copy link
Contributor Author

df7cb commented Sep 10, 2023

Afaict the problem is a5382d7 which rounds libpq_version() down to __libpq_version__.

@dvarrazzo
Copy link
Member

In short, this constant shouldn't be a constant, but be loaded at run time.

The runtime version is available in libpq_version(). The __libpq_version__ is meant exactly to represent the libpq version psycopg was compiled with, and hence which version-depending features were compiled at build time.

dvarrazzo added a commit that referenced this issue Sep 10, 2023
@dvarrazzo
Copy link
Member

@df7cb could you be so kind to test if the changeset in 55d8899 fixes the problem?

@df7cb
Copy link
Contributor Author

df7cb commented Sep 11, 2023

ERROR: test_ssl_attribute (tests.test_connection.TestConnectionInfo.test_ssl_attribute)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/myon/projects/postgresql/psycopg2/psycopg2/tests/test_connection.py", line 1927, in test_ssl_attribute
    return self.skipTest("libpq runtime version == %s", ext.libpq_version())
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: TestCase.skipTest() takes 2 positional arguments but 3 were given

it works when I change , to %:

return self.skipTest("libpq runtime version == %s" % ext.libpq_version())
test_ssl_attribute (tests.test_connection.TestConnectionInfo.test_ssl_attribute) ... skipped 'libpq runtime version == 160000'
test_ssl_in_use (tests.test_connection.TestConnectionInfo.test_ssl_in_use) ... ok
test_ssl_not_supported (tests.test_connection.TestConnectionInfo.test_ssl_not_supported) ... skipped 'skipped because libpq 150003'

Thanks!

@dvarrazzo
Copy link
Member

Oh, silly mistake. Thank you, will commit it with your fix.

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

No branches or pull requests

2 participants