[PATCH] oeqa/controllers/beaglebonetarget.py: fix conditions for files copied to /boot

[PATCH] oeqa/controllers/beaglebonetarget.py: fix conditions for files copied to /boot

[Corneliu Stoicescu]

Using '&&' as condition operator in '[ ! -e /mnt/testrootfs/boot/uImage ] && cp ~/test-kernel /mnt/testrootfs/boot/uImage' would result in exit code 1 if the first condition is not met.
Changing the code to handle the conditioning more cleanly and correctly return exit status 0 if [ ! -e /mnt/testrootfs/boot/uImage ] returns exit code 1.

Without this if the file existance check would fail then the image deploy task would stop.

Signed-off-by: Corneliu Stoicescu <corneliux.stoicescu@intel.com>
---
 meta-yocto-bsp/lib/oeqa/controllers/beaglebonetarget.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/meta-yocto-bsp/lib/oeqa/controllers/beaglebonetarget.py b/meta-yocto-bsp/lib/oeqa/controllers/beaglebonetarget.py
index 53f454b..af56bda 100644
--- a/meta-yocto-bsp/lib/oeqa/controllers/beaglebonetarget.py
+++ b/meta-yocto-bsp/lib/oeqa/controllers/beaglebonetarget.py
@@ -41,12 +41,12 @@ class BeagleBoneTarget(MasterImageHardwareTarget):
                 'mount -L testrootfs /mnt/testrootfs',
                 'rm -rf /mnt/testrootfs/*',
                 'tar xzvf ~/test-rootfs.tar.gz -C /mnt/testrootfs',
-                '[ ! -e /mnt/testrootfs/boot/uImage ] && cp ~/test-kernel /mnt/testrootfs/boot/uImage',
+                'if [ ! -e /mnt/testrootfs/boot/uImage ]; then cp ~/test-kernel /mnt/testrootfs/boot/uImage; fi',
                 ]
 
         for _, dtbfn in self.dtbs:
             # Kernel and dtb files may not be in the image, so copy them if not
-            self.deploy_cmds.append('[ ! -e /mnt/testrootfs/boot/{0} ] && cp ~/{0} /mnt/testrootfs/boot/'.format(dtbfn))
+            self.deploy_cmds.append('if [ ! -e /mnt/testrootfs/boot/{0} ]; then cp ~/{0} /mnt/testrootfs/boot/'.format(dtbfn) + '; fi')
 
         if not self.serialcontrol_cmd:
             bb.fatal("This TEST_TARGET needs a TEST_SERIALCONTROL_CMD defined in local.conf.")
-- 
1.8.3.2

Re: [PATCH] oeqa/controllers/beaglebonetarget.py: fix conditions for files copied to /boot

[Anders Darander]

Just nitpicking a little...

It should ideally have had a [PATCH v2] header, to make it easier to see
that it's an updated patch.

If Saul or someone else picks this up, there should be no need to
resubmit. But anyway, for the next time.

* Corneliu Stoicescu <corneliux.stoicescu@intel.com> [140527 09:46]:
> Using '&&' as condition operator in '[ ! -e /mnt/testrootfs/boot/uImage ] && cp ~/test-kernel /mnt/testrootfs/boot/uImage' would result in exit code 1 if the first condition is not met.
> Changing the code to handle the conditioning more cleanly and correctly return exit status 0 if [ ! -e /mnt/testrootfs/boot/uImage ] returns exit code 1.

Preferably, the commit message should be line wrapped at ~78 chars.

> Without this if the file existance check would fail then the image deploy task would stop.

> Signed-off-by: Corneliu Stoicescu <corneliux.stoicescu@intel.com>
> ---

A short changelog between version should go here. (In this case just
stating that the commit message was improved).

Cheers,
Anders

-- 
Anders Darander
ChargeStorm AB / eStorm AB

Re: [PATCH] oeqa/controllers/beaglebonetarget.py: fix conditions for files copied to /boot

[Stefan Stanacar]

On Tue, May 27, 2014 at 12:50 PM, Corneliu Stoicescu
<corneliux.stoicescu@intel.com> wrote:

> ---
>  meta-yocto-bsp/lib/oeqa/controllers/beaglebonetarget.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>

As this patch is for meta-yocto-bsp this should be sent to
poky@yoctoproject.org list...

