You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2020/05/19 01:20:40 UTC
[GitHub] [kafka] jiameixie opened a new pull request #8692: KAFKA-10018:Change sh to bash
jiameixie opened a new pull request #8692:
URL: https://github.com/apache/kafka/pull/8692
"#!/bin/sh" is used in kafka-server-stop.sh and zookeeper-server-stop.sh. [[ is a bash-builtin and used.
Modern Debian and Ubuntu systems, which symlink sh to dash by default. So "[[: not found" will occur.
Change "#!/bin/sh" into "#!/bin/bash" can avoid this error. Modify and make all scripts using bash.
Change-Id: I733c6e31f76d768e71ac0e040a33da8f4bd8f005
Signed-off-by: Jiamei Xie <ji...@arm.com>
*More detailed description of your change,
if necessary. The PR title and PR message become
the squashed commit message, so use a separate
comment to ping reviewers.*
*Summary of testing strategy (including rationale)
for the feature or bug fix. Unit and/or integration
tests are expected for any behaviour change and
system tests should be considered for larger changes.*
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] cmccabe commented on pull request #8692: KAFKA-10018: Change command line tools from /bin/sh to /bin/bash
Posted by GitBox <gi...@apache.org>.
cmccabe commented on pull request #8692:
URL: https://github.com/apache/kafka/pull/8692#issuecomment-634914243
Debian did make a big effort to strip bashisms out of scripts that used /bin/sh but I'm pretty sure that effectively everyone running Debian has bash installed. I think it's almost impossible to avoid installing bash when you're using Linux. (The exception would be in some weird embedded systems that aren't really relevant to Kafka.)
The places where I'd expect bash to be missing are the BSDs like FreeBSD, NetBSD, OpenBSD, and maybe some embedded systems.
Realistically it's probably best to just make any RPMs, DEBs, docker images, etc. that we generate for Kafka respins depend on bash, and just change the scripts to use /bin/bash. Unless we really want to test all the scripts with dash, which seems like a pain.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] cmccabe edited a comment on pull request #8692: KAFKA-10018: Change command line tools from /bin/sh to /bin/bash
Posted by GitBox <gi...@apache.org>.
cmccabe edited a comment on pull request #8692:
URL: https://github.com/apache/kafka/pull/8692#issuecomment-634914243
Debian did make a big effort to strip bashisms out of scripts that used /bin/sh but I'm pretty sure that effectively everyone running Debian has bash installed. I think it's almost impossible to avoid installing bash when you're using Linux.
The places where I'd expect bash to be missing are the BSDs like FreeBSD, NetBSD, OpenBSD, and maybe some embedded systems.
Realistically it's probably best to just make any RPMs, DEBs, docker images, etc. that we generate for Kafka respins depend on bash, and just change the scripts to use /bin/bash. Unless we really want to test all the scripts with dash, which seems like a pain.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mjsax merged pull request #8692: KAFKA-10018: Change command line tools from /bin/sh to /bin/bash
Posted by GitBox <gi...@apache.org>.
mjsax merged pull request #8692:
URL: https://github.com/apache/kafka/pull/8692
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mjsax commented on pull request #8692: KAFKA-10018: Change command line tools from /bin/sh to /bin/bash
Posted by GitBox <gi...@apache.org>.
mjsax commented on pull request #8692:
URL: https://github.com/apache/kafka/pull/8692#issuecomment-634953112
Thanks for the PR @jiameixie!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] tombentley commented on pull request #8692: KAFKA-10018:Change sh to bash
Posted by GitBox <gi...@apache.org>.
tombentley commented on pull request #8692:
URL: https://github.com/apache/kafka/pull/8692#issuecomment-630649523
Wouldn't it be better to change the `[[` to `[` and thus allow use of the script in environments which aren't using `bash`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on a change in pull request #8692: KAFKA-10018: Change command line tools from /bin/sh to /bin/bash
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #8692:
URL: https://github.com/apache/kafka/pull/8692#discussion_r428496868
##########
File path: bin/zookeeper-server-stop.sh
##########
@@ -1,4 +1,4 @@
-#!/bin/sh
Review comment:
Should we apply this change to ```zookeeper-shell.sh``` for consistency?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] glasser commented on pull request #8692: KAFKA-10018:Change sh to bash
Posted by GitBox <gi...@apache.org>.
glasser commented on pull request #8692:
URL: https://github.com/apache/kafka/pull/8692#issuecomment-631018459
I don't know why my name was mentioned here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mjsax commented on pull request #8692: KAFKA-10018: Change command line tools from /bin/sh to /bin/bash
Posted by GitBox <gi...@apache.org>.
mjsax commented on pull request #8692:
URL: https://github.com/apache/kafka/pull/8692#issuecomment-634951128
Thanks for the input @ijuma and @colinhicks. Seems we agree that this change is fine. Merging.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] jiameixie commented on a change in pull request #8692: KAFKA-10018: Change command line tools from /bin/sh to /bin/bash
Posted by GitBox <gi...@apache.org>.
jiameixie commented on a change in pull request #8692:
URL: https://github.com/apache/kafka/pull/8692#discussion_r428527359
##########
File path: bin/zookeeper-server-stop.sh
##########
@@ -1,4 +1,4 @@
-#!/bin/sh
Review comment:
@chia7712 Yes, I modified just now.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] jiameixie commented on pull request #8692: KAFKA-10018:Change sh to bash
Posted by GitBox <gi...@apache.org>.
jiameixie commented on pull request #8692:
URL: https://github.com/apache/kafka/pull/8692#issuecomment-630661883
> Wouldn't it be better to change the `[[` to `[` and thus allow use of the script in environments which aren't using `bash`?
Yeah, running with below statement can also avoid error. But there are also a lot of shell scripts under directory bin which use #!/bin/bash. And sh is symlink and not a sure thing. Sometime it's bash, sometime it's dash. So I think setting all scripts use the same shell interpreter bash is better.
` if [ $(uname -s) = "OS/390" ]
`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] jiameixie commented on pull request #8692: KAFKA-10018: Change command line tools from /bin/sh to /bin/bash
Posted by GitBox <gi...@apache.org>.
jiameixie commented on pull request #8692:
URL: https://github.com/apache/kafka/pull/8692#issuecomment-633334815
@ijuma Could you have a review for it? Thanks.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on pull request #8692: KAFKA-10018: Change command line tools from /bin/sh to /bin/bash
Posted by GitBox <gi...@apache.org>.
ijuma commented on pull request #8692:
URL: https://github.com/apache/kafka/pull/8692#issuecomment-634900285
I don't have a strong opinion here. Is `bash` always present? cc @cmccabe
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] jiameixie commented on pull request #8692: KAFKA-10018:Change sh to bash
Posted by GitBox <gi...@apache.org>.
jiameixie commented on pull request #8692:
URL: https://github.com/apache/kafka/pull/8692#issuecomment-630523741
@ijuma @abbccdda @mjsax @halorgium @astubbs @alexism @glasser PTAL, thanks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org