Patchwork [RFC,PATCHv2] bitbake.conf: include machine name in DEPLOY_DIR_IMAGE

login
register
mail settings
Submitter Koen Kooi
Date April 10, 2013, 8:05 a.m.
Message ID <1365581101-3810-1-git-send-email-koen@dominion.thruhere.net>
Download mbox | patch
Permalink /patch/47815/
State New
Headers show

Comments

Koen Kooi - April 10, 2013, 8:05 a.m.
This allows a clean seperation between all image outputs and making it possible to have convinience symlinks to make it ready to deploy. And while it isn't a valid reason, it must be mentioned: BSP layers which do use convenience symlinks already don't step on files owned by others anymore.

I assumed this was the default behaviour in OE-classic, but as it turns out every DISTRO set it to deploy/images/$MACHINE on its own.

Code inspections shows that the all references to the image deploy dir in classes and scripts in OE-core do the right thing and parse the DEPLOY_DIR_IMAGE variable, except runqemu.

Signed-off-by: Koen Kooi <koen@dominion.thruhere.net>
---
 meta/conf/bitbake.conf |  2 +-
 scripts/runqemu        | 37 ++++++++++++++++++++++++++++++++++---
 2 files changed, 35 insertions(+), 4 deletions(-)
Martin Jansa - April 14, 2013, 5:05 p.m.
On Wed, Apr 10, 2013 at 10:05:01AM +0200, Koen Kooi wrote:
> This allows a clean seperation between all image outputs and making it possible to have convinience symlinks to make it ready to deploy. And while it isn't a valid reason, it must be mentioned: BSP layers which do use convenience symlinks already don't step on files owned by others anymore.
> 
> I assumed this was the default behaviour in OE-classic, but as it turns out every DISTRO set it to deploy/images/$MACHINE on its own.
> 
> Code inspections shows that the all references to the image deploy dir in classes and scripts in OE-core do the right thing and parse the DEPLOY_DIR_IMAGE variable, except runqemu.

I'm not using runqemu scripts, but the change looks OK:

Acked-by: Martin Jansa <Martin.Jansa@gmail.com>

