Re: [bitbake-devel] [PATCH 1/2] fetch2: local files only in DL_DIR becomes fatal error

[Paulo Neves <ptsneves@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3181 bytes --]

On Sat, Jul 9, 2022, 08:52 Richard Purdie <
richard.purdie@linuxfoundation.org> wrote:

> On Fri, 2022-07-08 at 22:54 +0200, Paulo Neves wrote:
> > When trying to checksum local files, if a given file is not found
> > anywhere else than the DL_DIR then this means that the the build is
> > inconsistent, and unreproducible.
> >
> > This also means that if the DL_DIR is removed or not available the
> > build does not know how to fetch the file and will fail. With this
> > commit we fail earlier and consistently on this condition.
> >
> > Signed-off-by: Paulo Neves <ptsneves@gmail.com>
> > ---
> >  lib/bb/fetch2/__init__.py | 4 +++-
> >  lib/bb/tests/fetch.py     | 7 +++++++
> >  2 files changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
> > index ac557176..5f05278a 100644
> > --- a/lib/bb/fetch2/__init__.py
> > +++ b/lib/bb/fetch2/__init__.py
> > @@ -1233,7 +1233,9 @@ def get_checksum_file_list(d):
> >                  if f.startswith(dl_dir):
> >                      # The local fetcher's behaviour is to return a path
> under DL_DIR if it couldn't find the file anywhere else
> >                      if os.path.exists(f):
> > -                        bb.warn("Getting checksum for %s SRC_URI entry
> %s: file not found except in DL_DIR" % (d.getVar('PN'),
> os.path.basename(f)))
> > +                        bb.fatal(("Getting checksum for %s SRC_URI
> entry %s: file not found except in DL_DIR."
> > +                            " This means there is no way to get the
> file except for an orphaned copy"
> > +                            " in DL_DIR.") % (d.getVar('PN'),
> os.path.basename(f)))
> >                      else:
> >                          bb.warn("Unable to get checksum for %s SRC_URI
> entry %s: file could not be found" % (d.getVar('PN'), os.path.basename(f)))
> >                  filelist.append(f + ":" + str(os.path.exists(f)))
>
> Did you manage to trigger that error in a real world use case?
>
> I've just been looking at this code and it is horribly old and
> outdated. I can't help wonder if we shouldn't do something a bit more
> invasive to clean it up a bit more. I suspect it does some of these
> things for old/obsolete reasons...
>
> I ask since I'm wondering if anyone ever hits these code paths in a
> valid usecase.
>
> Thanks for working on it, we do need to improve some of these things.
>
> Cheers,
>
> Richard
>

Hey Richard,

I think these codepaths are hit on mostly on bugs(bad uri parsing) or bad
files(bad permissions).

The use case i added on the tests is also clearly bad, but I have seen
builds going on for very long in the state of warning, which becomes fatal
with the patch, because the dl_dir was stable and used by all users. So
expect this patch to increase the reports of latent issues people just
ignored.

Yesterday I also hit this warning organically exactly on the situation of
the test: a directory mentioned on the SRC_URI which did not exist in any
FILESPATHS but somehow existed on the dl_dir. So it is real.

I will investigate the whole default to dl_dir if not found and remove it
if I cannot find a good reason.

Paulo Neves

>

[-- Attachment #2: Type: text/html, Size: 4447 bytes --]