[PATCH] binman: Allow FIT binaries to have missing external blobs

[Alper Nebi Yasak]

On 01/09/2020 13:36, Simon Glass wrote:
> Hi Alper,
> 
> I found that evb-rk3288 fails to build with the final patch from your
> series. See u-boot-dm/testing for the tree.

I had seen your patch and assumed that nothing was already using the
allow-missing functionality in its FITs (since you're adding it now).
Looks like the version in your tree (aa3f219e) succeeds in the pipeline,
so I think there's nothing for me to do now. Thanks for fixing it.

> At present if a FIT references a missing external blob, binman reports an
> error, even if the image is supposed to allow this.
> 
> Propagate this setting down to the child sections of the FIT, so that the
> behaviour is consistent.
> 
> This is a fix-up patch to:
> 
>    binman: Build FIT image subentries with the section etype
> 
> and will be squashed into that.
> 
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
> 
>  tools/binman/etype/fit.py                  | 15 ++++++++
>  tools/binman/ftest.py                      |  8 +++++
>  tools/binman/test/168_fit_missing_blob.dts | 41 ++++++++++++++++++++++
>  3 files changed, 64 insertions(+)
>  create mode 100644 tools/binman/test/168_fit_missing_blob.dts
> 
> diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py
> index acba53aa92c..615b2fd8778 100644
> --- a/tools/binman/etype/fit.py
> +++ b/tools/binman/etype/fit.py
> @@ -169,3 +169,18 @@ class Entry_fit(Entry):
>          fdt.Sync(auto_resize=True)
>          data = fdt.GetContents()
>          return data
> +
> +    def CheckMissing(self, missing_list):
> +        """Check if any entries in this FIT have missing external blobs
> +
> +        If there are missing blobs, the entries are added to the list
> +
> +        Args:
> +            missing_list: List of Entry objects to be added to
> +        """
> +        for path, section in self._fit_sections.items():
> +            section.CheckMissing(missing_list)
> +
> +    def SetAllowMissing(self, allow_missing):
> +        for section in self._fit_sections.values():
> +            section.SetAllowMissing(allow_missing)

I saw GetAllowMissing() in section.py, doesn't this also need it?

> diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> index 762535b5a74..e672967dbaa 100644
> --- a/tools/binman/ftest.py
> +++ b/tools/binman/ftest.py
> @@ -3515,5 +3515,13 @@ class TestFunctional(unittest.TestCase):
>                      U_BOOT_DTB_DATA)
>          self.assertEqual(expected, data)
>  
> +    def testFitExtblobMissingOk(self):
> +        """Test a FIT with a missing external blob that is allowed"""
> +        with test_util.capture_sys_output() as (stdout, stderr):
> +            self._DoTestFile('168_fit_missing_blob.dts',
> +                             allow_missing=True)
> +        err = stderr.getvalue()
> +        self.assertRegex(err, "Image 'main-section'.*missing.*: blob-ext")
> +
>  if __name__ == "__main__":
>      unittest.main()
> diff --git a/tools/binman/test/168_fit_missing_blob.dts b/tools/binman/test/168_fit_missing_blob.dts
> new file mode 100644
> index 00000000000..e007aa41d8a
> --- /dev/null
> +++ b/tools/binman/test/168_fit_missing_blob.dts
> @@ -0,0 +1,41 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +/dts-v1/;
> +
> +/ {
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +
> +	binman {
> +		u-boot {
> +		};
> +		fit {
> +			description = "test-desc";
> +			#address-cells = <1>;
> +			fit,fdt-list = "of-list";
> +
> +			images {
> +				kernel {
> +					description = "ATF BL31";
> +					type = "kernel";
> +					arch = "ppc";
> +					os = "linux";
> +					compression = "gzip";
> +					load = <00000000>;
> +					entry = <00000000>;
> +					hash-1 {
> +						algo = "crc32";
> +					};
> +					hash-2 {
> +						algo = "sha1";
> +					};
> +					blob-ext {
> +						filename = "missing";
> +					};
> +				};
> +			};
> +		};
> +		u-boot-nodtb {
> +		};
> +	};
> +};

Anyway, feel free to add if you need to:

Reviewed-by: Alper Nebi Yasak <alpernebiyasak at gmail.com>

(or Acked-by, or neither)