> diff --git a/meta-yocto-bsp/lib/oeqa/controllers/beaglebonetarget.py b/meta-yocto-bsp/lib/oeqa/controllers/beaglebonetarget.py
> index 53f454b..af56bda 100644
> --- a/meta-yocto-bsp/lib/oeqa/controllers/beaglebonetarget.py
> +++ b/meta-yocto-bsp/lib/oeqa/controllers/beaglebonetarget.py
> @@ -41,12 +41,12 @@ class BeagleBoneTarget(MasterImageHardwareTarget):
>                  'mount -L testrootfs /mnt/testrootfs',
>                  'rm -rf /mnt/testrootfs/*',
>                  'tar xzvf ~/test-rootfs.tar.gz -C /mnt/testrootfs',
> -                '[ ! -e /mnt/testrootfs/boot/uImage ] && cp ~/test-kernel /mnt/testrootfs/boot/uImage',
> +                'if [ ! -e /mnt/testrootfs/boot/uImage ]; then cp ~/test-kernel /mnt/testrootfs/boot/uImage; fi',

How about reversing the check: [ -e /mnt/testrootfs/boot/uImage] || cp
~/test-kernel /mnt/testrootfs/boot/uImage
That would work in both cases, if it exists it won't copy the kernel
and returns 0, if it doesn't it will copy and returns the result of
cp.

>                  ]
>
>          for _, dtbfn in self.dtbs:
>              # Kernel and dtb files may not be in the image, so copy them if not
> -            self.deploy_cmds.append('[ ! -e /mnt/testrootfs/boot/{0} ] && cp ~/{0} /mnt/testrootfs/boot/'.format(dtbfn))
> +            self.deploy_cmds.append('if [ ! -e /mnt/testrootfs/boot/{0} ]; then cp ~/{0} /mnt/testrootfs/boot/'.format(dtbfn) + '; fi')
>

Same here.


Cheers,
Stefan

>          if not self.serialcontrol_cmd:
>              bb.fatal("This TEST_TARGET needs a TEST_SERIALCONTROL_CMD defined in local.conf.")
> --
> 1.8.3.2
>
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core

Re: [PATCH] oeqa/controllers/beaglebonetarget.py: fix conditions for files copied to /boot

[Saul Wold]

On 05/27/2014 04:14 AM, Stefan Stanacar wrote:
> On Tue, May 27, 2014 at 12:50 PM, Corneliu Stoicescu
> <corneliux.stoicescu@intel.com> wrote:
>
>> ---
>>   meta-yocto-bsp/lib/oeqa/controllers/beaglebonetarget.py | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>
> As this patch is for meta-yocto-bsp this should be sent to
> poky@yoctoproject.org list...
>
>> diff --git a/meta-yocto-bsp/lib/oeqa/controllers/beaglebonetarget.py b/meta-yocto-bsp/lib/oeqa/controllers/beaglebonetarget.py
>> index 53f454b..af56bda 100644
>> --- a/meta-yocto-bsp/lib/oeqa/controllers/beaglebonetarget.py
>> +++ b/meta-yocto-bsp/lib/oeqa/controllers/beaglebonetarget.py
>> @@ -41,12 +41,12 @@ class BeagleBoneTarget(MasterImageHardwareTarget):
>>                   'mount -L testrootfs /mnt/testrootfs',
>>                   'rm -rf /mnt/testrootfs/*',
>>                   'tar xzvf ~/test-rootfs.tar.gz -C /mnt/testrootfs',
>> -                '[ ! -e /mnt/testrootfs/boot/uImage ] && cp ~/test-kernel /mnt/testrootfs/boot/uImage',
>> +                'if [ ! -e /mnt/testrootfs/boot/uImage ]; then cp ~/test-kernel /mnt/testrootfs/boot/uImage; fi',
>
> How about reversing the check: [ -e /mnt/testrootfs/boot/uImage] || cp
> ~/test-kernel /mnt/testrootfs/boot/uImage
> That would work in both cases, if it exists it won't copy the kernel
> and returns 0, if it doesn't it will copy and returns the result of
> cp.
>
>>                   ]
>>
>>           for _, dtbfn in self.dtbs:
>>               # Kernel and dtb files may not be in the image, so copy them if not
>> -            self.deploy_cmds.append('[ ! -e /mnt/testrootfs/boot/{0} ] && cp ~/{0} /mnt/testrootfs/boot/'.format(dtbfn))
>> +            self.deploy_cmds.append('if [ ! -e /mnt/testrootfs/boot/{0} ]; then cp ~/{0} /mnt/testrootfs/boot/'.format(dtbfn) + '; fi')
>>
>
> Same here.
>

I agree with Stephan here along with Anders comments and this should 
really be on poky@ list, so a v3 to the poky@yoctoproject would be best.

