Hi Yannic,

On Fri, 14 Feb 2025 at 00:18, Yannic Moog <y.m...@phytec.de> wrote:
>
> Hi Simon,
>
> On Thu, 2025-02-13 at 07:01 -0700, Simon Glass wrote:
> > Hi Yannic,
> >
> > On Thu, 13 Feb 2025 at 00:15, Yannic Moog <y.m...@phytec.de> wrote:
> > >
> > > Hi Simon,
> > >
> > > On Mon, 2025-02-10 at 06:06 -0700, Simon Glass wrote:
> > > > Hi Yannic,
> > > >
> > > > On Wed, 29 Jan 2025 at 03:30, Yannic Moog <y.m...@phytec.de> wrote:
> > > > >
> > > > > The top level Makefile calls binman with fake-ext-blobs.
> > > > > The test setup should reflect this to spot potential bugs before
> > > > > reaching users.
> > > > >
> > > > > Signed-off-by: Yannic Moog <y.m...@phytec.de>
> > > > > ---
> > > > >  tools/binman/ftest.py | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> > > > > index a553ca9e564..1f194f9ecae 100644
> > > > > --- a/tools/binman/ftest.py
> > > > > +++ b/tools/binman/ftest.py
> > > > > @@ -372,7 +372,7 @@ class TestFunctional(unittest.TestCase):
> > > > >      def _DoTestFile(self, fname, debug=False, map=False, 
> > > > > update_dtb=False,
> > > > >                      entry_args=None, images=None, use_real_dtb=False,
> > > > >                      use_expanded=False, verbosity=None, 
> > > > > allow_missing=False,
> > > > > -                    allow_fake_blobs=False, extra_indirs=None, 
> > > > > threads=None,
> > > > > +                    allow_fake_blobs=True, extra_indirs=None, 
> > > > > threads=None,
> > > > >                      test_section_timeout=False, 
> > > > > update_fdt_in_elf=None,
> > > > >                      force_missing_bintools='', ignore_missing=False, 
> > > > > output_dir=None):
> > > > >          """Run binman with a given test file
> > > > >
> > > > > --
> > > > > 2.43.0
> > > > >
> > > >
> > > > I would like this default to stay the same (False) because that is the
> > > > normal case for Binman. We should expand the test-coverage as needed.
> > >
> > > Can you help me understand why we fake blobs in the call to binman when 
> > > building U-Boot?
> > > You say that not faking is the normal case so why are we doing something 
> > > abnormal in the top-
> > > level
> > > Makefile?
> >
> > I mean that not faking is the normal case for Binman's code base. Yes,
> > U-Boot requests faking.
> >
> > >
> > > I think that this inconsistency is a potential for bugs going unnoticed 
> > > so whatever we decide to
> > > do
> > > I would very much like to be consistent with testing and "normal" builds.
> >
> > Binman is tested by its own tests which handle both cases, so we don't
> > have a test gap there.
>
> Sorry, I am still stumped here. I feel like my question as to why U-Boot 
> requests faking has not
> been answered above. Can you please explain that again?

U-Boot needs to use fakes largely because in some cases the fake is
passed to mkimage, which sometimes cannot operate at all if there is a
missing or zero-sized file. So we create a fake just so that the build
will succeed (with warnings).

>
> And I am left a bit confused since I thought these RFC patches show that 
> there indeed was a test gap
> (in a single test case testFitFirmwareLoadables only, but still) due to not 
> faking external blobs.
> Is it not possible that there are other potential problems in regards to 
> faking blobs that we have
> not discovered, yet?

Yes, please fix the test gap and if there are other tests we need to
add, we should add them. The goal of Binman is 100% test coverage. But
faked blobs are more of an unfortunate workaround, not the primary
goal of Binman. So I would like tests to explicitly select faked blobs
when needed, rather than having this on everywhere. It should only
affect a very small amount of tests which deal with the pain of
external blobs.

Regards,
Simon

Reply via email to