Makefile, pg_amqp.c, librabbitmq: updating librabbitmq-c to current v…

[pmwebster]

This updates the lib to version 9 (current at the moment).
We're currently testing this internally, but thought it could use a second set of eyes.

[esatterwhite]

probably worthy of a version bump

[esatterwhite]

@xzilla @postwait Is there a test suite or test script that was used to test this?

Its a big change, we'd like to make sure we aren't missing anything.

[theory]

Uh…I have no idea. What’s the context for this?

[esatterwhite]

@theory Sorry, tagged the wrong person. Carry on.

[esatterwhite]

@theory Well maybe you can help?
We're just trying to update the libamqp version to the recent one. looking for some tests to verify if it still works correctly.

If not. we can write some.

[theory]

Yah, I don’t think I have any apps that use it today.

[xzilla]

@esatterwhite @pmwebster Best as I can remember, we always tested this by hand... install the extension into a fresh database, upgrade extension, then test basic sending of messages. I think we also did some a/b split testing in prod. In any case, if someone wants to write up some more official tests, we'd be happy to look into merging that in.

[pmwebster]

@xzilla I believe this is good to go. If you're running rabbit locally with the default admin, make && make install && make installcheck will do a basic sanity check using the default postgres extension tooling. Let me know if there's anything else we need to do to hopefully get this merged in.

[esatterwhite]

@postwait @xzilla is anyone able to do a sanity check, merge + tag?

[xzilla]

Speaking only for myself, I'm a bit bogged down with actual work and other more pressing community work in light of the pending PostgreSQL 12 release. If a couple of other folks can provide some patch review / testing / sanity checking and thumbsup this PR, I'd be happy to do a drive-by commit :-)

[vventirozos]

Hey @pmwebster , i tried to do some sanity checks but i fail to get it compiled. I tried with a couple of ubuntu versions and with rabbitmq docker image, with compiling rabbitmq from source with no luck, i am getting:

src/librabbitmq/amqp_socket.c: In function 'amqp_poll':
src/librabbitmq/amqp_socket.c:261:2: error: #error "poll() or select() is needed to compile rabbitmq-c"
 #error "poll() or select() is needed to compile rabbitmq-c"
  ^~~~~

I'm pretty sure that it's me missing something, but in any case, if it needs something "special" i think that we should add something in the documentation.

[pmwebster]

Hey @vventirozos,

Taking a nod from #20, here's a build using the current postgres 12 stable alpine image. FWIW, I'm able to build this locally on a normal OS install without having to add in the sys/types header.

FROM postgres:12-alpine as builder
RUN apk update \
    && apk add git make gcc linux-headers libc-dev libxml2-dev alpine-sdk musl-dev clang clang-dev llvm-dev \
    && git clone https://github.com/pmwebster/pg_amqp.git pg_amqp

ARG AMQP_H=src/librabbitmq/amqp.h

RUN cd pg_amqp \
        && head -n 2 ${AMQP_H} > ${AMQP_H}.temp \
        && echo "#include <sys/types.h>" >>  ${AMQP_H}.temp \
        && cat ${AMQP_H} | sed -e '1,3d' >> ${AMQP_H}.temp \
        && mv ${AMQP_H}.temp ${AMQP_H} \
        && make \
        && make install

[esatterwhite]

Anyone reasons not to merge this one?

[esatterwhite]

@xzilla @postwait is this still good? can we merge?

[kumy]

Hi,
with a base docker image as postgres:12, I had the same error as @vventirozos in #28 (comment). The proposed solution from @pmwebster in #28 (comment) didn't solved the build issue.

I had to patch the Makefile as such to get a successful build

--- /tmp/Makefile.orig	2022-12-03 12:33:44.880106544 +0100
+++ /tmp/Makefile	2022-12-03 12:34:11.427619640 +0100
@@ -5,7 +5,8 @@
 PG91         = $(shell $(PG_CONFIG) --version | grep -qE " 8\.| 9\.0" && echo no || echo yes)
 
 ifeq ($(PG91),yes)
-CFLAGS_SL='-D HAVE_POLL'
+PG_CFLAGS=-D HAVE_POLL
+PG_CPPFLAGS=-D HAVE_POLL
 # Windows Support
 # CFLAGS_SL='-D HAVE_SELECT'
 DOCS         = $(wildcard doc/*.*)