All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 00/12] NPM refactoring
@ 2020-01-17 16:45 Jean-Marie LEMETAYER
  2020-01-17 16:45 ` [PATCH v4 01/12] bitbake: utils: add sha384_file and sha512_file functions Jean-Marie LEMETAYER
                   ` (11 more replies)
  0 siblings, 12 replies; 14+ messages in thread
From: Jean-Marie LEMETAYER @ 2020-01-17 16:45 UTC (permalink / raw)
  To: bitbake-devel; +Cc: rennes-dev, paul.eggleton

Hi folks and happy new year,

These patches are part of a set which are mainly in OE-core.
More infos can be found on the openembedded-core list.

For readability here is a link if you want the history of this patchset:
http://lists.openembedded.org/pipermail/openembedded-core/2019-December/290298.html

--- V4

These patches can be found here:
https://github.com/savoirfairelinux/bitbake/tree/npm-refactoring-v4

 - As suggested by Richard, a new npmsw fetcher have been implemented to handle
   the npm dependencies. Now the fetch of the main package is separated from the
   fetch of its dependencies.

 - Add the support of "proxy" fetchers. These fetchers (npm and npmsw) just
   resolves an url and forwards it to a "raw" fetcher (wget, git, ...).
   For example an npm url like:
     npm://registry...;package=...;version=...
   is converted as:
     https://registry.../...tgz;downloadfilename=npm2/...tgz;sha256sum=...

 - Add more hash functions for checksum verification to be able to support the
   subresource integrity [1] returned by the npm registry. The supported hash
   functions are now: md5, sha1, sha256, sha384, sah512

 - More tests have been added for the npm and the npmsw fetchers including tests
   for the proxy forwarding thing (checksum, mirrors).

1: https://www.w3.org/TR/SRI/

Jean-Marie LEMETAYER (12):
  bitbake: utils: add sha384_file and sha512_file functions
  bitbake: utils: add is_semver function
  bitbake: fetch2: refactor checksum verification
  bitbake: fetch2: add more hash functions for checksum verification
  bitbake: fetch2: allow fetchers to forward the donestamp management
  bitbake: fetch2: allow fetchers to forward the mirrors management
  bitbake: fetch2: allow fetchers to forward the done condition
  bitbake: fetch2/wget: fix downloadfilename parameter
  bitbake: fetch2/npm: refactor the npm fetcher
  bitbake: tests/fetch: add npm tests
  bitbake: fetch2: add the npmsw fetcher
  bitbake: tests/fetch: add npmsw tests

 lib/bb/fetch2/__init__.py | 215 +++++++++------
 lib/bb/fetch2/npm.py      | 538 +++++++++++++++++++-------------------
 lib/bb/fetch2/npmsw.py    | 255 ++++++++++++++++++
 lib/bb/fetch2/wget.py     |   6 +-
 lib/bb/tests/fetch.py     | 434 ++++++++++++++++++++++++++++++
 lib/bb/utils.py           |  40 +++
 6 files changed, 1127 insertions(+), 361 deletions(-)
 create mode 100644 lib/bb/fetch2/npmsw.py

--
2.20.1



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v4 01/12] bitbake: utils: add sha384_file and sha512_file functions
  2020-01-17 16:45 [PATCH v4 00/12] NPM refactoring Jean-Marie LEMETAYER
@ 2020-01-17 16:45 ` Jean-Marie LEMETAYER
  2020-01-17 16:45 ` [PATCH v4 02/12] bitbake: utils: add is_semver function Jean-Marie LEMETAYER
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Jean-Marie LEMETAYER @ 2020-01-17 16:45 UTC (permalink / raw)
  To: bitbake-devel; +Cc: rennes-dev, paul.eggleton

The npm fetcher needs these functions to support the subresource
integrity: https://www.w3.org/TR/SRI/

Signed-off-by: Jean-Marie LEMETAYER <jean-marie.lemetayer@savoirfairelinux.com>
---
 lib/bb/utils.py | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/lib/bb/utils.py b/lib/bb/utils.py
index d65265c4..e5d19acf 100644
--- a/lib/bb/utils.py
+++ b/lib/bb/utils.py
@@ -557,6 +557,20 @@ def sha1_file(filename):
     import hashlib
     return _hasher(hashlib.sha1(), filename)
 
