All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 00/17] NPM refactoring
@ 2019-11-20  9:33 Jean-Marie LEMETAYER
  2019-11-20  9:33 ` [PATCH v3 01/17] devtool: npm: update command line options Jean-Marie LEMETAYER
                   ` (18 more replies)
  0 siblings, 19 replies; 29+ messages in thread
From: Jean-Marie LEMETAYER @ 2019-11-20  9:33 UTC (permalink / raw)
  To: openembedded-core; +Cc: jonaskgandersson, paul.eggleton, rennes, bunk

The current NPM support have several issues:
 - The current NPM fetcher downloads the dependency tree but not the other
   fetchers. The 'subdir' parameter was used to fix this issue.
 - They are multiple issues with package names (uppercase, exotic characters,
   scoped packages) even if they are inside the dependencies.
 - The lockdown file generation have issues. When a package depends on
   multiple version of the same package (all versions have the same checksum).

This patchset refactors the NPM support in Yocto:
 - As the NPM algorithm for dependency management is hard to handle, the new
   NPM fetcher downloads only the package source (and not the dependencies,
   like the other fetchers) (patch submitted in the bitbake-devel list).
 - The NPM class handles the dependencies using NPM (and not manually).
 - The NPM recipe creation is simplified to avoid issues.
 - The lockdown file is no more used as it is no longer relevant compared to the
   latest shrinkwrap file format.

This patchset may remove some features (lockdown file, license management for
dependencies) but fixes the majority of the NPM issues. All of these issues
from the bugzilla.yoctoproject.org are resolved by this patchset:
#10237, #10760, #11028, #11728, #11902, #12534

The fetcher and recipetool are now aware of a 'latest' keyword for the version
which allow to build the latest version available on the registry. This feature
fixes the two issues: #10515, #11029

Moreover the issue #13415 should also be fixed but I cannot test it.

I have tested the recipe creation and builds using a self made example to
generate build failures:
 - https://github.com/savoirfairelinux/node-server-example
 - https://npmjs.com/package/@savoirfairelinux/node-server-example

The test steps are these ones:
  $ source poky/oe-init-build-env
  $ bitbake-layers add-layer ../meta-openembedded/meta-oe
  $ devtool add "npm://registry.npmjs.org;name=@savoirfairelinux/node-server-example;version=latest"
  $ devtool build savoirfairelinux-node-server-example
  $ bitbake-layers create-layer ../meta-test
  $ bitbake-layers add-layer ../meta-test
  $ devtool finish savoirfairelinux-node-server-example ../meta-test
  $ echo IMAGE_INSTALL_append = '" savoirfairelinux-node-server-example"' >> conf/local.conf
  $ bitbake core-image-minimal

Also the 'devtool add' url could be one of these:
 - npm://registry.npmjs.org;name=@savoirfairelinux/node-server-example;version=latest
   This url uses the new npm fetcher and request the latest version available on
   the registry.
 - npm://registry.npmjs.org;name=@savoirfairelinux/node-server-example;version=1.0.0
   This url uses the new npm fetcher and request a fixed version.
 - git://github.com/savoirfairelinux/node-server-example.git;protocol=https
   This url uses the git fetcher. As the dependencies are managed by the NPM
   class, any fetcher can be used.

When this patchset will be merged, I have planned to update the NPM wiki:
  https://wiki.yoctoproject.org/wiki/TipsAndTricks/NPM

This patchset is also the base work for the full Angular support in Yocto that I
am preparing. These applications have a huge dependency tree which is ideal to
test the NPM support.

--- V2

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

 - Add a 'recipetool.RecipetoolTests.test_recipetool_create_npm' test case for
   'oe-selftest' to test the npm recipe creation.

 - Update the 'npm.bbclass' to fetch the dependencies in the 'do_fetch' task.
   The dependencies are cached in a npm cache stored in '${DL_DIR}/npm_cache'
   allowing the dependencies to be saved in the download directory and verify
   on insertion and extraction [1]:
      "All data that passes through the cache is fully verified
       for integrity on both insertion and extraction."

1: https://docs.npmjs.com/cli/cache.html

--- V3

 - Add a test regarding the 'devtool add' and 'devtool build' command:
     - devtool.DevtoolAddTests.test_devtool_add_npm

 - Add the license management for dependencies.

 - Split the npm workflow to use the bitbake default tasks (do_fetch, do_unpack,
   do_compile, do_install). Make sure that only the do_fetch task requires
   network access.

 - Split the commits for better understanding.

 - Force the rebuild of prebuild addons.

 - Use ${nonarch_libdir} instead of ${libdir} in the do_install task.

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

Jean-Marie LEMETAYER (17):
  devtool: npm: update command line options
  npm.bbclass: refactor the npm class
  recipetool/create_npm.py: refactor the npm recipe creation handler
  lib/oe/package.py: remove unneeded npm_split_package_dirs function
  devtool/standard.py: npm: update the append file
  recipetool/create.py: npm: remove the 'noverify' url parameter
  recipetool/create.py: npm: replace the 'latest' keyword
  npm.bbclass: split the do_compile task
  devtool/standard.py: npm: exclude the node_modules directory
  recipetool/create_npm.py: handle the licenses of the dependencies
  npm.bbclass: restrict the build to be offline
  npm.bbclass: use the local node headers
  recipetool/create_npm.py: convert the shrinkwrap file
  npm.bbclass: force to rebuild the prebuild addons
  devtool/standard.py: npm: configure the NPM_CACHE_DIR to be persistent
  oeqa/selftest/recipetool: add npm recipe creation test
  oeqa/selftest/devtool: add npm recipe build test

 meta/classes/npm.bbclass                   | 224 +++++---
 meta/lib/oe/package.py                     |  33 --
 meta/lib/oeqa/selftest/cases/devtool.py    |  20 +
 meta/lib/oeqa/selftest/cases/recipetool.py |  21 +
 scripts/lib/devtool/standard.py            |  31 +-
 scripts/lib/recipetool/create.py           |  16 +-
 scripts/lib/recipetool/create_npm.py       | 565 +++++++++++----------
 7 files changed, 510 insertions(+), 400 deletions(-)

--
2.20.1



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

* [PATCH v3 01/17] devtool: npm: update command line options
  2019-11-20  9:33 [PATCH v3 00/17] NPM refactoring Jean-Marie LEMETAYER
@ 2019-11-20  9:33 ` Jean-Marie LEMETAYER
  2019-11-20  9:33 ` [PATCH v3 02/17] npm.bbclass: refactor the npm class Jean-Marie LEMETAYER
                   ` (17 subsequent siblings)
  18 siblings, 0 replies; 29+ messages in thread
From: Jean-Marie LEMETAYER @ 2019-11-20  9:33 UTC (permalink / raw)
  To: openembedded-core; +Cc: jonaskgandersson, paul.eggleton, rennes, bunk

This commit renames the '--fetch-dev' option into '--npm-dev' which is
more easily understandable.

It also adds the '--npm-registry' option to allow creating a npm recipe
with a non default npm registry (e.g. if the SRC_URI is using git://).

Signed-off-by: Jean-Marie LEMETAYER <jean-marie.lemetayer@savoirfairelinux.com>
---
 scripts/lib/devtool/standard.py  |  9 ++++++---
 scripts/lib/recipetool/create.py | 12 +++++++-----
 2 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
index 8d9c1a3022..7068a02a01 100644
--- a/scripts/lib/devtool/standard.py
+++ b/scripts/lib/devtool/standard.py
@@ -145,8 +145,10 @@ def add(args, config, basepath, workspace):
         extracmdopts += ' --src-subdir "%s"' % args.src_subdir
     if args.autorev:
         extracmdopts += ' -a'
-    if args.fetch_dev:
-        extracmdopts += ' --fetch-dev'
+    if args.npm_dev:
+        extracmdopts += ' --npm-dev'
+    if args.npm_registry:
+        extracmdopts += ' --npm-registry "%s"' % args.npm_registry
     if args.mirrors:
         extracmdopts += ' --mirrors'
     if args.srcrev:
@@ -2197,7 +2199,8 @@ def register_commands(subparsers, context):
     group.add_argument('--same-dir', '-s', help='Build in same directory as source', action="store_true")
     group.add_argument('--no-same-dir', help='Force build in a separate build directory', action="store_true")
     parser_add.add_argument('--fetch', '-f', help='Fetch the specified URI and extract it to create the source tree (deprecated - pass as positional argument instead)', metavar='URI')
-    parser_add.add_argument('--fetch-dev', help='For npm, also fetch devDependencies', action="store_true")
+    parser_add.add_argument('--npm-dev', help='For npm, also fetch devDependencies', action="store_true")
+    parser_add.add_argument('--npm-registry', help='For npm, use the specified registry', type=str)
     parser_add.add_argument('--version', '-V', help='Version to use within recipe (PV)')
     parser_add.add_argument('--no-git', '-g', help='If fetching source, do not set up source tree as a git repository', action="store_true")
     group = parser_add.add_mutually_exclusive_group()
diff --git a/scripts/lib/recipetool/create.py b/scripts/lib/recipetool/create.py
index 1fb6b55530..932dc3f374 100644
--- a/scripts/lib/recipetool/create.py
+++ b/scripts/lib/recipetool/create.py
@@ -716,10 +716,11 @@ def create_recipe(args):
         lines_after.append('INSANE_SKIP_${PN} += "already-stripped"')
         lines_after.append('')
 
-    if args.fetch_dev:
-        extravalues['fetchdev'] = True
-    else:
-        extravalues['fetchdev'] = None
+    if args.npm_dev:
+        extravalues['NPM_INSTALL_DEV'] = 1
+
+    if args.npm_registry:
+        extravalues['NPM_REGISTRY'] = args.npm_registry
 
     # Find all plugins that want to register handlers
     logger.debug('Loading recipe handlers')
@@ -1315,7 +1316,8 @@ def register_commands(subparsers):
     group.add_argument('-S', '--srcrev', help='Source revision to fetch if fetching from an SCM such as git (default latest)')
     parser_create.add_argument('-B', '--srcbranch', help='Branch in source repository if fetching from an SCM such as git (default master)')
     parser_create.add_argument('--keep-temp', action="store_true", help='Keep temporary directory (for debugging)')
-    parser_create.add_argument('--fetch-dev', action="store_true", help='For npm, also fetch devDependencies')
+    parser_create.add_argument('--npm-dev', action="store_true", help='For npm, also fetch devDependencies')
+    parser_create.add_argument('--npm-registry', help='For npm, use the specified registry', type=str)
     parser_create.add_argument('--devtool', action="store_true", help=argparse.SUPPRESS)
     parser_create.add_argument('--mirrors', action="store_true", help='Enable PREMIRRORS and MIRRORS for source tree fetching (disabled by default).')
     parser_create.set_defaults(func=create_recipe)
-- 
2.20.1



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

* [PATCH v3 02/17] npm.bbclass: refactor the npm class
  2019-11-20  9:33 [PATCH v3 00/17] NPM refactoring Jean-Marie LEMETAYER
  2019-11-20  9:33 ` [PATCH v3 01/17] devtool: npm: update command line options Jean-Marie LEMETAYER
