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

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

[Chen Qi]

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;downloadfilename=${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)
-- 
2.33.0

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

[Richard Purdie]

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;downloadfilename=${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)

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.

Could we add another test case for this case so that this doesn't regress again
please?

Cheers,

Richard

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

[Scott Weaver]

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;downloadfilename=${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.

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

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

[Peter Kjellerstedt]

> -----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

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

[ChenQi]

On 10/15/2021 07:35 PM, 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;downloadfilename=${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)
> 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.
>
> Could we add another test case for this case so that this doesn't regress again
> please?

Yes. I'll do it.

Regards,
Qi

>
> Cheers,
>
> Richard
>
>
>
>