[PATCH v3 0/7] NPM refactoring

[PATCH v3 0/7] NPM refactoring

[Jean-Marie LEMETAYER]

These patches are part of a set which are mainly in OE-core.

More infos can be found on the openembedded-core list.

--- V2

 - Add the 'check_network_access' function before each network access to check
   for 'BB_NO_NETWORK' and 'BB_ALLOWED_NETWORKS' variables.

 - Add a 'bb.tests.fetch.NPMTest' test suite for 'bitbake-selftest' to test the
   npm fetcher. Here is the list of the new test cases:
     - bb.tests.fetch.NPMTest.test_npm
     - bb.tests.fetch.NPMTest.test_npm_name_invalid
     - bb.tests.fetch.NPMTest.test_npm_name_none
     - 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

--- V3

 - Add two more tests regarding the BB_NO_NETWORK variable:
     - bb.tests.fetch.NPMTest.test_npm_no_network_no_tarball
     - bb.tests.fetch.NPMTest.test_npm_no_network_with_tarball

 - Restrict the version parameter to allow only semver versions and the 'latest'
   tag (do not allow the npm range formats).

 - Split the commits for better understanding.

 - Add helper functions to handle dependency fetching and unpacking in the
   npm.bbclass and the recipetool/create_npm.py files.

 - Remove the progress handler as it does not work well while fetching the
   dependencies.

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

Jean-Marie LEMETAYER (7):
  bitbake: utils.py: add sha384_file and sha512_file functions
  fetch2/npm.py: refactor the npm fetcher
  fetch2/npm.py: restrict version parameter
  fetch2/npm.py: add utility functions to handle dependencies
  fetch2/npm.py: unpack the dependencies in a node_modules directory
  fetch2/npm.py: restrict the build to be offline
  tests/fetch.py: add npm tests

 lib/bb/fetch2/npm.py  | 576 +++++++++++++++++++++++-------------------
 lib/bb/tests/fetch.py | 103 ++++++++
 lib/bb/utils.py       |  24 ++
 3 files changed, 441 insertions(+), 262 deletions(-)

--
2.20.1

[PATCH v3 1/7] bitbake: utils.py: add sha384_file and sha512_file functions

[Jean-Marie LEMETAYER]

This commit adds the "sha384_file" and "sha512_file" functions in order
to check the integrity of the downloaded npm packages as npm now use
subresource integrity:

  https://w3c.github.io/webappsec-subresource-integrity

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

diff --git a/lib/bb/utils.py b/lib/bb/utils.py
index d035949b..34152855 100644
--- a/lib/bb/utils.py
+++ b/lib/bb/utils.py
@@ -562,6 +562,30 @@ def sha1_file(filename):
             s.update(line)
     return s.hexdigest()
 
+def sha384_file(filename):
+    """
+    Return the hex string representation of the SHA384 checksum of the filename
+    """
+    import hashlib
+
+    s = hashlib.sha384()
+    with open(filename, "rb") as f:
+        for line in f:
+            s.update(line)
+    return s.hexdigest()
+
+def sha512_file(filename):
+    """
+    Return the hex string representation of the SHA512 checksum of the filename
+    """
+    import hashlib
+
+    s = hashlib.sha512()
+    with open(filename, "rb") as f:
+        for line in f:
+            s.update(line)
+    return s.hexdigest()
+
 def preserved_envvars_exported():
     """Variables which are taken from the environment and placed in and exported
     from the metadata"""
-- 
2.20.1

[PATCH v3 2/7] fetch2/npm.py: refactor the npm fetcher

[Jean-Marie LEMETAYER]

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 now be fetched
   by the npm class.

 - 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
   validated with the 'integrity' and 'shasum' provided in the npm view
   of the package [1] [2].

 - The lockdown file had generation issues and is no longer relevant
   with the latest shrinkwrap files. The shrinkwrap file is now used
   by the npm class.

1: https://docs.npmjs.com/files/package-lock.json#integrity
2: https://w3c.github.io/webappsec/specs/subresourceintegrity

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

