Hi Yannic,

On Tue, 27 May 2025 at 14:24, Yannic Moog <y.m...@phytec.de> wrote:
>
> Optional blobs should mark themselves as absent to avoid being packed
> into an image.
> Extend the documentation of this behaviour. Although the documentation
> implied this before, the "optional" property had not been explained
> properly before.
> Note that the intended behaviour is the same, this patch just documents
> it.

This seems to indicate that it is just a doc change

> The actual behaviour will change as now absent entries are no longer
> packed into an image. The image map will also reflect this.

But this indicates that it changes the functionality. Can you clarify
this in your commit message?

> As a result, the CheckForProblems() function will no longer alert on
> optional (blob) entries. This is because the missing optional images
> were removed before CheckForProblems is called.
> This, as well, is intended behaviour.

This breaks testExtblobOptional() for me.

I applied your patch: tools: binman: drop "faked" return value from
check_fake_fname

Then I applied this one and got the error.

I could not apply patch 2 (ools: binman: ftest: pass allow_fake_blob
to _DoReadFileDtb) as it broke a lot of tests.

For testExtblobOptional() I see that you have fixed it by the end of
the series, but I get 11/12 test failures . Are you able to make the
series bisectable, so that 'binman test' works for each commit? It's
hard for me to comment on the series in this state.

Also there's no need to put an 'RFC' tag on this series. We do want to
resolve this, one way or another.

>
> Reported-by: Simon Glass <s...@chromium.org>
> Signed-off-by: Yannic Moog <y.m...@phytec.de>
> ---
>  tools/binman/binman.rst    | 7 +++++++
>  tools/binman/etype/blob.py | 2 ++
>  2 files changed, 9 insertions(+)
>
> diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst
> index 
> 84b1331df5c006f72167016b7f33de73f4a269fa..392e507d449c75eeba4431fe2db01d820bce8f61
>  100644
> --- a/tools/binman/binman.rst
> +++ b/tools/binman/binman.rst
> @@ -1143,6 +1143,13 @@ Optional entries
>
>  Some entries need to exist only if certain conditions are met. For example, 
> an
>  entry may want to appear in the image only if a file has a particular format.
> +Also, the ``optional`` property may be used to mark entries as optional::
> +
> +    tee-os {
> +        filename = "tee.bin";
> +        optional;
> +    };
> +
>  Obviously the entry must exist in the image description for it to be 
> processed
>  at all, so a way needs to be found to have the entry remove itself.
>
> diff --git a/tools/binman/etype/blob.py b/tools/binman/etype/blob.py
> index 
> 970fae91cd9b2a31b3f31d2ecdb77e27a89d69fb..acd9ae34074e4b4b9792b107cc80514dfdd88b3b
>  100644
> --- a/tools/binman/etype/blob.py
> +++ b/tools/binman/etype/blob.py
> @@ -52,6 +52,8 @@ class Entry_blob(Entry):
>                  fake_size = self.assume_size
>              self._pathname = self.check_fake_fname(self._filename, fake_size)
>              self.missing = True
> +            if self.optional:
> +                self.mark_absent("missing but optional")
>              if not self.faked:
>                  content_size = 0
>                  if self.assume_size: # Ensure we get test coverage on next 
> line
>
> --
> 2.43.0
>

Regards,
Simon

Reply via email to