[denzil-next BREAKAGE] Silent breakage in denzil-next --- uImage*.dts no longer generated

[denzil-next BREAKAGE] Silent breakage in denzil-next --- uImage*.dts no longer generated

[Leon Woestenberg]

[-- Attachment #1: Type: text/plain, Size: 2743 bytes --]

Hello Scott,

I tested your denzil-next branch but found breakage that will not fail the
(auto) build.

poky-contrib.git$ git branch
  master
* sgarman/denzil-next
poky-contrib.git$ git log -n1
commit 6a7d4c7dfc574669ea2eeacede1b74e2f55c4675

I'm hitting the "Warning: ${DTS_FILE} is not available!" case.

I'll follow-up later. This was just a heads up that even if the autobuilds
succeed, I would consider this failing.


This is the probable related diff between denzil and denzil-next, I think
fully from commit a40d795e (Zhenhua Luo).
meta/recipes-kernel/linux/linux-dtb.inc:


+++ poky-contrib.git/meta/recipes-kernel/linux/linux-dtb.inc    2012-08-01
12:03:42.502612844 +0200
@@ -15,13 +15,35 @@

 do_install_append() {
     if test -n "${KERNEL_DEVICETREE}"; then
-    dtc -I dts -O dtb ${KERNEL_DEVICETREE_FLAGS} -o devicetree
${KERNEL_DEVICETREE}
-    install -m 0644 devicetree ${D}/boot/devicetree-${KERNEL_VERSION}
-    install -d ${DEPLOYDIR}
-    install -m 0644 devicetree ${DEPLOYDIR}/${KERNEL_IMAGE_BASE_NAME}.dtb
-    cd ${DEPLOYDIR}
-    rm -f ${KERNEL_IMAGE_SYMLINK_NAME}.dtb
-    ln -sf ${KERNEL_IMAGE_BASE_NAME}.dtb ${KERNEL_IMAGE_SYMLINK_NAME}.dtb
+        for DTS_FILE in ${KERNEL_DEVICETREE}; do
+            if [ ! -f ${DTS_FILE} ]; then
+                echo "Warning: ${DTS_FILE} is not available!"
+                continue
+            fi
+            DTS_BASE_NAME=`basename ${DTS_FILE} | awk -F "." '{print $1}'`
+            DTB_NAME=`echo ${KERNEL_IMAGE_BASE_NAME} | sed
"s/${MACHINE}/${DTS_BASE_NAME}/g"`
+            DTB_SYMLINK_NAME=`echo ${KERNEL_IMAGE_SYMLINK_NAME} | sed
"s/${MACHINE}/${DTS_BASE_NAME}/g"`
+            dtc -I dts -O dtb ${KERNEL_DEVICETREE_FLAGS} -o
${DTS_BASE_NAME} ${DTS_FILE}
+            install -m 0644 ${DTS_BASE_NAME}
${D}/boot/devicetree-${DTB_SYMLINK_NAME}.dtb
+        done
     fi
 }

+do_deploy_append() {
+    if test -n "${KERNEL_DEVICETREE}"; then
+        for DTS_FILE in ${KERNEL_DEVICETREE}; do
+            if [ ! -f ${DTS_FILE} ]; then
+                echo "Warning: ${DTS_FILE} is not available!"
+                continue
+            fi
+            DTS_BASE_NAME=`basename ${DTS_FILE} | awk -F "." '{print $1}'`
+            DTB_NAME=`echo ${KERNEL_IMAGE_BASE_NAME} | sed
"s/${MACHINE}/${DTS_BASE_NAME}/g"`
+            DTB_SYMLINK_NAME=`echo ${KERNEL_IMAGE_SYMLINK_NAME} | sed
"s/${MACHINE}/${DTS_BASE_NAME}/g"`
+            install -d ${DEPLOYDIR}
+            install -m 0644 ${B}/${DTS_BASE_NAME}
${DEPLOYDIR}/${DTB_NAME}.dtb
+            cd ${DEPLOYDIR}
+            ln -sf ${DTB_NAME}.dtb ${DTB_SYMLINK_NAME}.dtb
+            cd -
+        done
+    fi
+}


Regards,

Leon.

[-- Attachment #2: Type: text/html, Size: 3031 bytes --]

Re: [denzil-next BREAKAGE] Silent breakage in denzil-next --- uImage*.dts no longer generated

[Leon Woestenberg]

[-- Attachment #1: Type: text/plain, Size: 675 bytes --]

Hello Scott, all,

On Wed, Aug 1, 2012 at 4:31 PM, Leon Woestenberg <
sidebranch.openembedded@gmail.com> wrote:

> Hello Scott,
>
> I tested your denzil-next branch but found breakage that will not fail the
> (auto) build.
>
> I'm hitting the "Warning: ${DTS_FILE} is not available!" case.
>
> Disregard the breakage.  Local user error; my KERNEL_DEVICETREE missed the
"${S}" prefix.

However, that being said, should we silently fail with a warning of the
specified device tree (DTS) is missing?  This is clearly a change of
behaviour that makes the build prone to user errors, as I just demonstrated.

I rather have the build fail.

Regards,

Leon.

[-- Attachment #2: Type: text/html, Size: 969 bytes --]

Re: [denzil-next BREAKAGE] Silent breakage in denzil-next --- uImage*.dts no longer generated

[Scott Garman]

On 08/01/2012 07:50 AM, Leon Woestenberg wrote:
> Hello Scott, all,
>
> On Wed, Aug 1, 2012 at 4:31 PM, Leon Woestenberg
> <sidebranch.openembedded@gmail.com
> <mailto:sidebranch.openembedded@gmail.com>> wrote:
>
>     Hello Scott,
>
>     I tested your denzil-next branch but found breakage that will not
>     fail the (auto) build.
>
>     I'm hitting the "Warning: ${DTS_FILE} is not available!" case.
>
> Disregard the breakage.  Local user error; my KERNEL_DEVICETREE missed
> the "${S}" prefix.
>
> However, that being said, should we silently fail with a warning of the
> specified device tree (DTS) is missing?  This is clearly a change of
> behaviour that makes the build prone to user errors, as I just demonstrated.
>
> I rather have the build fail.

Hi Leon,

Thanks for bringing this up. I believe Matthew made the original request 
for this commit (linux-dtb: add multi-dtb build support, 
a40d795ee97d8ada6a0b76c9741a8653fd646893). I'm cc:'ing him on this email 
as well as Richard Purdie, who has yet to review the new commits in my 
denzil-next branch and may also have some input on this issue.

The question in my mind right now is whether this change is too 
risky/change-inducing for a point-release.

Scott

-- 
Scott Garman
Embedded Linux Engineer - Yocto Project
Intel Open Source Technology Center

Re: [denzil-next BREAKAGE] Silent breakage in denzil-next --- uImage*.dts no longer generated

[McClintock Matthew-B29882]

On Wed, Aug 1, 2012 at 11:56 AM, Scott Garman <scott.a.garman@intel.com> wrote:
> On 08/01/2012 07:50 AM, Leon Woestenberg wrote:
>>
>> Hello Scott, all,
>>
>> On Wed, Aug 1, 2012 at 4:31 PM, Leon Woestenberg
>> <sidebranch.openembedded@gmail.com
>> <mailto:sidebranch.openembedded@gmail.com>> wrote:
>>
>>     Hello Scott,
>>
>>     I tested your denzil-next branch but found breakage that will not
>>     fail the (auto) build.
>>
>>     I'm hitting the "Warning: ${DTS_FILE} is not available!" case.
>>
>> Disregard the breakage.  Local user error; my KERNEL_DEVICETREE missed
>> the "${S}" prefix.
>>
>> However, that being said, should we silently fail with a warning of the
>> specified device tree (DTS) is missing?  This is clearly a change of
>> behaviour that makes the build prone to user errors, as I just
>> demonstrated.
>>
>> I rather have the build fail.
>
>
> Hi Leon,
>
> Thanks for bringing this up. I believe Matthew made the original request for
> this commit (linux-dtb: add multi-dtb build support,
> a40d795ee97d8ada6a0b76c9741a8653fd646893). I'm cc:'ing him on this email as
> well as Richard Purdie, who has yet to review the new commits in my
> denzil-next branch and may also have some input on this issue.
>
> The question in my mind right now is whether this change is too
> risky/change-inducing for a point-release.

I think the was made this way for people using different Linux tree's
(and Linux versions) specifically which lacked specific device trees
so switching between them causing problems for specific device tree
definitions so with this we could just list them all. For example
p1022ds -> (p1022ds_32b and p1022ds_36b). I could be convinced the
make this an error or at the very least a bb.warn instead of just an
echo since it does seem problematic.

Any change should go through master -> denzil

-M

Re: [denzil-next BREAKAGE] Silent breakage in denzil-next --- uImage*.dts no longer generated

[Leon Woestenberg]

[-- Attachment #1: Type: text/plain, Size: 1848 bytes --]

Hello,

On Wed, Aug 1, 2012 at 8:05 PM, McClintock Matthew-B29882 <
B29882@freescale.com> wrote:

> On Wed, Aug 1, 2012 at 11:56 AM, Scott Garman <scott.a.garman@intel.com>
> wrote:
> > On 08/01/2012 07:50 AM, Leon Woestenberg wrote:
> >> Disregard the breakage.  Local user error; my KERNEL_DEVICETREE missed
> >> the "${S}" prefix.
> >>
>

To summarize the change of behaviour of the patch:

1. Previously (before this patch) the "dtc" compiler was run from the ${S}
directory (linux/git) rather than the ${WORKDIR} directory (linux/),
so a missing ${S} in the KERNEL_DEVICETREE did not break it from compiling.

2. OpenEmbedded did not require the ${S} in KERNEL_DEVICETREE, and custom
device layers (people converting from OE Classic to Core or Poky/Yocto) may
still use it.

3. With this patch, not finding the DTS becomes a silent warning, instead
of an error.


> >
> > The question in my mind right now is whether this change is too
> > risky/change-inducing for a point-release.
>
> I think the was made this way for people using different Linux tree's
> (and Linux versions) specifically which lacked specific device trees
> so switching between them causing problems for specific device tree
> definitions so with this we could just list them all. For example
> p1022ds -> (p1022ds_32b and p1022ds_36b). I could be convinced the
> make this an error or at the very least a bb.warn instead of just an
> echo since it does seem problematic.
>
> Any change should go through master -> denzil
>
>
OK, I get the rationale. My proposal is a solution where the build should
fail if KERNEL_DEVICETREE was specified, but no dts was compiled at all (in
the for loop).

Warn if the dts specified was not an exact match.
Error out if no dts was compiled at all (in the for loop).

Regards,

Leon.

[-- Attachment #2: Type: text/html, Size: 2575 bytes --]