diff --git a/lib/bb/fetch2/npm.py b/lib/bb/fetch2/npm.py
index 9700e610..e377d18a 100644
--- a/lib/bb/fetch2/npm.py
+++ b/lib/bb/fetch2/npm.py
@@ -1,301 +1,195 @@
+# Copyright (C) 2019 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:
+- name
+   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.
 
 """
 
-import os
-import sys
-import urllib.request, urllib.parse, urllib.error
+import base64
 import json
-import subprocess
-import signal
+import os
+import re
 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 ChecksumError
+from bb.fetch2 import FetchError
+from bb.fetch2 import MissingParameterError
+from bb.fetch2 import ParameterError
+from bb.fetch2 import FetchMethod
+from bb.fetch2 import URI
+from bb.fetch2 import check_network_access
+from bb.fetch2 import runfetchcmd
 
 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
+            Check if a given url can be fetched with npm
         """
         return ud.type in ['npm']
 
-    def debug(self, msg):
-        logger.debug(1, "NpmFetch: %s", msg)
-
-    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)
-
     def urldata_init(self, ud, d):
         """
-        init NPM specific variable within url data
+            Init npm specific variables 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)
+
+        # Get the 'name' parameter
+        if "name" in ud.parm:
+            ud.name = ud.parm.get("name")
+
+        if not ud.name:
+            raise MissingParameterError("Parameter 'name' required", ud.url)
+
+        # Get the 'version' parameter
+        if "version" in ud.parm:
+            ud.version = ud.parm.get("version")
+
         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]
-
-    def need_update(self, ud, d):
-        if os.path.exists(ud.localpath):
-            return False
-        return True
+            raise MissingParameterError("Parameter 'version' required", ud.url)
 
-    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'))
+        # Get the 'registry' part of the url
+        registry = re.sub(r"^npm://", "", ud.url.split(";")[0])
+        ud.registry = "http://" + registry
+
+        # Update the NPM_REGISTRY in the environment
+        d.setVar("NPM_REGISTRY", ud.registry)
+
+        # Using the 'downloadfilename' parameter as local filename or the
+        # npm package name.
+        if "downloadfilename" in ud.parm:
+            ud.basename = 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]
+            # Scoped package names (with the @) use the same naming convention
+            # as the 'npm pack' command.
+            if ud.name.startswith("@"):
+                ud.basename = re.sub("/", "-", ud.name[1:])
             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))
+                ud.basename = ud.name
+            ud.basename += "-" + ud.version + ".tgz"
+
+        ud.localfile = os.path.join("npm", registry, d.expand(ud.basename))
+
+        ud.basecmd = d.getVar("FETCHCMD_wget")
+
+        if not ud.basecmd:
+            ud.basecmd = "wget"
+            ud.basecmd += " --tries=2"
+            ud.basecmd += " --timeout=30"
+            ud.basecmd += " --passive-ftp"
+            ud.basecmd += " --no-check-certificate"
+
+    @staticmethod
+    def _run_npm_view(ud, d):
+        """
+            Run the 'npm view' command to get informations about a npm package.
+        """
+        cmd = "npm view '{}@{}'".format(ud.name, ud.version)
+        cmd += " --json"
+        cmd += " --registry={}".format(ud.registry)
+        check_network_access(d, cmd, ud.registry)
+        view_string = runfetchcmd(cmd, d)
+
+        if not view_string:
+            raise ParameterError("Invalid view from npm", ud.url)
+
+        view = json.loads(view_string)
+
+        if view.get("error") is not None:
+            raise ParameterError(view.get("error", {}).get("summary"), 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)
+        if isinstance(view, list):
+            return view[-1]
+
+        return view
+
+    @staticmethod
+    def _run_wget(ud, d, cmd):
+        """
+            Run the 'wget' command
+        """
+        check_network_access(d, cmd, ud.url)
+        runfetchcmd(cmd, d)
+
+    @staticmethod
+    def _check_integrity(integrity, filename):
+        """
+            Check the subresource integrity.
+
+            https://w3c.github.io/webappsec-subresource-integrity
+            https://www.w3.org/TR/CSP2/#source-list-syntax
+        """
+        algo, value_b64 = integrity.split("-", maxsplit=1)
+        value_hex = base64.b64decode(value_b64).hex()
+
+        if algo == "sha256":
+            return value_hex == bb.utils.sha256_file(filename)
+        elif algo == "sha384":
+            return value_hex == bb.utils.sha384_file(filename)
+        elif algo == "sha512":
+            return value_hex == bb.utils.sha512_file(filename)
+
+        return False
 
     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)
+        """
+            Fetch url
+        """
+        view = self._run_npm_view(ud, d)
+
+        uri = URI(view.get("dist", {}).get("tarball"))
+        integrity = view.get("dist", {}).get("integrity")
+        shasum = view.get("dist", {}).get("shasum")
+
+        cmd = ud.basecmd
+
+        bb.utils.mkdirhier(os.path.dirname(ud.localpath))
+        cmd += " --output-document='{}'".format(ud.localpath)
+
+        if os.path.exists(ud.localpath):
+            cmd += " --continue"
+
+        cmd += d.expand(" --directory-prefix=${DL_DIR}")
+        cmd += " '{}'".format(uri)
+
+        self._run_wget(ud, d, cmd)
+
+        if not os.path.exists(ud.localpath):
+            raise FetchError("The fetched file does not exist")
+
+        if os.path.getsize(ud.localpath) == 0:
+            os.remove(ud.localpath)
+            raise FetchError("The fetched file is empty")
+
+        if integrity is not None:
+            if not self._check_integrity(integrity, ud.localpath):
+                raise ChecksumError("The fetched file integrity mismatch")
+        elif shasum is not None:
+            if shasum != bb.utils.sha1_file(ud.localpath):
+                raise ChecksumError("The fetched file shasum mismatch")
+
+    def unpack(self, ud, rootdir, d):
+        """
+            Unpack the downloaded archive to rootdir
+        """
+        cmd = "tar --extract --gzip --file='{}'".format(ud.localpath)
+        cmd += " --no-same-owner"
+        cmd += " --transform 's:^package/:npm/:'"
+        runfetchcmd(cmd, d, workdir=rootdir)
-- 
2.20.1

[PATCH v3 3/7] fetch2/npm.py: restrict version parameter

[Jean-Marie LEMETAYER]

The npm command allows version to be actual versions, tags or ranges.
The fetcher must be more restrictive and only allows fixed versions
(which are following the semver specification [1]).

The 'latest' tag which is automatically set to the latest published
version is also allowed to make the use of the 'devtool add' command
easier.

1: https://semver.org/spec/v2.0.0.html

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

diff --git a/lib/bb/fetch2/npm.py b/lib/bb/fetch2/npm.py
index e377d18a..e253cf3e 100644
--- a/lib/bb/fetch2/npm.py
+++ b/lib/bb/fetch2/npm.py
@@ -46,6 +46,33 @@ class Npm(FetchMethod):
         """
         return ud.type in ['npm']
 
