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

pg_config.h: NetBSD support #172

Merged
merged 1 commit into from
Feb 6, 2023
Merged

pg_config.h: NetBSD support #172

merged 1 commit into from
Feb 6, 2023

Conversation

bsiegert
Copy link
Contributor

This is the same as pganalyze/pg_query_go#72 except one level higher.

NetBSD has strchrnul too but with a slightly different prototype. Without this change, there is a compiler error for a conflicting declaration.

Copy link
Member

@lfittl lfittl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for getting this change in here!

There is actually one more small detail (I wasn't fully clear in my initial ask I realize), once that's fixed we can merge it :)

# Ensure we don't fail on systems that have strchrnul support (FreeBSD)
echo "#ifdef __FreeBSD__" >> $(PGDIR)/src/include/pg_config.h
# Ensure we don't fail on systems that have strchrnul support (FreeBSD and NetBSD)
echo "#if defined(__FreeBSD__) || defined(__NetBSD__)" >> $(PGDIR)/src/include/pg_config.h
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do actually also need to apply it to the generated pg_config.h file in the repo (we cache the result of the Makefile and only run extract_source as needed) -- you can just make the edit directly in the file, or run the extract_source locally to generate it (but that might be a bit more involved):

https://github.com/pganalyze/libpg_query/blob/15-latest/src/postgres/include/pg_config.h#L1035

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. I did not manage to get make extract_source to work (neither on NetBSD nor on Debian) so this is a manual edit. PTAL :)

NetBSD has strchrnul too but with a slightly different prototype. Without this
change, there is a compiler error for a conflicting declaration.
@bsiegert
Copy link
Contributor Author

bsiegert commented Feb 5, 2023

Can someone merge this, because I cannot :)

@lfittl
Copy link
Member

lfittl commented Feb 6, 2023

Can someone merge this, because I cannot :)

Looks like there was a seemingly random build failure - will merge after thats resolved (hopefully on a re-run, that I started just now).

@msepga msepga merged commit 4c4f68d into pganalyze:15-latest Feb 6, 2023
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

3 participants