[1/2] doc swupdate.rst: clarify execution order for scripts

Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com>
---
Attempt at clarifying following discussion on the list:
https://groups.google.com/g/swupdate/c/Ih6Co_4CCq8/m/FmhCRWZtCQAJ

Rewriting someone's words is always difficult, feel free to adjust as
you see fit as I might not have kept your original intention through
very well.

 doc/source/swupdate.rst | 45 ++++++++++++++++++++++++-----------------
 1 file changed, 27 insertions(+), 18 deletions(-)

Hi Dominique,

On 08.04.21 09:32, Dominique Martinet wrote:
> Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com>
> ---
> Attempt at clarifying following discussion on the list:
> https://groups.google.com/g/swupdate/c/Ih6Co_4CCq8/m/FmhCRWZtCQAJ
> 
> Rewriting someone's words is always difficult, feel free to adjust as
> you see fit as I might not have kept your original intention through
> very well.
> 
>   doc/source/swupdate.rst | 45 ++++++++++++++++++++++++-----------------
>   1 file changed, 27 insertions(+), 18 deletions(-)
> 
> diff --git a/doc/source/swupdate.rst b/doc/source/swupdate.rst
> index 361f24840007..f53f4dbb18f5 100644
> --- a/doc/source/swupdate.rst
> +++ b/doc/source/swupdate.rst
> @@ -80,7 +80,13 @@ General Overview
>   
>   - Support for setting / erasing `EFI Boot Guard`_ variables
>   
> -- Support for preinstall scripts. They run before updating the images
> +- Support for pre and post update commands run before the update starts
> +  processing data and after the update has finished successfully.
> +
> +- Support for lua hooks, executed before any handler runs.
> +
> +- Support for preinstall scripts. They run after streamed handlers have
> +  handled their data, and before regular handlers.

Ok

>   
>   - Support for postinstall scripts. They run after updating the images.
>   
> @@ -345,24 +351,27 @@ A run of SWUpdate consists mainly of the following steps:
>     with the table in sw-description.
>   - Parse sw-description to determine which artefacts in the incoming SWU
>     are required. Not required artifacts are simply skipped.
> -  The meta file sw-description is declarative and executive. If "hooks"
> -  are defined in sw-description, they are executed at parse time. Hooks
> -  are executed before any artefact is evaluated and they are the best
> -  way to implement pre install functions.

Should we drop the whole stuff above ? This is not described later.

The point is the buggy CPIO archive distributed with Linux distro (this 
does not affect OE). cpio in distros does not set the checksum when a 
single file is bigger as 2GB. Having this note (maybe rewritten ?) is 
important for debian users.

> +  If an "embedded-script" is defined, it is executed at this point
> +  before parsing files.

This is not correct. The embedded-script is loaded, but it is executed 
when the parser hits a hook, so while an entry is parsed.

Entries could be also not a file - see for example partition, or toggler 
(ssbl_handler for example). Use a generic name here (entry or whatever) 
- in fact, it is foreseen that some handlers have no image / file 
associated to it. (NO_DATA_HANDLER).

> +  If "hooks" are defined, they are executed as each file is parsed,
> +  even if they will be skipped.

Well, the hook can decide if the file is skipped. So the entry is 
parsed, the hook is called, and then it is decided if it is skipped.

>     At the end of the parsing, SWUpdate builds an internal mapping for each artifact
>     to recognize which handler should be called for each of them.
> -- Reads the cpio archive and proofs the CPIO checksum of each single file
> -  (this can be disabled in the configuration) and computes the sha256 sum if foreseen or if Signed Images
> -  are activated.  SWUpdate stops if the archive is not complete verified.

Agree, this is wrong - it assumes that SWUpdate reads an ertifact, 
computes the sha256 and stops before installing. But in this way, it is 
duty to save temporary the data. SWUpdate works on the fly, and 
decryption / decompression / hash computation is done with buffers in 
memory on the fly. Nice to remove.

> -- check that all components described in sw-description are
> -  really in the cpio archive.
> -- run the partitions handlers, if required.
> -- runs pre-install scripts, if any. Please note: if artifacts are streamed, a preinstall script cannot be
> -  executed because it depends on the order the artifact (including the script) are packed into the CPIO archive.
> -  In this case, please use the "embedded-script" feature in sw-description to execute functions before any
> -  installation takes place.
> -- if an artifact is found in SWU and it is marked with "installed-directly", proceed with installation,
> -  else extract it temporarily into TMPDIR.
> +- runs the pre update command, if set
> +- runs partition handlers, if required.
> +- reads through the cpio archive one file at a time and either:
> +  * execute handlers for each file marked as "installed-directly".
> +    checksum is checked while the data is streamed to handler, and copy will
> +    be marked as having failed if checksum was not correct failing the rest
> +    of the install.
> +  * copy other files to a temporary location while checking checksums,
> +    stopping if there was a mismatch.