+def sha384_file(filename):
+    """
+    Return the hex string representation of the SHA384 checksum of the filename
+    """
+    import hashlib
+    return _hasher(hashlib.sha384(), filename)
+
+def sha512_file(filename):
+    """
+    Return the hex string representation of the SHA512 checksum of the filename
+    """
+    import hashlib
+    return _hasher(hashlib.sha512(), filename)
+
 def preserved_envvars_exported():
     """Variables which are taken from the environment and placed in and exported
     from the metadata"""
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 02/12] bitbake: utils: add is_semver function
  2020-01-17 16:45 [PATCH v4 00/12] NPM refactoring Jean-Marie LEMETAYER
  2020-01-17 16:45 ` [PATCH v4 01/12] bitbake: utils: add sha384_file and sha512_file functions Jean-Marie LEMETAYER
@ 2020-01-17 16:45 ` Jean-Marie LEMETAYER
  2020-01-17 16:45 ` [PATCH v4 03/12] bitbake: fetch2: refactor checksum verification Jean-Marie LEMETAYER
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Jean-Marie LEMETAYER @ 2020-01-17 16:45 UTC (permalink / raw)
  To: bitbake-devel; +Cc: rennes-dev, paul.eggleton

This function checks if a string is a semantic version:
    https://semver.org/spec/v2.0.0.html

The npm fetcher needs this function to validate its version parameter.

Signed-off-by: Jean-Marie LEMETAYER <jean-marie.lemetayer@savoirfairelinux.com>
---
 lib/bb/utils.py | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/lib/bb/utils.py b/lib/bb/utils.py
index e5d19acf..7571d8d9 100644
--- a/lib/bb/utils.py
+++ b/lib/bb/utils.py
@@ -1575,3 +1575,29 @@ class LogCatcher(logging.Handler):
         self.messages.append(bb.build.logformatter.format(record))
     def contains(self, message):
         return (message in self.messages)
+
+def is_semver(version):
+    """
+        Is the version string following the semver semantic?
+
+        https://semver.org/spec/v2.0.0.html
+    """
+    regex = re.compile(
+    r"""
+    ^
+    (0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)
+    (?:-(
+        (?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)
+        (?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*
+    ))?
+    (?:\+(
+        [0-9a-zA-Z-]+
+        (?:\.[0-9a-zA-Z-]+)*
+    ))?
+    $
+    """, re.VERBOSE)
+
+    if regex.match(version) is None:
+        return False
+
+    return True
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 03/12] bitbake: fetch2: refactor checksum verification
  2020-01-17 16:45 [PATCH v4 00/12] NPM refactoring Jean-Marie LEMETAYER
  2020-01-17 16:45 ` [PATCH v4 01/12] bitbake: utils: add sha384_file and sha512_file functions Jean-Marie LEMETAYER
  2020-01-17 16:45 ` [PATCH v4 02/12] bitbake: utils: add is_semver function Jean-Marie LEMETAYER
@ 2020-01-17 16:45 ` Jean-Marie LEMETAYER
  2020-01-17 16:45 ` [PATCH v4 04/12] bitbake: fetch2: add more hash functions for " Jean-Marie LEMETAYER
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Jean-Marie LEMETAYER @ 2020-01-17 16:45 UTC (permalink / raw)
  To: bitbake-devel; +Cc: rennes-dev, paul.eggleton

This commit refactors the way checksums are verified to be more generic.

The support of new hash functions is now limited to the update of the
CHECKSUM_LIST variable.

Signed-off-by: Jean-Marie LEMETAYER <jean-marie.lemetayer@savoirfairelinux.com>
---
 lib/bb/fetch2/__init__.py | 157 +++++++++++++++++++++-----------------
 1 file changed, 85 insertions(+), 72 deletions(-)

diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
index 731c1608..b4122aee 100644
--- a/lib/bb/fetch2/__init__.py
+++ b/lib/bb/fetch2/__init__.py
@@ -33,6 +33,8 @@ _checksum_cache = bb.checksum.FileChecksumCache()
 
 logger = logging.getLogger("BitBake.Fetcher")
 
+CHECKSUM_LIST = [ "md5", "sha256" ]
+
 class BBFetchException(Exception):
     """Class all fetch exceptions inherit from"""
     def __init__(self, message):
@@ -131,10 +133,9 @@ class NonLocalMethod(Exception):
         Exception.__init__(self)
 
 class MissingChecksumEvent(bb.event.Event):
-    def __init__(self, url, md5sum, sha256sum):
+    def __init__(self, url, **checksums):
         self.url = url
-        self.checksums = {'md5sum': md5sum,
-                          'sha256sum': sha256sum}
+        self.checksums = checksums
         bb.event.Event.__init__(self)
 
 
@@ -552,71 +553,81 @@ def verify_checksum(ud, d, precomputed={}):
     downloading. See https://bugzilla.yoctoproject.org/show_bug.cgi?id=5571.
     """
 
-    _MD5_KEY = "md5"
-    _SHA256_KEY = "sha256"
-
     if ud.ignore_checksums or not ud.method.supports_checksum(ud):
         return {}
 
-    if _MD5_KEY in precomputed:
-        md5data = precomputed[_MD5_KEY]
-    else:
-        md5data = bb.utils.md5_file(ud.localpath)
+    def compute_checksum_info(checksum_id):
+        checksum_name = getattr(ud, "%s_name" % checksum_id)
 
-    if _SHA256_KEY in precomputed:
-        sha256data = precomputed[_SHA256_KEY]
-    else:
-        sha256data = bb.utils.sha256_file(ud.localpath)
+        if checksum_id in precomputed:
+            checksum_data = precomputed[checksum_id]
+        else:
+            checksum_data = getattr(bb.utils, "%s_file" % checksum_id)(ud.localpath)
 
-    if ud.method.recommends_checksum(ud) and not ud.md5_expected and not ud.sha256_expected:
-        # If strict checking enabled and neither sum defined, raise error
-        strict = d.getVar("BB_STRICT_CHECKSUM") or "0"
-        if strict == "1":
-            logger.error('No checksum specified for %s, please add at least one to the recipe:\n'
-                             'SRC_URI[%s] = "%s"\nSRC_URI[%s] = "%s"' %
-                             (ud.localpath, ud.md5_name, md5data,
-                              ud.sha256_name, sha256data))
-            raise NoChecksumError('Missing SRC_URI checksum', ud.url)
+        checksum_expected = getattr(ud, "%s_expected" % checksum_id)
 
-        bb.event.fire(MissingChecksumEvent(ud.url, md5data, sha256data), d)
+        return {
+            "id": checksum_id,
+            "name": checksum_name,
+            "data": checksum_data,
+            "expected": checksum_expected
+        }
 
-        if strict == "ignore":
-            return {
-                _MD5_KEY: md5data,
-                _SHA256_KEY: sha256data
-            }
+    checksum_infos = []
+    for checksum_id in CHECKSUM_LIST:
+        checksum_infos.append(compute_checksum_info(checksum_id))
 
-        # Log missing sums so user can more easily add them
-        logger.warning('Missing md5 SRC_URI checksum for %s, consider adding to the recipe:\n'
-                       'SRC_URI[%s] = "%s"',
-                       ud.localpath, ud.md5_name, md5data)
-        logger.warning('Missing sha256 SRC_URI checksum for %s, consider adding to the recipe:\n'
-                       'SRC_URI[%s] = "%s"',
-                       ud.localpath, ud.sha256_name, sha256data)
+    checksum_dict = {ci["id"] : ci["data"] for ci in checksum_infos}
 
-    # We want to alert the user if a checksum is defined in the recipe but
-    # it does not match.
-    msg = ""
-    mismatch = False
-    if ud.md5_expected and ud.md5_expected != md5data:
-        msg = msg + "\nFile: '%s' has %s checksum %s when %s was expected" % (ud.localpath, 'md5', md5data, ud.md5_expected)
-        mismatch = True;
+    checksum_lines = ["SRC_URI[%s] = \"%s\"" % (ci["name"], ci["data"]) for ci in checksum_infos]
 
-    if ud.sha256_expected and ud.sha256_expected != sha256data:
-        msg = msg + "\nFile: '%s' has %s checksum %s when %s was expected" % (ud.localpath, 'sha256', sha256data, ud.sha256_expected)
-        mismatch = True;
+    # If no checksum has been provided
+    if ud.method.recommends_checksum(ud) and all(ci["expected"] is None for ci in checksum_infos):
+        strict = d.getVar("BB_STRICT_CHECKSUM") or "0"
 
-    if mismatch:
-        msg = msg + '\nIf this change is expected (e.g. you have upgraded to a new version without updating the checksums) then you can use these lines within the recipe:\nSRC_URI[%s] = "%s"\nSRC_URI[%s] = "%s"\nOtherwise you should retry the download and/or check with upstream to determine if the file has become corrupted or otherwise unexpectedly modified.\n' % (ud.md5_name, md5data, ud.sha256_name, sha256data)
+        # If strict checking enabled and neither sum defined, raise error
+        if strict == "1":
+            messages = []
+            messages.append("No checksum specified for '%s', please add at " \
+                            "least one to the recipe:" % ud.localpath)
+            messages.extend(checksum_lines)
+            logger.error("\n".join(messages))
+            raise NoChecksumError("Missing SRC_URI checksum", ud.url)
 
-    if len(msg):
-        raise ChecksumError('Checksum mismatch!%s' % msg, ud.url, md5data)
+        if strict == "ignore":
+            return checksum_dict
 
-    return {
-        _MD5_KEY: md5data,
-        _SHA256_KEY: sha256data
-    }
+        # Log missing sums so user can more easily add them
+        else:
+            messages = []
+            messages.append("Missing checksum for '%s', consider adding at " \
+                            "least one to the recipe:" % ud.localpath)
+            messages.extend(checksum_lines)
+            logger.warning("\n".join(messages))
 
+    # We want to alert the user if a checksum is defined in the recipe but
+    # it does not match.
+    messages = []
+    messages.append("Checksum mismatch!")
+    bad_checksum = None
+
+    for ci in checksum_infos:
+        if ci["expected"] and ci["expected"] != ci["data"]:
+            messages.append("File: '%s' has %s checksum %s when %s was " \
+                            "expected" % (ud.localpath, ci["id"], ci["data"], ci["expected"]))
+            bad_checksum = ci["data"]
+
+    if bad_checksum:
+        messages.append("If this change is expected (e.g. you have upgraded " \
+                        "to a new version without updating the checksums) " \
+                        "then you can use these lines within the recipe:")
+        messages.extend(checksum_lines)
+        messages.append("Otherwise you should retry the download and/or " \
+                        "check with upstream to determine if the file has " \
+                        "become corrupted or otherwise unexpectedly modified.")
+        raise ChecksumError("\n".join(messages), ud.url, bad_checksum)
+
+    return checksum_dict
 
 def verify_donestamp(ud, d, origud=None):
     """
@@ -1230,24 +1241,26 @@ class FetchData(object):
             self.pswd = self.parm["pswd"]
         self.setup = False
 
-        if "name" in self.parm:
-            self.md5_name = "%s.md5sum" % self.parm["name"]
-            self.sha256_name = "%s.sha256sum" % self.parm["name"]
-        else:
-            self.md5_name = "md5sum"
-            self.sha256_name = "sha256sum"
-        if self.md5_name in self.parm:
-            self.md5_expected = self.parm[self.md5_name]
-        elif self.type not in ["http", "https", "ftp", "ftps", "sftp", "s3"]:
-            self.md5_expected = None
-        else:
-            self.md5_expected = d.getVarFlag("SRC_URI", self.md5_name)
-        if self.sha256_name in self.parm:
-            self.sha256_expected = self.parm[self.sha256_name]
-        elif self.type not in ["http", "https", "ftp", "ftps", "sftp", "s3"]:
-            self.sha256_expected = None
-        else:
-            self.sha256_expected = d.getVarFlag("SRC_URI", self.sha256_name)
+        def configure_checksum(checksum_id):
+            if "name" in self.parm:
+                checksum_name = "%s.%ssum" % (self.parm["name"], checksum_id)
+            else:
+                checksum_name = "%ssum" % checksum_id
+
+            setattr(self, "%s_name" % checksum_id, checksum_name)
+
+            if checksum_name in self.parm:
+                checksum_expected = self.parm[checksum_name]
+            elif self.type not in ["http", "https", "ftp", "ftps", "sftp", "s3"]:
+                checksum_expected = None
+            else:
+                checksum_expected = d.getVarFlag("SRC_URI", checksum_name)
+
+            setattr(self, "%s_expected" % checksum_id, checksum_expected)
+
+        for checksum_id in CHECKSUM_LIST:
+            configure_checksum(checksum_id)
+
         self.ignore_checksums = False
 
         self.names = self.parm.get("name",'default').split(',')
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 04/12] bitbake: fetch2: add more hash functions for checksum verification
  2020-01-17 16:45 [PATCH v4 00/12] NPM refactoring Jean-Marie LEMETAYER
                   ` (2 preceding siblings ...)
  2020-01-17 16:45 ` [PATCH v4 03/12] bitbake: fetch2: refactor checksum verification Jean-Marie LEMETAYER
@ 2020-01-17 16:45 ` Jean-Marie LEMETAYER
  2020-01-17 16:45 ` [PATCH v4 05/12] bitbake: fetch2: allow fetchers to forward the donestamp management Jean-Marie LEMETAYER
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Jean-Marie LEMETAYER @ 2020-01-17 16:45 UTC (permalink / raw)
  To: bitbake-devel; +Cc: rennes-dev, paul.eggleton

This commit enables the "sha1", "sha384" and "sha512" hash functions in
the supported checksum list. This allows to use more SRC_URI checksums
functions for a url:

  SRC_URI[sha1sum] = "..."
  SRC_URI[sha384sum] = "..."
  SRC_URI[sha512sum] = "..."

The npm fetcher needs this to support subresource integrity:
  https://www.w3.org/TR/SRI/

Signed-off-by: Jean-Marie LEMETAYER <jean-marie.lemetayer@savoirfairelinux.com>
---
 lib/bb/fetch2/__init__.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
index b4122aee..0fbc0c8d 100644
--- a/lib/bb/fetch2/__init__.py
+++ b/lib/bb/fetch2/__init__.py
@@ -33,7 +33,7 @@ _checksum_cache = bb.checksum.FileChecksumCache()
 
 logger = logging.getLogger("BitBake.Fetcher")
 
-CHECKSUM_LIST = [ "md5", "sha256" ]
+CHECKSUM_LIST = [ "md5", "sha256", "sha1", "sha384", "sha512" ]
 
 class BBFetchException(Exception):
     """Class all fetch exceptions inherit from"""
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 05/12] bitbake: fetch2: allow fetchers to forward the donestamp management
  2020-01-17 16:45 [PATCH v4 00/12] NPM refactoring Jean-Marie LEMETAYER
                   ` (3 preceding siblings ...)
  2020-01-17 16:45 ` [PATCH v4 04/12] bitbake: fetch2: add more hash functions for " Jean-Marie LEMETAYER
@ 2020-01-17 16:45 ` Jean-Marie LEMETAYER
  2020-01-17 16:45 ` [PATCH v4 06/12] bitbake: fetch2: allow fetchers to forward the mirrors management Jean-Marie LEMETAYER
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Jean-Marie LEMETAYER @ 2020-01-17 16:45 UTC (permalink / raw)
  To: bitbake-devel; +Cc: rennes-dev, paul.eggleton

This commit is necessary to introduce proxy fetchers and do not modify
the behavior of existing fetchers.

This commit allows fetchers to forwards the "verify_donestamp" and
"update_stamp" functions to a proxy fetcher.

Signed-off-by: Jean-Marie LEMETAYER <jean-marie.lemetayer@savoirfairelinux.com>
---
 lib/bb/fetch2/__init__.py | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
index 0fbc0c8d..45596bef 100644
--- a/lib/bb/fetch2/__init__.py
+++ b/lib/bb/fetch2/__init__.py
@@ -1378,6 +1378,18 @@ class FetchMethod(object):
         """
         return False
 
+    def verify_donestamp(self, ud, d):
+        """
+        Verify the donestamp file
+        """
+        return verify_donestamp(ud, d)
+
+    def update_donestamp(self, ud, d):
+        """
+        Update the donestamp file
+        """
+        update_stamp(ud, d)
+
     def _strip_leading_slashes(self, relpath):
         """
         Remove leading slash as os.path.join can't cope
@@ -1662,7 +1674,7 @@ class Fetch(object):
             try:
                 self.d.setVar("BB_NO_NETWORK", network)
 
-                if verify_donestamp(ud, self.d) and not m.need_update(ud, self.d):
+                if m.verify_donestamp(ud, self.d) and not m.need_update(ud, self.d):
                     localpath = ud.localpath
                 elif m.try_premirror(ud, self.d):
                     logger.debug(1, "Trying PREMIRRORS")
@@ -1672,7 +1684,7 @@ class Fetch(object):
                         try:
                             # early checksum verification so that if the checksum of the premirror
                             # contents mismatch the fetcher can still try upstream and mirrors
-                            update_stamp(ud, self.d)
+                            m.update_donestamp(ud, self.d)
                         except ChecksumError as e:
                             logger.warning("Checksum failure encountered with premirror download of %s - will attempt other sources." % u)
                             logger.debug(1, str(e))
@@ -1682,7 +1694,7 @@ class Fetch(object):
                     self.d.setVar("BB_NO_NETWORK", "1")
 
                 firsterr = None
-                verified_stamp = verify_donestamp(ud, self.d)
+                verified_stamp = m.verify_donestamp(ud, self.d)
                 if not localpath and (not verified_stamp or m.need_update(ud, self.d)):
                     try:
                         if not trusted_network(self.d, ud.url):
@@ -1694,7 +1706,7 @@ class Fetch(object):
                         localpath = ud.localpath
                         # early checksum verify, so that if checksum mismatched,
                         # fetcher still have chance to fetch from mirror
-                        update_stamp(ud, self.d)
+                        m.update_donestamp(ud, self.d)
 
                     except bb.fetch2.NetworkAccess:
                         raise
@@ -1723,7 +1735,7 @@ class Fetch(object):
                         logger.error(str(firsterr))
                     raise FetchError("Unable to fetch URL from any source.", u)
 
-                update_stamp(ud, self.d)
+                m.update_donestamp(ud, self.d)
 
             except IOError as e:
                 if e.errno in [errno.ESTALE]:
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 06/12] bitbake: fetch2: allow fetchers to forward the mirrors management
  2020-01-17 16:45 [PATCH v4 00/12] NPM refactoring Jean-Marie LEMETAYER
                   ` (4 preceding siblings ...)
  2020-01-17 16:45 ` [PATCH v4 05/12] bitbake: fetch2: allow fetchers to forward the donestamp management Jean-Marie LEMETAYER
@ 2020-01-17 16:45 ` Jean-Marie LEMETAYER
  2020-01-17 16:45 ` [PATCH v4 07/12] bitbake: fetch2: allow fetchers to forward the done condition Jean-Marie LEMETAYER
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Jean-Marie LEMETAYER @ 2020-01-17 16:45 UTC (permalink / raw)
  To: bitbake-devel; +Cc: rennes-dev, paul.eggleton

This commit is necessary to introduce proxy fetchers and do not modify
the behavior of existing fetchers.

This commit allows fetchers to forwards the "try_mirrors" functions to
a proxy fetcher.

Signed-off-by: Jean-Marie LEMETAYER <jean-marie.lemetayer@savoirfairelinux.com>
---
 lib/bb/fetch2/__init__.py | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
index 45596bef..0d060e19 100644
--- a/lib/bb/fetch2/__init__.py
+++ b/lib/bb/fetch2/__init__.py
@@ -1564,6 +1564,12 @@ class FetchMethod(object):
         """
         return True
 
+    def try_mirrors(self, fetch, urldata, d, mirrors):
+        """
+        Try to use a mirror
+        """
+        return try_mirrors(fetch, d, urldata, mirrors)
+
     def checkstatus(self, fetch, urldata, d):
         """
         Check the status of a URL
@@ -1679,7 +1685,7 @@ class Fetch(object):
                 elif m.try_premirror(ud, self.d):
                     logger.debug(1, "Trying PREMIRRORS")
                     mirrors = mirror_from_string(self.d.getVar('PREMIRRORS'))
-                    localpath = try_mirrors(self, self.d, ud, mirrors, False)
+                    localpath = m.try_mirrors(self, ud, self.d, mirrors)
                     if localpath:
                         try:
                             # early checksum verification so that if the checksum of the premirror
@@ -1728,7 +1734,7 @@ class Fetch(object):
                             m.clean(ud, self.d)
                         logger.debug(1, "Trying MIRRORS")
                         mirrors = mirror_from_string(self.d.getVar('MIRRORS'))
-                        localpath = try_mirrors(self, self.d, ud, mirrors)
+                        localpath = m.try_mirrors(self, ud, self.d, mirrors)
 
                 if not localpath or ((not os.path.exists(localpath)) and localpath.find("*") == -1):
                     if firsterr:
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 07/12] bitbake: fetch2: allow fetchers to forward the done condition
  2020-01-17 16:45 [PATCH v4 00/12] NPM refactoring Jean-Marie LEMETAYER
                   ` (5 preceding siblings ...)
  2020-01-17 16:45 ` [PATCH v4 06/12] bitbake: fetch2: allow fetchers to forward the mirrors management Jean-Marie LEMETAYER
@ 2020-01-17 16:45 ` Jean-Marie LEMETAYER
  2020-01-17 16:45 ` [PATCH v4 08/12] bitbake: fetch2/wget: fix downloadfilename parameter Jean-Marie LEMETAYER
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Jean-Marie LEMETAYER @ 2020-01-17 16:45 UTC (permalink / raw)
  To: bitbake-devel; +Cc: rennes-dev, paul.eggleton

This commit is necessary to introduce proxy fetchers and do not modify
the behavior of existing fetchers.

This commit allows fetchers to forwards the done condition to a
proxy fetcher.

Signed-off-by: Jean-Marie LEMETAYER <jean-marie.lemetayer@savoirfairelinux.com>
---
 lib/bb/fetch2/__init__.py | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
index 0d060e19..5c89ef61 100644
--- a/lib/bb/fetch2/__init__.py
+++ b/lib/bb/fetch2/__init__.py
@@ -1568,7 +1568,7 @@ class FetchMethod(object):
         """
         Try to use a mirror
         """
-        return try_mirrors(fetch, d, urldata, mirrors)
+        return bool(try_mirrors(fetch, d, urldata, mirrors))
 
     def checkstatus(self, fetch, urldata, d):
         """
@@ -1609,6 +1609,16 @@ class FetchMethod(object):
         """
         return ('', '')
 
+    def done(self, ud, d):
+        """
+        Is the download done ?
+        """
+        if os.path.exists(ud.localpath):
+            return True
+        if ud.localpath.find("*") != -1:
+            return True
+        return False
+
 class Fetch(object):
     def __init__(self, urls, d, cache = True, localonly = False, connection_cache = None):
         if localonly and cache:
@@ -1672,7 +1682,7 @@ class Fetch(object):
             ud = self.ud[u]
             ud.setup_localpath(self.d)
             m = ud.method
-            localpath = ""
+            done = False
 
             if ud.lockfile:
                 lf = bb.utils.lockfile(ud.lockfile)
@@ -1681,12 +1691,12 @@ class Fetch(object):
                 self.d.setVar("BB_NO_NETWORK", network)
 
                 if m.verify_donestamp(ud, self.d) and not m.need_update(ud, self.d):
-                    localpath = ud.localpath
+                    done = True
                 elif m.try_premirror(ud, self.d):
                     logger.debug(1, "Trying PREMIRRORS")
                     mirrors = mirror_from_string(self.d.getVar('PREMIRRORS'))
-                    localpath = m.try_mirrors(self, ud, self.d, mirrors)
-                    if localpath:
+                    done = m.try_mirrors(self, ud, self.d, mirrors)
+                    if done:
                         try:
                             # early checksum verification so that if the checksum of the premirror
                             # contents mismatch the fetcher can still try upstream and mirrors
@@ -1694,14 +1704,14 @@ class Fetch(object):
                         except ChecksumError as e:
                             logger.warning("Checksum failure encountered with premirror download of %s - will attempt other sources." % u)
                             logger.debug(1, str(e))
-                            localpath = ""
+                            done = False
 
                 if premirroronly:
                     self.d.setVar("BB_NO_NETWORK", "1")
 
                 firsterr = None
                 verified_stamp = m.verify_donestamp(ud, self.d)
-                if not localpath and (not verified_stamp or m.need_update(ud, self.d)):
+                if not done and (not verified_stamp or m.need_update(ud, self.d)):
                     try:
                         if not trusted_network(self.d, ud.url):
                             raise UntrustedUrl(ud.url)
@@ -1709,7 +1719,7 @@ class Fetch(object):
                         m.download(ud, self.d)
                         if hasattr(m, "build_mirror_data"):
                             m.build_mirror_data(ud, self.d)
-                        localpath = ud.localpath
+                        done = True
                         # early checksum verify, so that if checksum mismatched,
                         # fetcher still have chance to fetch from mirror
                         m.update_donestamp(ud, self.d)
@@ -1734,9 +1744,9 @@ class Fetch(object):
                             m.clean(ud, self.d)
                         logger.debug(1, "Trying MIRRORS")
                         mirrors = mirror_from_string(self.d.getVar('MIRRORS'))
-                        localpath = m.try_mirrors(self, ud, self.d, mirrors)
+                        done = m.try_mirrors(self, ud, self.d, mirrors)
 
-                if not localpath or ((not os.path.exists(localpath)) and localpath.find("*") == -1):
+                if not done or not m.done(ud, self.d):
                     if firsterr:
                         logger.error(str(firsterr))
                     raise FetchError("Unable to fetch URL from any source.", u)
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 08/12] bitbake: fetch2/wget: fix downloadfilename parameter
  2020-01-17 16:45 [PATCH v4 00/12] NPM refactoring Jean-Marie LEMETAYER
                   ` (6 preceding siblings ...)
  2020-01-17 16:45 ` [PATCH v4 07/12] bitbake: fetch2: allow fetchers to forward the done condition Jean-Marie LEMETAYER
@ 2020-01-17 16:45 ` Jean-Marie LEMETAYER
  2020-01-17 20:44   ` Christopher Larson
  2020-01-17 16:45 ` [PATCH v4 09/12] bitbake: fetch2/npm: refactor the npm fetcher Jean-Marie LEMETAYER
                   ` (3 subsequent siblings)
  11 siblings, 1 reply; 14+ messages in thread
From: Jean-Marie LEMETAYER @ 2020-01-17 16:45 UTC (permalink / raw)
  To: bitbake-devel; +Cc: rennes-dev, paul.eggleton

When using a download filename with characters which can be interpreted
by the shell ('(', ')', '&', ';', ...) the command fails. Escaping the
filename fixes the issue.

Signed-off-by: Jean-Marie LEMETAYER <jean-marie.lemetayer@savoirfairelinux.com>
---
 lib/bb/fetch2/wget.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/bb/fetch2/wget.py b/lib/bb/fetch2/wget.py
index 725586d2..6506e1e2 100644
--- a/lib/bb/fetch2/wget.py
+++ b/lib/bb/fetch2/wget.py
@@ -94,9 +94,9 @@ class Wget(FetchMethod):
         fetchcmd = self.basecmd
 
         if 'downloadfilename' in ud.parm:
-            dldir = d.getVar("DL_DIR")
-            bb.utils.mkdirhier(os.path.dirname(dldir + os.sep + ud.localfile))
-            fetchcmd += " -O " + dldir + os.sep + ud.localfile
+            localpath = os.path.join(d.getVar("DL_DIR"), ud.localfile)
+            bb.utils.mkdirhier(os.path.dirname(localpath))
+            fetchcmd += " -O '%s'" % localpath
 
         if ud.user and ud.pswd:
             fetchcmd += " --user=%s --password=%s --auth-no-challenge" % (ud.user, ud.pswd)
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 09/12] bitbake: fetch2/npm: refactor the npm fetcher
  2020-01-17 16:45 [PATCH v4 00/12] NPM refactoring Jean-Marie LEMETAYER
                   ` (7 preceding siblings ...)
  2020-01-17 16:45 ` [PATCH v4 08/12] bitbake: fetch2/wget: fix downloadfilename parameter Jean-Marie LEMETAYER
@ 2020-01-17 16:45 ` Jean-Marie LEMETAYER
  2020-01-17 16:45 ` [PATCH v4 10/12] bitbake: tests/fetch: add npm tests Jean-Marie LEMETAYER
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Jean-Marie LEMETAYER @ 2020-01-17 16:45 UTC (permalink / raw)
  To: bitbake-devel; +Cc: rennes-dev, paul.eggleton

This commit refactors the npm fetcher to improve some points and fix
others:

 - The big change is that the fetcher is only fetching the package
   source and no more the dependencies. Thus the npm fetcher act as the
   other fetchers e.g git, wget. The dependencies will be handled later.

 - The fetcher only resolves the url of the package using 'npm view' and
   then forwards it to a proxy fetcher.

 - This commit also fixes a lot of issues with the package names (exotic
   characters, scoped packages) which were badly handled.

 - The validation files - lockdown.json and npm-shrinkwrap.json - are no
   longer used by the fetcher. Instead, the downloaded tarball is
   verified with the 'integrity' and 'shasum' provided in the 'npm view'
   of the package [1][2].

1: https://docs.npmjs.com/files/package-lock.json#integrity
2: https://www.w3.org/TR/SRI

Signed-off-by: Jean-Marie LEMETAYER <jean-marie.lemetayer@savoirfairelinux.com>
---
 lib/bb/fetch2/npm.py | 538 +++++++++++++++++++++----------------------
 1 file changed, 266 insertions(+), 272 deletions(-)

diff --git a/lib/bb/fetch2/npm.py b/lib/bb/fetch2/npm.py
index 9700e610..15cdfefb 100644
--- a/lib/bb/fetch2/npm.py
+++ b/lib/bb/fetch2/npm.py
@@ -1,301 +1,295 @@
+# Copyright (C) 2020 Savoir-Faire Linux
 #
 # SPDX-License-Identifier: GPL-2.0-only
 #
 """
-BitBake 'Fetch' NPM implementation
+BitBake 'Fetch' npm implementation
 
-The NPM fetcher is used to retrieve files from the npmjs repository
+npm fetcher support the SRC_URI with format of:
+SRC_URI = "npm://some.registry.url;OptionA=xxx;OptionB=xxx;..."
 
-Usage in the recipe:
+Supported SRC_URI options are:
 
-    SRC_URI = "npm://registry.npmjs.org/;name=${PN};version=${PV}"
-    Suported SRC_URI options are:
+- package
+   The npm package name. This is a mandatory parameter.
 
-    - name
-    - version
+- version
+    The npm package version. This is a mandatory parameter.
 
-    npm://registry.npmjs.org/${PN}/-/${PN}-${PV}.tgz  would become npm://registry.npmjs.org;name=${PN};version=${PV}
-    The fetcher all triggers off the existence of ud.localpath. If that exists and has the ".done" stamp, its assumed the fetch is good/done
+- downloadfilename
+    Specifies the filename used when storing the downloaded file.
 
+- destsuffix
+    Specifies the directory to use to unpack the package (default: npm).
 """
 
-import os
-import sys
-import urllib.request, urllib.parse, urllib.error
+import base64
 import json
-import subprocess
-import signal
+import os
+import re
+import tempfile
 import bb
-from   bb.fetch2 import FetchMethod
-from   bb.fetch2 import FetchError
-from   bb.fetch2 import ChecksumError
-from   bb.fetch2 import runfetchcmd
-from   bb.fetch2 import logger
-from   bb.fetch2 import UnpackError
-from   bb.fetch2 import ParameterError
-
-def subprocess_setup():
-    # Python installs a SIGPIPE handler by default. This is usually not what
-    # non-Python subprocesses expect.
-    # SIGPIPE errors are known issues with gzip/bash
-    signal.signal(signal.SIGPIPE, signal.SIG_DFL)
+from bb.fetch2 import Fetch
+from bb.fetch2 import FetchError
+from bb.fetch2 import FetchMethod
+from bb.fetch2 import MissingParameterError
+from bb.fetch2 import ParameterError
+from bb.fetch2 import URI
+from bb.fetch2 import check_network_access
+from bb.fetch2 import runfetchcmd
+from bb.utils import is_semver
+
+def npm_package(package):
+    """Convert the npm package name to remove unsupported character"""
+    # Scoped package names (with the @) use the same naming convention
+    # as the 'npm pack' command.
+    if package.startswith("@"):
+        return re.sub("/", "-", package[1:])
+    return package
+
+def npm_filename(package, version):
+    """Get the filename of a npm package"""
+    return npm_package(package) + "-" + version + ".tgz"
+
+def npm_localfile(package, version):
+    """Get the local filename of a npm package"""
+    return os.path.join("npm2", npm_filename(package, version))
+
+def npm_integrity(integrity):
+    """
+    Get the checksum name and expected value from the subresource integrity
+        https://www.w3.org/TR/SRI/
+    """
+    algo, value = integrity.split("-", maxsplit=1)
+    return "%ssum" % algo, base64.b64decode(value).hex()
+
+def npm_unpack(tarball, destdir, d):
+    """Unpack a npm tarball"""
+    bb.utils.mkdirhier(destdir)
+    cmd = "tar --extract --gzip --file='%s'" % tarball
+    cmd += " --no-same-owner"
+    cmd += " --strip-components=1"
+    runfetchcmd(cmd, d, workdir=destdir)
+
+class NpmEnvironment(object):
+    """
+    Using a npm config file seems more reliable than using cli arguments.
+    This class allows to create a controlled environment for npm commands.
+    """
+    def __init__(self, d, configs=None):
+        self.d = d
+        self.configs = configs
+
+    def run(self, cmd, args=None, configs=None, workdir=None):
+        """Run npm command in a controlled environment"""
+        with tempfile.TemporaryDirectory() as tmpdir:
+            d = bb.data.createCopy(self.d)
+            d.setVar("HOME", tmpdir)
+
+            cfgfile = os.path.join(tmpdir, "npmrc")
+
+            if not workdir:
+                workdir = tmpdir
+
+            def _run(cmd):
+                cmd = "NPM_CONFIG_USERCONFIG=%s " % cfgfile + cmd
+                cmd = "NPM_CONFIG_GLOBALCONFIG=%s " % cfgfile + cmd
+                return runfetchcmd(cmd, d, workdir=workdir)
+
+            if self.configs:
+                for key, value in self.configs:
+                    _run("npm config set '%s' '%s'" % (key, value))
+
+            if configs:
+                for key, value in configs:
+                    _run("npm config set '%s' '%s'" % (key, value))
+
+            if args:
+                for key, value in args:
+                    cmd += " --%s='%s'" % (key, value)
+
+            return _run(cmd)
 
 class Npm(FetchMethod):
-
-    """Class to fetch urls via 'npm'"""
-    def init(self, d):
-        pass
+    """Class to fetch a package from a npm registry"""
 
     def supports(self, ud, d):
-        """
-        Check to see if a given url can be fetched with npm
-        """
-        return ud.type in ['npm']
+        """Check if a given url can be fetched with npm"""
+        return ud.type in ["npm"]
+
+    def urldata_init(self, ud, d):
+        """Init npm specific variables within url data"""
+        ud.package = None
+        ud.version = None
+        ud.registry = None
 
-    def debug(self, msg):
-        logger.debug(1, "NpmFetch: %s", msg)
+        # Get the 'package' parameter
+        if "package" in ud.parm:
+            ud.package = ud.parm.get("package")
 
-    def clean(self, ud, d):
-        logger.debug(2, "Calling cleanup %s" % ud.pkgname)
-        bb.utils.remove(ud.localpath, False)
-        bb.utils.remove(ud.pkgdatadir, True)
-        bb.utils.remove(ud.fullmirror, False)
+        if not ud.package:
+            raise MissingParameterError("Parameter 'package' required", ud.url)
+
+        # Get the 'version' parameter
+        if "version" in ud.parm:
+            ud.version = ud.parm.get("version")
 
-    def urldata_init(self, ud, d):
-        """
-        init NPM specific variable within url data
-        """
-        if 'downloadfilename' in ud.parm:
-            ud.basename = ud.parm['downloadfilename']
-        else:
-            ud.basename = os.path.basename(ud.path)
-
-        # can't call it ud.name otherwise fetcher base class will start doing sha1stuff
-        # TODO: find a way to get an sha1/sha256 manifest of pkg & all deps
-        ud.pkgname = ud.parm.get("name", None)
-        if not ud.pkgname:
-            raise ParameterError("NPM fetcher requires a name parameter", ud.url)
-        ud.version = ud.parm.get("version", None)
         if not ud.version:
-            raise ParameterError("NPM fetcher requires a version parameter", ud.url)
-        ud.bbnpmmanifest = "%s-%s.deps.json" % (ud.pkgname, ud.version)
-        ud.bbnpmmanifest = ud.bbnpmmanifest.replace('/', '-')
-        ud.registry = "http://%s" % (ud.url.replace('npm://', '', 1).split(';'))[0]
-        prefixdir = "npm/%s" % ud.pkgname
-        ud.pkgdatadir = d.expand("${DL_DIR}/%s" % prefixdir)
-        if not os.path.exists(ud.pkgdatadir):
-            bb.utils.mkdirhier(ud.pkgdatadir)
-        ud.localpath = d.expand("${DL_DIR}/npm/%s" % ud.bbnpmmanifest)
-
-        self.basecmd = d.getVar("FETCHCMD_wget") or "/usr/bin/env wget -O -t 2 -T 30 -nv --passive-ftp --no-check-certificate "
-        ud.prefixdir = prefixdir
-
-        ud.write_tarballs = ((d.getVar("BB_GENERATE_MIRROR_TARBALLS") or "0") != "0")
-        mirrortarball = 'npm_%s-%s.tar.xz' % (ud.pkgname, ud.version)
-        mirrortarball = mirrortarball.replace('/', '-')
-        ud.fullmirror = os.path.join(d.getVar("DL_DIR"), mirrortarball)
-        ud.mirrortarballs = [mirrortarball]
+            raise MissingParameterError("Parameter 'version' required", ud.url)
 
-    def need_update(self, ud, d):
-        if os.path.exists(ud.localpath):
-            return False
-        return True
-
-    def _runpack(self, ud, d, pkgfullname: str, quiet=False) -> str:
-        """
-        Runs npm pack on a full package name.
-        Returns the filename of the downloaded package
-        """
-        bb.fetch2.check_network_access(d, pkgfullname, ud.registry)
-        dldir = d.getVar("DL_DIR")
-        dldir = os.path.join(dldir, ud.prefixdir)
-
-        command = "npm pack {} --registry {}".format(pkgfullname, ud.registry)
-        logger.debug(2, "Fetching {} using command '{}' in {}".format(pkgfullname, command, dldir))
-        filename = runfetchcmd(command, d, quiet, workdir=dldir)
-        return filename.rstrip()
-
-    def _unpackdep(self, ud, pkg, data, destdir, dldir, d):
-        file = data[pkg]['tgz']
-        logger.debug(2, "file to extract is %s" % file)
-        if file.endswith('.tgz') or file.endswith('.tar.gz') or file.endswith('.tar.Z'):
-            cmd = 'tar xz --strip 1 --no-same-owner --warning=no-unknown-keyword -f %s/%s' % (dldir, file)
-        else:
-            bb.fatal("NPM package %s downloaded not a tarball!" % file)
-
-        # Change to subdir before executing command
-        if not os.path.exists(destdir):
-            os.makedirs(destdir)
-        path = d.getVar('PATH')
-        if path:
-            cmd = "PATH=\"%s\" %s" % (path, cmd)
-        bb.note("Unpacking %s to %s/" % (file, destdir))
-        ret = subprocess.call(cmd, preexec_fn=subprocess_setup, shell=True, cwd=destdir)
-
-        if ret != 0:
-            raise UnpackError("Unpack command %s failed with return value %s" % (cmd, ret), ud.url)
-
-        if 'deps' not in data[pkg]:
-            return
-        for dep in data[pkg]['deps']:
-            self._unpackdep(ud, dep, data[pkg]['deps'], "%s/node_modules/%s" % (destdir, dep), dldir, d)
-
-
-    def unpack(self, ud, destdir, d):
-        dldir = d.getVar("DL_DIR")
-        with open("%s/npm/%s" % (dldir, ud.bbnpmmanifest)) as datafile:
-            workobj = json.load(datafile)
-        dldir = "%s/%s" % (os.path.dirname(ud.localpath), ud.pkgname)
-
-        if 'subdir' in ud.parm:
-            unpackdir = '%s/%s' % (destdir, ud.parm.get('subdir'))
+        if not is_semver(ud.version) and not ud.version == "latest":
+            raise ParameterError("Invalid 'version' parameter", ud.url)
+
+        # Extract the 'registry' part of the url
+        ud.registry = re.sub(r"^npm://", "http://", ud.url.split(";")[0])
+
+        # Using the 'downloadfilename' parameter as local filename
+        # or the npm package name.
+        if "downloadfilename" in ud.parm:
+            ud.localfile = d.expand(ud.parm["downloadfilename"])
         else:
-            unpackdir = '%s/npmpkg' % destdir
-
-        self._unpackdep(ud, ud.pkgname, workobj, unpackdir, dldir, d)
-
-    def _parse_view(self, output):
-        '''
-        Parse the output of npm view --json; the last JSON result
-        is assumed to be the one that we're interested in.
-        '''
-        pdata = json.loads(output);
-        try:
-            return pdata[-1]
-        except:
-            return pdata
-
-    def _getdependencies(self, pkg, data, version, d, ud, optional=False, fetchedlist=None):
-        if fetchedlist is None:
-            fetchedlist = []
-        pkgfullname = pkg
-        if version != '*' and not '/' in version:
-            pkgfullname += "@'%s'" % version
-        if pkgfullname in fetchedlist:
-            return
-
-        logger.debug(2, "Calling getdeps on %s" % pkg)
-        fetchcmd = "npm view %s --json --registry %s" % (pkgfullname, ud.registry)
-        output = runfetchcmd(fetchcmd, d, True)
-        pdata = self._parse_view(output)
-        if not pdata:
-            raise FetchError("The command '%s' returned no output" % fetchcmd)
-        if optional:
-            pkg_os = pdata.get('os', None)
-            if pkg_os:
-                if not isinstance(pkg_os, list):
-                    pkg_os = [pkg_os]
-                blacklist = False
-                for item in pkg_os:
-                    if item.startswith('!'):
-                        blacklist = True
-                        break
-                if (not blacklist and 'linux' not in pkg_os) or '!linux' in pkg_os:
-                    logger.debug(2, "Skipping %s since it's incompatible with Linux" % pkg)
-                    return
-        filename = self._runpack(ud, d, pkgfullname)
-        data[pkg] = {}
-        data[pkg]['tgz'] = filename
-        fetchedlist.append(pkgfullname)
-
-        dependencies = pdata.get('dependencies', {})
-        optionalDependencies = pdata.get('optionalDependencies', {})
-        dependencies.update(optionalDependencies)
-        depsfound = {}
-        optdepsfound = {}
-        data[pkg]['deps'] = {}
-        for dep in dependencies:
-            if dep in optionalDependencies:
-                optdepsfound[dep] = dependencies[dep]
+            ud.localfile = npm_localfile(ud.package, ud.version)
+
+        # Get the base 'npm' command
+        ud.basecmd = d.getVar("FETCHCMD_npm") or "npm"
+
+        # This fetcher resolves a URI from a npm package name and version and
+        # then forwards it to a proxy fetcher. A resolve file containing the
+        # resolved URI is created to avoid unwanted network access (if the file
+        # already exists). The management of the donestamp file, the lockfile
+        # and the checksums are forwarded to the proxy fetcher.
+        ud.proxy = None
+        ud.needdonestamp = False
+        ud.resolvefile = self.localpath(ud, d) + ".resolved"
+
+    def _resolve_proxy_url(self, ud, d):
+        def _npm_view():
+            configs = []
+            configs.append(("json", "true"))
+            configs.append(("registry", ud.registry))
+            cmd = ud.basecmd + " view '%s@%s'" % (ud.package, ud.version)
+            env = NpmEnvironment(d)
+            check_network_access(d, cmd, ud.registry)
+            view_string = env.run(cmd, configs=configs)
+
+            if not view_string:
+                raise FetchError("Unavailable package '%s@%s'" % \
+                                 ud.package, ud.version, ud.registry, ud.url)
+
+            try:
+                view = json.loads(view_string)
+
+                error = view.get("error")
+                if error is not None:
+                    raise FetchError(error.get("summary"), ud.url)
+
+                if ud.version == "latest":
+                    bb.warn("The npm package '%s' is using the latest " \
+                            "version available. This could lead to " \
+                            "non-reproducible builds." % ud.package)
+                elif ud.version != view.get("version"):
+                    raise ParameterError("Invalid 'version' parameter", ud.url)
+
+                return view
+
+            except Exception as e:
+                raise FetchError("Invalid view from npm: %s" % str(e), ud.url)
+
+        def _get_url(view):
+            tarball_url = view.get("dist", {}).get("tarball")
+
+            if tarball_url is None:
+                raise FetchError("Invalid 'dist.tarball' in view", ud.url)
+
+            uri = URI(tarball_url)
+            uri.params["downloadfilename"] = ud.localfile
+
+            integrity = view.get("dist", {}).get("integrity")
+            shasum = view.get("dist", {}).get("shasum")
+
+            if integrity is not None:
+                checksum_name, checksum_expected = npm_integrity(integrity)
+                uri.params[checksum_name] = checksum_expected
+            elif shasum is not None:
+                uri.params["sha1sum"] = shasum
             else:
-                depsfound[dep] = dependencies[dep]
-        for dep, version in optdepsfound.items():
-            self._getdependencies(dep, data[pkg]['deps'], version, d, ud, optional=True, fetchedlist=fetchedlist)
-        for dep, version in depsfound.items():
-            self._getdependencies(dep, data[pkg]['deps'], version, d, ud, fetchedlist=fetchedlist)
-
-    def _getshrinkeddependencies(self, pkg, data, version, d, ud, lockdown, manifest, toplevel=True):
-        logger.debug(2, "NPM shrinkwrap file is %s" % data)
-        if toplevel:
-            name = data.get('name', None)
-            if name and name != pkg:
-                for obj in data.get('dependencies', []):
-                    if obj == pkg:
-                        self._getshrinkeddependencies(obj, data['dependencies'][obj], data['dependencies'][obj]['version'], d, ud, lockdown, manifest, False)
-                        return
-
-        pkgnameWithVersion = "{}@{}".format(pkg, version)
-        logger.debug(2, "Get dependencies for {}".format(pkgnameWithVersion))
-        filename = self._runpack(ud, d, pkgnameWithVersion)
-        manifest[pkg] = {}
-        manifest[pkg]['tgz'] = filename
-        manifest[pkg]['deps'] = {}
-
-        if pkg in lockdown:
-            sha1_expected = lockdown[pkg][version]
-            sha1_data = bb.utils.sha1_file("npm/%s/%s" % (ud.pkgname, manifest[pkg]['tgz']))
-            if sha1_expected != sha1_data:
-                msg = "\nFile: '%s' has %s checksum %s when %s was expected" % (manifest[pkg]['tgz'], 'sha1', sha1_data, sha1_expected)
-                raise ChecksumError('Checksum mismatch!%s' % msg)
-        else:
-            logger.debug(2, "No lockdown data for %s@%s" % (pkg, version))
+                raise FetchError("Invalid 'dist.integrity' in view", ud.url)
 
-        if 'dependencies' in data:
-            for obj in data['dependencies']:
-                logger.debug(2, "Found dep is %s" % str(obj))
-                self._getshrinkeddependencies(obj, data['dependencies'][obj], data['dependencies'][obj]['version'], d, ud, lockdown, manifest[pkg]['deps'], False)
+            return str(uri)
+
+        url = _get_url(_npm_view())
+
+        bb.utils.mkdirhier(os.path.dirname(ud.resolvefile))
+        with open(ud.resolvefile, "w") as f:
+            f.write(url)
+
+    def _setup_proxy(self, ud, d):
+        if ud.proxy is None:
+            if not os.path.exists(ud.resolvefile):
+                self._resolve_proxy_url(ud, d)
+
+            with open(ud.resolvefile, "r") as f:
+                url = f.read()
+
+            # Avoid conflicts between the environment data and:
+            # - the proxy url checksum
+            data = bb.data.createCopy(d)
+            data.delVarFlags("SRC_URI")
+            ud.proxy = Fetch([url], data)
+
+    def _get_proxy_method(self, ud, d):
+        self._setup_proxy(ud, d)
+        proxy_url = ud.proxy.urls[0]
+        proxy_ud = ud.proxy.ud[proxy_url]
+        proxy_d = ud.proxy.d
+        proxy_ud.setup_localpath(proxy_d)
+        return proxy_ud.method, proxy_ud, proxy_d
+
+    def verify_donestamp(self, ud, d):
+        """Verify the donestamp file"""
+        proxy_m, proxy_ud, proxy_d = self._get_proxy_method(ud, d)
+        return proxy_m.verify_donestamp(proxy_ud, proxy_d)
+
+    def update_donestamp(self, ud, d):
+        """Update the donestamp file"""
+        proxy_m, proxy_ud, proxy_d = self._get_proxy_method(ud, d)
+        proxy_m.update_donestamp(proxy_ud, proxy_d)
+
+    def need_update(self, ud, d):
+        """Force a fetch, even if localpath exists ?"""
+        if not os.path.exists(ud.resolvefile):
+            return True
+        if ud.version == "latest":
+            return True
+        proxy_m, proxy_ud, proxy_d = self._get_proxy_method(ud, d)
+        return proxy_m.need_update(proxy_ud, proxy_d)
+
+    def try_mirrors(self, fetch, ud, d, mirrors):
+        """Try to use a mirror"""
+        proxy_m, proxy_ud, proxy_d = self._get_proxy_method(ud, d)
+        return proxy_m.try_mirrors(fetch, proxy_ud, proxy_d, mirrors)
 
     def download(self, ud, d):
         """Fetch url"""
-        jsondepobj = {}
-        shrinkobj = {}
-        lockdown = {}
-
-        if not os.listdir(ud.pkgdatadir) and os.path.exists(ud.fullmirror):
-            dest = d.getVar("DL_DIR")
-            bb.utils.mkdirhier(dest)
-            runfetchcmd("tar -xJf %s" % (ud.fullmirror), d, workdir=dest)
-            return
-
-        if ud.parm.get("noverify", None) != '1':
-            shwrf = d.getVar('NPM_SHRINKWRAP')
-            logger.debug(2, "NPM shrinkwrap file is %s" % shwrf)
-            if shwrf:
-                try:
-                    with open(shwrf) as datafile:
-                        shrinkobj = json.load(datafile)
-                except Exception as e:
-                    raise FetchError('Error loading NPM_SHRINKWRAP file "%s" for %s: %s' % (shwrf, ud.pkgname, str(e)))
-            elif not ud.ignore_checksums:
-                logger.warning('Missing shrinkwrap file in NPM_SHRINKWRAP for %s, this will lead to unreliable builds!' % ud.pkgname)
-            lckdf = d.getVar('NPM_LOCKDOWN')
-            logger.debug(2, "NPM lockdown file is %s" % lckdf)
-            if lckdf:
-                try:
-                    with open(lckdf) as datafile:
-                        lockdown = json.load(datafile)
-                except Exception as e:
-                    raise FetchError('Error loading NPM_LOCKDOWN file "%s" for %s: %s' % (lckdf, ud.pkgname, str(e)))
-            elif not ud.ignore_checksums:
-                logger.warning('Missing lockdown file in NPM_LOCKDOWN for %s, this will lead to unreproducible builds!' % ud.pkgname)
-
-        if ('name' not in shrinkobj):
-            self._getdependencies(ud.pkgname, jsondepobj, ud.version, d, ud)
-        else:
-            self._getshrinkeddependencies(ud.pkgname, shrinkobj, ud.version, d, ud, lockdown, jsondepobj)
-
-        with open(ud.localpath, 'w') as outfile:
-            json.dump(jsondepobj, outfile)
-
-    def build_mirror_data(self, ud, d):
-        # Generate a mirror tarball if needed
-        if ud.write_tarballs and not os.path.exists(ud.fullmirror):
-            # it's possible that this symlink points to read-only filesystem with PREMIRROR
-            if os.path.islink(ud.fullmirror):
-                os.unlink(ud.fullmirror)
-
-            dldir = d.getVar("DL_DIR")
-            logger.info("Creating tarball of npm data")
-            runfetchcmd("tar -cJf %s npm/%s npm/%s" % (ud.fullmirror, ud.bbnpmmanifest, ud.pkgname), d,
-                        workdir=dldir)
-            runfetchcmd("touch %s.done" % (ud.fullmirror), d, workdir=dldir)
+        self._setup_proxy(ud, d)
+        ud.proxy.download()
+
+    def unpack(self, ud, rootdir, d):
+        """Unpack the downloaded archive"""
+        destsuffix = ud.parm.get("destsuffix", "npm")
+        destdir = os.path.join(rootdir, destsuffix)
+        npm_unpack(ud.localpath, destdir, d)
+
+    def clean(self, ud, d):
+        """Clean any existing full or partial download"""
+        if os.path.exists(ud.resolvefile):
+            self._setup_proxy(ud, d)
+            ud.proxy.clean()
+            bb.utils.remove(ud.resolvefile)
+
+    def done(self, ud, d):
+        """Is the download done ?"""
+        if not os.path.exists(ud.resolvefile):
+            return False
+        proxy_m, proxy_ud, proxy_d = self._get_proxy_method(ud, d)
+        return proxy_m.done(proxy_ud, proxy_d)
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 10/12] bitbake: tests/fetch: add npm tests
  2020-01-17 16:45 [PATCH v4 00/12] NPM refactoring Jean-Marie LEMETAYER
                   ` (8 preceding siblings ...)
  2020-01-17 16:45 ` [PATCH v4 09/12] bitbake: fetch2/npm: refactor the npm fetcher Jean-Marie LEMETAYER
@ 2020-01-17 16:45 ` Jean-Marie LEMETAYER
  2020-01-17 16:45 ` [PATCH v4 11/12] bitbake: fetch2: add the npmsw fetcher Jean-Marie LEMETAYER
  2020-01-17 16:45 ` [PATCH v4 12/12] bitbake: tests/fetch: add npmsw tests Jean-Marie LEMETAYER
  11 siblings, 0 replies; 14+ messages in thread