> 
> Signed-off-by: Koen Kooi <koen@dominion.thruhere.net>
> ---
>  meta/conf/bitbake.conf |  2 +-
>  scripts/runqemu        | 37 ++++++++++++++++++++++++++++++++++---
>  2 files changed, 35 insertions(+), 4 deletions(-)
> 
> diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
> index cd5d61e..0540766 100644
> --- a/meta/conf/bitbake.conf
> +++ b/meta/conf/bitbake.conf
> @@ -379,7 +379,7 @@ DEPLOY_DIR_TAR = "${DEPLOY_DIR}/tar"
>  DEPLOY_DIR_IPK = "${DEPLOY_DIR}/ipk"
>  DEPLOY_DIR_RPM = "${DEPLOY_DIR}/rpm"
>  DEPLOY_DIR_DEB = "${DEPLOY_DIR}/deb"
> -DEPLOY_DIR_IMAGE ?= "${DEPLOY_DIR}/images"
> +DEPLOY_DIR_IMAGE ?= "${DEPLOY_DIR}/images/${MACHINE}"
>  DEPLOY_DIR_TOOLS = "${DEPLOY_DIR}/tools"
>  
>  PKGDATA_DIR = "${TMPDIR}/pkgdata/${MULTIMACH_TARGET_SYS}"
> diff --git a/scripts/runqemu b/scripts/runqemu
> index 8ed1226..94aa054 100755
> --- a/scripts/runqemu
> +++ b/scripts/runqemu
> @@ -339,6 +339,34 @@ setup_tmpdir() {
>      fi
>  }
>  
> +setup_deploydir() {
> +    if [ -z "$DEPLOY_DIR_IMAGE" ]; then
> +        # Try to get DEPLOY_DIR_IMAGE 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";
> +            echo "bitbake in your PATH or to source oe-init-build-env";
> +            echo "before running this script" >&2;
> +            exit 1; }
> +
> +        # We have bitbake in PATH, get DEPLOY_DIR_IMAGE from bitbake
> +        OE_TMPDIR=`MACHINE=$MACHINE bitbake -e | sed -n 's/^DEPLOY_DIR_IMAGE=\"\(.*\)\"/\1/p'`
> +        if [ -z "$DEPLOY_DIR_IMAGE" ]; then
> +            # Check for errors from bitbake that the user needs to know about
> +            BITBAKE_OUTPUT=`bitbake -e | wc -l`
> +            if [ "$BITBAKE_OUTPUT" -eq "0" ]; then
> +                echo "Error: this script needs to be run from your build directory,"
> +                echo "or you need to explicitly set DEPLOY_DIR_IMAGE in your environment"
> +            else
> +                echo "There was an error running bitbake to determine DEPLOY_DIR_IMAGE"
> +                echo "Here is the output from 'bitbake -e':"
> +                bitbake -e
> +            fi
> +            exit 1
> +        fi
> +    fi
> +}
> +
>  setup_sysroot() {
>      # Toolchain installs set up $OECORE_NATIVE_SYSROOT in their
>      # environment script. If that variable isn't set, we're
> @@ -395,8 +423,9 @@ fi
>  
>  if [ -z "$KERNEL" -a "x$FSTYPE" != "xvmdk" ]; then
>      setup_tmpdir
> +    setup_deploydir
>      eval kernel_file=\$${machine2}_DEFAULT_KERNEL
> -    KERNEL=$OE_TMPDIR/deploy/images/$kernel_file
> +    KERNEL=$DEPLOY_DIR_IMAGE/$kernel_file
>  
>      if [ -z "$KERNEL" ]; then
>          error "Unable to determine default kernel for MACHINE [$MACHINE]"
> @@ -418,13 +447,15 @@ fi
>  # core-image-sato
>  if [ "$LAZY_ROOTFS" = "true" ]; then
>      setup_tmpdir
> +    setup_deploydir
>      echo "Assuming $ROOTFS really means $OE_TMPDIR/deploy/images/$ROOTFS-$MACHINE.$FSTYPE"
> -    ROOTFS=$OE_TMPDIR/deploy/images/$ROOTFS-$MACHINE.$FSTYPE
> +    ROOTFS=$DEPLOY_DIR_IMAGE/$ROOTFS-$MACHINE.$FSTYPE
>  fi
>  
>  if [ -z "$ROOTFS" -a "x$FSTYPE" != "xvmdk" ]; then
>      setup_tmpdir
> -    T=$OE_TMPDIR/deploy/images
> +    setup_deploydir
> +    T=$DEPLOY_DIR_IMAGE
>      eval rootfs_list=\$${machine2}_DEFAULT_ROOTFS
>      findimage $T $MACHINE $FSTYPE
>  
> -- 
> 1.8.1.4
> 
> 
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
Koen Kooi - April 14, 2013, 5:23 p.m.
Op 14 apr. 2013, om 19:05 heeft Martin Jansa <martin.jansa@gmail.com> het volgende geschreven:

> On Wed, Apr 10, 2013 at 10:05:01AM +0200, Koen Kooi wrote:
>> This allows a clean seperation between all image outputs and making it possible to have convinience symlinks to make it ready to deploy. And while it isn't a valid reason, it must be mentioned: BSP layers which do use convenience symlinks already don't step on files owned by others anymore.
>> 
>> I assumed this was the default behaviour in OE-classic, but as it turns out every DISTRO set it to deploy/images/$MACHINE on its own.
>> 
>> Code inspections shows that the all references to the image deploy dir in classes and scripts in OE-core do the right thing and parse the DEPLOY_DIR_IMAGE variable, except runqemu.
> 
> I'm not using runqemu scripts, but the change looks OK:
> 
> Acked-by: Martin Jansa <Martin.Jansa@gmail.com>

Thanks!

Since one part of this patch is a legitimate fix, should I split it and submit the runqemu fix for dylan?
Martin Jansa - Aug. 28, 2013, 8:29 a.m.
On Wed, Apr 10, 2013 at 10:05:01AM +0200, Koen Kooi wrote:
> This allows a clean seperation between all image outputs and making it possible to have convinience symlinks to make it ready to deploy. And while it isn't a valid reason, it must be mentioned: BSP layers which do use convenience symlinks already don't step on files owned by others anymore.
> 
> I assumed this was the default behaviour in OE-classic, but as it turns out every DISTRO set it to deploy/images/$MACHINE on its own.
> 
> Code inspections shows that the all references to the image deploy dir in classes and scripts in OE-core do the right thing and parse the DEPLOY_DIR_IMAGE variable, except runqemu.

Is there some reason why this patch didn't make it into 1.5?

> 
> Signed-off-by: Koen Kooi <koen@dominion.thruhere.net>
> ---
>  meta/conf/bitbake.conf |  2 +-
>  scripts/runqemu        | 37 ++++++++++++++++++++++++++++++++++---
>  2 files changed, 35 insertions(+), 4 deletions(-)
> 
> diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
> index cd5d61e..0540766 100644
> --- a/meta/conf/bitbake.conf
> +++ b/meta/conf/bitbake.conf
> @@ -379,7 +379,7 @@ DEPLOY_DIR_TAR = "${DEPLOY_DIR}/tar"
>  DEPLOY_DIR_IPK = "${DEPLOY_DIR}/ipk"
>  DEPLOY_DIR_RPM = "${DEPLOY_DIR}/rpm"
>  DEPLOY_DIR_DEB = "${DEPLOY_DIR}/deb"
> -DEPLOY_DIR_IMAGE ?= "${DEPLOY_DIR}/images"
> +DEPLOY_DIR_IMAGE ?= "${DEPLOY_DIR}/images/${MACHINE}"
>  DEPLOY_DIR_TOOLS = "${DEPLOY_DIR}/tools"
>  
>  PKGDATA_DIR = "${TMPDIR}/pkgdata/${MULTIMACH_TARGET_SYS}"
> diff --git a/scripts/runqemu b/scripts/runqemu
> index 8ed1226..94aa054 100755
> --- a/scripts/runqemu
> +++ b/scripts/runqemu
> @@ -339,6 +339,34 @@ setup_tmpdir() {
>      fi
>  }
>  
> +setup_deploydir() {
> +    if [ -z "$DEPLOY_DIR_IMAGE" ]; then
> +        # Try to get DEPLOY_DIR_IMAGE 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";
> +            echo "bitbake in your PATH or to source oe-init-build-env";
> +            echo "before running this script" >&2;
> +            exit 1; }
> +
> +        # We have bitbake in PATH, get DEPLOY_DIR_IMAGE from bitbake
> +        OE_TMPDIR=`MACHINE=$MACHINE bitbake -e | sed -n 's/^DEPLOY_DIR_IMAGE=\"\(.*\)\"/\1/p'`
> +        if [ -z "$DEPLOY_DIR_IMAGE" ]; then
> +            # Check for errors from bitbake that the user needs to know about
> +            BITBAKE_OUTPUT=`bitbake -e | wc -l`
> +            if [ "$BITBAKE_OUTPUT" -eq "0" ]; then
> +                echo "Error: this script needs to be run from your build directory,"
> +                echo "or you need to explicitly set DEPLOY_DIR_IMAGE in your environment"
> +            else
> +                echo "There was an error running bitbake to determine DEPLOY_DIR_IMAGE"
> +                echo "Here is the output from 'bitbake -e':"
> +                bitbake -e
> +            fi
> +            exit 1
> +        fi
> +    fi
> +}
> +
>  setup_sysroot() {
>      # Toolchain installs set up $OECORE_NATIVE_SYSROOT in their
>      # environment script. If that variable isn't set, we're
> @@ -395,8 +423,9 @@ fi
>  
>  if [ -z "$KERNEL" -a "x$FSTYPE" != "xvmdk" ]; then
>      setup_tmpdir
> +    setup_deploydir
>      eval kernel_file=\$${machine2}_DEFAULT_KERNEL
> -    KERNEL=$OE_TMPDIR/deploy/images/$kernel_file
> +    KERNEL=$DEPLOY_DIR_IMAGE/$kernel_file
>  
>      if [ -z "$KERNEL" ]; then
>          error "Unable to determine default kernel for MACHINE [$MACHINE]"
> @@ -418,13 +447,15 @@ fi
>  # core-image-sato
>  if [ "$LAZY_ROOTFS" = "true" ]; then
>      setup_tmpdir
> +    setup_deploydir
>      echo "Assuming $ROOTFS really means $OE_TMPDIR/deploy/images/$ROOTFS-$MACHINE.$FSTYPE"
> -    ROOTFS=$OE_TMPDIR/deploy/images/$ROOTFS-$MACHINE.$FSTYPE
> +    ROOTFS=$DEPLOY_DIR_IMAGE/$ROOTFS-$MACHINE.$FSTYPE
>  fi
>  
>  if [ -z "$ROOTFS" -a "x$FSTYPE" != "xvmdk" ]; then
>      setup_tmpdir
> -    T=$OE_TMPDIR/deploy/images
> +    setup_deploydir
> +    T=$DEPLOY_DIR_IMAGE
>      eval rootfs_list=\$${machine2}_DEFAULT_ROOTFS
>      findimage $T $MACHINE $FSTYPE
>  
> -- 
> 1.8.1.4
> 
> 
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
Paul Eggleton - Sept. 4, 2013, 1:44 p.m.
On Wednesday 28 August 2013 10:29:00 Martin Jansa wrote:
> On Wed, Apr 10, 2013 at 10:05:01AM +0200, Koen Kooi wrote:
> > This allows a clean seperation between all image outputs and making it
> > possible to have convinience symlinks to make it ready to deploy. And
> > while it isn't a valid reason, it must be mentioned: BSP layers which do
> > use convenience symlinks already don't step on files owned by others
> > anymore.
> > 
> > I assumed this was the default behaviour in OE-classic, but as it turns
> > out every DISTRO set it to deploy/images/$MACHINE on its own.
> > 
> > Code inspections shows that the all references to the image deploy dir in
> > classes and scripts in OE-core do the right thing and parse the
> > DEPLOY_DIR_IMAGE variable, except runqemu.
>
> Is there some reason why this patch didn't make it into 1.5?

I think it just got missed; looking at the changes made to runqemu I can see a 
couple of concerns (double execution of bitbake -e and one missed change to 
DEPLOY_DIR_IMAGE, albeit only in a message). I'm coming up with a revised 
version now.

Cheers,
Paul

Patch

diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
index cd5d61e..0540766 100644
--- a/meta/conf/bitbake.conf
+++ b/meta/conf/bitbake.conf
@@ -379,7 +379,7 @@  DEPLOY_DIR_TAR = "${DEPLOY_DIR}/tar"
 DEPLOY_DIR_IPK = "${DEPLOY_DIR}/ipk"
 DEPLOY_DIR_RPM = "${DEPLOY_DIR}/rpm"
 DEPLOY_DIR_DEB = "${DEPLOY_DIR}/deb"
-DEPLOY_DIR_IMAGE ?= "${DEPLOY_DIR}/images"
+DEPLOY_DIR_IMAGE ?= "${DEPLOY_DIR}/images/${MACHINE}"
 DEPLOY_DIR_TOOLS = "${DEPLOY_DIR}/tools"
 
 PKGDATA_DIR = "${TMPDIR}/pkgdata/${MULTIMACH_TARGET_SYS}"
diff --git a/scripts/runqemu b/scripts/runqemu
index 8ed1226..94aa054 100755
--- a/scripts/runqemu
+++ b/scripts/runqemu
@@ -339,6 +339,34 @@  setup_tmpdir() {
     fi
 }
 
+setup_deploydir() {
+    if [ -z "$DEPLOY_DIR_IMAGE" ]; then
+        # Try to get DEPLOY_DIR_IMAGE 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";
+            echo "bitbake in your PATH or to source oe-init-build-env";
+            echo "before running this script" >&2;
+            exit 1; }
+
+        # We have bitbake in PATH, get DEPLOY_DIR_IMAGE from bitbake
+        OE_TMPDIR=`MACHINE=$MACHINE bitbake -e | sed -n 's/^DEPLOY_DIR_IMAGE=\"\(.*\)\"/\1/p'`
+        if [ -z "$DEPLOY_DIR_IMAGE" ]; then
+            # Check for errors from bitbake that the user needs to know about
+            BITBAKE_OUTPUT=`bitbake -e | wc -l`
+            if [ "$BITBAKE_OUTPUT" -eq "0" ]; then
+                echo "Error: this script needs to be run from your build directory,"
+                echo "or you need to explicitly set DEPLOY_DIR_IMAGE in your environment"
+            else
+                echo "There was an error running bitbake to determine DEPLOY_DIR_IMAGE"
+                echo "Here is the output from 'bitbake -e':"
+                bitbake -e
+            fi
+            exit 1
+        fi
+    fi
+}
+
 setup_sysroot() {
     # Toolchain installs set up $OECORE_NATIVE_SYSROOT in their
     # environment script. If that variable isn't set, we're
@@ -395,8 +423,9 @@  fi
 
 if [ -z "$KERNEL" -a "x$FSTYPE" != "xvmdk" ]; then
     setup_tmpdir
+    setup_deploydir
     eval kernel_file=\$${machine2}_DEFAULT_KERNEL
-    KERNEL=$OE_TMPDIR/deploy/images/$kernel_file
+    KERNEL=$DEPLOY_DIR_IMAGE/$kernel_file
 
     if [ -z "$KERNEL" ]; then
         error "Unable to determine default kernel for MACHINE [$MACHINE]"
@@ -418,13 +447,15 @@  fi
 # core-image-sato
 if [ "$LAZY_ROOTFS" = "true" ]; then
     setup_tmpdir
+    setup_deploydir
     echo "Assuming $ROOTFS really means $OE_TMPDIR/deploy/images/$ROOTFS-$MACHINE.$FSTYPE"
-    ROOTFS=$OE_TMPDIR/deploy/images/$ROOTFS-$MACHINE.$FSTYPE
+    ROOTFS=$DEPLOY_DIR_IMAGE/$ROOTFS-$MACHINE.$FSTYPE
 fi
 
 if [ -z "$ROOTFS" -a "x$FSTYPE" != "xvmdk" ]; then
     setup_tmpdir
-    T=$OE_TMPDIR/deploy/images
+    setup_deploydir
+    T=$DEPLOY_DIR_IMAGE
     eval rootfs_list=\$${machine2}_DEFAULT_ROOTFS
     findimage $T $MACHINE $FSTYPE