Thanks
	Sau!

>
> Cheers,
> Stefan
>
>>           if not self.serialcontrol_cmd:
>>               bb.fatal("This TEST_TARGET needs a TEST_SERIALCONTROL_CMD defined in local.conf.")
>> --
>> 1.8.3.2
>>
>> --
>> _______________________________________________
>> Openembedded-core mailing list
>> Openembedded-core@lists.openembedded.org
>> http://lists.openembedded.org/mailman/listinfo/openembedded-core

Re: [PATCH] oeqa/controllers/beaglebonetarget.py: fix conditions for files copied to /boot

[Stoicescu, CorneliuX]

> -----Original Message-----
> From: Saul Wold [mailto:sgw@linux.intel.com]
> Sent: Tuesday, May 27, 2014 5:24 PM
> To: Stefan Stanacar; Stoicescu, CorneliuX
> Cc: openembedded-core@lists.openembedded.org
> Subject: Re: [OE-core] [PATCH] oeqa/controllers/beaglebonetarget.py: fix
> conditions for files copied to /boot
> 
> On 05/27/2014 04:14 AM, Stefan Stanacar wrote:
> > On Tue, May 27, 2014 at 12:50 PM, Corneliu Stoicescu
> > <corneliux.stoicescu@intel.com> wrote:
> >
> >> ---
> >>   meta-yocto-bsp/lib/oeqa/controllers/beaglebonetarget.py | 4 ++--
> >>   1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >
> > As this patch is for meta-yocto-bsp this should be sent to
> > poky@yoctoproject.org list...
> >
> >> diff --git a/meta-yocto-bsp/lib/oeqa/controllers/beaglebonetarget.py
> >> b/meta-yocto-bsp/lib/oeqa/controllers/beaglebonetarget.py
> >> index 53f454b..af56bda 100644
> >> --- a/meta-yocto-bsp/lib/oeqa/controllers/beaglebonetarget.py
> >> +++ b/meta-yocto-bsp/lib/oeqa/controllers/beaglebonetarget.py
> >> @@ -41,12 +41,12 @@ class
> BeagleBoneTarget(MasterImageHardwareTarget):
> >>                   'mount -L testrootfs /mnt/testrootfs',
> >>                   'rm -rf /mnt/testrootfs/*',
> >>                   'tar xzvf ~/test-rootfs.tar.gz -C /mnt/testrootfs',
> >> -                '[ ! -e /mnt/testrootfs/boot/uImage ] && cp ~/test-kernel
> /mnt/testrootfs/boot/uImage',
> >> +                'if [ ! -e /mnt/testrootfs/boot/uImage ]; then cp
> >> + ~/test-kernel /mnt/testrootfs/boot/uImage; fi',
> >
> > How about reversing the check: [ -e /mnt/testrootfs/boot/uImage] || cp
> > ~/test-kernel /mnt/testrootfs/boot/uImage That would work in both
> > cases, if it exists it won't copy the kernel and returns 0, if it
> > doesn't it will copy and returns the result of cp.
> >
> >>                   ]
> >>
> >>           for _, dtbfn in self.dtbs:
> >>               # Kernel and dtb files may not be in the image, so copy them if not
> >> -            self.deploy_cmds.append('[ ! -e /mnt/testrootfs/boot/{0} ] && cp
> ~/{0} /mnt/testrootfs/boot/'.format(dtbfn))
> >> +            self.deploy_cmds.append('if [ ! -e
> >> + /mnt/testrootfs/boot/{0} ]; then cp ~/{0}
> >> + /mnt/testrootfs/boot/'.format(dtbfn) + '; fi')
> >>
> >
> > Same here.
> >
> 
> I agree with Stephan here along with Anders comments and this should really
> be on poky@ list, so a v3 to the poky@yoctoproject would be best.
> 

Sure, I will test Stefan's version with core-image-sato and core-image-full-cmdline and send the patch tomorrow, to poky@ mailing list.
Thank you, Stefan! 

Regards,
Corneliu

> Thanks
> 	Sau!
> 
> >
> > Cheers,
> > Stefan
> >
> >>           if not self.serialcontrol_cmd:
> >>               bb.fatal("This TEST_TARGET needs a
> >> TEST_SERIALCONTROL_CMD defined in local.conf.")
> >> --
> >> 1.8.3.2
> >>
> >> --
> >> _______________________________________________
> >> Openembedded-core mailing list
> >> Openembedded-core@lists.openembedded.org
> >> http://lists.openembedded.org/mailman/listinfo/openembedded-core