From: Jean-Marie LEMETAYER @ 2020-01-17 16:45 UTC (permalink / raw)
  To: bitbake-devel; +Cc: rennes-dev, paul.eggleton

This commit adds some tests to validate the npm fetcher:

     - bb.tests.fetch.NPMTest.test_npm
     - bb.tests.fetch.NPMTest.test_npm_bad_checksum
     - bb.tests.fetch.NPMTest.test_npm_destsuffix_downloadfilename
     - bb.tests.fetch.NPMTest.test_npm_mirrors
     - bb.tests.fetch.NPMTest.test_npm_no_network_no_tarball
     - bb.tests.fetch.NPMTest.test_npm_no_network_with_tarball
     - bb.tests.fetch.NPMTest.test_npm_package_invalid
     - bb.tests.fetch.NPMTest.test_npm_package_none
     - bb.tests.fetch.NPMTest.test_npm_premirrors
     - bb.tests.fetch.NPMTest.test_npm_registry_alternate
     - bb.tests.fetch.NPMTest.test_npm_registry_invalid
     - bb.tests.fetch.NPMTest.test_npm_registry_none
     - bb.tests.fetch.NPMTest.test_npm_version_invalid
     - bb.tests.fetch.NPMTest.test_npm_version_latest
     - bb.tests.fetch.NPMTest.test_npm_version_none

