RE: [bitbake-devel][PATCH] fetch2: fix downloadfilename issue with premirror

[Peter Kjellerstedt <peter.kjellerstedt@axis.com>]

> -----Original Message-----
> From: bitbake-devel@lists.openembedded.org <bitbake-
> devel@lists.openembedded.org> On Behalf Of Scott Weaver
> Sent: den 15 oktober 2021 15:08
> To: Richard Purdie <richard.purdie@linuxfoundation.org>
> Cc: Chen Qi <Qi.Chen@windriver.com>; bitbake-devel@lists.openembedded.org
> Subject: Re: [bitbake-devel][PATCH] fetch2: fix downloadfilename issue
> with premirror
> 
> On 21-10-15 12:35:24, Richard Purdie wrote:
> > On Thu, 2021-10-14 at 23:17 -0700, Chen Qi wrote:
> > > The following commit to fix [Yocto #13039] causes regression of
> > > the behavior of PREMIRRORS.
> > >
> > >   "bitbake: fetch2: fix premirror URI when downloadfilename defined"
> > >
> > > Take meta-openembedded/meta-networking/recipes-
> protocols/freediameter/freediameter_1.4.0.bb
> > > as an example.
> > > SRC_URI = "\
> > >
> http://www.freediameter.net/hg/${fd_pkgname}/archive/${PV}.tar.gz;download
> filename=${fd_pkgname}-${PV}.tar.gz \
> > >     ...
> > > "
> > > With the above commit, it now tries to fetch 1.4.0.tar.gz instead of
> > > freeDiameter-1.4.0.tar.gz. This makes
> https://downloads.yoctoproject.org/mirror/sources
> > > not work for freediameter, as it holds freeDiameter-1.4.0.tar.gz.
> > >
> > > The commit above tries to avoid fetching from invalid url such as:
> > > https://<some_mirror>/1.4.0.tar.gz/freeDiameter-1.4.0.tar.gz.
> > > And its solution is to make basename to be 1.4.0.tar.gz, thus causing
> the
> > > regression.
> > >
> > > This patch fixes the above regression. For Yocto #13039, it now tries
> > > to fetch from url: https://<some_mirror>/freeDiameter-1.4.0.tar.gz.
> > >
> > > Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
> > > ---
> > >  lib/bb/fetch2/__init__.py | 10 +++++++---
> > >  1 file changed, 7 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
> > > index 666cc1306..000b49a50 100644
> > > --- a/lib/bb/fetch2/__init__.py
> > > +++ b/lib/bb/fetch2/__init__.py
> > > @@ -466,9 +466,13 @@ def uri_replace(ud, uri_find, uri_replace,
> replacements, d, mirrortarball=None):
> > >                      # Kill parameters, they make no sense for mirror
> tarballs
> > >                      uri_decoded[5] = {}
> > >                  elif ud.localpath and
> ud.method.supports_checksum(ud):
> > > -                    basename = os.path.basename(uri_decoded[loc])
> > > -                if basename and not
> result_decoded[loc].endswith(basename):
> > > -                    result_decoded[loc] =
> os.path.join(result_decoded[loc], basename)
> > > +                    basename = os.path.basename(ud.localpath)
> > > +                if basename:
> > > +                    uri_basename = os.path.basename(uri_decoded[loc])
> > > +                    if basename != uri_basename and
> result_decoded[loc].endswith(uri_basename):
> > > +                        result_decoded[loc] =
> result_decoded[loc].replace(uri_basename, basename)
> > > +                    elif not result_decoded[loc].endswith(basename):
> > > +                        result_decoded[loc] =
> os.path.join(result_decoded[loc], basename)
> > >          else:
> > >              return None
> > >      result = encodeurl(result_decoded)
> >
> 
> 
> Hi Richard and Chen,
> 
> > I've been meaning to reply to this thread as the regression wasn't good.
> We need
> > to be able to use DL_DIR as a mirror.
> 
> It's my understanding that DL_DIR is always the last mirror attempted if
> downloading the artifact from a SRC_URI, MIRRORS and PREMIRRORS fails.
> In my testing, this still works.
> 
> What we're saying is that we want to be able to copy or use an exiting
> DL_DIR as a PREMIRROR.

This is basically what the (relatively new) mirror mode of the 
archiver.bbclass does.

> If this is the use case then, yes, this is a regression but I'm just a
> little confused because I always think of a mirror as just being an
> alternate site where the artifact defined in the SRC_URI can be found.
> 
> Once that artifact is found and downloadfilename is defined it should
> save the artifact using the name defined in downloadfilename.
> 
> My apologies for not understanding that downloadfilename also needs to
> be tried when attempting to download from a premirror. I was going by this
> definition:
> 
> https://www.yoctoproject.org/docs/latest/mega-manual/mega-manual.html#var-
> SRC_URI
> 
> --
>   - Scott

What's important to remember is one of the reasons for using downloadfilename 
is to support different files with the same name but from different sites 
being downloaded. This needs to work whether the file is fetched from the 
original source, a (PRE)MIRROR or the DL_DIR, without the files overwriting 
each other.

//Peter