<sys/queue.h> portability

[rthalley]

While working on Solaris porting, we noticed that librdkafka only builds on Solaris if the SUNWhea package (kernel headers) is installed, because librdkafka is using sys/queue.h. The header is a BSD-ism, and may cause other portability problems in the future, so I thought I'd let you know.

[edenhill]

The dev branch has its own queue file, that might remedy the situation

[rthalley]

The dev branch queue.h works if I remove the #include <sys/cdefs.h>, which is not present on solaris either.

[edenhill]

Could you have a go with this on master branch?

[rthalley]

The queue.h seems to work. There are other issues with solaris, in particular at least the following:

The bash 3.2 requirement is a problem for us, but my workarounds still work if applied.

If you want a grep with -q on solaris, you need to use /usr/xpg4/bin/grep.

There are new problems too:

strndup() is not available on Solaris.

In file included from snappy.c:56:0:
snappy_compat.h: At top level:
snappy_compat.h:23:22: fatal error: endian.h: No such file or directory

include <endian.h>

                  ^

[edenhill]

Thanks!

• Re bash <3.2: I will most likely not put much effort into this since most people should be able to either update their bash to a > 2005 version.
• What Solaris version are you on? strndup() is in Solaris 11
• endian.h: fixed
• Bogus IOV_MAX: fixed

[rthalley]

Solaris 10 is the version I have to support.

[edenhill]

Ive made the necessary modifications for Solaris 10.
Can you verify on your end?

[rthalley]

On Dec 28, 2015, at 15:57, Magnus Edenhill notifications@github.com wrote:

Ive made the necessary modifications for Solaris 10.
Can you verify on your end?

All good except for the bash part (which you said you weren't changing, so OK :))

This is the change that I made to work around the bash version issue:

diff -u -r old/configure new/configure
--- old/configure Tue Dec 15 05:53:59 2015
+++ new/configure Mon Jan 11 11:55:52 2016
@@ -3,12 +3,6 @@

BASHVER=$(expr ${BASH_VERSINFO[0]} * 1000 + ${BASH_VERSINFO[1]})

-if [ "$BASHVER" -lt 3002 ]; then

• echo "ERROR: mklove requires bash version 3.2 or later but you are using $BASH_VERSION ($BASHVER)"
• echo " See Bash version edenhill/mklove#15"
• exit 1

-fi

MKL_CONFIGURE_ARGS="$0 $*"

Load base module

diff -u -r old/mklove/modules/configure.base new/mklove/modules/configure.base
--- old/mklove/modules/configure.base Mon Jan 11 09:24:16 2016
+++ new/mklove/modules/configure.base Mon Jan 11 11:56:04 2016
@@ -228,8 +228,8 @@
if [[ $val == code:* ]]; then
# Code block, copy verbatim without quotes, strip code: prefix
val=${val#code:}

• elif [[ ! ( "$val" =~ ^[0-9]+([lL]?[lL][dDuU]?)?$ || \

•    "$val" =~ ^0x[0-9a-fA-F]+([lL]?[lL][dDuU]?)?$ ) ]]; then
  

• elif [[ ! ( "$val" =~ '^[0-9]+([lL]?[lL][dDuU]?)?$' || \

•    "$val" =~ '^0x[0-9a-fA-F]+([lL]?[lL][dDuU]?)?$' ) ]]; then
   # String: quote
   val="\"$val\""
  

  fi

[edenhill]

@rthalley Mind creating a pull-request for that?
Are you sure the bash version check should be completely removed, or just adjusted?

[rthalley]

I'm not sure, which is why I didn't submit a pull request. I don't understand all the things that the version check is meant to be protecting against. I just removed the check and fixed what broke, and I got what looks like a good build out of it, though I haven't tested it much yet. I'm OK with leaving the check in and I can patch it out locally.

[edenhill]

Sorry but what's the status on this? Will current master build out of the box on your machines @rthalley, or do you still need a local patch?

[rthalley]

Just tried a build of the master branch. It does not build, for various reasons. First I have to fix the bash-too-old issue, but even if I do that I still have problems:

The linker complains about rd_kafka_message_errstr not being defined. Since this is supposed to be inline, I suspect there's some issue with __inline :) I don't have a patch for this because I hadn't experienced the problem until today (we're using an older snapshot of the master branch).

kafkatest_verifiable_client.cpp:635:35: error: 'rindex' was not declared in this scope
const char *t = rindex(val, '.') + 1;

You just need to #include <strings.h> here

[edenhill]

Apart from the bash issue (which you may PR if you like!) this should be fixed now.
Can you verify on your end?

[rthalley]

It now builds without any patches to .c or .h files. I still have a single "build" patch that changes things in mklove, namely

1. Quote the RHS of =~ to work with old bash. Fixes for this might include either setting compat31 and doing all matching with quotes, or putting the pattern into a variable. See http://stackoverflow.com/questions/218156/bash-regex-with-quotes for example.
2. Change RET=true (or false) type returns to exit 0 (or 1)
3. Change some INSTALL=install to INSTALL=/usr/ucb/install

I haven't submitted that as a patch as it would break other builds the way I do it currently, and I haven't had time to dig into mklove to figure out the right way.

[edenhill]

Thanks!
I'd be interested in a worked up patch for that eventually