Signed-off-by: Jean-Marie LEMETAYER <jean-marie.lemetayer@savoirfairelinux.com>
---
 lib/bb/tests/fetch.py | 183 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 183 insertions(+)

diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py
index 83fad3ff..e88eaa7e 100644
--- a/lib/bb/tests/fetch.py
+++ b/lib/bb/tests/fetch.py
@@ -2008,3 +2008,186 @@ class GitLfsTest(FetcherTest):
         ud.method._find_git_lfs = lambda d: False
         shutil.rmtree(self.gitdir, ignore_errors=True)
         fetcher.unpack(self.d.getVar('WORKDIR'))
+
+class NPMTest(FetcherTest):
+    def skipIfNoNpm():
+        import shutil
+        if not shutil.which('npm'):
+            return unittest.skip('npm not installed, tests being skipped')
+        return lambda f: f
+
+    @skipIfNoNpm()
+    @skipIfNoNetwork()
+    def test_npm(self):
+        url = 'npm://registry.npmjs.org;package=@savoirfairelinux/node-server-example;version=1.0.0'
+        fetcher = bb.fetch.Fetch([url], self.d)
+        ud = fetcher.ud[fetcher.urls[0]]
+        fetcher.download()
+        self.assertTrue(os.path.exists(ud.localpath))
+        self.assertTrue(os.path.exists(ud.localpath + '.done'))
+        self.assertTrue(os.path.exists(ud.resolvefile))
+        fetcher.unpack(self.unpackdir)
+        unpackdir = os.path.join(self.unpackdir, 'npm')
+        self.assertTrue(os.path.exists(os.path.join(unpackdir, 'package.json')))
+
+    @skipIfNoNpm()
+    @skipIfNoNetwork()
+    def test_npm_bad_checksum(self):
+        url = 'npm://registry.npmjs.org;package=@savoirfairelinux/node-server-example;version=1.0.0'
+        # Fetch once to get a tarball
+        fetcher = bb.fetch.Fetch([url], self.d)
+        ud = fetcher.ud[fetcher.urls[0]]
+        fetcher.download()
+        self.assertTrue(os.path.exists(ud.localpath))
+        # Modify the tarball
+        bad = b'bad checksum'
+        with open(ud.localpath, 'wb') as f:
+            f.write(bad)
+        # Verify that the tarball is fetched again
+        fetcher.download()
+        badsum = hashlib.sha512(bad).hexdigest()
+        self.assertTrue(os.path.exists(ud.localpath + '_bad-checksum_' + badsum))
+        self.assertTrue(os.path.exists(ud.localpath))
+
+    @skipIfNoNpm()
+    @skipIfNoNetwork()
+    def test_npm_premirrors(self):
+        url = 'npm://registry.npmjs.org;package=@savoirfairelinux/node-server-example;version=1.0.0'
+        # Fetch once to get a tarball
+        fetcher = bb.fetch.Fetch([url], self.d)
+        ud = fetcher.ud[fetcher.urls[0]]
+        fetcher.download()
+        self.assertTrue(os.path.exists(ud.localpath))
+        # Setup the mirror
+        mirrordir = os.path.join(self.tempdir, 'mirror')
+        bb.utils.mkdirhier(mirrordir)
+        os.replace(ud.localpath, os.path.join(mirrordir, os.path.basename(ud.localpath)))
+        self.d.setVar('PREMIRRORS', 'https?$://.*/.* file://%s/\n' % mirrordir)
+        self.d.setVar('BB_FETCH_PREMIRRORONLY', '1')
+        # Fetch again
+        self.assertFalse(os.path.exists(ud.localpath))
+        fetcher.download()
+        self.assertTrue(os.path.exists(ud.localpath))
+
+    @skipIfNoNpm()
+    @skipIfNoNetwork()
+    def test_npm_mirrors(self):
+        # Fetch once to get a tarball
+        url = 'npm://registry.npmjs.org;package=@savoirfairelinux/node-server-example;version=1.0.0'
+        fetcher = bb.fetch.Fetch([url], self.d)
+        ud = fetcher.ud[fetcher.urls[0]]
+        fetcher.download()
+        self.assertTrue(os.path.exists(ud.localpath))
+        # Setup the mirror
+        mirrordir = os.path.join(self.tempdir, 'mirror')
+        bb.utils.mkdirhier(mirrordir)
+        os.replace(ud.localpath, os.path.join(mirrordir, os.path.basename(ud.localpath)))
+        self.d.setVar('MIRRORS', 'https?$://.*/.* file://%s/\n' % mirrordir)
+        # Update the resolved url to an invalid url
+        with open(ud.resolvefile, 'r') as f:
+            url = f.read()
+        uri = URI(url)
+        uri.path = '/invalid'
+        with open(ud.resolvefile, 'w') as f:
+            f.write(str(uri))
+        # Fetch again
+        self.assertFalse(os.path.exists(ud.localpath))
+        fetcher.download()
+        self.assertTrue(os.path.exists(ud.localpath))
+
+    @skipIfNoNpm()
+    @skipIfNoNetwork()
+    def test_npm_destsuffix_downloadfilename(self):
+        url = 'npm://registry.npmjs.org;package=@savoirfairelinux/node-server-example;version=1.0.0;destsuffix=foo/bar;downloadfilename=foo-bar.tgz'
+        fetcher = bb.fetch.Fetch([url], self.d)
+        fetcher.download()
+        self.assertTrue(os.path.exists(os.path.join(self.dldir, 'foo-bar.tgz')))
+        fetcher.unpack(self.unpackdir)
+        unpackdir = os.path.join(self.unpackdir, 'foo', 'bar')
+        self.assertTrue(os.path.exists(os.path.join(unpackdir, 'package.json')))
+
+    def test_npm_no_network_no_tarball(self):
+        url = 'npm://registry.npmjs.org;package=@savoirfairelinux/node-server-example;version=1.0.0'
+        self.d.setVar('BB_NO_NETWORK', '1')
+        fetcher = bb.fetch.Fetch([url], self.d)
+        with self.assertRaises(bb.fetch2.NetworkAccess):
+            fetcher.download()
+
+    @skipIfNoNpm()
+    @skipIfNoNetwork()
+    def test_npm_no_network_with_tarball(self):
+        url = 'npm://registry.npmjs.org;package=@savoirfairelinux/node-server-example;version=1.0.0'
+        # Fetch once to get a tarball
+        fetcher = bb.fetch.Fetch([url], self.d)
+        fetcher.download()
+        # Disable network access
+        self.d.setVar('BB_NO_NETWORK', '1')
+        # Fetch again
+        fetcher.download()
+        fetcher.unpack(self.unpackdir)
+        unpackdir = os.path.join(self.unpackdir, 'npm')
+        self.assertTrue(os.path.exists(os.path.join(unpackdir, 'package.json')))
+
+    @skipIfNoNpm()
+    @skipIfNoNetwork()
+    def test_npm_registry_alternate(self):
+        url = 'npm://registry.freajs.org;package=@savoirfairelinux/node-server-example;version=1.0.0'
+        fetcher = bb.fetch.Fetch([url], self.d)
+        fetcher.download()
+        fetcher.unpack(self.unpackdir)
+        unpackdir = os.path.join(self.unpackdir, 'npm')
+        self.assertTrue(os.path.exists(os.path.join(unpackdir, 'package.json')))
+
+    @skipIfNoNpm()
+    @skipIfNoNetwork()
+    def test_npm_version_latest(self):
+        url = 'npm://registry.npmjs.org;package=@savoirfairelinux/node-server-example;version=latest'
+        fetcher = bb.fetch.Fetch([url], self.d)
+        fetcher.download()
+        fetcher.unpack(self.unpackdir)
+        unpackdir = os.path.join(self.unpackdir, 'npm')
+        self.assertTrue(os.path.exists(os.path.join(unpackdir, 'package.json')))
+
+    @skipIfNoNpm()
+    @skipIfNoNetwork()
+    def test_npm_registry_invalid(self):
+        url = 'npm://registry.invalid.org;package=@savoirfairelinux/node-server-example;version=1.0.0'
+        fetcher = bb.fetch.Fetch([url], self.d)
+        with self.assertRaises(bb.fetch2.FetchError):
+            fetcher.download()
+
+    @skipIfNoNpm()
+    @skipIfNoNetwork()
+    def test_npm_package_invalid(self):
+        url = 'npm://registry.npmjs.org;package=@savoirfairelinux/invalid;version=1.0.0'
+        fetcher = bb.fetch.Fetch([url], self.d)
+        with self.assertRaises(bb.fetch2.FetchError):
+            fetcher.download()
+
+    @skipIfNoNpm()
+    @skipIfNoNetwork()
+    def test_npm_version_invalid(self):
+        url = 'npm://registry.npmjs.org;package=@savoirfairelinux/node-server-example;version=invalid'
+        with self.assertRaises(bb.fetch2.ParameterError):
+            fetcher = bb.fetch.Fetch([url], self.d)
+
+    @skipIfNoNpm()
+    @skipIfNoNetwork()
+    def test_npm_registry_none(self):
+        url = 'npm://;package=@savoirfairelinux/node-server-example;version=1.0.0'
+        with self.assertRaises(bb.fetch2.MalformedUrl):
+            fetcher = bb.fetch.Fetch([url], self.d)
+
+    @skipIfNoNpm()
+    @skipIfNoNetwork()
+    def test_npm_package_none(self):
+        url = 'npm://registry.npmjs.org;version=1.0.0'
+        with self.assertRaises(bb.fetch2.MissingParameterError):
+            fetcher = bb.fetch.Fetch([url], self.d)
+
+    @skipIfNoNpm()
+    @skipIfNoNetwork()
+    def test_npm_version_none(self):
+        url = 'npm://registry.npmjs.org;package=@savoirfairelinux/node-server-example'
+        with self.assertRaises(bb.fetch2.MissingParameterError):
+            fetcher = bb.fetch.Fetch([url], self.d)
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 11/12] bitbake: fetch2: add the npmsw fetcher
  2020-01-17 16:45 [PATCH v4 00/12] NPM refactoring Jean-Marie LEMETAYER
                   ` (9 preceding siblings ...)
  2020-01-17 16:45 ` [PATCH v4 10/12] bitbake: tests/fetch: add npm tests Jean-Marie LEMETAYER
