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