ok

> +- iterates through all `scripts` and call the corresponding
> +  handler for pre-install scripts.
> +  Please note: if artifacts are streamed, they will be extracted
> +  before this runs. If earlier execution is required, please use
> +  the "embedded-script" or hooks features to ensure code is run
> +  before installation takes place.
>   - iterates through all `images` and call the corresponding
>     handler for installing on target.
>   - iterates through all `files` and call the corresponding
> @@ -372,7 +381,7 @@ A run of SWUpdate consists mainly of the following steps:
>   - iterates through all `bootenv` and updates the bootloader environment.
>   - reports the status to the operator through the notification interface
>     (logging, traces) and through the progress interface.
> -- if a postinstall command is foreseen (for example to reboot the device), call it.
> +- runs the post update command, if set.
>   
>   The first step that fails, stops the entire procedure and
>   an error is reported.
> 

Best regards,
Stefano Babic

Stefano Babic wrote on Thu, Apr 08, 2021 at 06:29:04PM +0200:
> >   - Support for postinstall scripts. They run after updating the images.
> > @@ -345,24 +351,27 @@ A run of SWUpdate consists mainly of the following steps:
> >     with the table in sw-description.
> >   - Parse sw-description to determine which artefacts in the incoming SWU
> >     are required. Not required artifacts are simply skipped.
> > -  The meta file sw-description is declarative and executive. If "hooks"
> > -  are defined in sw-description, they are executed at parse time. Hooks
> > -  are executed before any artefact is evaluated and they are the best
> > -  way to implement pre install functions.
> 
> Should we drop the whole stuff above ? This is not described later.
>
> The point is the buggy CPIO archive distributed with Linux distro (this does
> not affect OE). cpio in distros does not set the checksum when a single file
> is bigger as 2GB. Having this note (maybe rewritten ?) is important for
> debian users.

Sorry, I am not quite sure what exactly you are referring to?
In my head the whole few points below are part of parsing sw-description
and decribe execution happen while parsing it before looking at other
files; so they replace the above statement.
If you think this is not clear it might need an extra line?

I am also not sure what you mean with the cpio archive in linux distros;
I removed that the cpio checksum is checked below as well as the
sha256sum that might be worth keeping, was it that?

> > +  If an "embedded-script" is defined, it is executed at this point
> > +  before parsing files.
> 
> This is not correct. The embedded-script is loaded, but it is executed when
> the parser hits a hook, so while an entry is parsed.

Yes and no. the embedded-script is fully evaluated at this point before
hooks are hit. So if you just define functions then nothing happens
until hooks happen, but if you have declarating stuff then they will
just appear to have been executed e.g. the following sw-description will
print "embed" once when evaluating the embed script; then move on to
parsing sub items and run check_hook twice (once for each file) :

