Skip to content
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

Adds detection of BusyBox v1.30.0+ with updated timeout arguments #81

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 13 additions & 7 deletions wait-for-it.sh
Original file line number Diff line number Diff line change
Expand Up @@ -140,17 +140,23 @@ WAITFORIT_TIMEOUT=${WAITFORIT_TIMEOUT:-15}
WAITFORIT_STRICT=${WAITFORIT_STRICT:-0}
WAITFORIT_CHILD=${WAITFORIT_CHILD:-0}
WAITFORIT_QUIET=${WAITFORIT_QUIET:-0}

# check to see if timeout is from busybox?
WAITFORIT_ISBUSY=0
WAITFORIT_BUSYTIMEFLAG=""
WAITFORIT_TIMEOUT_PATH=$(type -p timeout)
WAITFORIT_TIMEOUT_PATH=$(realpath $WAITFORIT_TIMEOUT_PATH 2>/dev/null || readlink -f $WAITFORIT_TIMEOUT_PATH)

# check to see if we're using busybox?
if [[ $WAITFORIT_TIMEOUT_PATH =~ "busybox" ]]; then
WAITFORIT_ISBUSY=1
WAITFORIT_BUSYTIMEFLAG="-t"
WAITFORIT_ISBUSY=1
fi

else
WAITFORIT_ISBUSY=0
WAITFORIT_BUSYTIMEFLAG=""
# see if timeout.c args have been updated in busybox v1.30.0 or newer
Copy link

@robertp-indeed robertp-indeed Nov 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just read timeout --version?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's possible. It's just that timeout, at least on busybox, doesn't have a --version flag.

timeout --version
unrecognized option `--version'
BusyBox v1.22.1 (2015-03-20 11:09:37 GMT) multi-call binary.` 

However, it does output the busybox version info in the error, so it could still be used.

It was just a judgment call to use busybox itself to gather the version info since relying on timeout error output felt slightly worse.

Copy link

@robertp-indeed robertp-indeed Nov 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, that's a great reason. Looks good to me, then!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not get what is expected by "timeout.c" (".c")
Is it a misprint?

# note: this requires the use of bash on Alpine
Copy link
Contributor

@kolomenkin kolomenkin Jan 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I know wait-for-it.sh already needs bash in Alpine.
So there is no need to state this here additionally and frighten reviewers.

if [[ $WAITFORIT_ISBUSY && $(busybox | head -1) =~ ^.*v([[:digit:]]+)\.([[:digit:]]+)\..+$ ]]; then
if [[ ${BASH_REMATCH[1]} -le 1 && ${BASH_REMATCH[2]} -lt 30 ]]; then
Copy link
Contributor

@kolomenkin kolomenkin Jan 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This simplified version comparison looks not good since it can detect BusyBox 0.60 as new version.

However BusyBox 1.0.0-pre1 was release in 2003. It is unlikely somebody will use it in 2020 with latest wait-for-it.sh
https://busybox.net/oldnews.html

And for future versions of BusyBox this code will work OK.

# using pre 1.30.0 version with `-t SEC` arg
WAITFORIT_BUSYTIMEFLAG="-t"
fi
fi

if [[ $WAITFORIT_CHILD -gt 0 ]]; then
Expand Down