@ 2019-11-20  9:33 ` Jean-Marie LEMETAYER
  2019-11-20  9:33 ` [PATCH v3 03/17] recipetool/create_npm.py: refactor the npm recipe creation handler Jean-Marie LEMETAYER
                   ` (16 subsequent siblings)
  18 siblings, 0 replies; 29+ messages in thread
From: Jean-Marie LEMETAYER @ 2019-11-20  9:33 UTC (permalink / raw)
  To: openembedded-core; +Cc: jonaskgandersson, paul.eggleton, rennes, bunk

Many issues were related to npm dependencies badly handled: package
names, installation directories, ... In fact npm is using an install
algorithm [1] which is hard to reproduce / anticipate. Moreover some
npm packages use scopes [2] which adds more complexity.

The simplest solution is to let npm do its job. Assuming the fetcher
only get the sources of the package, the class will now run
'npm install' to create a build directory. The build directory is then
copied wisely to the destination.

1: https://docs.npmjs.com/cli/install#algorithm
2: https://docs.npmjs.com/about-scopes

Signed-off-by: Jean-Marie LEMETAYER <jean-marie.lemetayer@savoirfairelinux.com>
---
 meta/classes/npm.bbclass | 199 ++++++++++++++++++++++++++-------------
 1 file changed, 132 insertions(+), 67 deletions(-)

diff --git a/meta/classes/npm.bbclass b/meta/classes/npm.bbclass
index 4b1f0a39f0..b3f73d9b67 100644
--- a/meta/classes/npm.bbclass
+++ b/meta/classes/npm.bbclass
@@ -1,19 +1,40 @@
+# Copyright (C) 2019 Savoir-Faire Linux
+#
+# This bbclass builds and installs an npm package to the target. The package
+# sources files should be fetched in the calling recipe by using the SRC_URI
+# variable. The ${S} variable should be updated depending of your fetcher.
+#
+# Usage:
+#  SRC_URI = "..."
+#  inherit npm
+#
+# Optional variables:
+#  NPM_SHRINKWRAP:
+#       Provide a shrinkwrap file [1]. If available a shrinkwrap file in the
+#       sources has priority over the one provided. A shrinkwrap file is
+#       mandatory in order to ensure build reproducibility.
+#       1: https://docs.npmjs.com/files/shrinkwrap.json
+#
+#  NPM_INSTALL_DEV:
+#       Set to 1 to also install devDependencies.
+#
+#  NPM_REGISTRY:
+#       Use the specified registry.
+#
+#  NPM_ARCH:
+#       Override the auto generated npm architecture.
+
 DEPENDS_prepend = "nodejs-native "
 RDEPENDS_${PN}_prepend = "nodejs "
-S = "${WORKDIR}/npmpkg"
 
-def node_pkgname(d):
-    bpn = d.getVar('BPN')
-    if bpn.startswith("node-"):
-        return bpn[5:]
-    return bpn
+NPM_SHRINKWRAP ?= "${THISDIR}/${BPN}/npm-shrinkwrap.json"
 
-NPMPN ?= "${@node_pkgname(d)}"
+NPM_INSTALL_DEV ?= "0"
 
-NPM_INSTALLDIR = "${libdir}/node_modules/${NPMPN}"
+NPM_REGISTRY ?= "http://registry.npmjs.org"
 
 # function maps arch names to npm arch names
-def npm_oe_arch_map(target_arch, d):
+def npm_oe_arch_map(target_arch):
     import re
     if   re.match('p(pc|owerpc)(|64)', target_arch): return 'ppc'
     elif re.match('i.86$', target_arch): return 'ia32'
@@ -21,74 +42,118 @@ def npm_oe_arch_map(target_arch, d):
     elif re.match('arm64$', target_arch): return 'arm'
     return target_arch
 
-NPM_ARCH ?= "${@npm_oe_arch_map(d.getVar('TARGET_ARCH'), d)}"
-NPM_INSTALL_DEV ?= "0"
+NPM_ARCH ?= "${@npm_oe_arch_map(d.getVar('TARGET_ARCH'))}"
+
+NPM_CACHE_DIR ?= "${WORKDIR}/npm_cache"
+
+B = "${WORKDIR}/build"
+
+npm_install_shrinkwrap() {
+    # This function ensures that there is a shrinkwrap file in the specified
+    # directory. A shrinkwrap file is mandatory to have reproducible builds.
+    # If the shrinkwrap file is not already included in the sources,
+    # the recipe can provide one by using the NPM_SHRINKWRAP option.
+    # This function returns the filename of the installed file (if any).
+    if [ -f ${S}/npm-shrinkwrap.json ]
+    then
+        bbnote "Using the npm-shrinkwrap.json provided in the sources"
+    elif [ -f ${NPM_SHRINKWRAP} ]
+    then
+        install -m 644 ${NPM_SHRINKWRAP} ${S}/npm-shrinkwrap.json
+        echo ${S}/npm-shrinkwrap.json
+    else
+        bbfatal "No mandatory NPM_SHRINKWRAP file found"
+    fi
+}
 
 npm_do_compile() {
-	# Copy in any additionally fetched modules
-	if [ -d ${WORKDIR}/node_modules ] ; then
-		cp -a ${WORKDIR}/node_modules ${S}/
-	fi
-	# changing the home directory to the working directory, the .npmrc will
-	# be created in this directory
-	export HOME=${WORKDIR}
-	if [  "${NPM_INSTALL_DEV}" = "1" ]; then
-		npm config set dev true
-	else
-		npm config set dev false
-	fi
-	npm set cache ${WORKDIR}/npm_cache
-	# clear cache before every build
-	npm cache clear --force
-	# Install pkg into ${S} without going to the registry
-	if [  "${NPM_INSTALL_DEV}" = "1" ]; then
-		npm --arch=${NPM_ARCH} --target_arch=${NPM_ARCH} --no-registry install
-	else
-		npm --arch=${NPM_ARCH} --target_arch=${NPM_ARCH} --production --no-registry install
-	fi
+    # This function executes the 'npm install' command which builds and
+    # installs every dependencies needed for the package. All the files are
+    # installed in a build directory ${B} without filtering anything. To do so,
+    # a combination of 'npm pack' and 'npm install' is used to ensure that the
+    # files in ${B} are actual copies instead of symbolic links (which is the
+    # default npm behavior).
+
+    # First ensure that there is a shrinkwrap file in the sources.
+    local NPM_SHRINKWRAP_INSTALLED=$(npm_install_shrinkwrap)
+
+    # Then create a tarball from a npm package whose sources must be in ${S}.
+    local NPM_PACK_FILE=$(cd ${WORKDIR} && npm pack ${S}/)
+
+    # Clean source tree.
+    rm -f ${NPM_SHRINKWRAP_INSTALLED}
+
+    # Remove the build directory.
+    rm -rf ${B}
+
+    # Finally install and build the tarball package in ${B}.
+    local NPM_INSTALL_ARGS="${NPM_INSTALL_ARGS} --cache=${NPM_CACHE_DIR}"
+    local NPM_INSTALL_ARGS="${NPM_INSTALL_ARGS} --loglevel=silly"
+    local NPM_INSTALL_ARGS="${NPM_INSTALL_ARGS} --prefix=${B}"
+    local NPM_INSTALL_ARGS="${NPM_INSTALL_ARGS} --global"
+
+    if [ "${NPM_INSTALL_DEV}" != 1 ]
+    then
+        local NPM_INSTALL_ARGS="${NPM_INSTALL_ARGS} --production"
+    fi
+
+    local NPM_INSTALL_GYP_ARGS="${NPM_INSTALL_GYP_ARGS} --arch=${NPM_ARCH}"
+    local NPM_INSTALL_GYP_ARGS="${NPM_INSTALL_GYP_ARGS} --target_arch=${NPM_ARCH}"
+    local NPM_INSTALL_GYP_ARGS="${NPM_INSTALL_GYP_ARGS} --release"
+
+    cd ${WORKDIR} && npm install ${NPM_PACK_FILE} \
+        ${NPM_INSTALL_GYP_ARGS} \
+        ${NPM_INSTALL_ARGS}
 }
 
 npm_do_install() {
-	# changing the home directory to the working directory, the .npmrc will
-	# be created in this directory
-	export HOME=${WORKDIR}
-	mkdir -p ${D}${libdir}/node_modules
-	local NPM_PACKFILE=$(npm pack .)
-	npm install --prefix ${D}${prefix} -g --arch=${NPM_ARCH} --target_arch=${NPM_ARCH} --production --no-registry ${NPM_PACKFILE}
-	ln -fs node_modules ${D}${libdir}/node
-	find ${D}${NPM_INSTALLDIR} -type f \( -name "*.a" -o -name "*.d" -o -name "*.o" \) -delete
-	if [ -d ${D}${prefix}/etc ] ; then
-		# This will be empty
-		rmdir ${D}${prefix}/etc
-	fi
-}
+    # This function creates the destination directory from the pre installed
+    # files in the ${B} directory.
+
+    # Copy the entire lib and bin directories from ${B} to ${D}.
+    install -d ${D}/${nonarch_libdir}
+    cp --no-preserve=ownership --recursive ${B}/lib/. ${D}/${nonarch_libdir}
+
+    if [ -d "${B}/bin" ]
+    then
+        install -d ${D}/${bindir}
+        cp --no-preserve=ownership --recursive ${B}/bin/. ${D}/${bindir}
+    fi
+
+    # If the package (or its dependencies) uses node-gyp to build native addons,
+    # object files, static libraries or other temporary files can be hidden in
+    # the lib directory. To reduce the package size and to avoid QA issues
+    # (staticdev with static library files) these files must be removed.
+
+    # Remove any node-gyp directory in ${D} to remove temporary build files.
+    for GYP_D_FILE in $(find ${D} -regex ".*/build/Release/[^/]*.node")
+    do
+        local GYP_D_DIR=${GYP_D_FILE%/Release/*}
+
+        rm --recursive --force ${GYP_D_DIR}
+    done
+
+    # Copy only the node-gyp release files from ${B} to ${D}.
+    for GYP_B_FILE in $(find ${B} -regex ".*/build/Release/[^/]*.node")
+    do
+        local GYP_D_FILE=${D}/${prefix}/${GYP_B_FILE#${B}}
+
+        install -d ${GYP_D_FILE%/*}
+        install -m 755 ${GYP_B_FILE} ${GYP_D_FILE}
+    done
+
+    # Remove the shrinkwrap file which does not need to be packed.
+    rm -f ${D}/${nonarch_libdir}/node_modules/*/npm-shrinkwrap.json
+    rm -f ${D}/${nonarch_libdir}/node_modules/@*/*/npm-shrinkwrap.json
 
-python populate_packages_prepend () {
-    instdir = d.expand('${D}${NPM_INSTALLDIR}')
-    extrapackages = oe.package.npm_split_package_dirs(instdir)
-    pkgnames = extrapackages.keys()
-    d.prependVar('PACKAGES', '%s ' % ' '.join(pkgnames))
-    for pkgname in pkgnames:
-        pkgrelpath, pdata = extrapackages[pkgname]
-        pkgpath = '${NPM_INSTALLDIR}/' + pkgrelpath
-        # package names can't have underscores but npm packages sometimes use them
-        oe_pkg_name = pkgname.replace('_', '-')
-        expanded_pkgname = d.expand(oe_pkg_name)
-        d.setVar('FILES_%s' % expanded_pkgname, pkgpath)
-        if pdata:
-            version = pdata.get('version', None)
-            if version:
-                d.setVar('PKGV_%s' % expanded_pkgname, version)
-            description = pdata.get('description', None)
-            if description:
-                d.setVar('SUMMARY_%s' % expanded_pkgname, description.replace(u"\u2018", "'").replace(u"\u2019", "'"))
-    d.appendVar('RDEPENDS_%s' % d.getVar('PN'), ' %s' % ' '.join(pkgnames).replace('_', '-'))
+    # node(1) is using /usr/lib/node as default include directory and npm(1) is
+    # using /usr/lib/node_modules as install directory. Let's make both happy.
+    ln -fs node_modules ${D}/${nonarch_libdir}/node
 }
 
 FILES_${PN} += " \
     ${bindir} \
-    ${libdir}/node \
-    ${NPM_INSTALLDIR} \
+    ${nonarch_libdir} \
 "
 
 EXPORT_FUNCTIONS do_compile do_install
-- 
2.20.1



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

* [PATCH v3 03/17] recipetool/create_npm.py: refactor the npm recipe creation handler
  2019-11-20  9:33 [PATCH v3 00/17] NPM refactoring Jean-Marie LEMETAYER
  2019-11-20  9:33 ` [PATCH v3 01/17] devtool: npm: update command line options Jean-Marie LEMETAYER
  2019-11-20  9:33 ` [PATCH v3 02/17] npm.bbclass: refactor the npm class Jean-Marie LEMETAYER
@ 2019-11-20  9:33 ` Jean-Marie LEMETAYER
  2019-11-20  9:33 ` [PATCH v3 04/17] lib/oe/package.py: remove unneeded npm_split_package_dirs function Jean-Marie LEMETAYER
                   ` (15 subsequent siblings)
  18 siblings, 0 replies; 29+ messages in thread
From: Jean-Marie LEMETAYER @ 2019-11-20  9:33 UTC (permalink / raw)
  To: openembedded-core; +Cc: jonaskgandersson, paul.eggleton, rennes, bunk

This commit refactors the npm recipe creation handler to use the new npm
behavior. The process is kept as simple as possible and only generates
the shrinkwrap file.

To avoid naming issues the recipe name is now extracted from the npm
package name and not directly mapped.

Signed-off-by: Jean-Marie LEMETAYER <jean-marie.lemetayer@savoirfairelinux.com>
---
 scripts/lib/recipetool/create_npm.py | 475 +++++++++++----------------
 1 file changed, 192 insertions(+), 283 deletions(-)

diff --git a/scripts/lib/recipetool/create_npm.py b/scripts/lib/recipetool/create_npm.py
index 39429ebad3..ea3c3044fc 100644
--- a/scripts/lib/recipetool/create_npm.py
+++ b/scripts/lib/recipetool/create_npm.py
@@ -1,321 +1,230 @@
-# Recipe creation tool - node.js NPM module support plugin
-#
 # Copyright (C) 2016 Intel Corporation
+# Copyright (C) 2019 Savoir-Faire Linux
 #
 # SPDX-License-Identifier: GPL-2.0-only
 #
+"""
+    Recipe creation tool - npm module support plugin
+"""
 
+import json
 import os
+import re
+import shutil
 import sys
-import logging
-import subprocess
 import tempfile
-import shutil
-import json
-from recipetool.create import RecipeHandler, split_pkg_licenses, handle_license_vars
-
-logger = logging.getLogger('recipetool')
-
+import bb
+from bb.fetch2 import runfetchcmd
+from recipetool.create import RecipeHandler
 
 tinfoil = None
 
 def tinfoil_init(instance):
+    """
+        Initialize tinfoil.
+    """
     global tinfoil
     tinfoil = instance
 
-
 class NpmRecipeHandler(RecipeHandler):
-    lockdownpath = None
+    """
+        Class to handle the npm recipe creation
+    """
+
+    @staticmethod
+    def _get_registry(extravalues, lines_before):
+        """
+            Get the registry value from the '--npm-registry' option
+            or the 'npm://registry' url.
+        """
+        registry_option = extravalues.get("NPM_REGISTRY")
+
+        registry_fetch = None
+
+        def handle_metadata(name, value, *unused):
+            if name == "SRC_URI":
+                for uri in value.split():
+                    if uri.startswith("npm://"):
+                        nonlocal registry_fetch
+                        registry_fetch = re.sub(r"^npm://", "http://", uri.split(";")[0])
+            return value, None, 0, True
 
-    def _ensure_npm(self, fixed_setup=False):
+        bb.utils.edit_metadata(lines_before, ["SRC_URI"], handle_metadata)
+
+        if registry_fetch is not None:
+            registry = registry_fetch
+
+            if registry_option is not None:
+                bb.warn("The npm registry is specified multiple times")
+                bb.note("Using registry from the fetch url: '{}'".format(registry))
+                extravalues.pop("NPM_REGISTRY", None)
+
+        elif registry_option is not None:
+            registry = registry_option
+
+        else:
+            registry = "http://registry.npmjs.org"
+
+        return registry
+
+    @staticmethod
+    def _ensure_npm(d):
+        """
+            Check if the 'npm' command is available in the recipes, then build
+            it and add it to the PATH.
+        """
         if not tinfoil.recipes_parsed:
             tinfoil.parse_recipes()
+
         try:
             rd = tinfoil.parse_recipe('nodejs-native')
         except bb.providers.NoProvider:
-            if fixed_setup:
-                msg = 'nodejs-native is required for npm but is not available within this SDK'
-            else:
-                msg = 'nodejs-native is required for npm but is not available - you will likely need to add a layer that provides nodejs'
-            logger.error(msg)
-            return None
+            bb.error("Nothing provides 'nodejs-native' which is required for the build")
+            bb.note("You will likely need to add a layer that provides nodejs")
+            sys.exit(14)
+
         bindir = rd.getVar('STAGING_BINDIR_NATIVE')
         npmpath = os.path.join(bindir, 'npm')
+
         if not os.path.exists(npmpath):
             tinfoil.build_targets('nodejs-native', 'addto_recipe_sysroot')
+
             if not os.path.exists(npmpath):
-                logger.error('npm required to process specified source, but nodejs-native did not seem to populate it')
-                return None
-        return bindir
-
-    def _handle_license(self, data):
-        '''
-        Handle the license value from an npm package.json file
-        '''
-        license = None
-        if 'license' in data:
-            license = data['license']
-            if isinstance(license, dict):
-                license = license.get('type', None)
-            if license:
-                if 'OR' in license:
-                    license = license.replace('OR', '|')
-                    license = license.replace('AND', '&')
-                    license = license.replace(' ', '_')
-                    if not license[0] == '(':
-                        license = '(' + license + ')'
-                else:
-                    license = license.replace('AND', '&')
-                    if license[0] == '(':
-                        license = license[1:]
-                    if license[-1] == ')':
-                        license = license[:-1]
-                license = license.replace('MIT/X11', 'MIT')
-                license = license.replace('Public Domain', 'PD')
-                license = license.replace('SEE LICENSE IN EULA',
-                                          'SEE-LICENSE-IN-EULA')
-        return license
-
-    def _shrinkwrap(self, srctree, localfilesdir, extravalues, lines_before, d):
-        try:
-            runenv = dict(os.environ, PATH=d.getVar('PATH'))
-            bb.process.run('npm shrinkwrap', cwd=srctree, stderr=subprocess.STDOUT, env=runenv, shell=True)
-        except bb.process.ExecutionError as e:
-            logger.warning('npm shrinkwrap failed:\n%s' % e.stdout)
-            return
-
-        tmpfile = os.path.join(localfilesdir, 'npm-shrinkwrap.json')
-        shutil.move(os.path.join(srctree, 'npm-shrinkwrap.json'), tmpfile)
-        extravalues.setdefault('extrafiles', {})
-        extravalues['extrafiles']['npm-shrinkwrap.json'] = tmpfile
-        lines_before.append('NPM_SHRINKWRAP := "${THISDIR}/${PN}/npm-shrinkwrap.json"')
-
-    def _lockdown(self, srctree, localfilesdir, extravalues, lines_before, d):
-        runenv = dict(os.environ, PATH=d.getVar('PATH'))
-        if not NpmRecipeHandler.lockdownpath:
-            NpmRecipeHandler.lockdownpath = tempfile.mkdtemp('recipetool-npm-lockdown')
-            bb.process.run('npm install lockdown --prefix %s' % NpmRecipeHandler.lockdownpath,
-                           cwd=srctree, stderr=subprocess.STDOUT, env=runenv, shell=True)
-        relockbin = os.path.join(NpmRecipeHandler.lockdownpath, 'node_modules', 'lockdown', 'relock.js')
-        if not os.path.exists(relockbin):
-            logger.warning('Could not find relock.js within lockdown directory; skipping lockdown')
-            return
-        try:
-            bb.process.run('node %s' % relockbin, cwd=srctree, stderr=subprocess.STDOUT, env=runenv, shell=True)
-        except bb.process.ExecutionError as e:
-            logger.warning('lockdown-relock failed:\n%s' % e.stdout)
-            return
-
-        tmpfile = os.path.join(localfilesdir, 'lockdown.json')
-        shutil.move(os.path.join(srctree, 'lockdown.json'), tmpfile)
-        extravalues.setdefault('extrafiles', {})
-        extravalues['extrafiles']['lockdown.json'] = tmpfile
-        lines_before.append('NPM_LOCKDOWN := "${THISDIR}/${PN}/lockdown.json"')
-
-    def _handle_dependencies(self, d, deps, optdeps, devdeps, lines_before, srctree):
-        import scriptutils
-        # If this isn't a single module we need to get the dependencies
-        # and add them to SRC_URI
-        def varfunc(varname, origvalue, op, newlines):
-            if varname == 'SRC_URI':
-                if not origvalue.startswith('npm://'):
-                    src_uri = origvalue.split()
-                    deplist = {}
-                    for dep, depver in optdeps.items():
-                        depdata = self.get_npm_data(dep, depver, d)
-                        if self.check_npm_optional_dependency(depdata):
-                            deplist[dep] = depdata
-                    for dep, depver in devdeps.items():
-                        depdata = self.get_npm_data(dep, depver, d)
-                        if self.check_npm_optional_dependency(depdata):
-                            deplist[dep] = depdata
-                    for dep, depver in deps.items():
-                        depdata = self.get_npm_data(dep, depver, d)
-                        deplist[dep] = depdata
-
-                    extra_urls = []
-                    for dep, depdata in deplist.items():
-                        version = depdata.get('version', None)
-                        if version:
-                            url = 'npm://registry.npmjs.org;name=%s;version=%s;subdir=node_modules/%s' % (dep, version, dep)
-                            extra_urls.append(url)
-                    if extra_urls:
-                        scriptutils.fetch_url(tinfoil, ' '.join(extra_urls), None, srctree, logger)
-                        src_uri.extend(extra_urls)
-                        return src_uri, None, -1, True
-            return origvalue, None, 0, True
-        updated, newlines = bb.utils.edit_metadata(lines_before, ['SRC_URI'], varfunc)
-        if updated:
-            del lines_before[:]
-            for line in newlines:
-                # Hack to avoid newlines that edit_metadata inserts
-                if line.endswith('\n'):
-                    line = line[:-1]
-                lines_before.append(line)
-        return updated
+                bb.error("Failed to add 'npm' to sysroot")
+                sys.exit(14)
+
+        d.prependVar("PATH", "{}:".format(bindir))
+
+    @staticmethod
+    def _run_npm_install(d, development):
+        """
+            Run the 'npm install' command without building the addons (if any).
+            This is only needed to generate the initial shrinkwrap file.
+            The 'node_modules' directory is created and populated.
+        """
+        cmd = "npm install"
+        cmd += " --ignore-scripts"
+        cmd += " --no-shrinkwrap"
+        cmd += d.expand(" --cache=${NPM_CACHE_DIR}")
+        cmd += d.expand(" --registry=${NPM_REGISTRY}")
+
+        if development is None:
+            cmd += " --production"
+
+        bb.utils.remove(os.path.join(d.getVar("S"), "node_modules"), recurse=True)
+        runfetchcmd(cmd, d, workdir=d.getVar("S"))
+        bb.utils.remove(d.getVar("NPM_CACHE_DIR"), recurse=True)
+
+    @staticmethod
+    def _run_npm_shrinkwrap(d, development):
+        """
+            Run the 'npm shrinkwrap' command to generate the shrinkwrap file.
+        """
+        cmd = "npm shrinkwrap"
+
+        if development is not None:
+            cmd += " --development"
+
+        runfetchcmd(cmd, d, workdir=d.getVar("S"))
+
+    def _generate_shrinkwrap(self, d, lines, extravalues, development):
+        """
+            Check and generate the npm-shrinkwrap.json file if needed.
+        """
+        self._ensure_npm(d)
+        self._run_npm_install(d, development)
+
+        # Check if a shinkwrap file is already in the source
+        src_shrinkwrap = os.path.join(d.getVar("S"), "npm-shrinkwrap.json")
+        if os.path.exists(src_shrinkwrap):
+            bb.note("Using the npm-shrinkwrap.json provided in the sources")
+            return src_shrinkwrap
+
+        # Generate the 'npm-shrinkwrap.json' file
+        self._run_npm_shrinkwrap(d, development)
+
+        # Convert the shrinkwrap file and save it in a temporary location
+        tmpdir = tempfile.mkdtemp(prefix="recipetool-npm")
+        tmp_shrinkwrap = os.path.join(tmpdir, "npm-shrinkwrap.json")
+        shutil.move(src_shrinkwrap, tmp_shrinkwrap)
+
+        # Add the shrinkwrap file as 'extrafiles'
+        extravalues.setdefault("extrafiles", {})
+        extravalues["extrafiles"]["npm-shrinkwrap.json"] = tmp_shrinkwrap
+
+        # Add a line in the recipe to handle the shrinkwrap file
+        lines.append("NPM_SHRINKWRAP = \"${THISDIR}/${BPN}/npm-shrinkwrap.json\"")
+
+        # Clean the source tree
+        bb.utils.remove(os.path.join(d.getVar("S"), "node_modules"), recurse=True)
+        bb.utils.remove(src_shrinkwrap)
+
+        return tmp_shrinkwrap
+
+    @staticmethod
+    def _name_from_npm(npm_name, number=False):
+        """
+            Generate a name based on the npm package name.
+        """
+        name = npm_name
+        name = re.sub("/", "-", name)
+        name = name.lower()
+        if not number:
+            name = re.sub(r"[^\-a-z]", "", name)
+        else:
+            name = re.sub(r"[^\-a-z0-9]", "", name)
+        name = name.strip("-")
+        return name
 
     def process(self, srctree, classes, lines_before, lines_after, handled, extravalues):
