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.shevche...@linux.intel.com>
One note below.
>
> Signed-off-by: Simon Glass <s...@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
