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