omniti-labs / pg_amqp

AMQP Support for Postgres
https://labs.omniti.com/labs/pg_amqp
207 stars 63 forks source link

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

Open pmwebster opened 5 years ago

pmwebster commented 5 years ago

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 commented 5 years ago

probably worthy of a version bump

esatterwhite commented 5 years ago

@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 commented 5 years ago

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

esatterwhite commented 5 years ago

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

esatterwhite commented 5 years ago

@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 commented 5 years ago

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

xzilla commented 5 years ago

@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 commented 5 years ago

@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 commented 5 years ago

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

xzilla commented 5 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 3 years ago

Anyone reasons not to merge this one?

esatterwhite commented 3 years ago

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

kumy commented 1 year ago

Hi, with a base docker image as postgres:12, I had the same error as @vventirozos in https://github.com/omniti-labs/pg_amqp/pull/28#issuecomment-570584979. The proposed solution from @pmwebster in https://github.com/omniti-labs/pg_amqp/pull/28#issuecomment-570629491 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/*.*)