| Submitter | Bernhard Reutner-Fischer |
|---|---|
| Date | May 3, 2012, 5:12 p.m. |
| Message ID | <1336065154-8513-1-git-send-email-rep.dot.nop@gmail.com> |
| Download | mbox | patch |
| Permalink | /patch/26973/ |
| State | Accepted |
| Commit | 7b633d0a4cf9aa05e6243974bab2b780c246f8ba |
| Headers | show |
Comments
On 05/03/2012 10:12 AM, Bernhard Reutner-Fischer wrote: > The error message erroneously talked about TMPDIR. > Just use OE_TMPDIR everywhere to make the name of the variable obvious. > > Signed-off-by: Bernhard Reutner-Fischer<rep.dot.nop@gmail.com> Bernhard, these are great improvements - thanks for refactoring this! Full series Acked-by: Scott Garman <scott.a.garman@intel.com> Scott
On 05/03/2012 10:12 AM, Bernhard Reutner-Fischer wrote: > The error message erroneously talked about TMPDIR. > Just use OE_TMPDIR everywhere to make the name of the variable obvious. > > Signed-off-by: Bernhard Reutner-Fischer<rep.dot.nop@gmail.com> Bernhard, This change wasn't well-tested and has broken the runqemu script for both bash and dash. :( From what I can tell, the =~ regex operator is a bashism. It's also one that helps a lot with the code readability. So now that we're faced with re-writing the script to avoid using that operator, I'm having second thoughts about whether the runqemu script really needs to be shell-agnostic. The alternative of invoking grep or other commands to process the name patterns does not appeal to me. I can understand why we're trying to ensure our build system doesn't require /bin/sh to be bash, but I think support scripts like runqemu might be a special case. What do other people in the community think of this? The runqemu script isn't trivial, and it has to run in a lot of different contexts. Should we put the time in to make it shell-agnostic, or allow it to require bash? Scott
On Mon, 7 May 2012 16:56:11 -0700 Scott Garman <scott.a.garman@intel.com> wrote: > From what I can tell, the =~ regex operator is a bashism. It's also > one that helps a lot with the code readability. So now that we're > faced with re-writing the script to avoid using that operator, I'm > having second thoughts about whether the runqemu script really needs > to be shell-agnostic. The alternative of invoking grep or other > commands to process the name patterns does not appeal to me. > > I can understand why we're trying to ensure our build system doesn't > require /bin/sh to be bash, but I think support scripts like runqemu > might be a special case. > > What do other people in the community think of this? The runqemu > script isn't trivial, and it has to run in a lot of different > contexts. Should we put the time in to make it shell-agnostic, or > allow it to require bash? Hmm. I am honestly not a big fan of the =~, simply because I almost never remember it, and I can never think whether it's like perl's ~= or Lua's ~=. (One is "matches", the other is "is not".) I tend to write stuff like this as case $name in *pat1* | *pat2* | ... ) # code goes here ;; esac because that's the natural shell idiom. It can't do full regex processing, but we really don't need that here; we just want an unanchored pattern match. (And I'm not even sure we *want* a fully-unanchored match.) I think the bash [[ ]] thing is one of the kshisms, but "bash or ksh" is not much better. :P From a maintenance standpoint, I like the case construct better than [[]]. My interest in reading the bash man page to figure out what some unfamiliar bit of punctuation means this week has declined over the years. -s
On 8 May 2012 02:56, Scott Garman <scott.a.garman@intel.com> wrote: > > I can understand why we're trying to ensure our build system doesn't require > /bin/sh to be bash, but I think support scripts like runqemu might be a > special case. > > What do other people in the community think of this? The runqemu script > isn't trivial, and it has to run in a lot of different contexts. Should we > put the time in to make it shell-agnostic, or allow it to require bash? 1) Do not require /bin/sh to be bash 2) It's ok, and for "right tool for the job" -reasons often prefered, to require that development machine has also bash installed. So I'm happy with how runqemu currently has #!/bin/bash shebang. It requires bash to be present, but not necessarily as /bin/sh - ML
On 5/14/12 5:51 PM, Marko Lindqvist wrote: > On 8 May 2012 02:56, Scott Garman<scott.a.garman@intel.com> wrote: >> >> I can understand why we're trying to ensure our build system doesn't require >> /bin/sh to be bash, but I think support scripts like runqemu might be a >> special case. >> >> What do other people in the community think of this? The runqemu script >> isn't trivial, and it has to run in a lot of different contexts. Should we >> put the time in to make it shell-agnostic, or allow it to require bash? > > 1) Do not require /bin/sh to be bash > 2) It's ok, and for "right tool for the job" -reasons often prefered, > to require that development machine has also bash installed. > > So I'm happy with how runqemu currently has #!/bin/bash shebang. It > requires bash to be present, but not necessarily as /bin/sh I don't mind it using bash as long as it has bash-isms. Note, the recent =~ stuff didn't work on my machine, even with bash. --Mark > > - ML > > _______________________________________________ > Openembedded-core mailing list > Openembedded-core@lists.openembedded.org > http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
Patch
diff --git a/scripts/runqemu b/scripts/runqemu index caabf61..7c2c4b3 100755 --- a/scripts/runqemu +++ b/scripts/runqemu @@ -55,11 +55,6 @@ SCRIPT_QEMU_OPT="" SCRIPT_QEMU_EXTRA_OPT="" SCRIPT_KERNEL_OPT="" -# Don't use TMPDIR from the external environment, it may be a distro -# variable pointing to /tmp (e.g. within X on OpenSUSE) -# Instead, use OE_TMPDIR for passing this in externally. -TMPDIR="$OE_TMPDIR" - # Determine whether the file is a kernel or QEMU image, and set the # appropriate variables process_filename() { @@ -273,8 +268,8 @@ SPITZ_DEFAULT_KERNEL=zImage-spitz.bin SPITZ_DEFAULT_FSTYPE=ext3 setup_tmpdir() { - if [ -z "$TMPDIR" ]; then - # Try to get TMPDIR from bitbake + if [ -z "$OE_TMPDIR" ]; then + # Try to get OE_TMPDIR from bitbake type -P bitbake &>/dev/null || { echo "In order for this script to dynamically infer paths"; echo "to kernels or filesystem images, you either need"; @@ -282,11 +277,11 @@ setup_tmpdir() { echo "before running this script" >&2; exit 1; } - # We have bitbake in PATH, get TMPDIR from bitbake - TMPDIR=`bitbake -e | grep ^TMPDIR=\" | cut -d '=' -f2 | cut -d '"' -f2` - if [ -z "$TMPDIR" ]; then + # We have bitbake in PATH, get OE_TMPDIR from bitbake + OE_TMPDIR=`bitbake -e | grep ^TMPDIR=\" | cut -d '=' -f2 | cut -d '"' -f2` + if [ -z "$OE_TMPDIR" ]; then echo "Error: this script needs to be run from your build directory," - echo "or you need to explicitly set TMPDIR in your environment" + echo "or you need to explicitly set OE_TMPDIR in your environment" exit 1 fi fi @@ -303,7 +298,7 @@ setup_sysroot() { BUILD_OS=`uname | tr '[A-Z]' '[a-z]'` BUILD_SYS="$BUILD_ARCH-$BUILD_OS" - OECORE_NATIVE_SYSROOT=$TMPDIR/sysroots/$BUILD_SYS + OECORE_NATIVE_SYSROOT=$OE_TMPDIR/sysroots/$BUILD_SYS fi } @@ -348,7 +343,7 @@ fi if [ -z "$KERNEL" ]; then setup_tmpdir eval kernel_file=\$${machine2}_DEFAULT_KERNEL - KERNEL=$TMPDIR/deploy/images/$kernel_file + KERNEL=$OE_TMPDIR/deploy/images/$kernel_file if [ -z "$KERNEL" ]; then echo "Error: Unable to determine default kernel for MACHINE [$MACHINE]" @@ -372,13 +367,13 @@ fi # core-image-sato if [ "$LAZY_ROOTFS" = "true" ]; then setup_tmpdir - echo "Assuming $ROOTFS really means $TMPDIR/deploy/images/$ROOTFS-$MACHINE.$FSTYPE" - ROOTFS=$TMPDIR/deploy/images/$ROOTFS-$MACHINE.$FSTYPE + echo "Assuming $ROOTFS really means $OE_TMPDIR/deploy/images/$ROOTFS-$MACHINE.$FSTYPE" + ROOTFS=$OE_TMPDIR/deploy/images/$ROOTFS-$MACHINE.$FSTYPE fi if [ -z "$ROOTFS" ]; then setup_tmpdir - T=$TMPDIR/deploy/images + T=$OE_TMPDIR/deploy/images eval rootfs_list=\$${machine2}_DEFAULT_ROOTFS findimage $T $MACHINE $FSTYPE
The error message erroneously talked about TMPDIR. Just use OE_TMPDIR everywhere to make the name of the variable obvious. Signed-off-by: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> --- scripts/runqemu | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-)