diff mbox series

[meta-swupdate,V2] Add SWUPDATE_LIST_FOR_CPIO to allow explicit SWU file inclusion

Message ID 1554251241-86395-1-git-send-email-austin.phillips@planetinnovation.com.au
State Changes Requested
Headers show
Series [meta-swupdate,V2] Add SWUPDATE_LIST_FOR_CPIO to allow explicit SWU file inclusion | expand

Commit Message

'Darko Komljenovic' via swupdate April 3, 2019, 12:27 a.m. UTC
By default, swupdate.bbclass includes all files fetched by the Yocto
recipe in the SWU cpio archive.  This change adds a new variable
SWUPDATE_LIST_FOR_CPIO which, when specified, allows for explicit
control of which additional files are added to the SWU image

This is useful when the default fetch file list isn't suitable for
inclusion in the SWU image.

Fix inheritance of swupdate-common bbclass by removing unwanted .bbclass
suffix.

Signed-off-by: Austin Phillips <austin.phillips@planetinnovation.com.au>
---
 README                   | 42 ++++++++++++++++++++++++++++++++++++++++++
 classes/swupdate.bbclass | 14 +++++++++++---
 2 files changed, 53 insertions(+), 3 deletions(-)

Comments

Stefano Babic May 17, 2019, 8:58 a.m. UTC | #1
Hallo Austin,

sorry for late answer.

On 03/04/19 02:27, 'Austin Phillips' via swupdate wrote:
> By default, swupdate.bbclass includes all files fetched by the Yocto
> recipe in the SWU cpio archive.  This change adds a new variable
> SWUPDATE_LIST_FOR_CPIO which, when specified, allows for explicit
> control of which additional files are added to the SWU image
> 
> This is useful when the default fetch file list isn't suitable for
> inclusion in the SWU image.
> 
> Fix inheritance of swupdate-common bbclass by removing unwanted .bbclass
> suffix.

I am just thinking about if we can avoid to add an additional variable,
even if this is a straightforward approach - a lot of variablke
generates confusion.

There is already a variable to add images from DEPLOY directory, and
this is SWUPDATE_IMAGES. I would like to see the same approach used in
OE when something should be removed, something like
SWUPDATE_IMAGES_remove = "files to be removed" instead of bothering with
a new variable. This SWUPDATE_LIST_FOR_CPIO in fact conflicts with
SWUPDATE_IMAGES, just refers to files that are fetched instead of
getting them from the deployed.

Regards,
Stefano

