Message ID | 87f55606dfd70b12b5d7fe97ad889300a6884359.1754898959.git.liezhi.yang@windriver.com |
---|---|
State | New |
Headers | show |
Series | [1/1] fetch2: try_mirrors(): Skip invalid local url | expand |
On Mon, 11 Aug 2025 at 09:58, Robert Yang via lists.openembedded.org <liezhi.yang=windriver.com@lists.openembedded.org> wrote: > Skip fetching from an invalid local url since it would be failed. > - ret = try_mirror_url(fetch, origud, uds[index], ld, check) > + ud = uds[index] > + # Skip fetching it when the local url's path doesn't exist > + if ud.parm.get('protocol', '') == 'file' and not os.path.exists(ud.path): > + continue > + ret = try_mirror_url(fetch, origud, ud, ld, check) Can you please explain the use case? I'd think failing is the correct action, so that the invalid url can be removed from configuration. Alex
Hi Alex, On 8/11/25 16:04, Alexander Kanavin wrote: > On Mon, 11 Aug 2025 at 09:58, Robert Yang via lists.openembedded.org > <liezhi.yang=windriver.com@lists.openembedded.org> wrote: >> Skip fetching from an invalid local url since it would be failed. >> - ret = try_mirror_url(fetch, origud, uds[index], ld, check) >> + ud = uds[index] >> + # Skip fetching it when the local url's path doesn't exist >> + if ud.parm.get('protocol', '') == 'file' and not os.path.exists(ud.path): >> + continue >> + ret = try_mirror_url(fetch, origud, ud, ld, check) > > Can you please explain the use case? I'd think failing is the correct > action, so that the invalid url can be removed from configuration. There can be multiple PREMIRRORs each PREMIRROR conatains specifics sources for each layer, each recipe will try the PREMIRRORs one by one until succeed, but the trying would be failed if the PREMIRROR doesn't contain the required sources, so return it immediately to make log.do_fetch clean. // Robert > > Alex
On Mon, 11 Aug 2025 at 10:19, Robert Yang <liezhi.yang@windriver.com> wrote: > > Can you please explain the use case? I'd think failing is the correct > > action, so that the invalid url can be removed from configuration. > > There can be multiple PREMIRRORs each PREMIRROR conatains specifics sources > for each layer, each recipe will try the PREMIRRORs one > by one until succeed, but the trying would be failed if the PREMIRROR > doesn't contain the required sources, so return it immediately to make > log.do_fetch clean. But why is it specific to file protocol then? If the item doesn't exist on some remote server, there would be the same issue? Alex
On 8/11/25 16:26, Alexander Kanavin wrote: > On Mon, 11 Aug 2025 at 10:19, Robert Yang <liezhi.yang@windriver.com> wrote: >>> Can you please explain the use case? I'd think failing is the correct >>> action, so that the invalid url can be removed from configuration. >> >> There can be multiple PREMIRRORs each PREMIRROR conatains specifics sources >> for each layer, each recipe will try the PREMIRRORs one >> by one until succeed, but the trying would be failed if the PREMIRROR >> doesn't contain the required sources, so return it immediately to make >> log.do_fetch clean. > > But why is it specific to file protocol then? If the item doesn't > exist on some remote server, there would be the same issue? Yes, they same issues to other protocols, but there isn't an easy way to check that for other protocols, so we have to let the code go on for other protocols. // Robert > > Alex
On Mon, 11 Aug 2025 at 10:33, Robert Yang <liezhi.yang@windriver.com> wrote: > > But why is it specific to file protocol then? If the item doesn't > > exist on some remote server, there would be the same issue? > > Yes, they same issues to other protocols, but there isn't an easy way to check > that for other protocols, so we have to let the code go on for other protocols. So the goal of the patch is remove clutter from log.do_fetch, and nothing else? This should be spelled out in the commit message, with examples of logs before the change and after. I also wonder if this should be inside try_mirror_url (or further down), and not at the top level function. Otherwise other users of the function will have the same issue. Alex
On 8/11/25 16:37, Alexander Kanavin wrote: > On Mon, 11 Aug 2025 at 10:33, Robert Yang <liezhi.yang@windriver.com> wrote: >>> But why is it specific to file protocol then? If the item doesn't >>> exist on some remote server, there would be the same issue? >> >> Yes, they same issues to other protocols, but there isn't an easy way to check >> that for other protocols, so we have to let the code go on for other protocols. > > So the goal of the patch is remove clutter from log.do_fetch, and > nothing else? This should be spelled out in the commit message, with > examples of logs before the change and after. It also fixed a warning when BB_GIT_SHALLOW and is enabled and failed to fetch the source from the PREMIRROR: Fast shallow clone failed, try to skip fast mode now. This is because: def clone_shallow_local(self, ud, dest, d): """ Shallow fetch from ud.clonedir (${DL_DIR}/git2/<gitrepo> by default): - For BB_GIT_SHALLOW_DEPTH: git fetch --depth <depth> rev - For BB_GIT_SHALLOW_REVS: git fetch --shallow-exclude=<revs> rev """ progresshandler = GitProgressHandler(d) repourl = self._get_repo_url(ud) bb.utils.mkdirhier(dest) init_cmd = "%s init -q" % ud.basecmd if ud.bareclone: init_cmd += " --bare" runfetchcmd(init_cmd, d, workdir=dest) # Use repourl when creating a fast initial shallow clone # Prefer already existing full bare clones if available if not ud.shallow_skip_fast and not os.path.exists(ud.clonedir): remote = shlex.quote(repourl) The repourl is an invalid local url from PREMIRRORS which caused the warnings. > > I also wonder if this should be inside try_mirror_url (or further > down), and not at the top level function. Otherwise other users of the > function will have the same issue. Yes, that makes sense, I will send a V2 for it. // Robert > > Alex
diff --git a/bitbake/lib/bb/fetch2/__init__.py b/bitbake/lib/bb/fetch2/__init__.py index 0ad987c596..e4c9cecd86 100644 --- a/bitbake/lib/bb/fetch2/__init__.py +++ b/bitbake/lib/bb/fetch2/__init__.py @@ -1168,7 +1168,11 @@ def try_mirrors(fetch, d, origud, mirrors, check = False): uris, uds = build_mirroruris(origud, mirrors, ld) for index, uri in enumerate(uris): - ret = try_mirror_url(fetch, origud, uds[index], ld, check) + ud = uds[index] + # Skip fetching it when the local url's path doesn't exist + if ud.parm.get('protocol', '') == 'file' and not os.path.exists(ud.path): + continue + ret = try_mirror_url(fetch, origud, ud, ld, check) if ret: return ret return None