On 09/14/2016 04:02 PM, Thomas Huth wrote: > Well, /bin/bash is also not really portable ... I've seen systems in the > past where bash was installed in another directory or not at all...
True, but we already liberally use /bin/bash scripts elsewhere in qemu.git, so at least you wouldn't be the first, and if someone wants to build qemu on a platform where /bin/bash doesn't exist, they'd do a search-and-replace change to all affected scripts. > > Anyway, FYI, I've found two more nice ways to check for POSIX compliance: > > - There is a program called checkbashisms which reports bash related > style > > - "posh" is a very minimalistic POSIX compliant shell which hardly > supports any of the bash extras Yep, both of those are nice. > > And indeed, both pointed me to another bashism in my script: The > "function" keyword is not portable and should be avoided... oh well. In fact, even bash users discourage the use of the 'function' keyword. It exists because ksh has it, but ksh gives it different semantics than bash (what's worse, 'local' variables in bash functions always have dynamic scope; while in ksh, 'local' variables in POSIX-style functions have static scope and the only way to get dynamic scope is to use the 'function' keyword, except that the ksh maintainer says that he hates dynamic scope and wishes he hadn't done it). > Not > sure whether I really should do a v3 of my patch, convert it to python > or just give up the idea of releasing such a script to the public... I think keeping it as a shell script is probably okay (certainly easier than trying to convert it to python, at this point in the review process). And it does seem like a useful script. Sometimes, it's hard to see the forest (improving the ecosystem by accepting useful scripts into the project, even if only a handful of people ever run the script) for the trees (nitpicking on portability details that won't impact anyone who never runs the script). -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
