[PATCH] lib/bb/fetch2: Fix npm tarball fetch

[PATCH] lib/bb/fetch2: Fix npm tarball fetch

[Davis, Michael]

Tarballs generated by enabling mirror tarballs could not be found by the
fetch for the npm.  This fixes setting the path variable in decode url
so they can be matched.

Signed-off-by: Michael Davis <michael.davis@essvote.com>
---
 lib/bb/fetch2/__init__.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
index 136fc29..117c890 100644
--- a/lib/bb/fetch2/__init__.py
+++ b/lib/bb/fetch2/__init__.py
@@ -393,6 +393,8 @@ def decodeurl(url):
                     raise MalformedUrl(url, "The URL: '%s' is invalid: parameter %s does not specify a value (missing '=')" % (url, s))
                 s1, s2 = s.split('=')
                 p[s1] = s2
+                if type == 'npm' and s1 == 'name':
+                    path = '/' + s2
 
     return type, host, urllib.parse.unquote(path), user, pswd, p
 
-- 
2.9.3

Re: [PATCH] lib/bb/fetch2: Fix npm tarball fetch

[Richard Purdie]

On Fri, 2017-05-12 at 20:52 +0000, Davis, Michael wrote:
> Tarballs generated by enabling mirror tarballs could not be found by
> the
> fetch for the npm.  This fixes setting the path variable in decode
> url
> so they can be matched.

Would you be able to provide an example of a url which is incorrect and
what the correct version would be?

I get worried about fetcher specific url handling and we probably could
do with adding a test case for this too but its hard to understand
without seeing an example failure.

Cheers,

Richard

> Signed-off-by: Michael Davis <michael.davis@essvote.com>
> ---
>  lib/bb/fetch2/__init__.py | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
> index 136fc29..117c890 100644
> --- a/lib/bb/fetch2/__init__.py
> +++ b/lib/bb/fetch2/__init__.py
> @@ -393,6 +393,8 @@ def decodeurl(url):
>                      raise MalformedUrl(url, "The URL: '%s' is
> invalid: parameter %s does not specify a value (missing '=')" % (url,
> s))
>                  s1, s2 = s.split('=')
>                  p[s1] = s2
> +                if type == 'npm' and s1 == 'name':
> +                    path = '/' + s2
>  
>      return type, host, urllib.parse.unquote(path), user, pswd, p
>  
> -- 
> 2.9.3

Re: [PATCH] lib/bb/fetch2: Fix npm tarball fetch

[Davis, Michael]

I am not a 100% sure this is the best implementation.  It is my first work in python or bitbake, so I just took my best stab at it.
Basically the issue is recreated as such:
	1.  Generate npm tarballs using BB_GENERATE_MIRROR_TARBALLS = "1"
	2.  Set SOURCE_MIRROR_URL to some local dir containing the tarballs.
		a.  example: SOURCE_MIRROR_URL ?= "file:///build/downloads/"
	3.  The fetcher is not able to find the tarballs and instead attempts to download them again.
		a. This is due to the fact that for npms the "path" is not set and instead the package name is in the params.

I am open to any better ways to do it.  I pretty much compared what was happening with git and what was happening with npm.


-----Original Message-----
From: Richard Purdie [mailto:richard.purdie@linuxfoundation.org] 
Sent: Monday, May 15, 2017 10:23 AM
To: Davis, Michael; bitbake-devel@lists.openembedded.org
Subject: Re: [bitbake-devel] [PATCH] lib/bb/fetch2: Fix npm tarball fetch

On Fri, 2017-05-12 at 20:52 +0000, Davis, Michael wrote:
> Tarballs generated by enabling mirror tarballs could not be found by
> the
> fetch for the npm.  This fixes setting the path variable in decode
> url
> so they can be matched.

Would you be able to provide an example of a url which is incorrect and
what the correct version would be?

I get worried about fetcher specific url handling and we probably could
do with adding a test case for this too but its hard to understand
without seeing an example failure.

Cheers,

Richard

> Signed-off-by: Michael Davis <michael.davis@essvote.com>
> ---
>  lib/bb/fetch2/__init__.py | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
> index 136fc29..117c890 100644
> --- a/lib/bb/fetch2/__init__.py
> +++ b/lib/bb/fetch2/__init__.py
> @@ -393,6 +393,8 @@ def decodeurl(url):
>                      raise MalformedUrl(url, "The URL: '%s' is
> invalid: parameter %s does not specify a value (missing '=')" % (url,
> s))
>                  s1, s2 = s.split('=')
>                  p[s1] = s2
> +                if type == 'npm' and s1 == 'name':
> +                    path = '/' + s2
>  
>      return type, host, urllib.parse.unquote(path), user, pswd, p
>  
> -- 
> 2.9.3

Re: [PATCH] lib/bb/fetch2: Fix npm tarball fetch

[Davis, Michael]

I will also mention I am blocking network access with BB_NO_NETWORK = "1".
Disconnecting the network creates the same issue where the package can't be downloaded and errors out.


-----Original Message-----
From: Richard Purdie [mailto:richard.purdie@linuxfoundation.org] 
Sent: Monday, May 15, 2017 10:23 AM
To: Davis, Michael; bitbake-devel@lists.openembedded.org
Subject: Re: [bitbake-devel] [PATCH] lib/bb/fetch2: Fix npm tarball fetch

On Fri, 2017-05-12 at 20:52 +0000, Davis, Michael wrote:
> Tarballs generated by enabling mirror tarballs could not be found by
> the
> fetch for the npm.  This fixes setting the path variable in decode
> url
> so they can be matched.

