[bitbake][PATCH] bitbake: fetch2: correct PREMIRROR URI

[bitbake][PATCH] bitbake: fetch2: correct PREMIRROR URI

[Scott Weaver]

When downloadfilename is defined in a recipe's SRC_URI and a PREMIRROR is also
defined for that URI, the downloadfilename is appended to the mirror URI and it
should not be.

[YOCTO #13039]

Signed-off-by: Scott Weaver <weaverjs@gmail.com>
---
 bitbake/lib/bb/fetch2/__init__.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bitbake/lib/bb/fetch2/__init__.py b/bitbake/lib/bb/fetch2/__init__.py
index 914fa5c024..0bcd566e83 100644
--- a/bitbake/lib/bb/fetch2/__init__.py
+++ b/bitbake/lib/bb/fetch2/__init__.py
@@ -467,7 +467,7 @@ def uri_replace(ud, uri_find, uri_replace, replacements, d, mirrortarball=None):
                     uri_decoded[5] = {}
                 elif ud.localpath and ud.method.supports_checksum(ud):
                     basename = os.path.basename(ud.localpath)
-                if basename and not result_decoded[loc].endswith(basename):
+                if basename and result_decoded[loc].endswith('/'):
                     result_decoded[loc] = os.path.join(result_decoded[loc], basename)
         else:
             return None
-- 
2.25.1

Re: [bitbake-devel] [bitbake][PATCH] bitbake: fetch2: correct PREMIRROR URI

[Richard Purdie]

On Sun, 2021-08-22 at 08:34 -0400, Scott Weaver wrote:
> When downloadfilename is defined in a recipe's SRC_URI and a PREMIRROR is also
> defined for that URI, the downloadfilename is appended to the mirror URI and it
> should not be.
> 
> [YOCTO #13039]
> 
> Signed-off-by: Scott Weaver <weaverjs@gmail.com>
> ---
>  bitbake/lib/bb/fetch2/__init__.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/bitbake/lib/bb/fetch2/__init__.py b/bitbake/lib/bb/fetch2/__init__.py
> index 914fa5c024..0bcd566e83 100644
> --- a/bitbake/lib/bb/fetch2/__init__.py
> +++ b/bitbake/lib/bb/fetch2/__init__.py
> @@ -467,7 +467,7 @@ def uri_replace(ud, uri_find, uri_replace, replacements, d, mirrortarball=None):
>                      uri_decoded[5] = {}
>                  elif ud.localpath and ud.method.supports_checksum(ud):
>                      basename = os.path.basename(ud.localpath)
> -                if basename and not result_decoded[loc].endswith(basename):
> +                if basename and result_decoded[loc].endswith('/'):
>                      result_decoded[loc] = os.path.join(result_decoded[loc], basename)
>          else:
>              return None

Thanks for the patch. Should we be adding a test for this to
lib/bb/tests/fetch.py (run with bitbake-selftest)?

Cheers

Richard

Re: [bitbake-devel] [bitbake][PATCH] bitbake: fetch2: correct PREMIRROR URI

[Scott Weaver]

On 21-08-22 13:40:06, Richard Purdie wrote:
> On Sun, 2021-08-22 at 08:34 -0400, Scott Weaver wrote:
> > When downloadfilename is defined in a recipe's SRC_URI and a PREMIRROR is also
> > defined for that URI, the downloadfilename is appended to the mirror URI and it
> > should not be.
> > 
> > [YOCTO #13039]
> > 
> > Signed-off-by: Scott Weaver <weaverjs@gmail.com>
> > ---
> >  bitbake/lib/bb/fetch2/__init__.py | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/bitbake/lib/bb/fetch2/__init__.py b/bitbake/lib/bb/fetch2/__init__.py
> > index 914fa5c024..0bcd566e83 100644
> > --- a/bitbake/lib/bb/fetch2/__init__.py
> > +++ b/bitbake/lib/bb/fetch2/__init__.py
> > @@ -467,7 +467,7 @@ def uri_replace(ud, uri_find, uri_replace, replacements, d, mirrortarball=None):
> >                      uri_decoded[5] = {}
> >                  elif ud.localpath and ud.method.supports_checksum(ud):
> >                      basename = os.path.basename(ud.localpath)
> > -                if basename and not result_decoded[loc].endswith(basename):
> > +                if basename and result_decoded[loc].endswith('/'):
> >                      result_decoded[loc] = os.path.join(result_decoded[loc], basename)
> >          else:
> >              return None
> 
> Thanks for the patch. Should we be adding a test for this to
> lib/bb/tests/fetch.py (run with bitbake-selftest)?

I'll look into that. I'm not familiar with the selftest but it's a good
opportunity to learn. :)

> 
> Cheers
> 
> Richard
> 

-- 
  - Scott

Re: [bitbake-devel] [bitbake][PATCH] bitbake: fetch2: correct PREMIRROR URI

[Scott Weaver]

On 21-08-22 09:03:39, Scott Weaver wrote:
> On 21-08-22 13:40:06, Richard Purdie wrote:
> > On Sun, 2021-08-22 at 08:34 -0400, Scott Weaver wrote:
> > > When downloadfilename is defined in a recipe's SRC_URI and a PREMIRROR is also
> > > defined for that URI, the downloadfilename is appended to the mirror URI and it
> > > should not be.
> > > 
> > > [YOCTO #13039]
> > > 
> > > Signed-off-by: Scott Weaver <weaverjs@gmail.com>
> > > ---
> > >  bitbake/lib/bb/fetch2/__init__.py | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/bitbake/lib/bb/fetch2/__init__.py b/bitbake/lib/bb/fetch2/__init__.py
> > > index 914fa5c024..0bcd566e83 100644
> > > --- a/bitbake/lib/bb/fetch2/__init__.py
> > > +++ b/bitbake/lib/bb/fetch2/__init__.py
> > > @@ -467,7 +467,7 @@ def uri_replace(ud, uri_find, uri_replace, replacements, d, mirrortarball=None):
> > >                      uri_decoded[5] = {}
> > >                  elif ud.localpath and ud.method.supports_checksum(ud):
> > >                      basename = os.path.basename(ud.localpath)
> > > -                if basename and not result_decoded[loc].endswith(basename):
> > > +                if basename and result_decoded[loc].endswith('/'):
> > >                      result_decoded[loc] = os.path.join(result_decoded[loc], basename)
> > >          else:
> > >              return None
> > 
> > Thanks for the patch. Should we be adding a test for this to
> > lib/bb/tests/fetch.py (run with bitbake-selftest)?
> 
> I'll look into that. I'm not familiar with the selftest but it's a good
> opportunity to learn. :)

Well, I've learned that this patch breaks the selftest.
I was wondering what the usecase was for the original line I changed but
it seems this code is sensitive to whether PREMIRRORS is defined with a
trailing '/' or not.
I'll dig deeper.

> 
> > 
> > Cheers
> > 
> > Richard
> > 
> 

-- 
  - Scott

Re: [bitbake-devel] [bitbake][PATCH] bitbake: fetch2: correct PREMIRROR URI

[Richard Purdie]

On Sun, 2021-08-22 at 11:22 -0400, Scott Weaver wrote:
> On 21-08-22 09:03:39, Scott Weaver wrote:
> > On 21-08-22 13:40:06, Richard Purdie wrote:
> > > On Sun, 2021-08-22 at 08:34 -0400, Scott Weaver wrote:
> > > > When downloadfilename is defined in a recipe's SRC_URI and a PREMIRROR is also
> > > > defined for that URI, the downloadfilename is appended to the mirror URI and it
> > > > should not be.
> > > > 
> > > > [YOCTO #13039]
> > > > 
> > > > Signed-off-by: Scott Weaver <weaverjs@gmail.com>
> > > > ---
> > > >  bitbake/lib/bb/fetch2/__init__.py | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/bitbake/lib/bb/fetch2/__init__.py b/bitbake/lib/bb/fetch2/__init__.py
> > > > index 914fa5c024..0bcd566e83 100644
> > > > --- a/bitbake/lib/bb/fetch2/__init__.py
> > > > +++ b/bitbake/lib/bb/fetch2/__init__.py
> > > > @@ -467,7 +467,7 @@ def uri_replace(ud, uri_find, uri_replace, replacements, d, mirrortarball=None):
> > > >                      uri_decoded[5] = {}
> > > >                  elif ud.localpath and ud.method.supports_checksum(ud):
> > > >                      basename = os.path.basename(ud.localpath)
> > > > -                if basename and not result_decoded[loc].endswith(basename):
> > > > +                if basename and result_decoded[loc].endswith('/'):
> > > >                      result_decoded[loc] = os.path.join(result_decoded[loc], basename)
> > > >          else:
> > > >              return None
> > > 
> > > Thanks for the patch. Should we be adding a test for this to
> > > lib/bb/tests/fetch.py (run with bitbake-selftest)?
> > 
> > I'll look into that. I'm not familiar with the selftest but it's a good
> > opportunity to learn. :)
> 
> Well, I've learned that this patch breaks the selftest.

I had wondered but hadn't got to testing that yet. Where we find corner cases we
do try and teach the test suite so we try and avoid regressions. We've had a lot
of issues with this kind of thing in the past.

> I was wondering what the usecase was for the original line I changed but
> it seems this code is sensitive to whether PREMIRRORS is defined with a
> trailing '/' or not.
> I'll dig deeper.

Thanks, I don't doubt there are issues in there and hopefully the testsuite will
show it's value and help figure through things!

Cheers,

Richard