-        import bb.utils
-        import oe.package
-        from collections import OrderedDict
+        """
+            Handle the npm recipe creation
+        """
 
         if 'buildsystem' in handled:
             return False
 
-        def read_package_json(fn):
-            with open(fn, 'r', errors='surrogateescape') as f:
-                return json.loads(f.read())
+        files = RecipeHandler.checkfiles(srctree, ["package.json"])
 
-        files = RecipeHandler.checkfiles(srctree, ['package.json'])
-        if files:
-            d = bb.data.createCopy(tinfoil.config_data)
-            npm_bindir = self._ensure_npm()
-            if not npm_bindir:
-                sys.exit(14)
-            d.prependVar('PATH', '%s:' % npm_bindir)
-
-            data = read_package_json(files[0])
-            if 'name' in data and 'version' in data:
-                extravalues['PN'] = data['name']
-                extravalues['PV'] = data['version']
-                classes.append('npm')
-                handled.append('buildsystem')
-                if 'description' in data:
-                    extravalues['SUMMARY'] = data['description']
-                if 'homepage' in data:
-                    extravalues['HOMEPAGE'] = data['homepage']
-
-                fetchdev = extravalues['fetchdev'] or None
-                deps, optdeps, devdeps = self.get_npm_package_dependencies(data, fetchdev)
-                self._handle_dependencies(d, deps, optdeps, devdeps, lines_before, srctree)
-
-                # Shrinkwrap
-                localfilesdir = tempfile.mkdtemp(prefix='recipetool-npm')
-                self._shrinkwrap(srctree, localfilesdir, extravalues, lines_before, d)
-
-                # Lockdown
-                self._lockdown(srctree, localfilesdir, extravalues, lines_before, d)
-
-                # Split each npm module out to is own package
-                npmpackages = oe.package.npm_split_package_dirs(srctree)
-                licvalues = None
-                for item in handled:
-                    if isinstance(item, tuple):
-                        if item[0] == 'license':
-                            licvalues = item[1]
-                            break
-                if not licvalues:
-                    licvalues = handle_license_vars(srctree, lines_before, handled, extravalues, d)
-                if licvalues:
-                    # Augment the license list with information we have in the packages
-                    licenses = {}
-                    license = self._handle_license(data)
-                    if license:
-                        licenses['${PN}'] = license
-                    for pkgname, pkgitem in npmpackages.items():
-                        _, pdata = pkgitem
-                        license = self._handle_license(pdata)
-                        if license:
-                            licenses[pkgname] = license
-                    # Now write out the package-specific license values
-                    # We need to strip out the json data dicts for this since split_pkg_licenses
-                    # isn't expecting it
-                    packages = OrderedDict((x,y[0]) for x,y in npmpackages.items())
-                    packages['${PN}'] = ''
-                    pkglicenses = split_pkg_licenses(licvalues, packages, lines_after, licenses)
-                    all_licenses = list(set([item.replace('_', ' ') for pkglicense in pkglicenses.values() for item in pkglicense]))
-                    if '&' in all_licenses:
-                        all_licenses.remove('&')
-                    extravalues['LICENSE'] = ' & '.join(all_licenses)
-
-                # Need to move S setting after inherit npm
-                for i, line in enumerate(lines_before):
-                    if line.startswith('S ='):
-                        lines_before.pop(i)
-                        lines_after.insert(0, '# Must be set after inherit npm since that itself sets S')
-                        lines_after.insert(1, line)
-                        break
-
-                return True
-
-        return False
-
-    # FIXME this is duplicated from lib/bb/fetch2/npm.py
-    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 = None
-        outdeps = {}
-        datalines = []
-        bracelevel = 0
-        for line in output.splitlines():
-            if bracelevel:
-                datalines.append(line)
-            elif '{' in line:
-                datalines = []
-                datalines.append(line)
-            bracelevel = bracelevel + line.count('{') - line.count('}')
-        if datalines:
-            pdata = json.loads('\n'.join(datalines))
-        return pdata
-
-    # FIXME this is effectively duplicated from lib/bb/fetch2/npm.py
-    # (split out from _getdependencies())
-    def get_npm_data(self, pkg, version, d):
-        import bb.fetch2
-        pkgfullname = pkg
-        if version != '*' and not '/' in version:
-            pkgfullname += "@'%s'" % version
-        logger.debug(2, "Calling getdeps on %s" % pkg)
-        runenv = dict(os.environ, PATH=d.getVar('PATH'))
-        fetchcmd = "npm view %s --json" % pkgfullname
-        output, _ = bb.process.run(fetchcmd, stderr=subprocess.STDOUT, env=runenv, shell=True)
-        data = self._parse_view(output)
-        return data
-
-    # FIXME this is effectively duplicated from lib/bb/fetch2/npm.py
-    # (split out from _getdependencies())
-    def get_npm_package_dependencies(self, pdata, fetchdev):
-        dependencies = pdata.get('dependencies', {})
-        optionalDependencies = pdata.get('optionalDependencies', {})
-        dependencies.update(optionalDependencies)
-        if fetchdev:
-            devDependencies = pdata.get('devDependencies', {})
-            dependencies.update(devDependencies)
-        else:
-            devDependencies = {}
-        depsfound = {}
-        optdepsfound = {}
-        devdepsfound = {}
-        for dep in dependencies:
-            if dep in optionalDependencies:
-                optdepsfound[dep] = dependencies[dep]
-            elif dep in devDependencies:
-                devdepsfound[dep] = dependencies[dep]
-            else:
-                depsfound[dep] = dependencies[dep]
-        return depsfound, optdepsfound, devdepsfound
-
-    # FIXME this is effectively duplicated from lib/bb/fetch2/npm.py
-    # (split out from _getdependencies())
-    def check_npm_optional_dependency(self, pdata):
-        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:
-                pkg = pdata.get('name', 'Unnamed package')
-                logger.debug(2, "Skipping %s since it's incompatible with Linux" % pkg)
-                return False
-        return True
+        if not files:
+            return False
+
+        with open(files[0], "r") as f:
+            data = json.load(f)
 
+        if "name" not in data or "version" not in data:
+            return False
+
+        # Get the option values
+        development = extravalues.get("NPM_INSTALL_DEV")
+        registry = self._get_registry(extravalues, lines_before)
+
+        # Initialize the npm environment
+        d = bb.data.createCopy(tinfoil.config_data)
+        d.setVar("S", srctree)
+        d.setVar("NPM_REGISTRY", registry)
+        d.setVar("NPM_CACHE_DIR", "${S}/.npm_cache")
+
+        # Generate the shrinkwrap file
+        shrinkwrap = self._generate_shrinkwrap(d, lines_before,
+                                               extravalues, development)
+
+        extravalues["PN"] = self._name_from_npm(data["name"])
+        extravalues["PV"] = data["version"]
+
+        if "description" in data:
+            extravalues["SUMMARY"] = data["description"]
+
+        if "homepage" in data:
+            extravalues["HOMEPAGE"] = data["homepage"]
+
+        classes.append("npm")
+        handled.append("buildsystem")
+
+        return True
 
 def register_recipe_handlers(handlers):
+    """
+        Register the npm handler
+    """
     handlers.append((NpmRecipeHandler(), 60))
-- 
2.20.1



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

* [PATCH v3 04/17] lib/oe/package.py: remove unneeded npm_split_package_dirs function
  2019-11-20  9:33 [PATCH v3 00/17] NPM refactoring Jean-Marie LEMETAYER
                   ` (2 preceding siblings ...)
  2019-11-20  9:33 ` [PATCH v3 03/17] recipetool/create_npm.py: refactor the npm recipe creation handler Jean-Marie LEMETAYER
@ 2019-11-20  9:33 ` Jean-Marie LEMETAYER
  2019-11-20  9:33 ` [PATCH v3 05/17] devtool/standard.py: npm: update the append file Jean-Marie LEMETAYER
                   ` (14 subsequent siblings)
  18 siblings, 0 replies; 29+ messages in thread
From: Jean-Marie LEMETAYER @ 2019-11-20  9:33 UTC (permalink / raw)
  To: openembedded-core; +Cc: jonaskgandersson, paul.eggleton, rennes, bunk

The npm_split_package_dirs function was used by the recipetool when
creating npm recipes. This is not the case anymore.

Signed-off-by: Jean-Marie LEMETAYER <jean-marie.lemetayer@savoirfairelinux.com>
---
 meta/lib/oe/package.py | 33 ---------------------------------
 1 file changed, 33 deletions(-)

diff --git a/meta/lib/oe/package.py b/meta/lib/oe/package.py
index b8585d4253..dd700cbb0c 100644
--- a/meta/lib/oe/package.py
+++ b/meta/lib/oe/package.py
@@ -283,36 +283,3 @@ def read_shlib_providers(d):
                         shlib_provider[s[0]] = {}
                     shlib_provider[s[0]][s[1]] = (dep_pkg, s[2])
     return shlib_provider
-
-
-def npm_split_package_dirs(pkgdir):
-    """
-    Work out the packages fetched and unpacked by BitBake's npm fetcher
-    Returns a dict of packagename -> (relpath, package.json) ordered
-    such that it is suitable for use in PACKAGES and FILES
-    """
-    from collections import OrderedDict
-    import json
-    packages = {}
-    for root, dirs, files in os.walk(pkgdir):
-        if os.path.basename(root) == 'node_modules':
-            for dn in dirs:
-                relpth = os.path.relpath(os.path.join(root, dn), pkgdir)
-                pkgitems = ['${PN}']
-                for pathitem in relpth.split('/'):
-                    if pathitem == 'node_modules':
-                        continue
-                    pkgitems.append(pathitem)
-                pkgname = '-'.join(pkgitems).replace('_', '-')
-                pkgname = pkgname.replace('@', '')
-                pkgfile = os.path.join(root, dn, 'package.json')
-                data = None
-                if os.path.exists(pkgfile):
-                    with open(pkgfile, 'r') as f:
-                        data = json.loads(f.read())
-                    packages[pkgname] = (relpth, data)
-    # We want the main package for a module sorted *after* its subpackages
-    # (so that it doesn't otherwise steal the files for the subpackage), so
-    # this is a cheap way to do that whilst still having an otherwise
-    # alphabetical sort
-    return OrderedDict((key, packages[key]) for key in sorted(packages, key=lambda pkg: pkg + '~'))
-- 
2.20.1



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

* [PATCH v3 05/17] devtool/standard.py: npm: update the append file
  2019-11-20  9:33 [PATCH v3 00/17] NPM refactoring Jean-Marie LEMETAYER
                   ` (3 preceding siblings ...)
  2019-11-20  9:33 ` [PATCH v3 04/17] lib/oe/package.py: remove unneeded npm_split_package_dirs function Jean-Marie LEMETAYER
@ 2019-11-20  9:33 ` Jean-Marie LEMETAYER
  2019-11-20  9:33 ` [PATCH v3 06/17] recipetool/create.py: npm: remove the 'noverify' url parameter Jean-Marie LEMETAYER
                   ` (13 subsequent siblings)
  18 siblings, 0 replies; 29+ messages in thread
From: Jean-Marie LEMETAYER @ 2019-11-20  9:33 UTC (permalink / raw)
  To: openembedded-core; +Cc: jonaskgandersson, paul.eggleton, rennes, bunk

When creating a recipe using devtool, a workspace is created to store
the new recipe, the recipe source and some append files. These append
files are used by devtool to build the recipe using externalsrc (to use
the source which are in the workspace). They can also have some
additional actions according to the class of the recipe.

This commit updates the append file for the npm recipes. The
devtool / externalsrc files are removed in the npm build directory
instead of the install directory.

Signed-off-by: Jean-Marie LEMETAYER <jean-marie.lemetayer@savoirfairelinux.com>
---
 scripts/lib/devtool/standard.py | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
index 7068a02a01..2604b79be3 100644
--- a/scripts/lib/devtool/standard.py
+++ b/scripts/lib/devtool/standard.py
@@ -262,14 +262,11 @@ def add(args, config, basepath, workspace):
                 f.write('}\n')
 
             if bb.data.inherits_class('npm', rd):
-                f.write('do_install_append() {\n')
-                f.write('    # Remove files added to source dir by devtool/externalsrc\n')
-                f.write('    rm -f ${NPM_INSTALLDIR}/singletask.lock\n')
-                f.write('    rm -rf ${NPM_INSTALLDIR}/.git\n')
-                f.write('    rm -rf ${NPM_INSTALLDIR}/oe-local-files\n')
-                f.write('    for symlink in ${EXTERNALSRC_SYMLINKS} ; do\n')
-                f.write('        rm -f ${NPM_INSTALLDIR}/${symlink%%:*}\n')
-                f.write('    done\n')
+                f.write('do_compile_append() {\n')
+                f.write('    rm -rf ${B}/lib/node_modules/*/.git\n')
+                f.write('    rm -rf ${B}/lib/node_modules/@*/*/.git\n')
+                f.write('    rm -f ${B}/lib/node_modules/*/singletask.lock\n')
+                f.write('    rm -f ${B}/lib/node_modules/@*/*/singletask.lock\n')
                 f.write('}\n')
 
         # Check if the new layer provides recipes whose priorities have been
-- 
2.20.1



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

* [PATCH v3 06/17] recipetool/create.py: npm: remove the 'noverify' url parameter
  2019-11-20  9:33 [PATCH v3 00/17] NPM refactoring Jean-Marie LEMETAYER
                   ` (4 preceding siblings ...)
  2019-11-20  9:33 ` [PATCH v3 05/17] devtool/standard.py: npm: update the append file Jean-Marie LEMETAYER
