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

Remove obsolete and incorrect FreeBSD version condition #755

Closed
wants to merge 1 commit into from

Conversation

AMDmi3
Copy link
Contributor

@AMDmi3 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.

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
Copy link
Member

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

@AMDmi3
Copy link
Contributor Author

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
Copy link
Member

@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
Copy link
Member

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

@dvarrazzo
Copy link
Member

merged

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

Successfully merging this pull request may close these issues.

None yet

2 participants