librdkafka
librdkafka copied to clipboard
Remove small bashism from Makefile
The LICENSES.txt target makes a shell for loop, in which it tries to evalute the wildcard "LICENSE.*[^~]".
[^] is a bashism, and fails when /bin/sh is not bash (i.e. every Debian-based system by default):
$ /bin/bash -c "ls LICENSE.*[^~]" LICENSE.cjson LICENSE.fnv1a LICENSE.lz4 LICENSE.pycrc LICENSE.regexp LICENSE.tinycthread LICENSE.crc32c LICENSE.hdrhistogram LICENSE.murmur2 LICENSE.queue LICENSE.snappy LICENSE.wingetopt
$ /bin/sh -c "ls LICENSE.[^~]" ls: cannot access 'LICENSE.[^~]': No such file or directory
The equivalent POSIX way to do this is to use [!].
Tested with bash, dash and posh.
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.
It looks like section 2 of your CLA requires for me to assign (way too) many of my rights to Confluent, including the right to relicense the work to a proprietary license (i.e. the ability to pull a Hashicorp). I don't consider this reasonable for what is an unpaid/volunteer contribution, and as such I don't intend to sign such an agreement.
Feel free to consider this as a bug report rather than a PR and do a clean room implementation of it, or close without fixing it. (Note that in this case we'll have to carry this bug in Debian indefinitely, as it's necessary for librdkafka to build in Debian).
@paravoid thanks, we'll do the fix. We don't have this problem when we're building Confluent distributed packages, as we're using bash, but I acknowledge the need to distribute the package in Debian using the default repository.
Is that the only thing that prevents you from building the package?
@paravoid thanks, we'll do the fix. We don't have this problem when we're building Confluent distributed packages, as we're using bash, but I acknowledge the need to distribute the package in Debian using the default repository.
There are no guarantees that /bin/sh is "bash" in any system, really. If the Makefile depends on a bash-specific behavior, it can alternatively declare SHELL := /bin/bash. But given how limited this bashism is, the fix I've proposed is easier :)
Is that the only thing that prevents you from building the package?
Yes this is the only patch we carry. To be clear, it's not "preventing" us; we do have librdkafka in Debian, ever since I uploaded it in 2013 or so :wink: It's just that we've just been carrying this patch locally in the Debian package since the bashism was introduced last year, and I thought it'd be good to submit it here for others to benefit from it, too.
There are no guarantees that /bin/sh is "bash" in any system, really.
Sure, I never said so
@paravoid thanks, we'll do the fix.
@emasab Ping, since I have your attention :wink:
Done here: https://github.com/confluentinc/librdkafka/pull/5184
I'll need an internal review for this one and #5183 before merging.