@ 2019-11-20  9:33 ` Jean-Marie LEMETAYER
  2019-11-20  9:33 ` [PATCH v3 07/17] recipetool/create.py: npm: replace the 'latest' keyword Jean-Marie LEMETAYER
                   ` (12 subsequent siblings)
  18 siblings, 0 replies; 29+ messages in thread
From: Jean-Marie LEMETAYER @ 2019-11-20  9:33 UTC (permalink / raw)
  To: openembedded-core; +Cc: jonaskgandersson, paul.eggleton, rennes, bunk

This commit removes the 'noverify' parameter which was added to the url
to fix warnings with the shrinkwrap / lockdown file generation. This is
not needed anymore with the new npm fetcher.

Signed-off-by: Jean-Marie LEMETAYER <jean-marie.lemetayer@savoirfairelinux.com>
---
 scripts/lib/recipetool/create.py | 2 --
 1 file changed, 2 deletions(-)

diff --git a/scripts/lib/recipetool/create.py b/scripts/lib/recipetool/create.py
index 932dc3f374..7754668559 100644
--- a/scripts/lib/recipetool/create.py
+++ b/scripts/lib/recipetool/create.py
@@ -477,8 +477,6 @@ def create_recipe(args):
             storeTagName = params['tag']
             params['nobranch'] = '1'
             del params['tag']
-        if scheme == 'npm':
-            params['noverify'] = '1'
         fetchuri = bb.fetch2.encodeurl((scheme, network, path, user, passwd, params))
 
         tmpparent = tinfoil.config_data.getVar('BASE_WORKDIR')
-- 
2.20.1



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

* [PATCH v3 07/17] recipetool/create.py: npm: replace the 'latest' keyword
  2019-11-20  9:33 [PATCH v3 00/17] NPM refactoring Jean-Marie LEMETAYER
                   ` (5 preceding siblings ...)
  2019-11-20  9:33 ` [PATCH v3 06/17] recipetool/create.py: npm: remove the 'noverify' url parameter Jean-Marie LEMETAYER
@ 2019-11-20  9:33 ` Jean-Marie LEMETAYER
  2019-11-20  9:33 ` [PATCH v3 08/17] npm.bbclass: split the do_compile task Jean-Marie LEMETAYER
                   ` (11 subsequent siblings)
  18 siblings, 0 replies; 29+ messages in thread
From: Jean-Marie LEMETAYER @ 2019-11-20  9:33 UTC (permalink / raw)
  To: openembedded-core; +Cc: jonaskgandersson, paul.eggleton, rennes, bunk

The new npm fetcher allows the 'latest' keyword to be used to download
the latest version on the registry. But the keyword must be replace
as soon as the version is determined to have a stable generated recipe.

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

diff --git a/scripts/lib/recipetool/create.py b/scripts/lib/recipetool/create.py
index 7754668559..b4ab1117b8 100644
--- a/scripts/lib/recipetool/create.py
+++ b/scripts/lib/recipetool/create.py
@@ -831,6 +831,8 @@ def create_recipe(args):
         elif line.startswith('SRC_URI = '):
             if realpv and not pv_srcpv:
                 line = line.replace(realpv, '${PV}')
+            if scheme == 'npm':
+                line = line.replace('version=latest', 'version=${PV}')
         elif line.startswith('PV = '):
             if realpv:
                 # Replace the first part of the PV value
-- 
2.20.1



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

* [PATCH v3 08/17] npm.bbclass: split the do_compile task
  2019-11-20  9:33 [PATCH v3 00/17] NPM refactoring Jean-Marie LEMETAYER
                   ` (6 preceding siblings ...)
  2019-11-20  9:33 ` [PATCH v3 07/17] recipetool/create.py: npm: replace the 'latest' keyword Jean-Marie LEMETAYER
@ 2019-11-20  9:33 ` Jean-Marie LEMETAYER
  2019-11-25 13:58   ` Ross Burton
  2019-11-20  9:33 ` [PATCH v3 09/17] devtool/standard.py: npm: exclude the node_modules directory Jean-Marie LEMETAYER
                   ` (10 subsequent siblings)
  18 siblings, 1 reply; 29+ messages in thread
From: Jean-Marie LEMETAYER @ 2019-11-20  9:33 UTC (permalink / raw)
  To: openembedded-core; +Cc: jonaskgandersson, paul.eggleton, rennes, bunk

The do_compile task must be split in order to fit the yocto workflow.
The dependencies are fetched in the do_fetch task and cached in the
do_unpack task.

Signed-off-by: Jean-Marie LEMETAYER <jean-marie.lemetayer@savoirfairelinux.com>
---
 meta/classes/npm.bbclass | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/meta/classes/npm.bbclass b/meta/classes/npm.bbclass
index b3f73d9b67..e2a258afe1 100644
--- a/meta/classes/npm.bbclass
+++ b/meta/classes/npm.bbclass
@@ -48,6 +48,25 @@ NPM_CACHE_DIR ?= "${WORKDIR}/npm_cache"
 
 B = "${WORKDIR}/build"
 
