From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date: Fri, 4 Sep 2020 12:33:48 +0300
Subject: [PATCH 2/5] binman: Show an error when a file is missing
In-Reply-To: <20200904012856.1175506-3-sjg@chromium.org>
References: <20200904012856.1175506-1-sjg@chromium.org>
 <20200904012856.1175506-3-sjg@chromium.org>
Message-ID: <20200904093348.GH1891694@smile.fi.intel.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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@linux.intel.com>

One note below.

> 
> Signed-off-by: Simon Glass <sjg@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