+    @staticmethod
+    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
+
     def urldata_init(self, ud, d):
         """
             Init npm specific variables within url data
@@ -65,6 +92,9 @@ class Npm(FetchMethod):
         if not ud.version:
             raise MissingParameterError("Parameter 'version' required", ud.url)
 
+        if not self._is_semver(ud.version) and not ud.version == "latest":
+            raise ParameterError("Parameter 'version' is invalid", ud.url)
+
         # Get the 'registry' part of the url
         registry = re.sub(r"^npm://", "", ud.url.split(";")[0])
         ud.registry = "http://" + registry
@@ -96,6 +126,18 @@ class Npm(FetchMethod):
             ud.basecmd += " --passive-ftp"
             ud.basecmd += " --no-check-certificate"
 
+    def need_update(self, ud, d):
+        """
+            Force a fetch, even if localpath exists?
+        """
+        if ud.version == "latest":
+            return True
+
+        if os.path.exists(ud.localpath):
+            return False
+
+        return True
+
     @staticmethod
     def _run_npm_view(ud, d):
         """
@@ -158,6 +200,14 @@ class Npm(FetchMethod):
         integrity = view.get("dist", {}).get("integrity")
         shasum = view.get("dist", {}).get("shasum")
 
+        # Check if version is valid
+        if ud.version == "latest":
+            bb.warn("The npm package '{}' is using the latest version " \
+                    "available. This could lead to non-reproducible " \
+                    "builds.".format(ud.name))
+        elif ud.version != view.get("version"):
+            raise ParameterError("Parameter 'version' is invalid", ud.url)
+
         cmd = ud.basecmd
 
         bb.utils.mkdirhier(os.path.dirname(ud.localpath))