@ 2020-01-17 16:45 ` Jean-Marie LEMETAYER
  2020-01-17 16:45 ` [PATCH v4 12/12] bitbake: tests/fetch: add npmsw tests Jean-Marie LEMETAYER
  11 siblings, 0 replies; 14+ messages in thread
From: Jean-Marie LEMETAYER @ 2020-01-17 16:45 UTC (permalink / raw)
  To: bitbake-devel; +Cc: rennes-dev, paul.eggleton

This commit adds a new npmsw fetcher that fetches every npm dependencies
described in a npm shrinkwrap file:

  https://docs.npmjs.com/files/shrinkwrap.json.html

The main package must be fetched separately:

  SRC_URI = "npm://registry.url;package=foobar;version=1.0.0 \
             npmsw://${THISDIR}/npm-shrinkwrap.json"

Since a separation has been created between the package and its
dependencies, the package can also be fetched with a non npm fetcher
without impacting the general behavior:

  SRC_URI = "git://github.com/foo/bar.git;protocol=https \
             npmsw://${THISDIR}/npm-shrinkwrap.json"

Signed-off-by: Jean-Marie LEMETAYER <jean-marie.lemetayer@savoirfairelinux.com>
---
 lib/bb/fetch2/__init__.py |   2 +
 lib/bb/fetch2/npmsw.py    | 255 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 257 insertions(+)
 create mode 100644 lib/bb/fetch2/npmsw.py

diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
index 5c89ef61..ded67dbc 100644
--- a/lib/bb/fetch2/__init__.py
+++ b/lib/bb/fetch2/__init__.py
@@ -1894,6 +1894,7 @@ from . import osc
 from . import repo
 from . import clearcase
 from . import npm
+from . import npmsw
 
 methods.append(local.Local())
 methods.append(wget.Wget())
@@ -1912,3 +1913,4 @@ methods.append(osc.Osc())
 methods.append(repo.Repo())
 methods.append(clearcase.ClearCase())
 methods.append(npm.Npm())
+methods.append(npmsw.NpmShrinkWrap())
diff --git a/lib/bb/fetch2/npmsw.py b/lib/bb/fetch2/npmsw.py
new file mode 100644
index 00000000..0c3511d8
--- /dev/null
+++ b/lib/bb/fetch2/npmsw.py
@@ -0,0 +1,255 @@
+# Copyright (C) 2020 Savoir-Faire Linux
+#
+# SPDX-License-Identifier: GPL-2.0-only
+#
+"""
+BitBake 'Fetch' npm shrinkwrap implementation
+
+npm fetcher support the SRC_URI with format of:
+SRC_URI = "npmsw://some.registry.url;OptionA=xxx;OptionB=xxx;..."
+
+Supported SRC_URI options are:
+
+- dev
+   Set to 1 to also install devDependencies.
+
+- destsuffix
+    Specifies the directory to use to unpack the dependencies (default: ${S}).
+"""
+
+import json
+import os
+import re
+import bb
+from bb.fetch2 import Fetch
+from bb.fetch2 import FetchMethod
+from bb.fetch2 import ParameterError
+from bb.fetch2 import URI
+from bb.fetch2.npm import npm_integrity
+from bb.fetch2.npm import npm_localfile
+from bb.fetch2.npm import npm_unpack
+from bb.utils import is_semver
+
+def foreach_dependencies(shrinkwrap, callback=None, dev=False):
+    """
+        Run a callback for each dependencies of a shrinkwrap file.
+        The callback is using the format:
+            callback(name, params, deptree)
+        with:
+            name = the package name (string)
+            params = the package parameters (dictionary)
+            deptree = the package dependency tree (array of strings)
+    """
+    def _walk_deps(deps, deptree):
+        for name in deps:
+            subtree = [*deptree, name]
+            _walk_deps(deps[name].get("dependencies", {}), subtree)
+            if callback is not None:
+                if deps[name].get("dev", False) and not dev:
+                    continue
+                elif deps[name].get("bundled", False):
+                    continue
+                callback(name, deps[name], subtree)
+
+    _walk_deps(shrinkwrap.get("dependencies", {}), [])
+
+class NpmShrinkWrap(FetchMethod):
+    """Class to fetch all package from a shrinkwrap file"""
+
+    def supports(self, ud, d):
+        """Check if a given url can be fetched with npmsw"""
+        return ud.type in ["npmsw"]
+
+    def urldata_init(self, ud, d):
+        """Init npmsw specific variables within url data"""
+
+        # Get the 'shrinkwrap' parameter
+        ud.shrinkwrap_file = re.sub(r"^npmsw://", "", ud.url.split(";")[0])
+
+        # Get the 'dev' parameter
+        ud.dev = bb.utils.to_boolean(ud.parm.get("dev"), False)
+
+        # Resolve the dependencies
+        ud.deps = []
+
+        def _resolve_dependency(name, params, deptree):
+            url = None
+            localpath = None
+            extrapaths = []
+            destsubdirs = [os.path.join("node_modules", dep) for dep in deptree]
+            destsuffix = os.path.join(*destsubdirs)
+
+            integrity = params.get("integrity", None)
+            resolved = params.get("resolved", None)
+            version = params.get("version", None)
+
+            # Handle registry sources
+            if is_semver(version) and resolved and integrity:
+                localfile = npm_localfile(name, version)
+
+                uri = URI(resolved)
+                uri.params["downloadfilename"] = localfile
+
+                checksum_name, checksum_expected = npm_integrity(integrity)
+                uri.params[checksum_name] = checksum_expected
+
+                url = str(uri)
+
+                localpath = os.path.join(d.getVar("DL_DIR"), localfile)
+
+                # Create a resolve file to mimic the npm fetcher and allow
+                # re-usability of the downloaded file.
+                resolvefile = localpath + ".resolved"
+
+                bb.utils.mkdirhier(os.path.dirname(resolvefile))
+                with open(resolvefile, "w") as f:
+                    f.write(url)
+
+                extrapaths.append(resolvefile)
+
+            # Handle http tarball sources
+            elif version.startswith("http") and integrity:
+                localfile = os.path.join("npm2", os.path.basename(version))
+
+                uri = URI(version)
+                uri.params["downloadfilename"] = localfile
+
+                checksum_name, checksum_expected = npm_integrity(integrity)
+                uri.params[checksum_name] = checksum_expected
+
+                url = str(uri)
+
+                localpath = os.path.join(d.getVar("DL_DIR"), localfile)
+
+            # Handle git sources
+            elif version.startswith("git"):
+                regex = re.compile(r"""
+                    ^
+                    git\+
+                    (?P<protocol>[a-z]+)
+                    ://
+                    (?P<url>[^#]+)
+                    \#
+                    (?P<rev>[0-9a-f]+)
+                    $
+                    """, re.VERBOSE)
+
+                match = regex.match(version)
+
+                if not match:
+                    raise ParameterError("Invalid git url: %s" % version, ud.url)
+
+                groups = match.groupdict()
+
+                uri = URI("git://" + str(groups["url"]))
+                uri.params["protocol"] = str(groups["protocol"])
+                uri.params["rev"] = str(groups["rev"])
+                uri.params["destsuffix"] = destsuffix
+
+                url = str(uri)
+
+            # local tarball sources and local link sources are unsupported
+            else:
+                raise ParameterError("Unsupported dependency: %s" % name, ud.url)
+
+            ud.deps.append({
+                "url": url,
+                "localpath": localpath,
+                "extrapaths": extrapaths,
+                "destsuffix": destsuffix,
+            })
+
+        try:
+            with open(ud.shrinkwrap_file, "r") as f:
+                shrinkwrap = json.load(f)
+        except Exception as e:
+            raise ParameterError("Invalid shrinkwrap file: %s" % str(e), ud.url)
+
+        foreach_dependencies(shrinkwrap, _resolve_dependency, ud.dev)
+
+        # Avoid conflicts between the environment data and:
+        # - the proxy url revision
+        # - the proxy url checksum
+        data = bb.data.createCopy(d)
+        data.delVar("SRCREV")
+        data.delVarFlags("SRC_URI")
+
+        # This fetcher resolves multiple URIs from a shrinkwrap file and then
+        # forwards it to a proxy fetcher. The management of the donestamp file,
+        # the lockfile and the checksums are forwarded to the proxy fetcher.
+        ud.proxy = Fetch([dep["url"] for dep in ud.deps], data)
+        ud.needdonestamp = False
+
+    @staticmethod
+    def _foreach_proxy_method(ud, handle):
+        returns = []
+        for proxy_url in ud.proxy.urls:
+            proxy_ud = ud.proxy.ud[proxy_url]
+            proxy_d = ud.proxy.d
+            proxy_ud.setup_localpath(proxy_d)
+            returns.append(handle(proxy_ud.method, proxy_ud, proxy_d))
+        return returns
+
+    def verify_donestamp(self, ud, d):
+        """Verify the donestamp file"""
+        def _handle(m, ud, d):
+            return m.verify_donestamp(ud, d)
+        return all(self._foreach_proxy_method(ud, _handle))
+
+    def update_donestamp(self, ud, d):
+        """Update the donestamp file"""
+        def _handle(m, ud, d):
+            m.update_donestamp(ud, d)
+        self._foreach_proxy_method(ud, _handle)
+
+    def need_update(self, ud, d):
+        """Force a fetch, even if localpath exists ?"""
+        def _handle(m, ud, d):
+            return m.need_update(ud, d)
+        return all(self._foreach_proxy_method(ud, _handle))
+
+    def try_mirrors(self, fetch, ud, d, mirrors):
+        """Try to use a mirror"""
+        def _handle(m, ud, d):
+            return m.try_mirrors(fetch, ud, d, mirrors)
+        return all(self._foreach_proxy_method(ud, _handle))
+
+    def download(self, ud, d):
+        """Fetch url"""
+        ud.proxy.download()
+
+    def unpack(self, ud, rootdir, d):
+        """Unpack the downloaded dependencies"""
+        destdir = d.getVar("S")
+        destsuffix = ud.parm.get("destsuffix")
+        if destsuffix:
+            destdir = os.path.join(rootdir, destsuffix)
+
+        bb.utils.mkdirhier(destdir)
+        bb.utils.copyfile(ud.shrinkwrap_file,
+                          os.path.join(destdir, "npm-shrinkwrap.json"))
+
+        auto = [dep["url"] for dep in ud.deps if not dep["localpath"]]
+        manual = [dep for dep in ud.deps if dep["localpath"]]
+
+        if auto:
+            ud.proxy.unpack(destdir, auto)
+
+        for dep in manual:
+            depdestdir = os.path.join(destdir, dep["destsuffix"])
+            npm_unpack(dep["localpath"], depdestdir, d)
+
+    def clean(self, ud, d):
+        """Clean any existing full or partial download"""
+        ud.proxy.clean()
+
+        # Clean extra files
+        for dep in ud.deps:
+            for path in dep["extrapaths"]:
+                bb.utils.remove(path)
+
+    def done(self, ud, d):
+        """Is the download done ?"""
+        def _handle(m, ud, d):
+            return m.done(ud, d)
+        return all(self._foreach_proxy_method(ud, _handle))
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 12/12] bitbake: tests/fetch: add npmsw tests
  2020-01-17 16:45 [PATCH v4 00/12] NPM refactoring Jean-Marie LEMETAYER
                   ` (10 preceding siblings ...)
  2020-01-17 16:45 ` [PATCH v4 11/12] bitbake: fetch2: add the npmsw fetcher Jean-Marie LEMETAYER
