Helo Michael, On Thu, Oct 14, 2010 at 12:23 AM, Michael Tautschnig <m...@debian.org> wrote: >> There should be quotes around the $(command) for the -x test shell >> builtin: [ -x "$(command)" ] >> This is owing to the following: >> > > [...] > > Thanks a lot for debugging this; in fact I wonder whether you do some kind of > (automated?) code inspection or what is it that makes you discover all these > hidden issues!?
I don't use any kind of automated code inspection (unfortunately; it would be great if there was something like this, and perhaps there is something, but I don't know it). My task now is to understand how it is possible to do things that the FAI does. And all those discovered issues are just from looking through the code and probably (I suspect) from my previous working experience (sounds like self-advertisement for a mailing list, but nevertheless...) > Furthermore, thanks for providing the nice counter-example. I must, however, > admit that I don't quite like the patch. Or, rather, I'd like to go a bit > further. The quotes are fine and maybe even necessary to catch cases where the > path to rsync contains whitespace. This, however, will likely cause a number > of > other errors and won't be seen on too many systems. To catch the more likely > case of a missing rsync command I'd suggest the following: > > which rsync && [ -x $(which rsync) ] && echo ok > > (Well, maybe even a which rsync && echo ok would be just as fine.) > About the style of this correction. I agree that the solution is not the perfect one, but. If you looked through the whole codebase you can spot a lot of such constructions [ -x "$(command)" ]. And I tried to be consistent with the methods which has been used in the similar situations in this code. E.g.: debian07:~/.temp/fai-sources# svn info Path: . URL: svn://svn.debian.org/svn/fai/trunk Repository Root: svn://svn.debian.org/svn/fai Repository UUID: ba5ec265-b0fb-0310-8e1a-cf9e4c2b1591 Revision: 6123 Node Kind: directory Schedule: normal Last Changed Author: lange Last Changed Rev: 6118 Last Changed Date: 2010-10-06 10:45:09 -0400 (Wed, 06 Oct 2010) debian07:~/.temp/fai-sources# grep -rn " \-x " ./ | grep which ./lib/task_sysinfo:14:[ -x "$(which dmidecode)" ] && dmidecode ./lib/task_sysinfo:15:[ -x "$(which lshw)" ] && lshw -short ./lib/task_sysinfo:17:[ -x "$(which discover)" ] && { ./lib/task_sysinfo:24:[ -x "$(which hwinfo)" ] && hwinfo --short ./lib/task_sysinfo:33:[ -x "$(which sfdisk)" ] && sfdisk -d ./lib/task_sysinfo:41:[ -x "$(which blkid)" ] && blkid ./bin/make-fai-nfsroot:525:[ ! -x "$(which debootstrap)" ] && die 1 "Can't find debootstrap command. Aborting." ./bin/fai-cd:334:[ -x "$(which rsync)" ] && rsync=1 ./bin/fai-cd:376: [ -x "$(which mkisofs)" ] || die 8 "mkisofs not found. Please install package." ./examples/simple/class/10-base-classes:5:[ -x "`which dpkg`" ] && dpkg --print-architecture | tr a-z A-Z debian07:~/.temp/fai-sources# -- Regards, Michael