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

Remove obsolete and incorrect FreeBSD version condition #755

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@AMDmi3
Contributor

AMDmi3 commented Jul 26, 2018

The FreeBSD-related condition which enables custom round() implementation is incorrect: one must include <sys/param.h> to get __FreeBSD_version value, and since it's not included here, the check succeeds while it shouldn't. Before it worked somehow, but since python 3.7 it results in conflicting declarations of round(). The condition is also no longer needed since FreeBSD 5.3 is unsupported for 12 years.

This change thus fixes build of psycopg with python 3.7 on FreeBSD.

Remove obsolete and incorrect FreeBSD version condition
The FreeBSD-related condition which enables custom round() implementation is incorrect: one must include <sys/param.h> to get __FreeBSD_version value, and since it's not included here, the check succeeds while it shouldn't. Before it worked somehow, but since python 3.7 it results in conflicting declarations of round(). The condition is also no longer needed since FreeBSD 5.3 is unsupported for 12 years.
@dvarrazzo

This comment has been minimized.

Member

dvarrazzo commented Jul 26, 2018

Thank you for this. Do you know if it's possible to set up a CI system for BSD?

@AMDmi3

This comment has been minimized.

Contributor

AMDmi3 commented Jul 27, 2018

If you mean public CI service like Travis, there are none that I know of, unfortunately. Self hosting is always available with any opensource CI system, but maintaining this would be time wasting IMO. You could also just set a FreeBSD (or other BSD) VM and test your code there occasionally.

@dvarrazzo

This comment has been minimized.

Member

dvarrazzo commented Jul 27, 2018

@AMDmi3 self-hosting is not anything I fancy doing, no. As well as having my own VM. So this patch will be merged without further QA.

@dvarrazzo

This comment has been minimized.

Member

dvarrazzo commented Sep 5, 2018

@AMDmi3 can you please check if the MR #768 fixes the problem? Thank you

@dvarrazzo

This comment has been minimized.

Member

dvarrazzo commented Sep 7, 2018

merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment