[PATCH 2/5] binman: Show an error when a file is missing

[Andy Shevchenko]

On Thu, Sep 03, 2020 at 07:28:53PM -0600, Simon Glass wrote:
> The recent support for missing external binaries does not show an error
> message when a file is genuinely missing (i.e. it is missing but not
> marked as 'external'). This means that when -m is passed to binman, it
> will never report a missing file.
> 
> Fix this and add a test.

Acked-by: Andy Shevchenko <andriy.shevchenko at linux.intel.com>

One note below.

> 
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
> 
>  tools/binman/etype/blob.py             |  7 ++++---
>  tools/binman/ftest.py                  |  7 +++++++
>  tools/binman/test/173_missing_blob.dts | 14 ++++++++++++++
>  3 files changed, 25 insertions(+), 3 deletions(-)
>  create mode 100644 tools/binman/test/173_missing_blob.dts
> 
> diff --git a/tools/binman/etype/blob.py b/tools/binman/etype/blob.py
> index c5f97c85a38..66cf243976e 100644
> --- a/tools/binman/etype/blob.py
> +++ b/tools/binman/etype/blob.py
> @@ -37,13 +37,14 @@ class Entry_blob(Entry):
>  
>      def ObtainContents(self):
>          self._filename = self.GetDefaultFilename()
> -        self._pathname = tools.GetInputFilename(self._filename,
> -                                                self.section.GetAllowMissing())
> +        self._pathname = tools.GetInputFilename(
> +            self._filename, self.external and self.section.GetAllowMissing())

I hope you know that 'and' has a bit different semantics in Python than in C for example.
And why to move first parameter to new line?

>          # Allow the file to be missing
> -        if self.external and not self._pathname:
> +        if not self._pathname:
>              self.SetContents(b'')
>              self.missing = True
>              return True
> +
>          self.ReadBlobContents()
>          return True
>  
> diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> index 95b17d0b749..91225459162 100644
> --- a/tools/binman/ftest.py
> +++ b/tools/binman/ftest.py
> @@ -3708,5 +3708,12 @@ class TestFunctional(unittest.TestCase):
>          self.assertIn('Wibble test', err)
>          self.assertIn('Another test', err)
>  
> +    def testMissingBlob(self):
> +        """Test handling of a blob containing a missing file"""
> +        with self.assertRaises(ValueError) as e:
> +            self._DoTestFile('173_missing_blob.dts', allow_missing=True)
> +        self.assertIn("Filename 'missing' not found in input path",
> +                      str(e.exception))
> +
>  if __name__ == "__main__":
>      unittest.main()
> diff --git a/tools/binman/test/173_missing_blob.dts b/tools/binman/test/173_missing_blob.dts
> new file mode 100644
> index 00000000000..ffb655a1cb4
> --- /dev/null
> +++ b/tools/binman/test/173_missing_blob.dts
> @@ -0,0 +1,14 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +/dts-v1/;
> +
> +/ {
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +
> +	binman {
> +		blob {
> +			filename = "missing";
> +		};
> +	};
> +};
> -- 
> 2.28.0.526.ge36021eeef-goog
> 

-- 
With Best Regards,
Andy Shevchenko