> 
> Signed-off-by: Austin Phillips <austin.phillips@planetinnovation.com.au>
> ---
>  README                   | 42 ++++++++++++++++++++++++++++++++++++++++++
>  classes/swupdate.bbclass | 14 +++++++++++---
>  2 files changed, 53 insertions(+), 3 deletions(-)
> 
> diff --git a/README b/README
> index ffc8f33..31baa11 100644
> --- a/README
> +++ b/README
> @@ -57,6 +57,48 @@ which is included in the SWU file.
>  Encrypted private keys are not currently supported since a secure
>  mechanism must exist to provide the passphrase.
>  
> +SWU image contents
> +----------
> +By default the SWU image will include all files fetched by the Yocto
> +fetcher as specified in recipe SRC_URI variable.  If this behaviour is not
> +suitable, an explicit list of file inclusions can be specified by
> +setting `SWUPDATE_LIST_FOR_CPIO` to a list of source file paths to be
> +included in the SWU image.
> +
> +This may be useful in cases where additional build steps are
> +introduced prior to the swuimage task to pre-process fetched source files
> +or files are generated as part of the recipe for inclusion in the SWU
> +image.
> +
> +e.g.
> +
> +Let's assume an additional pre-processing step is required to
> +inject some content to sw-description before its inclusion in the SWU.
> +
> +A Yocto recipe may include a configuration like the following:
> +
> +...
> +
> +SRC_URI := "\
> +    file://sw-description.conf" \
> +    file://my-version-file \
> +"
> +
> +inherit swupdate
> +
> +python do_swuimage_prepend () {
> +    # Preprocess sw-description.conf, injecting version information
> +    # from my-version-file to produce sw-description.
> +    # Generate another file 'foo' for inclusion in SWU.
> +}
> +
> +# Explicit list of additional files to include in SWU archive.
> +# Prevent sw-description.conf and my-version-file from being included
> +# in the archive by using an explicit list of file inclusions.
> +# sw-description is included by default.
> +SWUPDATE_LIST_FOR_CPIO = "${WORKDIR}/foo"
> +
> +
>  Maintainer
>  ----------
>  
> diff --git a/classes/swupdate.bbclass b/classes/swupdate.bbclass
> index 1d74eef..8167479 100644
> --- a/classes/swupdate.bbclass
> +++ b/classes/swupdate.bbclass
> @@ -11,7 +11,7 @@
>  # To use, add swupdate to the inherit clause and set
>  # set the images (all of them must be found in deploy directory)
>  # that are part of the compound image.
> -inherit swupdate-common.bbclass
> +inherit swupdate-common
>  
>  S = "${WORKDIR}/${PN}"
>  
> @@ -74,8 +74,16 @@ python do_swuimage () {
>      if d.getVar('SWUPDATE_SIGNING', True):
>          list_for_cpio.append('sw-description.sig')
>  
> -    for url in fetch.urls:
> -        local = fetch.localpath(url)
> +    # Default to obtaining list of additional cpio members from fetch sources.
> +    # The SWUPDATE_LIST_FOR_CPIO can be used to override the default list
> +    # to specify a custom set of cpio members.
> +    swupdate_list_for_cpio = d.getVar('SWUPDATE_LIST_FOR_CPIO', True)
> +    if swupdate_list_for_cpio is None:
> +        swupdate_list_for_cpio = [fetch.localpath(url) for url in fetch.urls]
> +    else:
> +        swupdate_list_for_cpio = swupdate_list_for_cpio.split()
> +
> +    for local in swupdate_list_for_cpio:
>          filename = os.path.basename(local)
>          if (filename != 'sw-description'):
>              shutil.copyfile(local, os.path.join(s, "%s" % filename ))
>
'Darko Komljenovic' via swupdate May 22, 2019, 11:09 a.m. UTC | #2
Hi Stefano,

Thanks for reviewing.

>
> Hallo Austin,
>
> sorry for late answer.
>
> On 03/04/19 02:27, 'Austin Phillips' via swupdate wrote:
> > By default, swupdate.bbclass includes all files fetched by the Yocto
> > recipe in the SWU cpio archive.  This change adds a new variable
> > SWUPDATE_LIST_FOR_CPIO which, when specified, allows for explicit
> > control of which additional files are added to the SWU image
> >
> > This is useful when the default fetch file list isn't suitable for
> > inclusion in the SWU image.
> >
> > Fix inheritance of swupdate-common bbclass by removing unwanted
> > .bbclass suffix.
>
> I am just thinking about if we can avoid to add an additional variable,
> even if
> this is a straightforward approach - a lot of variablke generates
> confusion.

Agree in general  addition of variables should be avoided where possible.

>
> There is already a variable to add images from DEPLOY directory, and this
> is
> SWUPDATE_IMAGES. I would like to see the same approach used in OE when
> something should be removed, something like SWUPDATE_IMAGES_remove
> = "files to be removed" instead of bothering with a new variable. This
> SWUPDATE_LIST_FOR_CPIO in fact conflicts with SWUPDATE_IMAGES, just
> refers to files that are fetched instead of getting them from the
> deployed.

The issue I've had with the current approach is that all fetched files are
added to the cpio archive.  There is an implicit link between files which
are fetched and those which should exist in the archive, I'd rather this
list were explicit.

While SWUPDATE_IMAGES provides a mechanism to add Yocto images from the
deploy directory, it doesn't handle the case where the archive is to hold
items which are constructed locally. e.g.  I've got an update.sh script
which is fetched by the fetcher, but I want to encrypt it before placing it
in the image.

Perhaps SWUPDATE_IMAGES should  be an explicit list of files which are
expected to be added to the archive, and it would be up to the recipe using
swupdate bbclass to fetch deploy images.  i.e. Instead of having
swupdate.bbclass pull images from deploy, make this the responsibility of
the recipe.  swupdate.bbclass could still be used to provide functions
useful for this purpose.

One of the reasons I chose this implementation was that I didn't want to
break existing recipes using this bbclass.

Regards
Austin

>
> Regards,
> Stefano
>
> >
> > Signed-off-by: Austin Phillips
> > <austin.phillips@planetinnovation.com.au>
> > ---
> >  README                   | 42
> ++++++++++++++++++++++++++++++++++++++++++
> >  classes/swupdate.bbclass | 14 +++++++++++---
> >  2 files changed, 53 insertions(+), 3 deletions(-)
> >
> > diff --git a/README b/README
> > index ffc8f33..31baa11 100644
> > --- a/README
> > +++ b/README
> > @@ -57,6 +57,48 @@ which is included in the SWU file.
> >  Encrypted private keys are not currently supported since a secure
> > mechanism must exist to provide the passphrase.
> >
> > +SWU image contents
> > +----------
> > +By default the SWU image will include all files fetched by the Yocto
> > +fetcher as specified in recipe SRC_URI variable.  If this behaviour
> > +is not suitable, an explicit list of file inclusions can be specified
> > +by setting `SWUPDATE_LIST_FOR_CPIO` to a list of source file paths to
> > +be included in the SWU image.
> > +
> > +This may be useful in cases where additional build steps are
> > +introduced prior to the swuimage task to pre-process fetched source
> > +files or files are generated as part of the recipe for inclusion in
> > +the SWU image.
> > +
> > +e.g.
> > +
> > +Let's assume an additional pre-processing step is required to inject
> > +some content to sw-description before its inclusion in the SWU.
> > +
> > +A Yocto recipe may include a configuration like the following:
> > +
> > +...
> > +
> > +SRC_URI := "\
> > +    file://sw-description.conf" \
> > +    file://my-version-file \
> > +"
> > +
> > +inherit swupdate
> > +
> > +python do_swuimage_prepend () {
> > +    # Preprocess sw-description.conf, injecting version information
> > +    # from my-version-file to produce sw-description.
> > +    # Generate another file 'foo' for inclusion in SWU.
> > +}
> > +
> > +# Explicit list of additional files to include in SWU archive.
> > +# Prevent sw-description.conf and my-version-file from being included
> > +# in the archive by using an explicit list of file inclusions.
> > +# sw-description is included by default.
> > +SWUPDATE_LIST_FOR_CPIO = "${WORKDIR}/foo"
> > +
> > +
> >  Maintainer
> >  ----------
> >
> > diff --git a/classes/swupdate.bbclass b/classes/swupdate.bbclass index
> > 1d74eef..8167479 100644
> > --- a/classes/swupdate.bbclass
> > +++ b/classes/swupdate.bbclass
> > @@ -11,7 +11,7 @@
> >  # To use, add swupdate to the inherit clause and set  # set the
> > images (all of them must be found in deploy directory)  # that are
> > part of the compound image.
> > -inherit swupdate-common.bbclass
> > +inherit swupdate-common
> >
> >  S = "${WORKDIR}/${PN}"
> >
> > @@ -74,8 +74,16 @@ python do_swuimage () {
> >      if d.getVar('SWUPDATE_SIGNING', True):
> >          list_for_cpio.append('sw-description.sig')
> >
> > -    for url in fetch.urls:
> > -        local = fetch.localpath(url)
> > +    # Default to obtaining list of additional cpio members from fetch
> sources.
> > +    # The SWUPDATE_LIST_FOR_CPIO can be used to override the default
> list
> > +    # to specify a custom set of cpio members.
> > +    swupdate_list_for_cpio = d.getVar('SWUPDATE_LIST_FOR_CPIO', True)
> > +    if swupdate_list_for_cpio is None:
> > +        swupdate_list_for_cpio = [fetch.localpath(url) for url in
> > fetch.urls]
> > +    else:
> > +        swupdate_list_for_cpio = swupdate_list_for_cpio.split()
> > +
> > +    for local in swupdate_list_for_cpio:
> >          filename = os.path.basename(local)
> >          if (filename != 'sw-description'):
> >              shutil.copyfile(local, os.path.join(s, "%s" % filename ))
> >
>
>
> --
> ==========================================================
> ===========
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de
> ==========================================================
> ===========
Stefano Babic May 22, 2019, 1:48 p.m. UTC | #3
Hi Austin,

On 22/05/19 13:09, 'Austin Phillips' via swupdate wrote:
> Hi Stefano,
> 
> Thanks for reviewing.
> 
>>
>> Hallo Austin,
>>
>> sorry for late answer.
>>
>> On 03/04/19 02:27, 'Austin Phillips' via swupdate wrote:
>>> By default, swupdate.bbclass includes all files fetched by the Yocto
>>> recipe in the SWU cpio archive.  This change adds a new variable
>>> SWUPDATE_LIST_FOR_CPIO which, when specified, allows for explicit
>>> control of which additional files are added to the SWU image
>>>
>>> This is useful when the default fetch file list isn't suitable for
>>> inclusion in the SWU image.
>>>
>>> Fix inheritance of swupdate-common bbclass by removing unwanted
>>> .bbclass suffix.
>>
>> I am just thinking about if we can avoid to add an additional variable,
>> even if
>> this is a straightforward approach - a lot of variablke generates
>> confusion.
> 
> Agree in general  addition of variables should be avoided where possible.
> 
>>
>> There is already a variable to add images from DEPLOY directory, and this
>> is
>> SWUPDATE_IMAGES. I would like to see the same approach used in OE when
>> something should be removed, something like SWUPDATE_IMAGES_remove
>> = "files to be removed" instead of bothering with a new variable. This
>> SWUPDATE_LIST_FOR_CPIO in fact conflicts with SWUPDATE_IMAGES, just
>> refers to files that are fetched instead of getting them from the
>> deployed.
> 
> The issue I've had with the current approach is that all fetched files are
> added to the cpio archive.  There is an implicit link between files which
> are fetched and those which should exist in the archive, I'd rather this
> list were explicit.

Yes, I understand your issue. This is done to easy add files (scripts ?)
and sw-descripĆ¼tion to the SWU without introducing another variable.

> 
> While SWUPDATE_IMAGES provides a mechanism to add Yocto images from the
> deploy directory, it doesn't handle 

yet

> the case where the archive is to hold
> items which are constructed locally. e.g.  I've got an update.sh script
> which is fetched by the fetcher, but I want to encrypt it before placing it
> in the image.

In the specific case, IMHO this could be the wrong way, Why is the SWU
recipe responsible to do this ? This recipe should just take images /
results and packed together in a suitable way, signing the images, and
so on. If an image like a script must be encrypted, it should be done
before.

What about to do in this way:

- add a recipe for the script(s)
- the recipe encrypts, write a package (not used) and has a do_deploy to
put the results in DEPLOYDIR
- your encrypted script is simply added to SWUPDATE_IMAGES

> 
> Perhaps SWUPDATE_IMAGES should  be an explicit list of files

ok, but somewhere we should know where the files are stored and this
cannot be a list of absolute and not portable paths.


My idea is that SWUPDATE_IMAGES is changed to contain the list of
fetched files and using the _remove, the list is updated.

> which are
> expected to be added to the archive, and it would be up to the recipe using
> swupdate bbclass to fetch deploy images.  i.e. Instead of having
> swupdate.bbclass pull images from deploy, make this the responsibility of
> the recipe.

However, this makes work for a common user more difficult. And there
will be a lot of recipes that are just a copy&paste duplicating the way
to get images (kernel, bootloader, rootfs,..) that are simply stored in
DEPLOYDIR.

>  swupdate.bbclass could still be used to provide functions
> useful for this purpose.
> 
> One of the reasons I chose this implementation was that I didn't want to
> break existing recipes using this bbclass.
> 

Best regards,
Stefano

> Regards
> Austin
> 
>>
>> Regards,
>> Stefano
>>
>>>
>>> Signed-off-by: Austin Phillips
>>> <austin.phillips@planetinnovation.com.au>
>>> ---
>>>  README                   | 42
>> ++++++++++++++++++++++++++++++++++++++++++
>>>  classes/swupdate.bbclass | 14 +++++++++++---
>>>  2 files changed, 53 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/README b/README
>>> index ffc8f33..31baa11 100644
>>> --- a/README
>>> +++ b/README
>>> @@ -57,6 +57,48 @@ which is included in the SWU file.
>>>  Encrypted private keys are not currently supported since a secure
>>> mechanism must exist to provide the passphrase.
>>>
>>> +SWU image contents
>>> +----------
>>> +By default the SWU image will include all files fetched by the Yocto
>>> +fetcher as specified in recipe SRC_URI variable.  If this behaviour
>>> +is not suitable, an explicit list of file inclusions can be specified
>>> +by setting `SWUPDATE_LIST_FOR_CPIO` to a list of source file paths to
>>> +be included in the SWU image.
>>> +
>>> +This may be useful in cases where additional build steps are
>>> +introduced prior to the swuimage task to pre-process fetched source
>>> +files or files are generated as part of the recipe for inclusion in
>>> +the SWU image.
>>> +
>>> +e.g.
>>> +
>>> +Let's assume an additional pre-processing step is required to inject
>>> +some content to sw-description before its inclusion in the SWU.
>>> +
>>> +A Yocto recipe may include a configuration like the following:
>>> +
>>> +...
>>> +
>>> +SRC_URI := "\
>>> +    file://sw-description.conf" \
>>> +    file://my-version-file \
>>> +"
>>> +
>>> +inherit swupdate
>>> +
>>> +python do_swuimage_prepend () {
>>> +    # Preprocess sw-description.conf, injecting version information
>>> +    # from my-version-file to produce sw-description.
>>> +    # Generate another file 'foo' for inclusion in SWU.
>>> +}
>>> +
>>> +# Explicit list of additional files to include in SWU archive.
>>> +# Prevent sw-description.conf and my-version-file from being included
>>> +# in the archive by using an explicit list of file inclusions.
>>> +# sw-description is included by default.
>>> +SWUPDATE_LIST_FOR_CPIO = "${WORKDIR}/foo"
>>> +
>>> +
>>>  Maintainer
>>>  ----------
>>>
>>> diff --git a/classes/swupdate.bbclass b/classes/swupdate.bbclass index
>>> 1d74eef..8167479 100644
>>> --- a/classes/swupdate.bbclass
>>> +++ b/classes/swupdate.bbclass
>>> @@ -11,7 +11,7 @@
>>>  # To use, add swupdate to the inherit clause and set  # set the
>>> images (all of them must be found in deploy directory)  # that are
>>> part of the compound image.
>>> -inherit swupdate-common.bbclass
>>> +inherit swupdate-common
>>>
>>>  S = "${WORKDIR}/${PN}"
>>>
>>> @@ -74,8 +74,16 @@ python do_swuimage () {
>>>      if d.getVar('SWUPDATE_SIGNING', True):
>>>          list_for_cpio.append('sw-description.sig')
>>>
>>> -    for url in fetch.urls:
>>> -        local = fetch.localpath(url)
>>> +    # Default to obtaining list of additional cpio members from fetch
>> sources.
>>> +    # The SWUPDATE_LIST_FOR_CPIO can be used to override the default
>> list
>>> +    # to specify a custom set of cpio members.
>>> +    swupdate_list_for_cpio = d.getVar('SWUPDATE_LIST_FOR_CPIO', True)
>>> +    if swupdate_list_for_cpio is None:
>>> +        swupdate_list_for_cpio = [fetch.localpath(url) for url in
>>> fetch.urls]
>>> +    else:
>>> +        swupdate_list_for_cpio = swupdate_list_for_cpio.split()
>>> +
>>> +    for local in swupdate_list_for_cpio:
>>>          filename = os.path.basename(local)
>>>          if (filename != 'sw-description'):
>>>              shutil.copyfile(local, os.path.join(s, "%s" % filename ))
>>>
>>
>>
>> --
>> ==========================================================
>> ===========
>> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
>> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
>> Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de
>> ==========================================================
>> ===========
>
diff mbox series

Patch

diff --git a/README b/README
index ffc8f33..31baa11 100644
--- a/README
+++ b/README
@@ -57,6 +57,48 @@  which is included in the SWU file.
 Encrypted private keys are not currently supported since a secure
 mechanism must exist to provide the passphrase.
 
+SWU image contents
+----------
+By default the SWU image will include all files fetched by the Yocto
+fetcher as specified in recipe SRC_URI variable.  If this behaviour is not
+suitable, an explicit list of file inclusions can be specified by
+setting `SWUPDATE_LIST_FOR_CPIO` to a list of source file paths to be
+included in the SWU image.
+
+This may be useful in cases where additional build steps are
+introduced prior to the swuimage task to pre-process fetched source files
+or files are generated as part of the recipe for inclusion in the SWU
+image.
+
+e.g.
+
+Let's assume an additional pre-processing step is required to
+inject some content to sw-description before its inclusion in the SWU.
+
+A Yocto recipe may include a configuration like the following:
+
+...
+
+SRC_URI := "\
+    file://sw-description.conf" \
+    file://my-version-file \
+"
+
+inherit swupdate
+
+python do_swuimage_prepend () {
+    # Preprocess sw-description.conf, injecting version information
+    # from my-version-file to produce sw-description.
+    # Generate another file 'foo' for inclusion in SWU.
+}
+
+# Explicit list of additional files to include in SWU archive.
+# Prevent sw-description.conf and my-version-file from being included
+# in the archive by using an explicit list of file inclusions.
+# sw-description is included by default.
+SWUPDATE_LIST_FOR_CPIO = "${WORKDIR}/foo"
+
+
 Maintainer
 ----------
 
diff --git a/classes/swupdate.bbclass b/classes/swupdate.bbclass
index 1d74eef..8167479 100644
--- a/classes/swupdate.bbclass
+++ b/classes/swupdate.bbclass
@@ -11,7 +11,7 @@ 
 # To use, add swupdate to the inherit clause and set
 # set the images (all of them must be found in deploy directory)
 # that are part of the compound image.
-inherit swupdate-common.bbclass
+inherit swupdate-common
 
 S = "${WORKDIR}/${PN}"
 
@@ -74,8 +74,16 @@  python do_swuimage () {
     if d.getVar('SWUPDATE_SIGNING', True):
         list_for_cpio.append('sw-description.sig')
 
-    for url in fetch.urls:
-        local = fetch.localpath(url)
+    # Default to obtaining list of additional cpio members from fetch sources.
+    # The SWUPDATE_LIST_FOR_CPIO can be used to override the default list
+    # to specify a custom set of cpio members.
+    swupdate_list_for_cpio = d.getVar('SWUPDATE_LIST_FOR_CPIO', True)
+    if swupdate_list_for_cpio is None:
+        swupdate_list_for_cpio = [fetch.localpath(url) for url in fetch.urls]
+    else:
+        swupdate_list_for_cpio = swupdate_list_for_cpio.split()
+
+    for local in swupdate_list_for_cpio:
         filename = os.path.basename(local)
         if (filename != 'sw-description'):
             shutil.copyfile(local, os.path.join(s, "%s" % filename ))