diff mbox series

[1/1] fetch2: try_mirrors(): Skip invalid local url

Message ID 87f55606dfd70b12b5d7fe97ad889300a6884359.1754898959.git.liezhi.yang@windriver.com
State New
Headers show
Series [1/1] fetch2: try_mirrors(): Skip invalid local url | expand

Commit Message

Robert Yang Aug. 11, 2025, 7:58 a.m. UTC
From: Robert Yang <liezhi.yang@windriver.com>

Skip fetching from an invalid local url since it would be failed.

Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
---
 bitbake/lib/bb/fetch2/__init__.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Alexander Kanavin Aug. 11, 2025, 8:04 a.m. UTC | #1
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
Robert Yang Aug. 11, 2025, 8:19 a.m. UTC | #2
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
Alexander Kanavin Aug. 11, 2025, 8:26 a.m. UTC | #3
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
Robert Yang Aug. 11, 2025, 8:33 a.m. UTC | #4
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
Alexander Kanavin Aug. 11, 2025, 8:37 a.m. UTC | #5
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
Robert Yang Aug. 11, 2025, 9:04 a.m. UTC | #6
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 mbox series

Patch

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