On Fri, 2025-02-14 at 06:48 -0700, Simon Glass wrote: > 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.
Thanks for taking the time to explain this to me, I got it now. I will take your suggestion and review the test cases to see if any should explicitly set fake-blobs . And with that will drop the default fake blobs per you request. Yannic > > Regards, > Simon