Would you be able to provide an example of a url which is incorrect and
what the correct version would be?

I get worried about fetcher specific url handling and we probably could
do with adding a test case for this too but its hard to understand
without seeing an example failure.

Cheers,

Richard

> Signed-off-by: Michael Davis <michael.davis@essvote.com>
> ---
>  lib/bb/fetch2/__init__.py | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
> index 136fc29..117c890 100644
> --- a/lib/bb/fetch2/__init__.py
> +++ b/lib/bb/fetch2/__init__.py
> @@ -393,6 +393,8 @@ def decodeurl(url):
>                      raise MalformedUrl(url, "The URL: '%s' is
> invalid: parameter %s does not specify a value (missing '=')" % (url,
> s))
>                  s1, s2 = s.split('=')
>                  p[s1] = s2
> +                if type == 'npm' and s1 == 'name':
> +                    path = '/' + s2
>  
>      return type, host, urllib.parse.unquote(path), user, pswd, p
>  
> -- 
> 2.9.3

Re: [PATCH] lib/bb/fetch2: Fix npm tarball fetch

[Davis, Michael]

Do you have any recommendations or tips to better implement this?
I would really like to get offline npm downloads working properly.

Thanks,
Mike


-----Original Message-----
From: Richard Purdie [mailto:richard.purdie@linuxfoundation.org] 
Sent: Monday, May 15, 2017 10:23 AM
To: Davis, Michael; bitbake-devel@lists.openembedded.org
Subject: Re: [bitbake-devel] [PATCH] lib/bb/fetch2: Fix npm tarball fetch

On Fri, 2017-05-12 at 20:52 +0000, Davis, Michael wrote:
> Tarballs generated by enabling mirror tarballs could not be found by
> the
> fetch for the npm.  This fixes setting the path variable in decode
> url
> so they can be matched.

Would you be able to provide an example of a url which is incorrect and
what the correct version would be?

I get worried about fetcher specific url handling and we probably could
do with adding a test case for this too but its hard to understand
without seeing an example failure.

Cheers,

Richard

> Signed-off-by: Michael Davis <michael.davis@essvote.com>
> ---
>  lib/bb/fetch2/__init__.py | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
> index 136fc29..117c890 100644
> --- a/lib/bb/fetch2/__init__.py
> +++ b/lib/bb/fetch2/__init__.py
> @@ -393,6 +393,8 @@ def decodeurl(url):
>                      raise MalformedUrl(url, "The URL: '%s' is
> invalid: parameter %s does not specify a value (missing '=')" % (url,
> s))
>                  s1, s2 = s.split('=')
>                  p[s1] = s2
> +                if type == 'npm' and s1 == 'name':
> +                    path = '/' + s2
>  
>      return type, host, urllib.parse.unquote(path), user, pswd, p
>  
> -- 
> 2.9.3

Re: [PATCH] lib/bb/fetch2: Fix npm tarball fetch

[Davis, Michael]

I created a patch that is a little less ugly, but it is still fetcher specific url handling.
The basic issue is that the path is set to blank for an npm link.
When this compare "elif (re.match(regexp, uri_decoded[loc])):" happens it is comparing "/*" to "".
Since they don't match it ends up skipping the check for a mirror tarball.

The elif line could test if uri_decoded[loc] is empty or matches.
It could also strip off the leading "/" from path and compare "*" to "" which I assume would pass.
My main concern is either of those might break something else unrelated.
I am trying to find the smallest change that won't run the risk of breaking everything else.

Still open to more suggestions on a better solution, but if you like this one I can send a full email diff.

Thanks,
Mike



diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
index 79a8906..0975408 100644
--- a/lib/bb/fetch2/__init__.py
+++ b/lib/bb/fetch2/__init__.py
@@ -373,6 +373,9 @@ def decodeurl(url):
     elif type.lower() == 'file':
         host = ""
         path = location
+    elif type.lower() == 'npm':
+        host = location
+        path = "/*"
     else:
         host = location
         path = ""

-----Original Message-----
From: Richard Purdie [mailto:richard.purdie@linuxfoundation.org] 
Sent: Monday, May 15, 2017 10:23 AM
To: Davis, Michael; bitbake-devel@lists.openembedded.org
Subject: Re: [bitbake-devel] [PATCH] lib/bb/fetch2: Fix npm tarball fetch

On Fri, 2017-05-12 at 20:52 +0000, Davis, Michael wrote:
> Tarballs generated by enabling mirror tarballs could not be found by
> the
> fetch for the npm.  This fixes setting the path variable in decode
> url
> so they can be matched.

Would you be able to provide an example of a url which is incorrect and
what the correct version would be?

I get worried about fetcher specific url handling and we probably could
do with adding a test case for this too but its hard to understand
without seeing an example failure.

Cheers,

Richard

> Signed-off-by: Michael Davis <michael.davis@essvote.com>
> ---
>  lib/bb/fetch2/__init__.py | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
> index 136fc29..117c890 100644
> --- a/lib/bb/fetch2/__init__.py
> +++ b/lib/bb/fetch2/__init__.py
> @@ -393,6 +393,8 @@ def decodeurl(url):
>                      raise MalformedUrl(url, "The URL: '%s' is
> invalid: parameter %s does not specify a value (missing '=')" % (url,
> s))
>                  s1, s2 = s.split('=')
>                  p[s1] = s2
> +                if type == 'npm' and s1 == 'name':
> +                    path = '/' + s2
>  
>      return type, host, urllib.parse.unquote(path), user, pswd, p
>  
> -- 
> 2.9.3