-- 
2.20.1

[PATCH v3 4/7] fetch2/npm.py: add utility functions to handle dependencies

[Jean-Marie LEMETAYER]

These utility functions are used parse a shrinkwrap file and run actions
for each dependencies. Some actions are also provided like fetching and
unpacking. They will be useful in the class and recipetool.

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

diff --git a/lib/bb/fetch2/npm.py b/lib/bb/fetch2/npm.py
index e253cf3e..bd6e32ca 100644
--- a/lib/bb/fetch2/npm.py
+++ b/lib/bb/fetch2/npm.py
@@ -243,3 +243,98 @@ class Npm(FetchMethod):
         cmd += " --no-same-owner"
         cmd += " --transform 's:^package/:npm/:'"
         runfetchcmd(cmd, d, workdir=rootdir)
+
+def _parse_shrinkwrap(d, shrinkwrap_file=None):
+    """
+        Find and parse the shrinkwrap file to use.
+    """
+
+    def get_shrinkwrap_file(d):
+        src_shrinkwrap = d.expand("${S}/npm-shrinkwrap.json")
+        npm_shrinkwrap = d.getVar("NPM_SHRINKWRAP")
+
+        if os.path.exists(src_shrinkwrap):
+            bb.note("Using the npm-shrinkwrap.json provided in the sources")
+            return src_shrinkwrap
+        elif os.path.exists(npm_shrinkwrap):
+            return npm_shrinkwrap
+
+        bb.fatal("No mandatory NPM_SHRINKWRAP file found")
+
+    if shrinkwrap_file is None:
+        shrinkwrap_file = get_shrinkwrap_file(d)
+
+    with open(shrinkwrap_file, "r") as f:
+        shrinkwrap = json.load(f)
+
+    return shrinkwrap
+
+def foreach_dependencies(d, callback=None, shrinkwrap_file=None):
+    """
+        Run a callback for each dependencies of a shrinkwrap file.
+        The callback is using the format:
+            callback(name, version, deptree)
+        with:
+            name = the packet name (string)
+            version = the packet version (string)
+            deptree = the package dependency tree (array of strings)
+    """
+    shrinkwrap = _parse_shrinkwrap(d, shrinkwrap_file)
+
+    def walk_deps(deps, deptree):
+        out = []
+
+        for name in deps:
+            version = deps[name]["version"]
+            subtree = [*deptree, name]
+            out.extend(walk_deps(deps[name].get("dependencies", {}), subtree))
+            if callback is not None:
+                out.append(callback(name, version, subtree))
+
+        return out
+
+    return walk_deps(shrinkwrap.get("dependencies", {}), [])
+
+def _get_url(d, name, version):
+    registry = re.sub(r"https?://", "npm://", d.getVar("NPM_REGISTRY"))
+    url = "{};name={};version={}".format(registry, name, version)
+    return url
+
+def fetch_dependency(d, name, version):
+    """
+        Fetch a dependency and return the tarball path.
+    """
+    url = _get_url(d, name, version)
+    fetcher = bb.fetch2.Fetch([url], d)
+    fetcher.download()
+    return fetcher.localpath(url)
+
+def fetch_dependencies(d, shrinkwrap_file=None):
+    """
+        Fetch all dependencies of a shrinkwrap file.
+    """
+
+    def handle_dependency(name, version, *unused):
+        fetch_dependency(d, name, version)
+
+    foreach_dependencies(d, handle_dependency, shrinkwrap_file)
+
+def unpack_dependencies(d, shrinkwrap_file=None):
+    """
+        Unpack all dependencies of a shrinkwrap file. The dependencies are
+        unpacked in the source tree and added to the npm cache.
+    """
+    bb.utils.remove(d.getVar("NPM_CACHE_DIR"), recurse=True)
+
+    def cache_dependency(tarball):
+        cmd = "npm cache add '{}'".format(tarball)
+        cmd += d.expand(" --cache=${NPM_CACHE_DIR}")
+        runfetchcmd(cmd, d)
+
+    def handle_dependency(name, version, *unused):
+        url = _get_url(d, name, version)
+        fetcher = bb.fetch2.Fetch([url], d)
+        tarball = fetcher.localpath(url)
+        cache_dependency(tarball)
+
+    foreach_dependencies(d, handle_dependency, shrinkwrap_file)
-- 
2.20.1

