[Yocto,3/4] bootimg: Skip build iso image for aarch64

Message ID 1425397584-5154-4-git-send-email-naresh.bhat@linaro.org
State New
Headers show

Commit Message

naresh.bhat@linaro.org March 3, 2015, 3:46 p.m.
Build iso image required syslinux package.  We have already skip the syslinux
package.  Hence just skip the iso image build too.

Signed-off-by: Naresh Bhat <naresh.bhat@linaro.org>
Reviewed-by: Matt Fleming <matt.fleming@intel.com>
---
 meta/classes/bootimg.bbclass |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

naresh.bhat@linaro.org March 4, 2015, 7:15 a.m. | #1
On 3 March 2015 at 23:27, Bernhard Reutner-Fischer
<rep.dot.nop@gmail.com> wrote:
> On 3 March 2015 at 16:46, Naresh Bhat <naresh.bhat@linaro.org> wrote:
>> Build iso image required syslinux package.  We have already skip the syslinux
>> package.  Hence just skip the iso image build too.
>>
>> Signed-off-by: Naresh Bhat <naresh.bhat@linaro.org>
>> Reviewed-by: Matt Fleming <matt.fleming@intel.com>
>> ---
>>  meta/classes/bootimg.bbclass |    6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/meta/classes/bootimg.bbclass b/meta/classes/bootimg.bbclass
>> index a8e0c19..2edba3a 100644
>> --- a/meta/classes/bootimg.bbclass
>> +++ b/meta/classes/bootimg.bbclass
>> @@ -266,7 +266,11 @@ python do_bootimg() {
>>      if d.getVar("EFI", True) == "1":
>>          bb.build.exec_func('build_efi_cfg', d)
>>      bb.build.exec_func('build_hddimg', d)
>> -    bb.build.exec_func('build_iso', d)
>> +
>> +    if [ "${TARGET_ARCH}" == "aarch64" ]:
>> +             return
>
> erm is this shell-code or python-code?
>
> For the former there is no "==" operator for POSIX-compliant test(1)
> implementations, for the latter the "[" looks suspicious.
>
> What am i missing?
> thanks,

Ah my mistake, Probably I could have implemented as below

if d.getVar('TARGET_ARCH', True) == "aarch64":
               return

Does it make sense ?

>> +    else:
>> +            bb.build.exec_func('build_iso', d)
>>  }
>>
>>  IMAGE_TYPEDEP_iso = "ext3"
naresh.bhat@linaro.org March 5, 2015, 1:56 p.m. | #2
On 5 March 2015 at 02:07, Burton, Ross <ross.burton@intel.com> wrote:
> Hi Naresh,
>
> On 3 March 2015 at 15:46, Naresh Bhat <naresh.bhat@linaro.org> wrote:
>>
>> +    if [ "${TARGET_ARCH}" == "aarch64" ]:
>> +             return
>> +    else:
>> +            bb.build.exec_func('build_iso', d)
>
>
> As Bernhard says, that isn't valid Python.  Were these patches actually
> tested?
Yes, agreed.  I will change it in my next series. I have tested with
luvOS build.
>
> Ross
naresh.bhat@linaro.org March 5, 2015, 2:06 p.m. | #3
On 4 March 2015 at 12:56, Bernhard Reutner-Fischer
<rep.dot.nop@gmail.com> wrote:
> On March 4, 2015 8:15:15 AM GMT+01:00, Naresh Bhat <naresh.bhat@linaro.org> wrote:
>>On 3 March 2015 at 23:27, Bernhard Reutner-Fischer
>><rep.dot.nop@gmail.com> wrote:
>>> On 3 March 2015 at 16:46, Naresh Bhat <naresh.bhat@linaro.org> wrote:
>>>> Build iso image required syslinux package.  We have already skip the
>>syslinux
>>>> package.  Hence just skip the iso image build too.
>>>>
>>>> Signed-off-by: Naresh Bhat <naresh.bhat@linaro.org>
>>>> Reviewed-by: Matt Fleming <matt.fleming@intel.com>
>>>> ---
>>>>  meta/classes/bootimg.bbclass |    6 +++++-
>>>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/meta/classes/bootimg.bbclass
>>b/meta/classes/bootimg.bbclass
>>>> index a8e0c19..2edba3a 100644
>>>> --- a/meta/classes/bootimg.bbclass
>>>> +++ b/meta/classes/bootimg.bbclass
>>>> @@ -266,7 +266,11 @@ python do_bootimg() {
>>>>      if d.getVar("EFI", True) == "1":
>>>>          bb.build.exec_func('build_efi_cfg', d)
>>>>      bb.build.exec_func('build_hddimg', d)
>>>> -    bb.build.exec_func('build_iso', d)
>>>> +
>>>> +    if [ "${TARGET_ARCH}" == "aarch64" ]:
>>>> +             return
>>>
>>> erm is this shell-code or python-code?
>>>
>>> For the former there is no "==" operator for POSIX-compliant test(1)
>>> implementations, for the latter the "[" looks suspicious.
>>>
>>> What am i missing?
>>> thanks,
>>
>>Ah my mistake, Probably I could have implemented as below
>>
>>if d.getVar('TARGET_ARCH', True) == "aarch64":
>>               return
>>
>>Does it make sense ?
>
> At least the parser should grok it.
> The change itself does not make sense to me either way.
I did this change because it does not make any sense to build the ISO
image for aarch64 architecture.  The HDD image should be sufficient.
Correct me if I am wrong.  I find a dependency of syslinux package to
build ISO image (isolinux.bin).  I have skipped the syslinux package
dependency @luvOS distribution.  The syslinux package contains lot of
x86 assembly code. That's the reason I just want to skip the ISO image
build for aarch64 architecture.

I will try to explain as much as possible in my next series patch
>
> Thanks,
>>
>>>> +    else:
>>>> +            bb.build.exec_func('build_iso', d)
>>>>  }
>>>>
>>>>  IMAGE_TYPEDEP_iso = "ext3"
>
>
naresh.bhat@linaro.org March 9, 2015, 8:11 a.m. | #4
Hi Bernhard Reutner-Fischer,