software = {
  embedded-script = "
    require(\"swupdate\")

    function check_hook(image)
      swupdate.trace(\"test hook\")
      return true, image
    end

    swupdate.trace(\"embed\")"
  ...
  files: (
    { ... hook = "check_hook"; ... },
    { ... hook = "check_hook"; ... },
  );
};


I found about this "feature" will writing the documentation.

> Entries could be also not a file - see for example partition, or toggler
> (ssbl_handler for example). Use a generic name here (entry or whatever) - in
> fact, it is foreseen that some handlers have no image / file associated to
> it. (NO_DATA_HANDLER).

Yes for this, sorry for wording.

> > +  If "hooks" are defined, they are executed as each file is parsed,
> > +  even if they will be skipped.
> 
> Well, the hook can decide if the file is skipped. So the entry is parsed,
> the hook is called, and then it is decided if it is skipped.

Yes; I understand how this works, but there are other skip mechanisms
(e.g. version comparison) and people might be surprised if hooks are
executed even if install-if-different is set and version match installed
software for example.
The hook could decide to install anyway, so it makes sense to do it that
way; it just needs mentioning.

Help with the wording is appreciated if you think this is confusing.



I will send v2 after reply

On 09.04.21 02:12, Dominique Martinet wrote:
> Stefano Babic wrote on Thu, Apr 08, 2021 at 06:29:04PM +0200:
>>>    - Support for postinstall scripts. They run after updating the images.
>>> @@ -345,24 +351,27 @@ A run of SWUpdate consists mainly of the following steps:
>>>      with the table in sw-description.
>>>    - Parse sw-description to determine which artefacts in the incoming SWU
>>>      are required. Not required artifacts are simply skipped.
>>> -  The meta file sw-description is declarative and executive. If "hooks"
>>> -  are defined in sw-description, they are executed at parse time. Hooks
>>> -  are executed before any artefact is evaluated and they are the best
>>> -  way to implement pre install functions.
>>
>> Should we drop the whole stuff above ? This is not described later.
>>
>> The point is the buggy CPIO archive distributed with Linux distro (this does
>> not affect OE). cpio in distros does not set the checksum when a single file
>> is bigger as 2GB. Having this note (maybe rewritten ?) is important for
>> debian users.
> 
> Sorry, I am not quite sure what exactly you are referring to?
> In my head the whole few points below are part of parsing sw-description
> and decribe execution happen while parsing it before looking at other
> files; so they replace the above statement.

Yes - the information above is at the wrong place, so I agree it should 
be removed. I do not know where to put it back, the topic comes more as 
once on ML.

> If you think this is not clear it might need an extra line?

No, I reread, this part is for parsing and the issue related to CPIO is 
not part of parsing.

> 
> I am also not sure what you mean with the cpio archive in linux distros;
> I removed that the cpio checksum is checked below as well as the
> sha256sum that might be worth keeping, was it that?

SWUpdate uses cpio "new ascii format with checksum". Checksum is very 
weak, but it still something. However, cpio (a very old program) 
delivered with Distros (Debian, Fedora, ...) has a bug and the checksum 
is set to zero if the file is bigger than 2GB. When SWUpdate chekcs it, 
it raises the error.

On OE this is fixed in meta-swupdate. There is quite no CPIO maintainer, 
I sent the patch to cpio ML, but this list is quite dead. There are a 
large set of patches that are simply ignored. So if you are not using OE 
or buildroot but Debian, you have to patch CPIO yourself or configure 
SWUpdate to ignore CPIO checksum.

> 
>>> +  If an "embedded-script" is defined, it is executed at this point
>>> +  before parsing files.
>>
>> This is not correct. The embedded-script is loaded, but it is executed when
>> the parser hits a hook, so while an entry is parsed.
> 
> Yes and no. the embedded-script is fully evaluated at this point before
> hooks are hit. So if you just define functions then nothing happens
> until hooks happen, but if you have declarating stuff then they will
> just appear to have been executed e.g.

Oh yes, that is correct - as in any script. It is also a use case.


> the following sw-description will
> print "embed" once when evaluating the embed script; then move on to
> parsing sub items and run check_hook twice (once for each file) :
> 
> software = {
>    embedded-script = "
>      require(\"swupdate\")
> 
>      function check_hook(image)
>        swupdate.trace(\"test hook\")
>        return true, image
>      end
> 
>      swupdate.trace(\"embed\")"
>    ...
>    files: (
>      { ... hook = "check_hook"; ... },
>      { ... hook = "check_hook"; ... },
>    );
> };
> 
> 
> I found about this "feature" will writing the documentation.

Agree - it is a "hidden" feature, but it is correct what you say.

> 
>> Entries could be also not a file - see for example partition, or toggler
>> (ssbl_handler for example). Use a generic name here (entry or whatever) - in
>> fact, it is foreseen that some handlers have no image / file associated to
>> it. (NO_DATA_HANDLER).
> 
> Yes for this, sorry for wording.
> 
>>> +  If "hooks" are defined, they are executed as each file is parsed,
>>> +  even if they will be skipped.
>>
>> Well, the hook can decide if the file is skipped. So the entry is parsed,
>> the hook is called, and then it is decided if it is skipped.
> 
> Yes; I understand how this works, but there are other skip mechanisms
> (e.g. version comparison) and people might be surprised if hooks are
> executed even if install-if-different is set and version match installed
> software for example.

Well, this is part of the parsing. Anyway it is ok for me if you think 
that enforcing this makes things clearer.

> The hook could decide to install anyway, so it makes sense to do it that
> way; it just needs mentioning.

Fine.

> 
> Help with the wording is appreciated if you think this is confusing.

I guess I cannot do a lot. My phrases looked confusing to you (and I 
admit, one reason is that some additional text was added in later patch 
without checking if the whole makes sense), so appreciated that you want 
to fix it. It will be nice to have more comments to check if what we are 
writing is just clear to us or there is a common understanding.



> 
> 
> 
> I will send v2 after reply
> 

Best regards,
Stefano Babic