+python do_fetch_append() {
+    """
+        Fetch all dependencies tarball to DL_DIR.
+    """
+    bb.fetch2.npm.fetch_dependencies(d)
+}
+
+python do_unpack_append() {
+    """
+        Unpack all dependencies tarball to the 'node_modules' directory and add
+        them to the npm cache. The dependencies needs to be unpacked to have
+        access to the licenses files checksum feature (LIC_FILES_CHKSUM). They
+        also have to be in the cache to properly execute the 'npm install'.
+    """
+    bb.fetch2.npm.unpack_dependencies(d)
+}
+
+do_unpack[depends] = "nodejs-native:do_populate_sysroot"
+
 npm_install_shrinkwrap() {
     # This function ensures that there is a shrinkwrap file in the specified
     # directory. A shrinkwrap file is mandatory to have reproducible builds.
-- 
2.20.1



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

* [PATCH v3 09/17] devtool/standard.py: npm: exclude the node_modules directory
  2019-11-20  9:33 [PATCH v3 00/17] NPM refactoring Jean-Marie LEMETAYER
                   ` (7 preceding siblings ...)
  2019-11-20  9:33 ` [PATCH v3 08/17] npm.bbclass: split the do_compile task Jean-Marie LEMETAYER
@ 2019-11-20  9:33 ` Jean-Marie LEMETAYER
  2019-11-20  9:33 ` [PATCH v3 10/17] recipetool/create_npm.py: handle the licenses of the dependencies Jean-Marie LEMETAYER
                   ` (9 subsequent siblings)
  18 siblings, 0 replies; 29+ messages in thread
From: Jean-Marie LEMETAYER @ 2019-11-20  9:33 UTC (permalink / raw)
  To: openembedded-core; +Cc: jonaskgandersson, paul.eggleton, rennes, bunk

If 'devtool add' was executed without the '--no-git' option a git
repository have been initialized in the source directory in order to
check if the sources stay unchanged after the 'devtool build' command.
The 'devtool finish' command will fail if any modification is found.

As the node_modules directory is added to the source tree in order to
manage the dependency license, it needs to be explicitly excluded from
this git repository.

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

diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
index 2604b79be3..31f0c44b20 100644
--- a/scripts/lib/devtool/standard.py
+++ b/scripts/lib/devtool/standard.py
@@ -262,7 +262,14 @@ def add(args, config, basepath, workspace):
                 f.write('}\n')
 
             if bb.data.inherits_class('npm', rd):
+                f.write('exclude_git() {\n')
+                f.write('    local exclude="${S}/.git/info/exclude"\n')
+                f.write('    if [ -f "${exclude}" ] && ! grep -q "${1}" "${exclude}" ; then\n')
+                f.write('        echo "${1}" >> "${exclude}"\n')
+                f.write('    fi\n')
+                f.write('}\n')
                 f.write('do_compile_append() {\n')
+                f.write('    exclude_git "/node_modules"\n')
                 f.write('    rm -rf ${B}/lib/node_modules/*/.git\n')
                 f.write('    rm -rf ${B}/lib/node_modules/@*/*/.git\n')
                 f.write('    rm -f ${B}/lib/node_modules/*/singletask.lock\n')
-- 
2.20.1



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

* [PATCH v3 10/17] recipetool/create_npm.py: handle the licenses of the dependencies
  2019-11-20  9:33 [PATCH v3 00/17] NPM refactoring Jean-Marie LEMETAYER
                   ` (8 preceding siblings ...)
  2019-11-20  9:33 ` [PATCH v3 09/17] devtool/standard.py: npm: exclude the node_modules directory Jean-Marie LEMETAYER
@ 2019-11-20  9:33 ` Jean-Marie LEMETAYER
  2019-11-20  9:33 ` [PATCH v3 11/17] npm.bbclass: restrict the build to be offline Jean-Marie LEMETAYER
                   ` (8 subsequent siblings)
  18 siblings, 0 replies; 29+ messages in thread
From: Jean-Marie LEMETAYER @ 2019-11-20  9:33 UTC (permalink / raw)
  To: openembedded-core; +Cc: jonaskgandersson, paul.eggleton, rennes, bunk

As usual the 'LICENSE' and the 'LIC_FILES_CHKSUM' values reflects all
the license files discovered in the source tree (including the
dependencies).

For npm recipes the 'LIC_FILES_CHKSUM' value contains also the status of
the 'package.json' file of every packages as it contains license
informations.

Finally each package has a separate 'LICENSE_${PN}-package-name' value
which describes its license.

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

diff --git a/scripts/lib/recipetool/create_npm.py b/scripts/lib/recipetool/create_npm.py
index ea3c3044fc..e6eb002251 100644
--- a/scripts/lib/recipetool/create_npm.py
+++ b/scripts/lib/recipetool/create_npm.py
@@ -15,7 +15,12 @@ import sys
 import tempfile
 import bb
 from bb.fetch2 import runfetchcmd
+from bb.fetch2.npm import fetch_dependencies
+from bb.fetch2.npm import foreach_dependencies
+from bb.fetch2.npm import unpack_dependencies
 from recipetool.create import RecipeHandler
+from recipetool.create import guess_license
+from recipetool.create import split_pkg_licenses
 
 tinfoil = None
 
@@ -176,6 +181,43 @@ class NpmRecipeHandler(RecipeHandler):
         name = name.strip("-")
         return name
 
+    def _handle_licenses(self, d, shrinkwrap_file, lines, extravalues):
+        """
+            This function have 2 goals:
+                - Add all the package.json files as license files.
+                - Split the license into sub packages.
+        """
+
+        lic_files_chksum = []
+        packages = {}
+
+        def lic_files_chksum_append(licfile):
+            licfilepath = os.path.join(d.getVar("S"), licfile)
+            licmd5 = bb.utils.md5_file(licfilepath)
+            lic_files_chksum.append("file://{};md5={}".format(licfile, licmd5))
+
+        # Handle the parent package
+        lic_files_chksum_append("package.json")
+        packages["${PN}"] = ""
+
+        # Handle the dependencies
+        def handle_dependency(name, version, deptree):
+            prefix = "-".join([self._name_from_npm(x, number=True) for x in deptree])
+            relpath = os.path.join(*[os.path.join("node_modules", x) for x in deptree])
+            lic_files_chksum_append(os.path.join(relpath, "package.json"))
+            packages["${PN}-" + prefix] = relpath
+
+        foreach_dependencies(d, handle_dependency, shrinkwrap_file)
+
+        # Add the extra package.json license files. They will be handled by
+        # the handle_license_vars() function later.
+        extravalues["LIC_FILES_CHKSUM"] = lic_files_chksum
+
+        # Split the license into sub packages. The global LICENSE will be
+        # processed by the handle_license_vars() function later.
+        licvalues = guess_license(d.getVar("S"), d)
+        split_pkg_licenses(licvalues, packages, lines, [])
+
     def process(self, srctree, classes, lines_before, lines_after, handled, extravalues):
         """
             Handle the npm recipe creation
@@ -209,6 +251,13 @@ class NpmRecipeHandler(RecipeHandler):
         shrinkwrap = self._generate_shrinkwrap(d, lines_before,
                                                extravalues, development)
 
+        # Fetch and unpack the dependencies
+        fetch_dependencies(d, shrinkwrap)
+        unpack_dependencies(d, shrinkwrap)
+
+        # Handle the licenses
+        self._handle_licenses(d, shrinkwrap, lines_after, extravalues)
+
         extravalues["PN"] = self._name_from_npm(data["name"])
         extravalues["PV"] = data["version"]
 
-- 
2.20.1



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

* [PATCH v3 11/17] npm.bbclass: restrict the build to be offline
  2019-11-20  9:33 [PATCH v3 00/17] NPM refactoring Jean-Marie LEMETAYER
                   ` (9 preceding siblings ...)
  2019-11-20  9:33 ` [PATCH v3 10/17] recipetool/create_npm.py: handle the licenses of the dependencies Jean-Marie LEMETAYER
@ 2019-11-20  9:33 ` Jean-Marie LEMETAYER
  2019-11-20  9:33 ` [PATCH v3 12/17] npm.bbclass: use the local node headers Jean-Marie LEMETAYER
                   ` (7 subsequent siblings)
  18 siblings, 0 replies; 29+ messages in thread
From: Jean-Marie LEMETAYER @ 2019-11-20  9:33 UTC (permalink / raw)
  To: openembedded-core; +Cc: jonaskgandersson, paul.eggleton, rennes, bunk

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>
---
 meta/classes/npm.bbclass | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/meta/classes/npm.bbclass b/meta/classes/npm.bbclass
index e2a258afe1..b13dca7eb8 100644
--- a/meta/classes/npm.bbclass
+++ b/meta/classes/npm.bbclass
@@ -97,7 +97,7 @@ npm_do_compile() {
     local NPM_SHRINKWRAP_INSTALLED=$(npm_install_shrinkwrap)
 
     # Then create a tarball from a npm package whose sources must be in ${S}.
-    local NPM_PACK_FILE=$(cd ${WORKDIR} && npm pack ${S}/)
+    local NPM_PACK_FILE=$(cd ${WORKDIR} && npm pack ${S}/ --offline --proxy=http://invalid.org)
 
     # Clean source tree.
     rm -f ${NPM_SHRINKWRAP_INSTALLED}
@@ -106,6 +106,8 @@ npm_do_compile() {
     rm -rf ${B}
 
     # Finally install and build the tarball package in ${B}.
+    local NPM_INSTALL_ARGS="${NPM_INSTALL_ARGS} --offline"
+    local NPM_INSTALL_ARGS="${NPM_INSTALL_ARGS} --proxy=http://invalid.org"
     local NPM_INSTALL_ARGS="${NPM_INSTALL_ARGS} --cache=${NPM_CACHE_DIR}"
     local NPM_INSTALL_ARGS="${NPM_INSTALL_ARGS} --loglevel=silly"
     local NPM_INSTALL_ARGS="${NPM_INSTALL_ARGS} --prefix=${B}"
-- 
2.20.1



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

* [PATCH v3 12/17] npm.bbclass: use the local node headers
  2019-11-20  9:33 [PATCH v3 00/17] NPM refactoring Jean-Marie LEMETAYER
                   ` (10 preceding siblings ...)
  2019-11-20  9:33 ` [PATCH v3 11/17] npm.bbclass: restrict the build to be offline Jean-Marie LEMETAYER
@ 2019-11-20  9:33 ` Jean-Marie LEMETAYER
  2019-11-20  9:33 ` [PATCH v3 13/17] recipetool/create_npm.py: convert the shrinkwrap file Jean-Marie LEMETAYER
                   ` (6 subsequent siblings)
  18 siblings, 0 replies; 29+ messages in thread
From: Jean-Marie LEMETAYER @ 2019-11-20  9:33 UTC (permalink / raw)
  To: openembedded-core; +Cc: jonaskgandersson, paul.eggleton, rennes, bunk

When building addons, the node headers are needed to be able to compile
properly. Usually they are downloaded by npm but network access in the
do_compile task are unauthorized. Hopefully the local node headers are
available in the native sysroot so lets use them.

Signed-off-by: Jean-Marie LEMETAYER <jean-marie.lemetayer@savoirfairelinux.com>
---
 meta/classes/npm.bbclass | 1 +
 1 file changed, 1 insertion(+)

diff --git a/meta/classes/npm.bbclass b/meta/classes/npm.bbclass
index b13dca7eb8..63b1d9f624 100644
--- a/meta/classes/npm.bbclass
+++ b/meta/classes/npm.bbclass
@@ -121,6 +121,7 @@ npm_do_compile() {
     local NPM_INSTALL_GYP_ARGS="${NPM_INSTALL_GYP_ARGS} --arch=${NPM_ARCH}"
     local NPM_INSTALL_GYP_ARGS="${NPM_INSTALL_GYP_ARGS} --target_arch=${NPM_ARCH}"
     local NPM_INSTALL_GYP_ARGS="${NPM_INSTALL_GYP_ARGS} --release"
+    local NPM_INSTALL_GYP_ARGS="${NPM_INSTALL_GYP_ARGS} --nodedir=${RECIPE_SYSROOT_NATIVE}${prefix_native}"
 
     cd ${WORKDIR} && npm install ${NPM_PACK_FILE} \
         ${NPM_INSTALL_GYP_ARGS} \
-- 
2.20.1



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

* [PATCH v3 13/17] recipetool/create_npm.py: convert the shrinkwrap file
  2019-11-20  9:33 [PATCH v3 00/17] NPM refactoring Jean-Marie LEMETAYER
                   ` (11 preceding siblings ...)
  2019-11-20  9:33 ` [PATCH v3 12/17] npm.bbclass: use the local node headers Jean-Marie LEMETAYER
@ 2019-11-20  9:33 ` Jean-Marie LEMETAYER
  2019-11-20  9:33 ` [PATCH v3 14/17] npm.bbclass: force to rebuild the prebuild addons Jean-Marie LEMETAYER
                   ` (5 subsequent siblings)
  18 siblings, 0 replies; 29+ messages in thread
From: Jean-Marie LEMETAYER @ 2019-11-20  9:33 UTC (permalink / raw)
  To: openembedded-core; +Cc: jonaskgandersson, paul.eggleton, rennes, bunk

A local npm cache pre filled with the dependencies is used to be able to
install the npm package without doing any unauthorized network access.

When it is offline, the npm cache identify a package using its integrity
metadata [1] but only the sha512 algorithm is used (this is the
default cacache algorithm [2]). And to create the shrinkwrap file, npm
use the integrity set in the registry which is not necessarily using
the sha512 algorithm.

The generated shrinkwrap file needs to be converted to use only sha512
integrity to ensure that the packages will be fetched from the cache and
not from the network.

1: https://www.w3.org/TR/SRI/
2: https://www.npmjs.com/package/cacache#optsalgorithms

Signed-off-by: Jean-Marie LEMETAYER <jean-marie.lemetayer@savoirfairelinux.com>
---
 scripts/lib/recipetool/create_npm.py | 47 ++++++++++++++++++++++++++--
 1 file changed, 45 insertions(+), 2 deletions(-)

diff --git a/scripts/lib/recipetool/create_npm.py b/scripts/lib/recipetool/create_npm.py
index e6eb002251..dec9b741c5 100644
--- a/scripts/lib/recipetool/create_npm.py
+++ b/scripts/lib/recipetool/create_npm.py
@@ -7,15 +7,17 @@
     Recipe creation tool - npm module support plugin
 """
 
+import base64
+import copy
 import json
 import os
 import re
-import shutil
 import sys
 import tempfile
 import bb
 from bb.fetch2 import runfetchcmd
 from bb.fetch2.npm import fetch_dependencies
+from bb.fetch2.npm import fetch_dependency
 from bb.fetch2.npm import foreach_dependencies
 from bb.fetch2.npm import unpack_dependencies
 from recipetool.create import RecipeHandler
@@ -132,6 +134,47 @@ class NpmRecipeHandler(RecipeHandler):
 
         runfetchcmd(cmd, d, workdir=d.getVar("S"))
 
+    @staticmethod
+    def _convert_shrinkwrap(d, src_shrinkwrap, dst_shrinkwrap):
+        """
+            When adding local tarball to the npm cache, only the sha512
+            algorithm is used to create the cache metadata. The shrinkwrap file
+            must be converted to use only sha512 integrity to be able to
+            retrieve dependencies from the npm cache.
+        """
+
+        def sha512_integrity(name, version):
+            tarball = fetch_dependency(d, name, version)
+            sha512 = bb.utils.sha512_file(tarball)
+            return "sha512-" + base64.b64encode(bytes.fromhex(sha512)).decode()
+
+        def convert_deps(src):
+            if src is None:
+                return None
+            dst = copy.deepcopy(src)
+            for name in src:
+                version = src[name].get("version")
+                integrity = src[name].get("integrity")
+                if integrity is not None and not integrity.startswith("sha512"):
+                    dst[name]["integrity"] = sha512_integrity(name, version)
+                deps = src[name].get("dependencies")
+                if deps is not None:
+                    dst[name]["dependencies"] = convert_deps(deps)
+            return dst
+
+        def convert(src):
+            dst = copy.deepcopy(src)
+            deps = src.get("dependencies")
+            if deps is not None:
+                dst["dependencies"] = convert_deps(deps)
+            return dst
+
+        with open(src_shrinkwrap, "r") as f:
+            src = json.load(f)
+
+        with open(dst_shrinkwrap, "w") as f:
+            print(json.dumps(convert(src), indent=2), file=f)
+
     def _generate_shrinkwrap(self, d, lines, extravalues, development):
         """
             Check and generate the npm-shrinkwrap.json file if needed.
@@ -151,7 +194,7 @@ class NpmRecipeHandler(RecipeHandler):
         # Convert the shrinkwrap file and save it in a temporary location
         tmpdir = tempfile.mkdtemp(prefix="recipetool-npm")
         tmp_shrinkwrap = os.path.join(tmpdir, "npm-shrinkwrap.json")
-        shutil.move(src_shrinkwrap, tmp_shrinkwrap)
+        self._convert_shrinkwrap(d, src_shrinkwrap, tmp_shrinkwrap)
 
         # Add the shrinkwrap file as 'extrafiles'
         extravalues.setdefault("extrafiles", {})
-- 
2.20.1



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

* [PATCH v3 14/17] npm.bbclass: force to rebuild the prebuild addons
  2019-11-20  9:33 [PATCH v3 00/17] NPM refactoring Jean-Marie LEMETAYER
                   ` (12 preceding siblings ...)
  2019-11-20  9:33 ` [PATCH v3 13/17] recipetool/create_npm.py: convert the shrinkwrap file Jean-Marie LEMETAYER
@ 2019-11-20  9:33 ` Jean-Marie LEMETAYER
  2019-11-20  9:33 ` [PATCH v3 15/17] devtool/standard.py: npm: configure the NPM_CACHE_DIR to be persistent Jean-Marie LEMETAYER
                   ` (4 subsequent siblings)
  18 siblings, 0 replies; 29+ messages in thread
From: Jean-Marie LEMETAYER @ 2019-11-20  9:33 UTC (permalink / raw)
  To: openembedded-core; +Cc: jonaskgandersson, paul.eggleton, rennes, bunk

This commit forces to rebuild the prebuild addons which are using
node-gyp-build.

  https://www.npmjs.com/package/node-gyp-build

Signed-off-by: Jean-Marie LEMETAYER <jean-marie.lemetayer@savoirfairelinux.com>
---
 meta/classes/npm.bbclass | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/meta/classes/npm.bbclass b/meta/classes/npm.bbclass
index 63b1d9f624..13f6b2b158 100644
--- a/meta/classes/npm.bbclass
+++ b/meta/classes/npm.bbclass
@@ -123,7 +123,10 @@ npm_do_compile() {
     local NPM_INSTALL_GYP_ARGS="${NPM_INSTALL_GYP_ARGS} --release"
     local NPM_INSTALL_GYP_ARGS="${NPM_INSTALL_GYP_ARGS} --nodedir=${RECIPE_SYSROOT_NATIVE}${prefix_native}"
 
+    local NPM_INSTALL_GYP_BUILD_ARGS="${NPM_INSTALL_GYP_BUILD_ARGS} --build-from-source"
+
     cd ${WORKDIR} && npm install ${NPM_PACK_FILE} \
+        ${NPM_INSTALL_GYP_BUILD_ARGS} \
         ${NPM_INSTALL_GYP_ARGS} \
         ${NPM_INSTALL_ARGS}
 }
-- 
2.20.1



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

* [PATCH v3 15/17] devtool/standard.py: npm: configure the NPM_CACHE_DIR to be persistent
  2019-11-20  9:33 [PATCH v3 00/17] NPM refactoring Jean-Marie LEMETAYER
                   ` (13 preceding siblings ...)
  2019-11-20  9:33 ` [PATCH v3 14/17] npm.bbclass: force to rebuild the prebuild addons Jean-Marie LEMETAYER
@ 2019-11-20  9:33 ` Jean-Marie LEMETAYER
  2019-11-20  9:33 ` [PATCH v3 16/17] oeqa/selftest/recipetool: add npm recipe creation test Jean-Marie LEMETAYER
                   ` (3 subsequent siblings)
  18 siblings, 0 replies; 29+ messages in thread
From: Jean-Marie LEMETAYER @ 2019-11-20  9:33 UTC (permalink / raw)
  To: openembedded-core; +Cc: jonaskgandersson, paul.eggleton, rennes, bunk

When using devtool the ${WORKDIR} is not persistent as the creation
workflow is using temporary directories and external workspace. For npm
recipes the NPM_CACHE_DIR needs to be persistent to make the link
between the first do_fetch, do_unpack tasks and the do_compile task.

As the only persistent directory is the source directory, it is used to
store the npm cache. And it is also excluded from the git repository to
allow the 'devtool finish' command to succeed.

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

diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
index 31f0c44b20..1e68d37db9 100644
--- a/scripts/lib/devtool/standard.py
+++ b/scripts/lib/devtool/standard.py
@@ -262,6 +262,7 @@ def add(args, config, basepath, workspace):
                 f.write('}\n')
 
             if bb.data.inherits_class('npm', rd):
+                f.write('NPM_CACHE_DIR = "${S}/.npm_cache"\n')
                 f.write('exclude_git() {\n')
                 f.write('    local exclude="${S}/.git/info/exclude"\n')
                 f.write('    if [ -f "${exclude}" ] && ! grep -q "${1}" "${exclude}" ; then\n')
@@ -269,6 +270,7 @@ def add(args, config, basepath, workspace):
                 f.write('    fi\n')
                 f.write('}\n')
                 f.write('do_compile_append() {\n')
+                f.write('    exclude_git "/.npm_cache"\n')
                 f.write('    exclude_git "/node_modules"\n')
                 f.write('    rm -rf ${B}/lib/node_modules/*/.git\n')
                 f.write('    rm -rf ${B}/lib/node_modules/@*/*/.git\n')
-- 
2.20.1



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

* [PATCH v3 16/17] oeqa/selftest/recipetool: add npm recipe creation test
  2019-11-20  9:33 [PATCH v3 00/17] NPM refactoring Jean-Marie LEMETAYER
                   ` (14 preceding siblings ...)
  2019-11-20  9:33 ` [PATCH v3 15/17] devtool/standard.py: npm: configure the NPM_CACHE_DIR to be persistent Jean-Marie LEMETAYER
@ 2019-11-20  9:33 ` Jean-Marie LEMETAYER
  2019-11-20  9:33 ` [PATCH v3 17/17] oeqa/selftest/devtool: add npm recipe build test Jean-Marie LEMETAYER
                   ` (2 subsequent siblings)
  18 siblings, 0 replies; 29+ messages in thread
From: Jean-Marie LEMETAYER @ 2019-11-20  9:33 UTC (permalink / raw)
  To: openembedded-core; +Cc: jonaskgandersson, paul.eggleton, rennes, bunk

This commit adds a recipetool creation test for npm recipe:

 - recipetool.RecipetoolTests.test_recipetool_create_npm

Signed-off-by: Jean-Marie LEMETAYER <jean-marie.lemetayer@savoirfairelinux.com>
---
 meta/lib/oeqa/selftest/cases/recipetool.py | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/meta/lib/oeqa/selftest/cases/recipetool.py b/meta/lib/oeqa/selftest/cases/recipetool.py
index c1562c63b2..fd993ab260 100644
--- a/meta/lib/oeqa/selftest/cases/recipetool.py
+++ b/meta/lib/oeqa/selftest/cases/recipetool.py
@@ -421,6 +421,27 @@ class RecipetoolTests(RecipetoolBase):
         inherits = ['cmake']
         self._test_recipe_contents(recipefile, checkvars, inherits)
 
+    def test_recipetool_create_npm(self):
+        temprecipe = os.path.join(self.tempdir, 'recipe')
+        os.makedirs(temprecipe)
+        recipefile = os.path.join(temprecipe, 'savoirfairelinux-node-server-example_1.0.0.bb')
+        shrinkwrap = os.path.join(temprecipe, 'savoirfairelinux-node-server-example', 'npm-shrinkwrap.json')
+        srcuri = 'npm://registry.npmjs.org;name=@savoirfairelinux/node-server-example;version=1.0.0'
+        result = runCmd('recipetool create -o %s \'%s\'' % (temprecipe, srcuri))
+        self.assertTrue(os.path.isfile(recipefile))
+        self.assertTrue(os.path.isfile(shrinkwrap))
+        checkvars = {}
+        checkvars['SUMMARY'] = 'Node Server Example'
+        checkvars['HOMEPAGE'] = 'https://github.com/savoirfairelinux/node-server-example#readme'
+        checkvars['LICENSE'] = set(['MIT', 'ISC', 'Unknown'])
+        checkvars['SRC_URI'] = 'npm://registry.npmjs.org/;name=@savoirfairelinux/node-server-example;version=${PV}'
+        checkvars['S'] = '${WORKDIR}/npm'
+        checkvars['NPM_SHRINKWRAP'] = '${THISDIR}/${BPN}/npm-shrinkwrap.json'
+        checkvars['LICENSE_${PN}-execute'] = 'Unknown'
+        checkvars['LICENSE_${PN}-base64'] = 'Unknown'
+        inherits = ['npm']
+        self._test_recipe_contents(recipefile, checkvars, inherits)
+
     def test_recipetool_create_github(self):
         # Basic test to see if github URL mangling works
         temprecipe = os.path.join(self.tempdir, 'recipe')
-- 
2.20.1



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

* [PATCH v3 17/17] oeqa/selftest/devtool: add npm recipe build test
  2019-11-20  9:33 [PATCH v3 00/17] NPM refactoring Jean-Marie LEMETAYER
                   ` (15 preceding siblings ...)
  2019-11-20  9:33 ` [PATCH v3 16/17] oeqa/selftest/recipetool: add npm recipe creation test Jean-Marie LEMETAYER
@ 2019-11-20  9:33 ` Jean-Marie LEMETAYER
  2019-11-21  6:27 ` [PATCH v3 00/17] NPM refactoring Tim Orling
  2019-11-21 18:26 ` Richard Purdie
  18 siblings, 0 replies; 29+ messages in thread
From: Jean-Marie LEMETAYER @ 2019-11-20  9:33 UTC (permalink / raw)
  To: openembedded-core; +Cc: jonaskgandersson, paul.eggleton, rennes, bunk

This commit adds a devtool build test for npm recipe:

 - devtool.DevtoolAddTests.test_devtool_add_npm

Signed-off-by: Jean-Marie LEMETAYER <jean-marie.lemetayer@savoirfairelinux.com>
---
 meta/lib/oeqa/selftest/cases/devtool.py | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/meta/lib/oeqa/selftest/cases/devtool.py b/meta/lib/oeqa/selftest/cases/devtool.py
index 3a25da2033..d89a33e3c8 100644
--- a/meta/lib/oeqa/selftest/cases/devtool.py
+++ b/meta/lib/oeqa/selftest/cases/devtool.py
@@ -510,6 +510,26 @@ class DevtoolAddTests(DevtoolBase):
         checkvars['SRC_URI'] = url.replace(testver, '${PV}')
         self._test_recipe_contents(recipefile, checkvars, [])
 
+    def test_devtool_add_npm(self):
+        pn = 'savoirfairelinux-node-server-example'
+        pv = '1.0.0'
+        url = 'npm://registry.npmjs.org;name=@savoirfairelinux/node-server-example;version=1.0.0'
+        # Test devtool add
+        self.track_for_cleanup(self.workspacedir)
+        self.add_command_to_tearDown('bitbake -c cleansstate %s' % pn)
+        self.add_command_to_tearDown('bitbake-layers remove-layer */workspace')
+        result = runCmd('devtool add \'%s\'' % url)
+        self.assertExists(os.path.join(self.workspacedir, 'conf', 'layer.conf'), 'Workspace directory not created')
+        self.assertExists(os.path.join(self.workspacedir, 'recipes', pn, '%s_%s.bb' % (pn, pv)), 'Recipe not created')
+        self.assertExists(os.path.join(self.workspacedir, 'recipes', pn, pn, 'npm-shrinkwrap.json'), 'Shrinkwrap not created')
+        # Test devtool status
+        result = runCmd('devtool status')
+        self.assertIn(pn, result.output)
+        # Clean up anything in the workdir/sysroot/sstate cache (have to do this *after* devtool add since the recipe only exists then)
+        bitbake('%s -c cleansstate' % pn)
+        # Test devtool build
+        result = runCmd('devtool build %s' % pn)
+
 class DevtoolModifyTests(DevtoolBase):
 
     def test_devtool_modify(self):
-- 
2.20.1



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

* Re: [PATCH v3 00/17] NPM refactoring
  2019-11-20  9:33 [PATCH v3 00/17] NPM refactoring Jean-Marie LEMETAYER
                   ` (16 preceding siblings ...)
  2019-11-20  9:33 ` [PATCH v3 17/17] oeqa/selftest/devtool: add npm recipe build test Jean-Marie LEMETAYER
@ 2019-11-21  6:27 ` Tim Orling
  2019-11-21  6:36   ` Tim Orling
  2019-11-21 18:26 ` Richard Purdie
  18 siblings, 1 reply; 29+ messages in thread
From: Tim Orling @ 2019-11-21  6:27 UTC (permalink / raw)
  To: Jean-Marie LEMETAYER
  Cc: jonaskgandersson, paul.eggleton, rennes, openembedded-core, bunk

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



> On Nov 20, 2019, at 1:33 AM, Jean-Marie LEMETAYER <jean-marie.lemetayer@savoirfairelinux.com> wrote:
> 
> The current NPM support have several issues:
> - The current NPM fetcher downloads the dependency tree but not the other
>   fetchers. The 'subdir' parameter was used to fix this issue.
> - They are multiple issues with package names (uppercase, exotic characters,
>   scoped packages) even if they are inside the dependencies.
> - The lockdown file generation have issues. When a package depends on
>   multiple version of the same package (all versions have the same checksum).
> 
> This patchset refactors the NPM support in Yocto:
> - As the NPM algorithm for dependency management is hard to handle, the new
>   NPM fetcher downloads only the package source (and not the dependencies,
>   like the other fetchers) (patch submitted in the bitbake-devel list).
> - The NPM class handles the dependencies using NPM (and not manually).
> - The NPM recipe creation is simplified to avoid issues.
> - The lockdown file is no more used as it is no longer relevant compared to the
>   latest shrinkwrap file format.
> 
> This patchset may remove some features (lockdown file, license management for
> dependencies) but fixes the majority of the NPM issues. All of these issues
> from the bugzilla.yoctoproject.org are resolved by this patchset:
> #10237, #10760, #11028, #11728, #11902, #12534
> 
> The fetcher and recipetool are now aware of a 'latest' keyword for the version
> which allow to build the latest version available on the registry. This feature
> fixes the two issues: #10515, #11029
> 
> Moreover the issue #13415 should also be fixed but I cannot test it.
> 
> I have tested the recipe creation and builds using a self made example to
> generate build failures:
> - https://github.com/savoirfairelinux/node-server-example
> - https://npmjs.com/package/@savoirfairelinux/node-server-example
> 
> The test steps are these ones:
>  $ source poky/oe-init-build-env
>  $ bitbake-layers add-layer ../meta-openembedded/meta-oe
>  $ devtool add "npm://registry.npmjs.org;name=@savoirfairelinux/node-server-example;version=latest”

I noticed this before but forgot to report it:

ERROR: Nothing PROVIDES 'c-ares-native' (but virtual:native:/build/meta-openembedded/meta-oe/recipes-devtools/nodejs/nodejs_10.17.0.bb DEPENDS on or otherwise requires it). ...
http://layers.openembedded.org/layerindex/recipe/50318/ <http://layers.openembedded.org/layerindex/recipe/50318/>
$ bitbake-layers add-layer ../meta-openembedded/meta-networking
Which requires
$ bitbake-layers add-layer ../meta-openembedded/meta-python


Using tip of bitbake with V3, openembedded-core with V3 and meta-openembedded master, somehow nodejs-native do_compile broke (at least on qemux86-64) since the last time I built it shortly after ELC-E…
So now I’ll have to git bisect that…more to come…

FWIW I had previously successfully run your instructions and the tests for both bitbake and oe-selftest for V2 (distroless, e.g. not with poky). Something unrelated has broken I suspect.

>  $ devtool build savoirfairelinux-node-server-example
>  $ bitbake-layers create-layer ../meta-test
>  $ bitbake-layers add-layer ../meta-test
>  $ devtool finish savoirfairelinux-node-server-example ../meta-test
>  $ echo IMAGE_INSTALL_append = '" savoirfairelinux-node-server-example"' >> conf/local.conf
>  $ bitbake core-image-minimal
> 
> Also the 'devtool add' url could be one of these:
> - npm://registry.npmjs.org;name=@savoirfairelinux/node-server-example;version=latest
>   This url uses the new npm fetcher and request the latest version available on
>   the registry.
> - npm://registry.npmjs.org;name=@savoirfairelinux/node-server-example;version=1.0.0
>   This url uses the new npm fetcher and request a fixed version.
> - git://github.com/savoirfairelinux/node-server-example.git;protocol=https
>   This url uses the git fetcher. As the dependencies are managed by the NPM
>   class, any fetcher can be used.
> 
> When this patchset will be merged, I have planned to update the NPM wiki:
>  https://wiki.yoctoproject.org/wiki/TipsAndTricks/NPM
> 
> This patchset is also the base work for the full Angular support in Yocto that I
> am preparing. These applications have a huge dependency tree which is ideal to
> test the NPM support.
> 
> --- V2
> 
> - Add the 'check_network_access' function before each network access to check
>   for 'BB_NO_NETWORK' and 'BB_ALLOWED_NETWORKS' variables.
> 
> - Add a 'recipetool.RecipetoolTests.test_recipetool_create_npm' test case for
>   'oe-selftest' to test the npm recipe creation.
> 
> - Update the 'npm.bbclass' to fetch the dependencies in the 'do_fetch' task.
>   The dependencies are cached in a npm cache stored in '${DL_DIR}/npm_cache'
>   allowing the dependencies to be saved in the download directory and verify
>   on insertion and extraction [1]:
>      "All data that passes through the cache is fully verified
>       for integrity on both insertion and extraction."
> 
> 1: https://docs.npmjs.com/cli/cache.html
> 
> --- V3
> 
> - Add a test regarding the 'devtool add' and 'devtool build' command:
>     - devtool.DevtoolAddTests.test_devtool_add_npm
> 
> - Add the license management for dependencies.
> 
> - Split the npm workflow to use the bitbake default tasks (do_fetch, do_unpack,
>   do_compile, do_install). Make sure that only the do_fetch task requires
>   network access.
> 
> - Split the commits for better understanding.
> 
> - Force the rebuild of prebuild addons.
> 
> - Use ${nonarch_libdir} instead of ${libdir} in the do_install task.
> 
>  - These patches can be found here:
>     - https://github.com/savoirfairelinux/openembedded-core/tree/npm-refactoring-v3
>     - https://github.com/savoirfairelinux/poky/tree/npm-refactoring-v3
> 
> Jean-Marie LEMETAYER (17):
>  devtool: npm: update command line options
>  npm.bbclass: refactor the npm class
>  recipetool/create_npm.py: refactor the npm recipe creation handler
>  lib/oe/package.py: remove unneeded npm_split_package_dirs function
>  devtool/standard.py: npm: update the append file
>  recipetool/create.py: npm: remove the 'noverify' url parameter
>  recipetool/create.py: npm: replace the 'latest' keyword
>  npm.bbclass: split the do_compile task
>  devtool/standard.py: npm: exclude the node_modules directory
>  recipetool/create_npm.py: handle the licenses of the dependencies
>  npm.bbclass: restrict the build to be offline
>  npm.bbclass: use the local node headers
>  recipetool/create_npm.py: convert the shrinkwrap file
>  npm.bbclass: force to rebuild the prebuild addons
>  devtool/standard.py: npm: configure the NPM_CACHE_DIR to be persistent
>  oeqa/selftest/recipetool: add npm recipe creation test
>  oeqa/selftest/devtool: add npm recipe build test
> 
> meta/classes/npm.bbclass                   | 224 +++++---
> meta/lib/oe/package.py                     |  33 --
> meta/lib/oeqa/selftest/cases/devtool.py    |  20 +
> meta/lib/oeqa/selftest/cases/recipetool.py |  21 +
> scripts/lib/devtool/standard.py            |  31 +-
> scripts/lib/recipetool/create.py           |  16 +-
> scripts/lib/recipetool/create_npm.py       | 565 +++++++++++----------
> 7 files changed, 510 insertions(+), 400 deletions(-)
> 
> --
> 2.20.1
> 


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

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

* Re: [PATCH v3 00/17] NPM refactoring
  2019-11-21  6:27 ` [PATCH v3 00/17] NPM refactoring Tim Orling
@ 2019-11-21  6:36   ` Tim Orling
  2019-11-21 13:21     ` Jean-Marie LEMETAYER
  2019-11-21 14:07     ` André Draszik
  0 siblings, 2 replies; 29+ messages in thread
From: Tim Orling @ 2019-11-21  6:36 UTC (permalink / raw)
  To: Jean-Marie LEMETAYER
  Cc: paul.eggleton, openembedded-core, rennes, bunk, jonaskgandersson

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



> On Nov 20, 2019, at 10:27 PM, Tim Orling <timothy.t.orling@linux.intel.com> wrote:
> 
> 
> 
>> On Nov 20, 2019, at 1:33 AM, Jean-Marie LEMETAYER <jean-marie.lemetayer@savoirfairelinux.com <mailto:jean-marie.lemetayer@savoirfairelinux.com>> wrote:
>> 
>> The current NPM support have several issues:
>> - The current NPM fetcher downloads the dependency tree but not the other
>>   fetchers. The 'subdir' parameter was used to fix this issue.
>> - They are multiple issues with package names (uppercase, exotic characters,
>>   scoped packages) even if they are inside the dependencies.
>> - The lockdown file generation have issues. When a package depends on
>>   multiple version of the same package (all versions have the same checksum).
>> 
>> This patchset refactors the NPM support in Yocto:
>> - As the NPM algorithm for dependency management is hard to handle, the new
>>   NPM fetcher downloads only the package source (and not the dependencies,
>>   like the other fetchers) (patch submitted in the bitbake-devel list).
>> - The NPM class handles the dependencies using NPM (and not manually).
>> - The NPM recipe creation is simplified to avoid issues.
>> - The lockdown file is no more used as it is no longer relevant compared to the
>>   latest shrinkwrap file format.
>> 
>> This patchset may remove some features (lockdown file, license management for
>> dependencies) but fixes the majority of the NPM issues. All of these issues
>> from the bugzilla.yoctoproject.org <http://bugzilla.yoctoproject.org/> are resolved by this patchset:
>> #10237, #10760, #11028, #11728, #11902, #12534
>> 
>> The fetcher and recipetool are now aware of a 'latest' keyword for the version
>> which allow to build the latest version available on the registry. This feature
>> fixes the two issues: #10515, #11029
>> 
>> Moreover the issue #13415 should also be fixed but I cannot test it.
>> 
>> I have tested the recipe creation and builds using a self made example to
>> generate build failures:
>> - https://github.com/savoirfairelinux/node-server-example <https://github.com/savoirfairelinux/node-server-example>
>> - https://npmjs.com/package/@savoirfairelinux/node-server-example <https://npmjs.com/package/@savoirfairelinux/node-server-example>
>> 
>> The test steps are these ones:
>>  $ source poky/oe-init-build-env
>>  $ bitbake-layers add-layer ../meta-openembedded/meta-oe
>>  $ devtool add "npm://registry.npmjs.org;name=@savoirfairelinux/node-server-example;version=latest <npm://registry.npmjs.org%3Bname%3D@savoirfairelinux/node-server-example;version=latest>”
> 
> I noticed this before but forgot to report it:
> 
> ERROR: Nothing PROVIDES 'c-ares-native' (but virtual:native:/build/meta-openembedded/meta-oe/recipes-devtools/nodejs/nodejs_10.17.0.bb DEPENDS on or otherwise requires it). ...
> http://layers.openembedded.org/layerindex/recipe/50318/ <http://layers.openembedded.org/layerindex/recipe/50318/>
> $ bitbake-layers add-layer ../meta-openembedded/meta-networking
> Which requires
> $ bitbake-layers add-layer ../meta-openembedded/meta-python
> 
> 
> Using tip of bitbake with V3, openembedded-core with V3 and meta-openembedded master, somehow nodejs-native do_compile broke (at least on qemux86-64) since the last time I built it shortly after ELC-E…
> So now I’ll have to git bisect that…more to come…

See https://errors.yoctoproject.org/Errors/Details/274939/ <https://errors.yoctoproject.org/Errors/Details/274939/>
The same error I am seeing.

> 
> FWIW I had previously successfully run your instructions and the tests for both bitbake and oe-selftest for V2 (distroless, e.g. not with poky). Something unrelated has broken I suspect.
> 
>>  $ devtool build savoirfairelinux-node-server-example
>>  $ bitbake-layers create-layer ../meta-test
>>  $ bitbake-layers add-layer ../meta-test
>>  $ devtool finish savoirfairelinux-node-server-example ../meta-test
>>  $ echo IMAGE_INSTALL_append = '" savoirfairelinux-node-server-example"' >> conf/local.conf
>>  $ bitbake core-image-minimal
>> 
>> Also the 'devtool add' url could be one of these:
>> - npm://registry.npmjs.org;name=@savoirfairelinux/node-server-example;version=latest <npm://registry.npmjs.org%3Bname%3D@savoirfairelinux/node-server-example;version=latest>
>>   This url uses the new npm fetcher and request the latest version available on
>>   the registry.
>> - npm://registry.npmjs.org;name=@savoirfairelinux/node-server-example;version=1.0.0 <npm://registry.npmjs.org%3Bname%3D@savoirfairelinux/node-server-example;version=1.0.0>
>>   This url uses the new npm fetcher and request a fixed version.
>> - git://github.com/savoirfairelinux/node-server-example.git;protocol=https <git://github.com/savoirfairelinux/node-server-example.git;protocol=https>
>>   This url uses the git fetcher. As the dependencies are managed by the NPM
>>   class, any fetcher can be used.
>> 
>> When this patchset will be merged, I have planned to update the NPM wiki:
>>  https://wiki.yoctoproject.org/wiki/TipsAndTricks/NPM <https://wiki.yoctoproject.org/wiki/TipsAndTricks/NPM>
>> 
>> This patchset is also the base work for the full Angular support in Yocto that I
>> am preparing. These applications have a huge dependency tree which is ideal to
>> test the NPM support.
>> 
>> --- V2
>> 
>> - Add the 'check_network_access' function before each network access to check
>>   for 'BB_NO_NETWORK' and 'BB_ALLOWED_NETWORKS' variables.
>> 
>> - Add a 'recipetool.RecipetoolTests.test_recipetool_create_npm' test case for
>>   'oe-selftest' to test the npm recipe creation.
>> 
>> - Update the 'npm.bbclass' to fetch the dependencies in the 'do_fetch' task.
>>   The dependencies are cached in a npm cache stored in '${DL_DIR}/npm_cache'
>>   allowing the dependencies to be saved in the download directory and verify
>>   on insertion and extraction [1]:
>>      "All data that passes through the cache is fully verified
>>       for integrity on both insertion and extraction."
>> 
>> 1: https://docs.npmjs.com/cli/cache.html <https://docs.npmjs.com/cli/cache.html>
>> 
>> --- V3
>> 
>> - Add a test regarding the 'devtool add' and 'devtool build' command:
>>     - devtool.DevtoolAddTests.test_devtool_add_npm
>> 
>> - Add the license management for dependencies.
>> 
>> - Split the npm workflow to use the bitbake default tasks (do_fetch, do_unpack,
>>   do_compile, do_install). Make sure that only the do_fetch task requires
>>   network access.
>> 
>> - Split the commits for better understanding.
>> 
>> - Force the rebuild of prebuild addons.
>> 
>> - Use ${nonarch_libdir} instead of ${libdir} in the do_install task.
>> 
>>  - These patches can be found here:
>>     - https://github.com/savoirfairelinux/openembedded-core/tree/npm-refactoring-v3 <https://github.com/savoirfairelinux/openembedded-core/tree/npm-refactoring-v3>
>>     - https://github.com/savoirfairelinux/poky/tree/npm-refactoring-v3 <https://github.com/savoirfairelinux/poky/tree/npm-refactoring-v3>
>> 
>> Jean-Marie LEMETAYER (17):
>>  devtool: npm: update command line options
>>  npm.bbclass: refactor the npm class
>>  recipetool/create_npm.py: refactor the npm recipe creation handler
>>  lib/oe/package.py: remove unneeded npm_split_package_dirs function
>>  devtool/standard.py: npm: update the append file
>>  recipetool/create.py: npm: remove the 'noverify' url parameter
>>  recipetool/create.py: npm: replace the 'latest' keyword
>>  npm.bbclass: split the do_compile task
>>  devtool/standard.py: npm: exclude the node_modules directory
>>  recipetool/create_npm.py: handle the licenses of the dependencies
>>  npm.bbclass: restrict the build to be offline
>>  npm.bbclass: use the local node headers
>>  recipetool/create_npm.py: convert the shrinkwrap file
>>  npm.bbclass: force to rebuild the prebuild addons
>>  devtool/standard.py: npm: configure the NPM_CACHE_DIR to be persistent
>>  oeqa/selftest/recipetool: add npm recipe creation test
>>  oeqa/selftest/devtool: add npm recipe build test
>> 
>> meta/classes/npm.bbclass                   | 224 +++++---
>> meta/lib/oe/package.py                     |  33 --
>> meta/lib/oeqa/selftest/cases/devtool.py    |  20 +
>> meta/lib/oeqa/selftest/cases/recipetool.py |  21 +
>> scripts/lib/devtool/standard.py            |  31 +-
>> scripts/lib/recipetool/create.py           |  16 +-
>> scripts/lib/recipetool/create_npm.py       | 565 +++++++++++----------
>> 7 files changed, 510 insertions(+), 400 deletions(-)
>> 
>> --
>> 2.20.1
>> 
> 
> -- 
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org <mailto:Openembedded-core@lists.openembedded.org>
> http://lists.openembedded.org/mailman/listinfo/openembedded-core <http://lists.openembedded.org/mailman/listinfo/openembedded-core>

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

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

* Re: [PATCH v3 00/17] NPM refactoring
  2019-11-21  6:36   ` Tim Orling
@ 2019-11-21 13:21     ` Jean-Marie LEMETAYER
  2019-11-21 14:07     ` André Draszik
  1 sibling, 0 replies; 29+ messages in thread
From: Jean-Marie LEMETAYER @ 2019-11-21 13:21 UTC (permalink / raw)
  To: Tim Orling
  Cc: paul eggleton, OE-core, Savoir-faire Linux Rennes, bunk, Jonas Andersson

Hi Tim,

On Nov 21, 2019, at 7:36 AM, Tim Orling timothy.t.orling@linux.intel.com wrote:
>> On Nov 20, 2019, at 10:27 PM, Tim Orling <timothy.t.orling@linux.intel.com>
>> wrote:
>>> On Nov 20, 2019, at 1:33 AM, Jean-Marie LEMETAYER
>>> <jean-marie.lemetayer@savoirfairelinux.com
>>> <mailto:jean-marie.lemetayer@savoirfairelinux.com>> wrote:
>>> 
>>> The test steps are these ones:
>>>  $ source poky/oe-init-build-env
>>>  $ bitbake-layers add-layer ../meta-openembedded/meta-oe
>>>  $ devtool add
>>>  "npm://registry.npmjs.org;name=@savoirfairelinux/node-server-example;version=latest
>>>  <npm://registry.npmjs.org%3Bname%3D@savoirfairelinux/node-server-example;version=latest>”
>> 
>> I noticed this before but forgot to report it:
>> 
>> ERROR: Nothing PROVIDES 'c-ares-native' (but
>> virtual:native:/build/meta-openembedded/meta-oe/recipes-devtools/nodejs/nodejs_10.17.0.bb
>> DEPENDS on or otherwise requires it). ...
>> http://layers.openembedded.org/layerindex/recipe/50318/
>> <http://layers.openembedded.org/layerindex/recipe/50318/>
>> $ bitbake-layers add-layer ../meta-openembedded/meta-networking
>> Which requires
>> $ bitbake-layers add-layer ../meta-openembedded/meta-python
>> 
>> 
>> Using tip of bitbake with V3, openembedded-core with V3 and meta-openembedded
>> master, somehow nodejs-native do_compile broke (at least on qemux86-64) since
>> the last time I built it shortly after ELC-E…
>> So now I’ll have to git bisect that…more to come…
> 
> See https://errors.yoctoproject.org/Errors/Details/274939/
> <https://errors.yoctoproject.org/Errors/Details/274939/>
> The same error I am seeing.
> 
>> 
>> FWIW I had previously successfully run your instructions and the tests for both
>> bitbake and oe-selftest for V2 (distroless, e.g. not with poky). Something
>> unrelated has broken I suspect.

Thanks for your feedback.

Since ELCE the nodejs recipe have received some patches including this one:
http://lists.openembedded.org/pipermail/openembedded-devel/2019-October/202636.html

And was updated to 10.17.0:
http://lists.openembedded.org/pipermail/openembedded-devel/2019-October/202639.html

But since it is concerning the nodejs recipe and not the npm fetcher/class
do you think it should block the patchset ?

>>>  $ devtool build savoirfairelinux-node-server-example
>>>  $ bitbake-layers create-layer ../meta-test
>>>  $ bitbake-layers add-layer ../meta-test
>>>  $ devtool finish savoirfairelinux-node-server-example ../meta-test
>>>  $ echo IMAGE_INSTALL_append = '" savoirfairelinux-node-server-example"' >>
>>>  conf/local.conf
>>>  $ bitbake core-image-minimal
>>> 
>>> --- V2
>>> 
>>> - Add a 'recipetool.RecipetoolTests.test_recipetool_create_npm' test case for
>>>   'oe-selftest' to test the npm recipe creation.
>>> 
>>> --- V3
>>> 
>>> - Add a test regarding the 'devtool add' and 'devtool build' command:
>>>     - devtool.DevtoolAddTests.test_devtool_add_npm

I had a lot of requests about tests. What do you think about these ?
FYI some fetcher tests have also been posted in the bitbake mailing list.

Regards,
Jean-Marie






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

* Re: [PATCH v3 00/17] NPM refactoring
  2019-11-21  6:36   ` Tim Orling
  2019-11-21 13:21     ` Jean-Marie LEMETAYER
@ 2019-11-21 14:07     ` André Draszik
  1 sibling, 0 replies; 29+ messages in thread
From: André Draszik @ 2019-11-21 14:07 UTC (permalink / raw)
  To: openembedded-core

Hi,

On Wed, 2019-11-20 at 22:36 -0800, Tim Orling wrote:
> 
> 
> > On Nov 20, 2019, at 10:27 PM, Tim Orling <timothy.t.orling@linux.intel.com> wrote:
> > 
> > 
> > 
> > > On Nov 20, 2019, at 1:33 AM, Jean-Marie LEMETAYER <jean-marie.lemetayer@savoirfairelinux.com> wrote:
> > > 
> > > The current NPM support have several issues:
> > > - The current NPM fetcher downloads the dependency tree but not the other
> > >   fetchers. The 'subdir' parameter was used to fix this issue.
> > > - They are multiple issues with package names (uppercase, exotic characters,
> > >   scoped packages) even if they are inside the dependencies.
> > > - The lockdown file generation have issues. When a package depends on
> > >   multiple version of the same package (all versions have the same checksum).
> > > 
> > > This patchset refactors the NPM support in Yocto:
> > > - As the NPM algorithm for dependency management is hard to handle, the new
> > >   NPM fetcher downloads only the package source (and not the dependencies,
> > >   like the other fetchers) (patch submitted in the bitbake-devel list).
> > > - The NPM class handles the dependencies using NPM (and not manually).
> > > - The NPM recipe creation is simplified to avoid issues.
> > > - The lockdown file is no more used as it is no longer relevant compared to the
> > >   latest shrinkwrap file format.
> > > 
> > > This patchset may remove some features (lockdown file, license management for
> > > dependencies) but fixes the majority of the NPM issues. All of these issues
> > > from the bugzilla.yoctoproject.org are resolved by this patchset:
> > > #10237, #10760, #11028, #11728, #11902, #12534
> > > 
> > > The fetcher and recipetool are now aware of a 'latest' keyword for the version
> > > which allow to build the latest version available on the registry. This feature
> > > fixes the two issues: #10515, #11029
> > > 
> > > Moreover the issue #13415 should also be fixed but I cannot test it.
> > > 
> > > I have tested the recipe creation and builds using a self made example to
> > > generate build failures:
> > > - https://github.com/savoirfairelinux/node-server-example
> > > - https://npmjs.com/package/@savoirfairelinux/node-server-example
> > > 
> > > The test steps are these ones:
> > >  $ source poky/oe-init-build-env
> > >  $ bitbake-layers add-layer ../meta-openembedded/meta-oe
> > >  $ devtool add "npm://registry.npmjs.org;name=@savoirfairelinux/node-server-example;version=latest”
> > 
> > I noticed this before but forgot to report it:
> > 
> > ERROR: Nothing PROVIDES 'c-ares-native' (but virtual:native:/build/meta-openembedded/meta-oe/recipes-
> > devtools/nodejs/nodejs_10.17.0.bb DEPENDS on or otherwise requires it). ...
> > http://layers.openembedded.org/layerindex/recipe/50318/
> > $ bitbake-layers add-layer ../meta-openembedded/meta-networking
> > Which requires
> > $ bitbake-layers add-layer ../meta-openembedded/meta-python
> > 
> > 
> > Using tip of bitbake with V3, openembedded-core with V3 and meta-openembedded master, somehow nodejs-native
> > do_compile broke (at least on qemux86-64) since the last time I built it shortly after ELC-E…
> > So now I’ll have to git bisect that…more to come…
> 
> See https://errors.yoctoproject.org/Errors/Details/274939/
> The same error I am seeing.

Might be caused by 0005-Link-atomic-library.patch which unconditionally makes it
link against libatomic.so
I'd say this should be done conditionally on those arches that need it, exclusively.
Ideally using some kind of auto-detection...

Not sure why it's a problem only now, though.

Cheers,
Andre'

> 
> > FWIW I had previously successfully run your instructions and the tests for both bitbake and oe-selftest for V2
> > (distroless, e.g. not with poky). Something unrelated has broken I suspect.
> > 
> > >  $ devtool build savoirfairelinux-node-server-example
> > >  $ bitbake-layers create-layer ../meta-test
> > >  $ bitbake-layers add-layer ../meta-test
> > >  $ devtool finish savoirfairelinux-node-server-example ../meta-test
> > >  $ echo IMAGE_INSTALL_append = '" savoirfairelinux-node-server-example"' >> conf/local.conf
> > >  $ bitbake core-image-minimal
> > > 
> > > Also the 'devtool add' url could be one of these:
> > > - npm://registry.npmjs.org;name=@savoirfairelinux/node-server-example;version=latest
> > >   This url uses the new npm fetcher and request the latest version available on
> > >   the registry.
> > > - npm://registry.npmjs.org;name=@savoirfairelinux/node-server-example;version=1.0.0
> > >   This url uses the new npm fetcher and request a fixed version.
> > > - git://github.com/savoirfairelinux/node-server-example.git;protocol=https
> > >   This url uses the git fetcher. As the dependencies are managed by the NPM
> > >   class, any fetcher can be used.
> > > 
> > > When this patchset will be merged, I have planned to update the NPM wiki:
> > >  https://wiki.yoctoproject.org/wiki/TipsAndTricks/NPM
> > > 
> > > This patchset is also the base work for the full Angular support in Yocto that I
> > > am preparing. These applications have a huge dependency tree which is ideal to
> > > test the NPM support.
> > > 
> > > --- V2
> > > 
> > > - Add the 'check_network_access' function before each network access to check
> > >   for 'BB_NO_NETWORK' and 'BB_ALLOWED_NETWORKS' variables.
> > > 
> > > - Add a 'recipetool.RecipetoolTests.test_recipetool_create_npm' test case for
> > >   'oe-selftest' to test the npm recipe creation.
> > > 
> > > - Update the 'npm.bbclass' to fetch the dependencies in the 'do_fetch' task.
> > >   The dependencies are cached in a npm cache stored in '${DL_DIR}/npm_cache'
> > >   allowing the dependencies to be saved in the download directory and verify
> > >   on insertion and extraction [1]:
> > >      "All data that passes through the cache is fully verified
> > >       for integrity on both insertion and extraction."
> > > 
> > > 1: https://docs.npmjs.com/cli/cache.html
> > > 
> > > --- V3
> > > 
> > > - Add a test regarding the 'devtool add' and 'devtool build' command:
> > >     - devtool.DevtoolAddTests.test_devtool_add_npm
> > > 
> > > - Add the license management for dependencies.
> > > 
> > > - Split the npm workflow to use the bitbake default tasks (do_fetch, do_unpack,
> > >   do_compile, do_install). Make sure that only the do_fetch task requires
> > >   network access.
> > > 
> > > - Split the commits for better understanding.
> > > 
> > > - Force the rebuild of prebuild addons.
> > > 
> > > - Use ${nonarch_libdir} instead of ${libdir} in the do_install task.
> > > 
> > >  - These patches can be found here:
> > >     - https://github.com/savoirfairelinux/openembedded-core/tree/npm-refactoring-v3
> > >     - https://github.com/savoirfairelinux/poky/tree/npm-refactoring-v3
> > > 
> > > Jean-Marie LEMETAYER (17):
> > >  devtool: npm: update command line options
> > >  npm.bbclass: refactor the npm class
> > >  recipetool/create_npm.py: refactor the npm recipe creation handler
> > >  lib/oe/package.py: remove unneeded npm_split_package_dirs function
> > >  devtool/standard.py: npm: update the append file
> > >  recipetool/create.py: npm: remove the 'noverify' url parameter
> > >  recipetool/create.py: npm: replace the 'latest' keyword
> > >  npm.bbclass: split the do_compile task
> > >  devtool/standard.py: npm: exclude the node_modules directory
> > >  recipetool/create_npm.py: handle the licenses of the dependencies
> > >  npm.bbclass: restrict the build to be offline
> > >  npm.bbclass: use the local node headers
> > >  recipetool/create_npm.py: convert the shrinkwrap file
> > >  npm.bbclass: force to rebuild the prebuild addons
> > >  devtool/standard.py: npm: configure the NPM_CACHE_DIR to be persistent
> > >  oeqa/selftest/recipetool: add npm recipe creation test
> > >  oeqa/selftest/devtool: add npm recipe build test
> > > 
> > > meta/classes/npm.bbclass                   | 224 +++++---
> > > meta/lib/oe/package.py                     |  33 --
> > > meta/lib/oeqa/selftest/cases/devtool.py    |  20 +
> > > meta/lib/oeqa/selftest/cases/recipetool.py |  21 +
> > > scripts/lib/devtool/standard.py            |  31 +-
> > > scripts/lib/recipetool/create.py           |  16 +-
> > > scripts/lib/recipetool/create_npm.py       | 565 +++++++++++----------
> > > 7 files changed, 510 insertions(+), 400 deletions(-)
> > > 
> > > --
> > > 2.20.1
> > > 
> > 
> > -- 
> > _______________________________________________
> > Openembedded-core mailing list
> > Openembedded-core@lists.openembedded.org
> > http://lists.openembedded.org/mailman/listinfo/openembedded-core
> 
> 



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

* Re: [PATCH v3 00/17] NPM refactoring
  2019-11-20  9:33 [PATCH v3 00/17] NPM refactoring Jean-Marie LEMETAYER
                   ` (17 preceding siblings ...)
  2019-11-21  6:27 ` [PATCH v3 00/17] NPM refactoring Tim Orling
@ 2019-11-21 18:26 ` Richard Purdie
  2019-11-21 19:53   ` Jean-Marie LEMETAYER
  18 siblings, 1 reply; 29+ messages in thread
From: Richard Purdie @ 2019-11-21 18:26 UTC (permalink / raw)
  To: Jean-Marie LEMETAYER, openembedded-core
  Cc: jonaskgandersson, paul.eggleton, rennes, bunk

On Wed, 2019-11-20 at 10:33 +0100, Jean-Marie LEMETAYER wrote:
> The current NPM support have several issues:
>  - The current NPM fetcher downloads the dependency tree but not the other
>    fetchers. The 'subdir' parameter was used to fix this issue.
>  - They are multiple issues with package names (uppercase, exotic characters,
>    scoped packages) even if they are inside the dependencies.
>  - The lockdown file generation have issues. When a package depends on
>    multiple version of the same package (all versions have the same checksum).
> 
> This patchset refactors the NPM support in Yocto:
>  - As the NPM algorithm for dependency management is hard to handle, the new
>    NPM fetcher downloads only the package source (and not the dependencies,
>    like the other fetchers) (patch submitted in the bitbake-devel list).

I'm a little confused by this item.

If the NPM fetcher only downloads a given package's source and it has
dependencies, where/when are the dependencies downloaded?

My worry here is that if the fetcher isn't downloading them, DL_DIR
can't contain all the needed pieces needed to reproduce a build at a
later date?

I suspect I'm missing something obvious...

Cheers,

Richard



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

* Re: [PATCH v3 00/17] NPM refactoring
  2019-11-21 18:26 ` Richard Purdie
@ 2019-11-21 19:53   ` Jean-Marie LEMETAYER
  2019-11-21 20:25     ` Richard Purdie
  0 siblings, 1 reply; 29+ messages in thread
From: Jean-Marie LEMETAYER @ 2019-11-21 19:53 UTC (permalink / raw)
  To: Richard Purdie
  Cc: Jonas Andersson, paul eggleton, Savoir-faire Linux Rennes, OE-core, bunk



Hi Richard,

On Nov 21, 2019, at 7:26 PM, Richard Purdie richard.purdie@linuxfoundation.org wrote:
> On Wed, 2019-11-20 at 10:33 +0100, Jean-Marie LEMETAYER wrote:
>> The current NPM support have several issues:
>>  - The current NPM fetcher downloads the dependency tree but not the other
>>    fetchers. The 'subdir' parameter was used to fix this issue.
>>  - They are multiple issues with package names (uppercase, exotic characters,
>>    scoped packages) even if they are inside the dependencies.
>>  - The lockdown file generation have issues. When a package depends on
>>    multiple version of the same package (all versions have the same checksum).
>> 
>> This patchset refactors the NPM support in Yocto:
>>  - As the NPM algorithm for dependency management is hard to handle, the new
>>    NPM fetcher downloads only the package source (and not the dependencies,
>>    like the other fetchers) (patch submitted in the bitbake-devel list).
> 
> I'm a little confused by this item.
> 
> If the NPM fetcher only downloads a given package's source and it has
> dependencies, where/when are the dependencies downloaded?
> 
> My worry here is that if the fetcher isn't downloading them, DL_DIR
> can't contain all the needed pieces needed to reproduce a build at a
> later date?
> 
> I suspect I'm missing something obvious...

The magic is in the bbclass, in the do_fetch_append() function:
https://github.com/savoirfairelinux/poky/blob/npm-refactoring-v3/meta/classes/npm.bbclass#L51

Which calls:
https://github.com/savoirfairelinux/poky/blob/npm-refactoring-v3/bitbake/lib/bb/fetch2/npm.py#L312

Which runs the npm fetcher for each dependencies described in the
shrinkwrap file. This is the same behavior for the do_unpack task.


To be more clear, a recipe like this:

```
SRC_URI = "npm://registry.npmjs.org/;name=my-package;version=1.0.0"
S = "${WORKDIR}/npm"
```

Will only fetch/unpack the package source without handling the
dependencies.

But a recipe like this:

```
SRC_URI = "npm://registry.npmjs.org/;name=my-package;version=1.0.0"
S = "${WORKDIR}/npm"

NPM_SHRINKWRAP = "${THISDIR}/${BPN}/npm-shrinkwrap.json"
inherit npm
```

Will fetch/unpack the package and all the dependencies which are
described in the shrinkwrap file.


Bonus tips: you can have your base package SRC_URI using another
fetcher than npm (e.g. git) without breaking anything. The behavior
will still be the same.


Best regards,
Jean-Marie


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

* Re: [PATCH v3 00/17] NPM refactoring
  2019-11-21 19:53   ` Jean-Marie LEMETAYER
@ 2019-11-21 20:25     ` Richard Purdie
  2019-11-22 11:08       ` Jean-Marie LEMETAYER
  0 siblings, 1 reply; 29+ messages in thread
From: Richard Purdie @ 2019-11-21 20:25 UTC (permalink / raw)
  To: Jean-Marie LEMETAYER
  Cc: Jonas Andersson, paul eggleton, Savoir-faire Linux Rennes, OE-core, bunk

On Thu, 2019-11-21 at 14:53 -0500, Jean-Marie LEMETAYER wrote:
> On Nov 21, 2019, at 7:26 PM, Richard Purdie 
> richard.purdie@linuxfoundation.org wrote:
> > On Wed, 2019-11-20 at 10:33 +0100, Jean-Marie LEMETAYER wrote:
> > > The current NPM support have several issues:
> > >  - The current NPM fetcher downloads the dependency tree but not
> > > the other
> > >    fetchers. The 'subdir' parameter was used to fix this issue.
> > >  - They are multiple issues with package names (uppercase, exotic
> > > characters,
> > >    scoped packages) even if they are inside the dependencies.
> > >  - The lockdown file generation have issues. When a package
> > > depends on
> > >    multiple version of the same package (all versions have the
> > > same checksum).
> > > 
> > > This patchset refactors the NPM support in Yocto:
> > >  - As the NPM algorithm for dependency management is hard to
> > > handle, the new
> > >    NPM fetcher downloads only the package source (and not the
> > > dependencies,
> > >    like the other fetchers) (patch submitted in the bitbake-devel 
> > > list).
> > 
> > I'm a little confused by this item.
> > 
> > If the NPM fetcher only downloads a given package's source and it
> > has
> > dependencies, where/when are the dependencies downloaded?
> > 
> > My worry here is that if the fetcher isn't downloading them, DL_DIR
> > can't contain all the needed pieces needed to reproduce a build at
> > a
> > later date?
> > 
> > I suspect I'm missing something obvious...
> 
> The magic is in the bbclass, in the do_fetch_append() function:
> https://github.com/savoirfairelinux/poky/blob/npm-refactoring-v3/meta/classes/npm.bbclass#L51
> 
> Which calls:
> https://github.com/savoirfairelinux/poky/blob/npm-refactoring-v3/bitbake/lib/bb/fetch2/npm.py#L312
> 
> Which runs the npm fetcher for each dependencies described in the
> shrinkwrap file. This is the same behavior for the do_unpack task.
> 
> 
> To be more clear, a recipe like this:
> 
> ```
> SRC_URI = "npm://registry.npmjs.org/;name=my-package;version=1.0.0"
> S = "${WORKDIR}/npm"
> ```
> 
> Will only fetch/unpack the package source without handling the
> dependencies.
> 
> But a recipe like this:
> 
> ```
> SRC_URI = "npm://registry.npmjs.org/;name=my-package;version=1.0.0"
> S = "${WORKDIR}/npm"
> 
> NPM_SHRINKWRAP = "${THISDIR}/${BPN}/npm-shrinkwrap.json"
> inherit npm
> ```
> 
> Will fetch/unpack the package and all the dependencies which are
> described in the shrinkwrap file.

That does remove some of my worries, thanks for explaining!

It does make me wonder if we shouldn't have two fetch classes and then
support a url like:
 
SRC_URI = "npm://registry.npmjs.org/;name=my-package;version=1.0.0 \
           npmsw://${THISDIR}/${BPN}/npm-shrinkwrap.json"

where the "npmsw" handler would handle the shrinkwrap file?

Basically I don't like the way the current code has to tag the
shrinkwrap file handling on to the fetcher...

> Bonus tips: you can have your base package SRC_URI using another
> fetcher than npm (e.g. git) without breaking anything. The behavior
> will still be the same.

That does start to make it clearer why this is advantageous.

Cheers,

Richard



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

* Re: [PATCH v3 00/17] NPM refactoring
  2019-11-21 20:25     ` Richard Purdie
@ 2019-11-22 11:08       ` Jean-Marie LEMETAYER
  2019-11-25 13:55         ` Ross Burton
  0 siblings, 1 reply; 29+ messages in thread
From: Jean-Marie LEMETAYER @ 2019-11-22 11:08 UTC (permalink / raw)
  To: Richard Purdie
  Cc: Jonas Andersson, paul eggleton, Savoir-faire Linux Rennes, OE-core, bunk

Hi Richard,

On Nov 21, 2019, at 9:25 PM, Richard Purdie richard.purdie@linuxfoundation.org wrote:
> On Thu, 2019-11-21 at 14:53 -0500, Jean-Marie LEMETAYER wrote:
>> On Nov 21, 2019, at 7:26 PM, Richard Purdie
>> richard.purdie@linuxfoundation.org wrote:
>> > On Wed, 2019-11-20 at 10:33 +0100, Jean-Marie LEMETAYER wrote:
>> > > The current NPM support have several issues:
>> > >  - The current NPM fetcher downloads the dependency tree but not
>> > > the other
>> > >    fetchers. The 'subdir' parameter was used to fix this issue.
>> > >  - They are multiple issues with package names (uppercase, exotic
>> > > characters,
>> > >    scoped packages) even if they are inside the dependencies.
>> > >  - The lockdown file generation have issues. When a package
>> > > depends on
>> > >    multiple version of the same package (all versions have the
>> > > same checksum).
>> > > 
>> > > This patchset refactors the NPM support in Yocto:
>> > >  - As the NPM algorithm for dependency management is hard to
>> > > handle, the new
>> > >    NPM fetcher downloads only the package source (and not the
>> > > dependencies,
>> > >    like the other fetchers) (patch submitted in the bitbake-devel
>> > > list).
>> > 
>> > I'm a little confused by this item.
>> > 
>> > If the NPM fetcher only downloads a given package's source and it
>> > has
>> > dependencies, where/when are the dependencies downloaded?
>> > 
>> > My worry here is that if the fetcher isn't downloading them, DL_DIR
>> > can't contain all the needed pieces needed to reproduce a build at
>> > a
>> > later date?
>> > 
>> > I suspect I'm missing something obvious...
>> 
>> The magic is in the bbclass, in the do_fetch_append() function:
>> https://github.com/savoirfairelinux/poky/blob/npm-refactoring-v3/meta/classes/npm.bbclass#L51
>> 
>> Which calls:
>> https://github.com/savoirfairelinux/poky/blob/npm-refactoring-v3/bitbake/lib/bb/fetch2/npm.py#L312
>> 
>> Which runs the npm fetcher for each dependencies described in the
>> shrinkwrap file. This is the same behavior for the do_unpack task.
>> 
>> 
>> To be more clear, a recipe like this:
>> 
>> ```
>> SRC_URI = "npm://registry.npmjs.org/;name=my-package;version=1.0.0"
>> S = "${WORKDIR}/npm"
>> ```
>> 
>> Will only fetch/unpack the package source without handling the
>> dependencies.
>> 
>> But a recipe like this:
>> 
>> ```
>> SRC_URI = "npm://registry.npmjs.org/;name=my-package;version=1.0.0"
>> S = "${WORKDIR}/npm"
>> 
>> NPM_SHRINKWRAP = "${THISDIR}/${BPN}/npm-shrinkwrap.json"
>> inherit npm
>> ```
>> 
>> Will fetch/unpack the package and all the dependencies which are
>> described in the shrinkwrap file.
> 
> That does remove some of my worries, thanks for explaining!
> 
> It does make me wonder if we shouldn't have two fetch classes and then
> support a url like:
> 
> SRC_URI = "npm://registry.npmjs.org/;name=my-package;version=1.0.0 \
>           npmsw://${THISDIR}/${BPN}/npm-shrinkwrap.json"
> 
> where the "npmsw" handler would handle the shrinkwrap file?
> 
> Basically I don't like the way the current code has to tag the
> shrinkwrap file handling on to the fetcher...

This definitely sounds like a great idea !

I based my work on the current implementation, but I never thought of
refactoring as much. I have however some concerns about the management
of the registry and the development dependencies. I will try to come
back with a v4.

>> Bonus tips: you can have your base package SRC_URI using another
>> fetcher than npm (e.g. git) without breaking anything. The behavior
>> will still be the same.
> 
> That does start to make it clearer why this is advantageous.

Regards,
Jean-Marie


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

* Re: [PATCH v3 00/17] NPM refactoring
  2019-11-22 11:08       ` Jean-Marie LEMETAYER
@ 2019-11-25 13:55         ` Ross Burton
  2019-11-25 15:03           ` Jean-Marie LEMETAYER
  0 siblings, 1 reply; 29+ messages in thread
From: Ross Burton @ 2019-11-25 13:55 UTC (permalink / raw)
  To: Jean-Marie LEMETAYER
  Cc: paul eggleton, bunk, Savoir-faire Linux Rennes, OE-core

On 22/11/2019 11:08, Jean-Marie LEMETAYER wrote:
> I based my work on the current implementation, but I never thought of
> refactoring as much. I have however some concerns about the management
> of the registry and the development dependencies. I will try to come
> back with a v4.

There's a lot of NPM patches flying around now: are any of the patches 
in this 17-long series able to be merged ahead of a full refactor?

Ross


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

* Re: [PATCH v3 08/17] npm.bbclass: split the do_compile task
  2019-11-20  9:33 ` [PATCH v3 08/17] npm.bbclass: split the do_compile task Jean-Marie LEMETAYER
@ 2019-11-25 13:58   ` Ross Burton
  0 siblings, 0 replies; 29+ messages in thread
From: Ross Burton @ 2019-11-25 13:58 UTC (permalink / raw)
  To: openembedded-core

On 20/11/2019 09:33, Jean-Marie LEMETAYER wrote:
> +python do_fetch_append() {
> +    """
> +        Fetch all dependencies tarball to DL_DIR.
> +    """
> +    bb.fetch2.npm.fetch_dependencies(d)
> +}
> +
> +python do_unpack_append() {
> +    """
> +        Unpack all dependencies tarball to the 'node_modules' directory and add
> +        them to the npm cache. The dependencies needs to be unpacked to have
> +        access to the licenses files checksum feature (LIC_FILES_CHKSUM). They
> +        also have to be in the cache to properly execute the 'npm install'.
> +    """
> +    bb.fetch2.npm.unpack_dependencies(d)
> +}

I'd say that appends are a little ugly for introduction of core 
behaviour, maybe using do_fetch[postfuncs] would be neater.

Ross


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

* Re: [PATCH v3 00/17] NPM refactoring
  2019-11-25 13:55         ` Ross Burton
@ 2019-11-25 15:03           ` Jean-Marie LEMETAYER
  0 siblings, 0 replies; 29+ messages in thread
From: Jean-Marie LEMETAYER @ 2019-11-25 15:03 UTC (permalink / raw)
  To: Ross Burton; +Cc: Savoir-faire Linux Rennes, OE-core

Hi Ross,

On Nov 25, 2019, at 2:55 PM, Ross Burton ross.burton@intel.com wrote:
> On 22/11/2019 11:08, Jean-Marie LEMETAYER wrote:
>> I based my work on the current implementation, but I never thought of
>> refactoring as much. I have however some concerns about the management
>> of the registry and the development dependencies. I will try to come
>> back with a v4.
> 
> There's a lot of NPM patches flying around now: are any of the patches
> in this 17-long series able to be merged ahead of a full refactor?

No every patches are more or less linked. Moreover Richard points me
some possible enhancements. I will come back with a v4.

Thanks,
Jean-Marie


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

end of thread, other threads:[~2019-11-25 15:03 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-20  9:33 [PATCH v3 00/17] NPM refactoring Jean-Marie LEMETAYER
2019-11-20  9:33 ` [PATCH v3 01/17] devtool: npm: update command line options Jean-Marie LEMETAYER
2019-11-20  9:33 ` [PATCH v3 02/17] npm.bbclass: refactor the npm class Jean-Marie LEMETAYER
2019-11-20  9:33 ` [PATCH v3 03/17] recipetool/create_npm.py: refactor the npm recipe creation handler Jean-Marie LEMETAYER
2019-11-20  9:33 ` [PATCH v3 04/17] lib/oe/package.py: remove unneeded npm_split_package_dirs function Jean-Marie LEMETAYER
2019-11-20  9:33 ` [PATCH v3 05/17] devtool/standard.py: npm: update the append file Jean-Marie LEMETAYER
2019-11-20  9:33 ` [PATCH v3 06/17] recipetool/create.py: npm: remove the 'noverify' url parameter Jean-Marie LEMETAYER
2019-11-20  9:33 ` [PATCH v3 07/17] recipetool/create.py: npm: replace the 'latest' keyword Jean-Marie LEMETAYER
2019-11-20  9:33 ` [PATCH v3 08/17] npm.bbclass: split the do_compile task Jean-Marie LEMETAYER
2019-11-25 13:58   ` Ross Burton
2019-11-20  9:33 ` [PATCH v3 09/17] devtool/standard.py: npm: exclude the node_modules directory Jean-Marie LEMETAYER
2019-11-20  9:33 ` [PATCH v3 10/17] recipetool/create_npm.py: handle the licenses of the dependencies Jean-Marie LEMETAYER
2019-11-20  9:33 ` [PATCH v3 11/17] npm.bbclass: restrict the build to be offline Jean-Marie LEMETAYER
2019-11-20  9:33 ` [PATCH v3 12/17] npm.bbclass: use the local node headers Jean-Marie LEMETAYER
2019-11-20  9:33 ` [PATCH v3 13/17] recipetool/create_npm.py: convert the shrinkwrap file Jean-Marie LEMETAYER
2019-11-20  9:33 ` [PATCH v3 14/17] npm.bbclass: force to rebuild the prebuild addons Jean-Marie LEMETAYER
2019-11-20  9:33 ` [PATCH v3 15/17] devtool/standard.py: npm: configure the NPM_CACHE_DIR to be persistent Jean-Marie LEMETAYER
2019-11-20  9:33 ` [PATCH v3 16/17] oeqa/selftest/recipetool: add npm recipe creation test Jean-Marie LEMETAYER
2019-11-20  9:33 ` [PATCH v3 17/17] oeqa/selftest/devtool: add npm recipe build test Jean-Marie LEMETAYER
2019-11-21  6:27 ` [PATCH v3 00/17] NPM refactoring Tim Orling
2019-11-21  6:36   ` Tim Orling
2019-11-21 13:21     ` Jean-Marie LEMETAYER
2019-11-21 14:07     ` André Draszik
2019-11-21 18:26 ` Richard Purdie
2019-11-21 19:53   ` Jean-Marie LEMETAYER
2019-11-21 20:25     ` Richard Purdie
2019-11-22 11:08       ` Jean-Marie LEMETAYER
2019-11-25 13:55         ` Ross Burton
2019-11-25 15:03           ` 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.