On 5 March 2015 at 21:48, Bernhard Reutner-Fischer
<rep.dot.nop@gmail.com> wrote:
> On 5 March 2015 at 15:06, Naresh Bhat <naresh.bhat@linaro.org> wrote:
>
>>>>if d.getVar('TARGET_ARCH', True) == "aarch64":
>>>>               return
>>>>
>>>>Does it make sense ?
>>>
>>> At least the parser should grok it.
>>> The change itself does not make sense to me either way.
>> I did this change because it does not make any sense to build the ISO
>> image for aarch64 architecture.  The HDD image should be sufficient.
>> Correct me if I am wrong.  I find a dependency of syslinux package to
>> build ISO image (isolinux.bin).  I have skipped the syslinux package
>> dependency @luvOS distribution.  The syslinux package contains lot of
>> x86 assembly code. That's the reason I just want to skip the ISO image
>> build for aarch64 architecture.
>
> COMPATIBLE_HOST of syslinux should have prevented you from building
> syslinux for aarch64 in the first place, no?
>
> My assumption is that building this part of bootimg class for aarch64
> is wrong, you should have gotten an error when you attempt to build
> the HDD-image for non-i386 compatible targets.
> So, maybe, NOHDD should be (automagically) set based on
> COMPATIBLE_HOST or something like this?
> But then i don't really see why your aarch64 is the first to encounter
> this so maybe your config is just wrong instead?
>
> Not my homework though ;)

Yes, I do agree.  I did my homework.  I have missed a patch in this
series.  I will push the modified patches next series by
today/tomorrow.  Can we please continue the discussion on my next
series.  Probably you can give me few more points on my next patch
series.

>
> thanks,
naresh.bhat@linaro.org March 10, 2015, 9:24 a.m. | #5
On 10 March 2015 at 14:42, Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
wrote:

> On March 9, 2015 9:11:29 AM GMT+01:00, Naresh Bhat <naresh.bhat@linaro.org>
> wrote:
>
> >today/tomorrow.  Can we please continue the discussion on my next
> >series.  Probably you can give me few more points on my next patch
> >series.
>
> Please CC me so i do not overlook the new series, TIA.
>
Oops my apologies I forgot to CC you..:(  I have already posted the v1
series @oe-core.  I will keep in mind while posting next series.

Patch

diff --git a/meta/classes/bootimg.bbclass b/meta/classes/bootimg.bbclass
index a8e0c19..2edba3a 100644
--- a/meta/classes/bootimg.bbclass
+++ b/meta/classes/bootimg.bbclass
@@ -266,7 +266,11 @@  python do_bootimg() {
     if d.getVar("EFI", True) == "1":
         bb.build.exec_func('build_efi_cfg', d)
     bb.build.exec_func('build_hddimg', d)
-    bb.build.exec_func('build_iso', d)
+
+    if [ "${TARGET_ARCH}" == "aarch64" ]:
+             return
+    else:
+            bb.build.exec_func('build_iso', d)
 }
 
 IMAGE_TYPEDEP_iso = "ext3"