[PATCH v3 5/7] fetch2/npm.py: unpack the dependencies in a node_modules directory

[Jean-Marie LEMETAYER]

This commit unpacks the dependencies in a node_modules directory in the
source tree. This is needed to allow license file management for all the
dependencies. This way the license files of the dependencies can be
tracked in the LIC_FILES_CHKSUM variable.

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

diff --git a/lib/bb/fetch2/npm.py b/lib/bb/fetch2/npm.py
index bd6e32ca..9beb84bf 100644
--- a/lib/bb/fetch2/npm.py
+++ b/lib/bb/fetch2/npm.py
@@ -325,16 +325,27 @@ def unpack_dependencies(d, shrinkwrap_file=None):
         unpacked in the source tree and added to the npm cache.
     """
     bb.utils.remove(d.getVar("NPM_CACHE_DIR"), recurse=True)
+    bb.utils.remove(d.expand("${S}/node_modules/"), recurse=True)
 
     def cache_dependency(tarball):
         cmd = "npm cache add '{}'".format(tarball)
         cmd += d.expand(" --cache=${NPM_CACHE_DIR}")
         runfetchcmd(cmd, d)
 
-    def handle_dependency(name, version, *unused):
+    def unpack_dependency(tarball, srctree):
+        cmd = "tar --extract --gzip --file='{}'".format(tarball)
+        cmd += " --no-same-owner"
+        cmd += " --transform 's:^package/::'"
+        bb.utils.mkdirhier(srctree)
+        runfetchcmd(cmd, d, workdir=srctree)
+
+    def handle_dependency(name, version, deptree):
         url = _get_url(d, name, version)
         fetcher = bb.fetch2.Fetch([url], d)
         tarball = fetcher.localpath(url)
         cache_dependency(tarball)
+        relpath = os.path.join(*[os.path.join("node_modules", d) for d in deptree])
+        abspath = os.path.join(d.getVar("S"), relpath)
+        unpack_dependency(tarball, abspath)
 
     foreach_dependencies(d, handle_dependency, shrinkwrap_file)
-- 
2.20.1

[PATCH v3 6/7] fetch2/npm.py: restrict the build to be offline

[Jean-Marie LEMETAYER]

After the do_fetch task, every other tasks must not access the network.
In order to ensure this point every npm command must use the '--offline'
argument. In addition, setting an invalid proxy is used as a safety.

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

diff --git a/lib/bb/fetch2/npm.py b/lib/bb/fetch2/npm.py
index 9beb84bf..b365d5fc 100644
--- a/lib/bb/fetch2/npm.py
+++ b/lib/bb/fetch2/npm.py
@@ -329,6 +329,8 @@ def unpack_dependencies(d, shrinkwrap_file=None):
 
     def cache_dependency(tarball):
         cmd = "npm cache add '{}'".format(tarball)
+        cmd += " --offline"
+        cmd += " --proxy=http://invalid.org"
         cmd += d.expand(" --cache=${NPM_CACHE_DIR}")
         runfetchcmd(cmd, d)
 
-- 
2.20.1

[PATCH v3 7/7] tests/fetch.py: add npm tests

[Jean-Marie LEMETAYER]

This commit adds some basic tests for the npm fetcher:

 - bb.tests.fetch.NPMTest.test_npm
 - bb.tests.fetch.NPMTest.test_npm_name_invalid
 - bb.tests.fetch.NPMTest.test_npm_name_none
 - 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_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 | 103 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 103 insertions(+)

diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py
index 83fad3ff..d1091fa7 100644
--- a/lib/bb/tests/fetch.py
+++ b/lib/bb/tests/fetch.py
@@ -2008,3 +2008,106 @@ 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;name=@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_no_network_no_tarball(self):
+        url = 'npm://registry.npmjs.org;name=@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;name=@savoirfairelinux/node-server-example;version=1.0.0'
+        fetcher = bb.fetch.Fetch([url], self.d)
+        fetcher.download()
+        self.d.setVar('BB_NO_NETWORK', '1')
+        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_alternate(self):
+        url = 'npm://registry.freajs.org;name=@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;name=@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;name=@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_name_invalid(self):
+        url = 'npm://registry.npmjs.org;name=@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;name=@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://;name=@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_name_none(self):
+        url = 'npm://registry.npmjs.org;version=1.0.0'
+        with self.assertRaises(AttributeError):
+            fetcher = bb.fetch.Fetch([url], self.d)
+
+    @skipIfNoNpm()
+    @skipIfNoNetwork()
+    def test_npm_version_none(self):
+        url = 'npm://registry.npmjs.org;name=@savoirfairelinux/node-server-example'
+        with self.assertRaises(AttributeError):
+            fetcher = bb.fetch.Fetch([url], self.d)
-- 
2.20.1

Re: [PATCH v3 7/7] tests/fetch.py: add npm tests

[Mark Hatle]

First thanks for the tests!  A few comments inline

On 11/20/19 3:34 AM, Jean-Marie LEMETAYER wrote:
> This commit adds some basic tests for the npm fetcher:
> 
>  - bb.tests.fetch.NPMTest.test_npm
>  - bb.tests.fetch.NPMTest.test_npm_name_invalid
>  - bb.tests.fetch.NPMTest.test_npm_name_none
>  - 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_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 | 103 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 103 insertions(+)
> 
> diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py
> index 83fad3ff..d1091fa7 100644
> --- a/lib/bb/tests/fetch.py
> +++ b/lib/bb/tests/fetch.py
> @@ -2008,3 +2008,106 @@ 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;name=@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_no_network_no_tarball(self):
> +        url = 'npm://registry.npmjs.org;name=@savoirfairelinux/node-server-example;version=1.0.0'
> +        self.d.setVar('BB_NO_NETWORK', '1')

Since this is a BB_NO_NETWORK=1 case, the @skipIfNoNetwork() should not be here,
since it should not need the network to perform the test.

> +        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;name=@savoirfairelinux/node-server-example;version=1.0.0'
> +        fetcher = bb.fetch.Fetch([url], self.d)
> +        fetcher.download()
> +        self.d.setVar('BB_NO_NETWORK', '1')

Same here.

If any other the ones below work off local files, then similarly we shouldn't
skip running them on a no network config.

--Mark

> +        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_alternate(self):
> +        url = 'npm://registry.freajs.org;name=@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;name=@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;name=@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_name_invalid(self):
> +        url = 'npm://registry.npmjs.org;name=@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;name=@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://;name=@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_name_none(self):
> +        url = 'npm://registry.npmjs.org;version=1.0.0'
> +        with self.assertRaises(AttributeError):
> +            fetcher = bb.fetch.Fetch([url], self.d)
> +
> +    @skipIfNoNpm()
> +    @skipIfNoNetwork()
> +    def test_npm_version_none(self):
> +        url = 'npm://registry.npmjs.org;name=@savoirfairelinux/node-server-example'
> +        with self.assertRaises(AttributeError):
> +            fetcher = bb.fetch.Fetch([url], self.d)
> 

Re: [PATCH v3 7/7] tests/fetch.py: add npm tests

[Jean-Marie LEMETAYER]

Hi Mark,

On Nov 20, 2019, at 7:29 PM, Mark Hatle mark.hatle@kernel.crashing.org wrote:
> First thanks for the tests!  A few comments inline
> 
> On 11/20/19 3:34 AM, Jean-Marie LEMETAYER wrote:
>> +
>> +    @skipIfNoNpm()
>> +    @skipIfNoNetwork()
>> +    def test_npm_no_network_no_tarball(self):
>> +        url =
>> 'npm://registry.npmjs.org;name=@savoirfairelinux/node-server-example;version=1.0.0'
>> +        self.d.setVar('BB_NO_NETWORK', '1')
> 
> Since this is a BB_NO_NETWORK=1 case, the @skipIfNoNetwork() should not be here,
> since it should not need the network to perform the test.

Yes it is correct. In fact the @skipIfNoNpm() is not useful either as the
bb.fetch2.NetworkAccess exception should be raised before any npm commands.

>> +        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;name=@savoirfairelinux/node-server-example;version=1.0.0'
>> +        fetcher = bb.fetch.Fetch([url], self.d)
>> +        fetcher.download()
>> +        self.d.setVar('BB_NO_NETWORK', '1')
> 
> Same here.
> 
> If any other the ones below work off local files, then similarly we shouldn't
> skip running them on a no network config.

As the DL_DIR point to a temporary directory created in the FetcherTest::setUp()
function when starting the test suite I think there is no way to work with local
files if you haven't download it first (which implies a network access).
Or did I miss something ?

>> +        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'))

Regards,
Jean-Marie

Re: [PATCH v3 7/7] tests/fetch.py: add npm tests

[Mark Hatle]

On 11/21/19 6:54 AM, Jean-Marie LEMETAYER wrote:
> Hi Mark,
> 
> On Nov 20, 2019, at 7:29 PM, Mark Hatle mark.hatle@kernel.crashing.org wrote:
>> First thanks for the tests!  A few comments inline
>>
>> On 11/20/19 3:34 AM, Jean-Marie LEMETAYER wrote:
>>> +
>>> +    @skipIfNoNpm()
>>> +    @skipIfNoNetwork()
>>> +    def test_npm_no_network_no_tarball(self):
>>> +        url =
>>> 'npm://registry.npmjs.org;name=@savoirfairelinux/node-server-example;version=1.0.0'
>>> +        self.d.setVar('BB_NO_NETWORK', '1')
>>
>> Since this is a BB_NO_NETWORK=1 case, the @skipIfNoNetwork() should not be here,
>> since it should not need the network to perform the test.
> 
> Yes it is correct. In fact the @skipIfNoNpm() is not useful either as the
> bb.fetch2.NetworkAccess exception should be raised before any npm commands.
> 
>>> +        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;name=@savoirfairelinux/node-server-example;version=1.0.0'
>>> +        fetcher = bb.fetch.Fetch([url], self.d)
>>> +        fetcher.download()
>>> +        self.d.setVar('BB_NO_NETWORK', '1')
>>
>> Same here.
>>
>> If any other the ones below work off local files, then similarly we shouldn't
>> skip running them on a no network config.
> 
> As the DL_DIR point to a temporary directory created in the FetcherTest::setUp()
> function when starting the test suite I think there is no way to work with local
> files if you haven't download it first (which implies a network access).
> Or did I miss something ?

I didn't notice thi one has the BB_NO_NETWORK -after- the download.  So ya, I
see how it requires the network.

--Mark

>>> +        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'))
> 
> Regards,
> Jean-Marie
> 

Re: [PATCH v3 1/7] bitbake: utils.py: add sha384_file and sha512_file functions

[Richard Purdie]

On Wed, 2019-11-20 at 10:34 +0100, Jean-Marie LEMETAYER wrote:
> This commit adds the "sha384_file" and "sha512_file" functions in
> order
> to check the integrity of the downloaded npm packages as npm now use
> subresource integrity:
> 
>   https://w3c.github.io/webappsec-subresource-integrity
> 
> Signed-off-by: Jean-Marie LEMETAYER <
> jean-marie.lemetayer@savoirfairelinux.com>
> ---
>  lib/bb/utils.py | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/lib/bb/utils.py b/lib/bb/utils.py
> index d035949b..34152855 100644
> --- a/lib/bb/utils.py
> +++ b/lib/bb/utils.py
> @@ -562,6 +562,30 @@ def sha1_file(filename):
>              s.update(line)
>      return s.hexdigest()
>  
> +def sha384_file(filename):
> +    """
> +    Return the hex string representation of the SHA384 checksum of
> the filename
> +    """
> +    import hashlib
> +
> +    s = hashlib.sha384()
> +    with open(filename, "rb") as f:
> +        for line in f:
> +            s.update(line)
> +    return s.hexdigest()
> +
> +def sha512_file(filename):
> +    """
> +    Return the hex string representation of the SHA512 checksum of
> the filename
> +    """
> +    import hashlib
> +
> +    s = hashlib.sha512()
> +    with open(filename, "rb") as f:
> +        for line in f:
> +            s.update(line)
> +    return s.hexdigest()
> +
>  def preserved_envvars_exported():

Note that:
http://git.yoctoproject.org/cgit.cgi/poky/commit/?id=9d2fd91844fe387186b780b5457d0c02bea998e7 
merged, so this patch probably needs tweaking to match, thanks.

Cheers,

Richard