FS#16623 - Replace the old [ test with the bash builtin [[ and (( for numeric stuff

Attached to Project: Pacman
Opened by Isaac G (IsaacG) - Tuesday, 13 October 2009, 19:16 GMT
Last edited by Xavier (shining) - Saturday, 28 November 2009, 16:04 GMT
Task Type General Gripe
Category makepkg
Status Closed
Assigned To Dan McGee (toofishes)
Allan McRae (Allan)
Architecture All
Severity Low
Priority Normal
Reported Version 3.3.1
Due in Version 3.4.0
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

To quote from the #bash factoids,
[[ is a bash keyword similar to (but more powerful than) the [ command.
See http://mywiki.wooledge.org/BashFAQ/031 and
http://mywiki.wooledge.org/BashGuide#Bash_Tests Unless you are scripting
for POSIX sh we recommend [[.
and
When evaluating arithmetic expressions, use (( expr )), not [ ] or [[ ]].
See http://mywiki.wooledge.org/ArithmeticExpression

The [[ runs about twice as fast as [ and is cleaner. bash also has (( for numeric tests. Some examples for the makepkg that can be fixed:

if [ -z "$1" ]; then
to
if [[ ! $1 ]]; then

if [ "$LOGGING" -eq 1 ]; then
to
if (( LOGGING == 1 )); then

if [ $ret -gt 0 ]; then
to
if (( ret > 1 )); then
or
if (( ret )); then

if [ $EXIT_CODE -eq 0 -a "$CLEANUP" -eq 1 ]; then
to
if (( EXIT_CODE == 0 && CLEANUP == 1 )); then
or possibly
if (( EXIT_CODE == 0 && CLEANUP )); then
This task depends upon

Closed by  Xavier (shining)
Saturday, 28 November 2009, 16:04 GMT
Reason for closing:  Implemented
Additional comments about closing:  implemented in git; commits 966c815 and c299961
Comment by Xavier (shining) - Tuesday, 13 October 2009, 19:32 GMT
That sounds reasonable to me. Do we have any reasons for not using these ?

IsaacG might be willing to make a patch if everyone is fine with it.
Comment by Allan McRae (Allan) - Tuesday, 13 October 2009, 23:33 GMT
I'd be fine with it. Are there available in bash-3.x?
Comment by Isaac G (IsaacG) - Tuesday, 13 October 2009, 23:59 GMT
[[ was introduced in bash-2.02-alpha1 and (( in bash-2.0-beta2
Comment by Allan McRae (Allan) - Wednesday, 14 October 2009, 00:09 GMT
Well then... I sure all the other OS will support that by now :P

Are you willing to make us a patch for this?
Comment by Isaac G (IsaacG) - Wednesday, 14 October 2009, 00:20 GMT
I am willing. It just might take a week or two to find time. Or one single night if I get bored...
Comment by Dan McGee (toofishes) - Thursday, 15 October 2009, 03:22 GMT
So this is great and all, but the BS/FUD/whatever should be left out of any argument. Where did you pull that "twice as fast" comment from? And cleaner? I personally like the sanity of quotes, and we depend on globbing in places as well (which [[ does not do), so be careful.

$ type [
[ is a shell builtin

$ type [[
[[ is a shell keyword
Comment by Isaac G (IsaacG) - Thursday, 15 October 2009, 03:40 GMT
Yeah, the speed improvement is minimal. The time spent on [ compared to anything else is about zero.
But [[ is faster than [.

$ s=hi; time for i in {1..100000} ; do [[ $s = "hi" ]] ; done ; time for i in {1..100000} ; do [ "$s" = "hi" ] ; done

The user times I get for [ and double for [[. bash does parse the two differently.
Comment by Isaac G (IsaacG) - Thursday, 15 October 2009, 04:15 GMT
FWIW the first 30% (link valid for 30 days): http://dpaste.com/hold/107454/
Comment by Xavier (shining) - Thursday, 15 October 2009, 05:52 GMT
You should create a patch :) Ideally using git format-patch against git master branch :
http://www.archlinux.org/pacman/submitting-patches.html
Comment by Isaac G (IsaacG) - Thursday, 15 October 2009, 16:04 GMT
When I get to 100% I'll be sure to submit a git-patch!
Comment by Isaac G (IsaacG) - Sunday, 25 October 2009, 23:42 GMT
Patch attached.

I did the git commit -s and git format-patch but I didn't see any patch file produced. git show [commit hash] produced this file...

I'm supposed to also email this to the dev list?
   makepkg.patch (48.6 KiB)
Comment by xduugu (xduugu) - Sunday, 25 October 2009, 23:56 GMT
You have to use something like

git format-patch HEAD^

to get the patch file, and yes, you should send this to pacman-dev (via git send-email or inline).
See also doc/submitting-patches.txt in pacman's git tree.

Loading...