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
Do not ship rabbitmq-c with syslog-ng (remove as submodule) #2052
Conversation
f2452e0
to
b71984f
Compare
Build SUCCESS |
1 similar comment
Build SUCCESS |
.travis.yml
Outdated
@@ -54,7 +55,7 @@ before_script: | |||
--prefix=$HOME/install/syslog-ng | |||
--with-ivykis=internal | |||
--with-mongoc=internal | |||
--with-librabbitmq=internal | |||
--with-librabbitmq=system |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In configure.ac you dropped the whole option, so you can just delete the line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right, if it is a go I'll update this.
I wholeheartedly support the idea! \o/ |
Hi,
Apart from ubuntu trusty, all relevant debian/ubuntu versions meet the
librabbitmq_min_version criteria.
Centos6/7 is a question still and how we would address the trusty case in
our obs packages.
@czanik @lbudai
…On May 23, 2018 08:27, "furiel" ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In .travis.yml
<#2052 (comment)>:
> @@ -54,7 +55,7 @@ before_script:
--prefix=$HOME/install/syslog-ng
--with-ivykis=internal
--with-mongoc=internal
- --with-librabbitmq=internal
+ --with-librabbitmq=system
In configure.ac you dropped the whole option, so you can just delete the
line.
------------------------------
In configure.ac
<#2052 (comment)>:
> -if test "x$with_librabbitmq_client" = "xinternal"; then
- if $LIBRABBITMQ_INTERNAL_SOURCE_EXISTS; then
- # these can only be used in modules/amqp as it assumes
- # the current directory just one below rabbitmq-c
-
- LIBRABBITMQ_LIBS="-L\$(top_builddir)/modules/afamqp/rabbitmq-c/librabbitmq -lrabbitmq"
- LIBRABBITMQ_CFLAGS="-I\$(top_srcdir)/modules/afamqp/rabbitmq-c/librabbitmq -I\$(top_builddir)/modules/afamqp/rabbitmq-c/librabbitmq"
- else
- AC_MSG_WARN([Internal librabbitmq-client sources not found in modules/afamqp/rabbitmq-c])
- with_librabbitmq_client="no"
- fi
-elif test "x$with_librabbitmq_client" = "xsystem"; then
- PKG_CHECK_MODULES(LIBRABBITMQ, librabbitmq >= $LIBRABBITMQ_MIN_VERSION,, enable_amqp="no")
-fi
-
-if test "x$with_librabbitmq_client" = "xno"; then
Similar validation still needed: if amqp is enabled, librabbitmq-client
must be available.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#2052 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AArldtoDJuZYJvXctsjNAA8JuSM8ShQGks5t1QFfgaJpZM4UJysk>
.
|
Hi, |
RHEL7 has librabbitmq 0.8.0 in the base repository. |
Now submouldes are half baked, some dependency are added as submodule and some are not. As the AMQP is not a core functionality, so it is not a requirement to compile syslog-ng. Also this is the last commit, where the autotools are supported by rabbitmq-c, unless we include the cmake into the build (integrate with autotools) there is no way to update the submodule, thus no fix can came from upstream. That's really bad. Signed-off-by: kokan <peter.kokai@balabit.com>
b71984f
to
c055959
Compare
Build SUCCESS |
Could you please check and add librabbitmq-dev package into dbld's package lists. |
Build FAILURE |
OBS won't be a problem as I'm building the rabbitmq package inside OBS. |
@kira-syslogng retest this please |
Build SUCCESS |
This removes the internal version of rabbitmq-c so its source won't be shipped with syslog-ng anymore.
At this point we are stuck with the current commit for rabbitmq-c, because it only supports
cmake
based build after this commit, the autotools have been dropped. The autotools probably won't be supported anymore, and I do not think that syslog-ng project should continue to support it either. Thus I am proposing to drop as a submodule and everybody that needs amqp just get themself as other dependencies.Fixes #2051
Disclaimer: although it is not a real fix, I believe this is a way forward, and with this that ticket can be closed.