@ 2020-01-17 16:45 ` Jean-Marie LEMETAYER
  11 siblings, 0 replies; 14+ messages in thread
From: Jean-Marie LEMETAYER @ 2020-01-17 16:45 UTC (permalink / raw)
  To: bitbake-devel; +Cc: rennes-dev, paul.eggleton

This commit adds some tests to validate the npmsw fetcher:

     - bb.tests.fetch.NPMTest.test_npmsw
     - bb.tests.fetch.NPMTest.test_npmsw_bad_checksum
     - bb.tests.fetch.NPMTest.test_npmsw_destsuffix
     - bb.tests.fetch.NPMTest.test_npmsw_dev
     - bb.tests.fetch.NPMTest.test_npmsw_mirrors
     - bb.tests.fetch.NPMTest.test_npmsw_no_network_no_tarball
     - bb.tests.fetch.NPMTest.test_npmsw_no_network_with_tarball
     - bb.tests.fetch.NPMTest.test_npmsw_npm_reusability
     - bb.tests.fetch.NPMTest.test_npmsw_premirrors

Signed-off-by: Jean-Marie LEMETAYER <jean-marie.lemetayer@savoirfairelinux.com>
---
 lib/bb/tests/fetch.py | 251 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 251 insertions(+)

diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py
index e88eaa7e..f33c4c0b 100644
--- a/lib/bb/tests/fetch.py
+++ b/lib/bb/tests/fetch.py
@@ -2191,3 +2191,254 @@ class NPMTest(FetcherTest):
         url = 'npm://registry.npmjs.org;package=@savoirfairelinux/node-server-example'
         with self.assertRaises(bb.fetch2.MissingParameterError):
             fetcher = bb.fetch.Fetch([url], self.d)
+
+    def create_shrinkwrap_file(self, data):
+        import json
+        datadir = os.path.join(self.tempdir, 'data')
+        swfile = os.path.join(datadir, 'npm-shrinkwrap.json')
+        bb.utils.mkdirhier(datadir)
+        with open(swfile, 'w') as f:
+            json.dump(data, f)
+        # Also configure the S directory
+        self.sdir = os.path.join(self.unpackdir, 'S')
+        self.d.setVar('S', self.sdir)
+        return swfile
+
+    @skipIfNoNpm()
+    @skipIfNoNetwork()
+    def test_npmsw(self):
+        swfile = self.create_shrinkwrap_file({
+            'dependencies': {
+                'array-flatten': {
+                    'version': '1.1.1',
+                    'resolved': 'https://registry.npmjs.org/array-flatten/-/array-flatten-1.1.1.tgz',
+                    'integrity': 'sha1-ml9pkFGx5wczKPKgCJaLZOopVdI=',
+                    'dependencies': {
+                        'content-type': {
+                            'version': 'https://registry.npmjs.org/content-type/-/content-type-1.0.4.tgz',
+                            'integrity': 'sha512-hIP3EEPs8tB9AT1L+NUqtwOAps4mk2Zob89MWXMHjHWg9milF/j4osnnQLXBCBFBk/tvIG/tUc9mOUJiPBhPXA==',
+                            'dependencies': {
+                                'cookie': {
+                                    'version': 'git+https://github.com/jshttp/cookie.git#aec1177c7da67e3b3273df96cf476824dbc9ae09',
+                                    'from': 'git+https://github.com/jshttp/cookie.git'
+                                }
+                            }
+                        }
+                    }
+                }
+            }
+        })
+        fetcher = bb.fetch.Fetch(['npmsw://' + swfile], self.d)
+        fetcher.download()
+        self.assertTrue(os.path.exists(os.path.join(self.dldir, 'npm2', 'array-flatten-1.1.1.tgz')))
+        self.assertTrue(os.path.exists(os.path.join(self.dldir, 'npm2', 'content-type-1.0.4.tgz')))
+        self.assertTrue(os.path.exists(os.path.join(self.dldir, 'git2', 'github.com.jshttp.cookie.git')))
+        fetcher.unpack(self.unpackdir)
+        self.assertTrue(os.path.exists(os.path.join(self.sdir, 'npm-shrinkwrap.json')))
+        self.assertTrue(os.path.exists(os.path.join(self.sdir, 'node_modules', 'array-flatten', 'package.json')))
+        self.assertTrue(os.path.exists(os.path.join(self.sdir, 'node_modules', 'array-flatten', 'node_modules', 'content-type', 'package.json')))
+        self.assertTrue(os.path.exists(os.path.join(self.sdir, 'node_modules', 'array-flatten', 'node_modules', 'content-type', 'node_modules', 'cookie', 'package.json')))
+
+    @skipIfNoNpm()
+    @skipIfNoNetwork()
+    def test_npmsw_dev(self):
+        swfile = self.create_shrinkwrap_file({
+            'dependencies': {
+                'array-flatten': {
+                    'version': '1.1.1',
+                    'resolved': 'https://registry.npmjs.org/array-flatten/-/array-flatten-1.1.1.tgz',
+                    'integrity': 'sha1-ml9pkFGx5wczKPKgCJaLZOopVdI='
+                },
+                'content-type': {
+                    'version': '1.0.4',
+                    'resolved': 'https://registry.npmjs.org/content-type/-/content-type-1.0.4.tgz',
+                    'integrity': 'sha512-hIP3EEPs8tB9AT1L+NUqtwOAps4mk2Zob89MWXMHjHWg9milF/j4osnnQLXBCBFBk/tvIG/tUc9mOUJiPBhPXA==',
+                    'dev': True
+                }
+            }
+        })
+        # Fetch with dev disabled
+        fetcher = bb.fetch.Fetch(['npmsw://' + swfile], self.d)
+        fetcher.download()
+        self.assertTrue(os.path.exists(os.path.join(self.dldir, 'npm2', 'array-flatten-1.1.1.tgz')))
+        self.assertFalse(os.path.exists(os.path.join(self.dldir, 'npm2', 'content-type-1.0.4.tgz')))
+        # Fetch with dev enabled
+        fetcher = bb.fetch.Fetch(['npmsw://' + swfile + ';dev=1'], self.d)
+        fetcher.download()
+        self.assertTrue(os.path.exists(os.path.join(self.dldir, 'npm2', 'array-flatten-1.1.1.tgz')))
+        self.assertTrue(os.path.exists(os.path.join(self.dldir, 'npm2', 'content-type-1.0.4.tgz')))
+
+    @skipIfNoNpm()
+    @skipIfNoNetwork()
+    def test_npmsw_destsuffix(self):
+        swfile = self.create_shrinkwrap_file({
+            'dependencies': {
+                'array-flatten': {
+                    'version': '1.1.1',
+                    'resolved': 'https://registry.npmjs.org/array-flatten/-/array-flatten-1.1.1.tgz',
+                    'integrity': 'sha1-ml9pkFGx5wczKPKgCJaLZOopVdI='
+                }
+            }
+        })
+        fetcher = bb.fetch.Fetch(['npmsw://' + swfile + ';destsuffix=foo/bar'], self.d)
+        fetcher.download()
+        fetcher.unpack(self.unpackdir)
+        self.assertTrue(os.path.exists(os.path.join(self.unpackdir, 'foo', 'bar', 'node_modules', 'array-flatten', 'package.json')))
+
+    def test_npmsw_no_network_no_tarball(self):
+        swfile = self.create_shrinkwrap_file({
+            'dependencies': {
+                'array-flatten': {
+                    'version': '1.1.1',
+                    'resolved': 'https://registry.npmjs.org/array-flatten/-/array-flatten-1.1.1.tgz',
+                    'integrity': 'sha1-ml9pkFGx5wczKPKgCJaLZOopVdI='
+                }
+            }
+        })
+        self.d.setVar('BB_NO_NETWORK', '1')
+        fetcher = bb.fetch.Fetch(['npmsw://' + swfile], self.d)
+        with self.assertRaises(bb.fetch2.NetworkAccess):
+            fetcher.download()
+
+    @skipIfNoNpm()
+    @skipIfNoNetwork()
+    def test_npmsw_no_network_with_tarball(self):
+        # Fetch once to get a tarball
+        fetcher = bb.fetch.Fetch(['npm://registry.npmjs.org;package=array-flatten;version=1.1.1'], self.d)
+        fetcher.download()
+        # Disable network access
+        self.d.setVar('BB_NO_NETWORK', '1')
+        # Fetch again
+        swfile = self.create_shrinkwrap_file({
+            'dependencies': {
+                'array-flatten': {
+                    'version': '1.1.1',
+                    'resolved': 'https://registry.npmjs.org/array-flatten/-/array-flatten-1.1.1.tgz',
+                    'integrity': 'sha1-ml9pkFGx5wczKPKgCJaLZOopVdI='
+                }
+            }
+        })
+        fetcher = bb.fetch.Fetch(['npmsw://' + swfile], self.d)
+        fetcher.download()
+        fetcher.unpack(self.unpackdir)
+        self.assertTrue(os.path.exists(os.path.join(self.sdir, 'node_modules', 'array-flatten', 'package.json')))
+
+    @skipIfNoNpm()
+    @skipIfNoNetwork()
+    def test_npmsw_npm_reusability(self):
+        # Fetch once with npmsw
+        swfile = self.create_shrinkwrap_file({
+            'dependencies': {
+                'array-flatten': {
+                    'version': '1.1.1',
+                    'resolved': 'https://registry.npmjs.org/array-flatten/-/array-flatten-1.1.1.tgz',
+                    'integrity': 'sha1-ml9pkFGx5wczKPKgCJaLZOopVdI='
+                }
+            }
+        })
+        fetcher = bb.fetch.Fetch(['npmsw://' + swfile], self.d)
+        fetcher.download()
+        # Disable network access
+        self.d.setVar('BB_NO_NETWORK', '1')
+        # Fetch again with npm
+        fetcher = bb.fetch.Fetch(['npm://registry.npmjs.org;package=array-flatten;version=1.1.1'], self.d)
+        fetcher.download()
+        fetcher.unpack(self.unpackdir)
+        self.assertTrue(os.path.exists(os.path.join(self.unpackdir, 'npm', 'package.json')))
+
+    @skipIfNoNpm()
+    @skipIfNoNetwork()
+    def test_npmsw_bad_checksum(self):
+        # Try to fetch with bad checksum
+        swfile = self.create_shrinkwrap_file({
+            'dependencies': {
+                'array-flatten': {
+                    'version': '1.1.1',
+                    'resolved': 'https://registry.npmjs.org/array-flatten/-/array-flatten-1.1.1.tgz',
+                    'integrity': 'sha1-gfNEp2hqgLTFKT6P3AsBYMgsBqg='
+                }
+            }
+        })
+        fetcher = bb.fetch.Fetch(['npmsw://' + swfile], self.d)
+        with self.assertRaises(bb.fetch2.FetchError):
+            fetcher.download()
+        # Fetch correctly to get a tarball
+        swfile = self.create_shrinkwrap_file({
+            'dependencies': {
+                'array-flatten': {
+                    'version': '1.1.1',
+                    'resolved': 'https://registry.npmjs.org/array-flatten/-/array-flatten-1.1.1.tgz',
+                    'integrity': 'sha1-ml9pkFGx5wczKPKgCJaLZOopVdI='
+                }
+            }
+        })
+        fetcher = bb.fetch.Fetch(['npmsw://' + swfile], self.d)
+        fetcher.download()
+        localpath = os.path.join(self.dldir, 'npm2', 'array-flatten-1.1.1.tgz')
+        self.assertTrue(os.path.exists(localpath))
+        # Modify the tarball
+        bad = b'bad checksum'
+        with open(localpath, 'wb') as f:
+            f.write(bad)
+        # Verify that the tarball is fetched again
+        fetcher.download()
+        badsum = hashlib.sha1(bad).hexdigest()
+        self.assertTrue(os.path.exists(localpath + '_bad-checksum_' + badsum))
+        self.assertTrue(os.path.exists(localpath))
+
+    @skipIfNoNpm()
+    @skipIfNoNetwork()
+    def test_npmsw_premirrors(self):
+        # Fetch once to get a tarball
+        fetcher = bb.fetch.Fetch(['npm://registry.npmjs.org;package=array-flatten;version=1.1.1'], self.d)
+        ud = fetcher.ud[fetcher.urls[0]]
+        fetcher.download()
+        self.assertTrue(os.path.exists(ud.localpath))
+        # Setup the mirror
+        mirrordir = os.path.join(self.tempdir, 'mirror')
+        bb.utils.mkdirhier(mirrordir)
+        os.replace(ud.localpath, os.path.join(mirrordir, os.path.basename(ud.localpath)))
+        self.d.setVar('PREMIRRORS', 'https?$://.*/.* file://%s/\n' % mirrordir)
+        self.d.setVar('BB_FETCH_PREMIRRORONLY', '1')
+        # Fetch again
+        self.assertFalse(os.path.exists(ud.localpath))
+        swfile = self.create_shrinkwrap_file({
+            'dependencies': {
+                'array-flatten': {
+                    'version': '1.1.1',
+                    'resolved': 'https://registry.npmjs.org/array-flatten/-/array-flatten-1.1.1.tgz',
+                    'integrity': 'sha1-ml9pkFGx5wczKPKgCJaLZOopVdI='
+                }
+            }
+        })
+        fetcher = bb.fetch.Fetch(['npmsw://' + swfile], self.d)
+        fetcher.download()
+        self.assertTrue(os.path.exists(ud.localpath))
+
+    @skipIfNoNpm()
+    @skipIfNoNetwork()
+    def test_npmsw_mirrors(self):
+        # Fetch once to get a tarball
+        fetcher = bb.fetch.Fetch(['npm://registry.npmjs.org;package=array-flatten;version=1.1.1'], self.d)
+        ud = fetcher.ud[fetcher.urls[0]]
+        fetcher.download()
+        self.assertTrue(os.path.exists(ud.localpath))
+        # Setup the mirror
+        mirrordir = os.path.join(self.tempdir, 'mirror')
+        bb.utils.mkdirhier(mirrordir)
+        os.replace(ud.localpath, os.path.join(mirrordir, os.path.basename(ud.localpath)))
+        self.d.setVar('MIRRORS', 'https?$://.*/.* file://%s/\n' % mirrordir)
+        # Fetch again with invalid url
+        self.assertFalse(os.path.exists(ud.localpath))
+        swfile = self.create_shrinkwrap_file({
+            'dependencies': {
+                'array-flatten': {
+                    'version': '1.1.1',
+                    'resolved': 'https://invalid',
+                    'integrity': 'sha1-ml9pkFGx5wczKPKgCJaLZOopVdI='
+                }
+            }
+        })
+        fetcher = bb.fetch.Fetch(['npmsw://' + swfile], self.d)
+        fetcher.download()
+        self.assertTrue(os.path.exists(ud.localpath))
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 08/12] bitbake: fetch2/wget: fix downloadfilename parameter
  2020-01-17 16:45 ` [PATCH v4 08/12] bitbake: fetch2/wget: fix downloadfilename parameter Jean-Marie LEMETAYER
