pg_amqp icon indicating copy to clipboard operation
pg_amqp copied to clipboard

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

Open pmwebster opened this issue 6 years ago • 15 comments

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.

pmwebster avatar Aug 21 '19 16:08 pmwebster

probably worthy of a version bump

esatterwhite avatar Aug 21 '19 19:08 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.

esatterwhite avatar Aug 21 '19 19:08 esatterwhite

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

theory avatar Aug 22 '19 15:08 theory

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

esatterwhite avatar Aug 22 '19 15:08 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.

esatterwhite avatar Aug 22 '19 15:08 esatterwhite

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

theory avatar Aug 22 '19 15:08 theory

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

xzilla avatar Aug 22 '19 19:08 xzilla

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

pmwebster avatar Aug 26 '19 18:08 pmwebster

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

esatterwhite avatar Sep 21 '19 23:09 esatterwhite

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 :-)

xzilla avatar Sep 27 '19 05:09 xzilla

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.

vventirozos avatar Jan 03 '20 14:01 vventirozos

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

pmwebster avatar Jan 03 '20 16:01 pmwebster

Anyone reasons not to merge this one?

esatterwhite avatar Feb 15 '21 01:02 esatterwhite

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

esatterwhite avatar Sep 14 '21 04:09 esatterwhite

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/*.*)

kumy avatar Dec 03 '22 11:12 kumy