@ 2020-01-17 20:44   ` Christopher Larson
  0 siblings, 0 replies; 14+ messages in thread
From: Christopher Larson @ 2020-01-17 20:44 UTC (permalink / raw)
  To: Jean-Marie LEMETAYER; +Cc: Paul Eggleton, bitbake-devel, rennes-dev

[-- Attachment #1: Type: text/plain, Size: 2014 bytes --]

This doesn't escape the filename, it quotes it, so should probably correct
the message, and will almost certainly break if the name contains a single
quote. I'd suggest using subprocess.list2cmdline(), pipes.quote(), or
similar instead of doing it manually. Definitely a good fix, just likely
best done with an existing mechanism :) Thanks for the contribution.

On Fri, Jan 17, 2020 at 9:52 AM Jean-Marie LEMETAYER <
jean-marie.lemetayer@savoirfairelinux.com> wrote:

> When using a download filename with characters which can be interpreted
> by the shell ('(', ')', '&', ';', ...) the command fails. Escaping the
> filename fixes the issue.
>
> Signed-off-by: Jean-Marie LEMETAYER <
> jean-marie.lemetayer@savoirfairelinux.com>
> ---
>  lib/bb/fetch2/wget.py | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/lib/bb/fetch2/wget.py b/lib/bb/fetch2/wget.py
> index 725586d2..6506e1e2 100644
> --- a/lib/bb/fetch2/wget.py
> +++ b/lib/bb/fetch2/wget.py
> @@ -94,9 +94,9 @@ class Wget(FetchMethod):
>          fetchcmd = self.basecmd
>
>          if 'downloadfilename' in ud.parm:
> -            dldir = d.getVar("DL_DIR")
> -            bb.utils.mkdirhier(os.path.dirname(dldir + os.sep +
> ud.localfile))
> -            fetchcmd += " -O " + dldir + os.sep + ud.localfile
> +            localpath = os.path.join(d.getVar("DL_DIR"), ud.localfile)
> +            bb.utils.mkdirhier(os.path.dirname(localpath))
> +            fetchcmd += " -O '%s'" % localpath
>
>          if ud.user and ud.pswd:
>              fetchcmd += " --user=%s --password=%s --auth-no-challenge" %
> (ud.user, ud.pswd)
> --
> 2.20.1
>
> --
> _______________________________________________
> bitbake-devel mailing list
> bitbake-devel@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/bitbake-devel
>


-- 
Christopher Larson
kergoth at gmail dot com
Founder - BitBake, OpenEmbedded, OpenZaurus
Senior Software Engineer, Mentor Graphics

[-- Attachment #2: Type: text/html, Size: 2977 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2020-01-17 20:44 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-17 16:45 [PATCH v4 00/12] NPM refactoring Jean-Marie LEMETAYER
2020-01-17 16:45 ` [PATCH v4 01/12] bitbake: utils: add sha384_file and sha512_file functions Jean-Marie LEMETAYER
2020-01-17 16:45 ` [PATCH v4 02/12] bitbake: utils: add is_semver function Jean-Marie LEMETAYER
2020-01-17 16:45 ` [PATCH v4 03/12] bitbake: fetch2: refactor checksum verification Jean-Marie LEMETAYER
2020-01-17 16:45 ` [PATCH v4 04/12] bitbake: fetch2: add more hash functions for " Jean-Marie LEMETAYER
2020-01-17 16:45 ` [PATCH v4 05/12] bitbake: fetch2: allow fetchers to forward the donestamp management Jean-Marie LEMETAYER
2020-01-17 16:45 ` [PATCH v4 06/12] bitbake: fetch2: allow fetchers to forward the mirrors management Jean-Marie LEMETAYER
2020-01-17 16:45 ` [PATCH v4 07/12] bitbake: fetch2: allow fetchers to forward the done condition Jean-Marie LEMETAYER
2020-01-17 16:45 ` [PATCH v4 08/12] bitbake: fetch2/wget: fix downloadfilename parameter Jean-Marie LEMETAYER
2020-01-17 20:44   ` Christopher Larson
2020-01-17 16:45 ` [PATCH v4 09/12] bitbake: fetch2/npm: refactor the npm fetcher Jean-Marie LEMETAYER
2020-01-17 16:45 ` [PATCH v4 10/12] bitbake: tests/fetch: add npm tests Jean-Marie LEMETAYER
2020-01-17 16:45 ` [PATCH v4 11/12] bitbake: fetch2: add the npmsw fetcher Jean-Marie LEMETAYER
2020-01-17 16:45 ` [PATCH v4 12/12] bitbake: tests/fetch: add npmsw tests Jean-Marie LEMETAYER

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.