[PATCH 0/4] devtool/recipetool fixes

[PATCH 0/4] devtool/recipetool fixes

[Paul Eggleton]

Some fixes for devtool/recipetool, plus one related fix to the npm
class.


The following changes since commit b3a74335e1f9701ce2d6217080800baf3a61acb4:

  Revert "yocto-bsps: add 4.9 bbappend" (2017-02-06 14:45:48 +0000)

are available in the git repository at:

  git://git.openembedded.org/openembedded-core-contrib paule/devtool25
  http://cgit.openembedded.org/openembedded-core-contrib/log/?h=paule/devtool25

Paul Eggleton (4):
  classes/npm: set HOME during do_install
  recipetool: create: properly handle npm optional dependencies
  recipetool: create: do not treat numbers in SCM URLs as versions
  devtool: improve parse failure handling

 meta/classes/npm.bbclass             |  3 ++
 scripts/create-pull-request          |  1 +
 scripts/lib/devtool/__init__.py      |  7 ++++-
 scripts/lib/devtool/standard.py      | 11 ++++++-
 scripts/lib/recipetool/create.py     |  3 +-
 scripts/lib/recipetool/create_npm.py | 56 +++++++++++++++++++++++++++++++-----
 6 files changed, 70 insertions(+), 11 deletions(-)

-- 
2.9.3

[PATCH 1/4] classes/npm: set HOME during do_install

[Paul Eggleton]

In do_compile we set HOME so that ~/.npm* only get created in the work
directory; we need to do the same in do_install as well or they'll go
into the user's home directory which we do not want.

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 meta/classes/npm.bbclass | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/meta/classes/npm.bbclass b/meta/classes/npm.bbclass
index 1c778a7..c538040 100644
--- a/meta/classes/npm.bbclass
+++ b/meta/classes/npm.bbclass
@@ -32,6 +32,9 @@ npm_do_compile() {
 }
 
 npm_do_install() {
+	# changing the home directory to the working directory, the .npmrc will
+	# be created in this directory
+	export HOME=${WORKDIR}
 	mkdir -p ${NPM_INSTALLDIR}/
 	npm install --prefix ${D}${prefix} -g --arch=${NPM_ARCH} --target_arch=${NPM_ARCH} --production --no-registry
 	if [ -d ${D}${prefix}/etc ] ; then
-- 
2.9.3

[PATCH 2/4] recipetool: create: properly handle npm optional dependencies

[Paul Eggleton]

npm's package.json supports two types of dependencies -
optionalDependencies and dependencies; in the code for creating a recipe
from a non-npm source (e.g. a git repository) we were not handling
optionalDependencies and thus when pointed at a node.js application
outside of npm we weren't taking care of all dependencies.

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 scripts/lib/recipetool/create_npm.py | 56 +++++++++++++++++++++++++++++++-----
 1 file changed, 49 insertions(+), 7 deletions(-)

diff --git a/scripts/lib/recipetool/create_npm.py b/scripts/lib/recipetool/create_npm.py
index 888aa2b..3ba6de0 100644
--- a/scripts/lib/recipetool/create_npm.py
+++ b/scripts/lib/recipetool/create_npm.py
@@ -83,7 +83,7 @@ class NpmRecipeHandler(RecipeHandler):
         extravalues['extrafiles']['lockdown.json'] = tmpfile
         lines_before.append('NPM_LOCKDOWN := "${THISDIR}/${PN}/lockdown.json"')
 
-    def _handle_dependencies(self, d, deps, lines_before, srctree):
+    def _handle_dependencies(self, d, deps, optdeps, lines_before, srctree):
         import scriptutils
         # If this isn't a single module we need to get the dependencies
         # and add them to SRC_URI
@@ -92,8 +92,17 @@ class NpmRecipeHandler(RecipeHandler):
                 if not origvalue.startswith('npm://'):
                     src_uri = origvalue.split()
                     changed = False
-                    for dep, depdata in deps.items():
-                        version = self.get_node_version(dep, depdata, d)
+                    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 deps.items():
+                        depdata = self.get_npm_data(dep, depver, d)
+                        deplist[dep] = depdata
+
+                    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)
                             scriptutils.fetch_uri(d, url, srctree)
@@ -170,8 +179,8 @@ class NpmRecipeHandler(RecipeHandler):
                 if 'homepage' in data:
                     extravalues['HOMEPAGE'] = data['homepage']
 
-                deps = data.get('dependencies', {})
-                updated = self._handle_dependencies(tinfoil.config_data, deps, lines_before, srctree)
+                deps, optdeps = self.get_npm_package_dependencies(data)
+                updated = self._handle_dependencies(tinfoil.config_data, deps, optdeps, lines_before, srctree)
                 if updated:
                     # We need to redo the license stuff
                     self._replace_license_vars(srctree, lines_before, handled, extravalues, tinfoil.config_data)
@@ -251,7 +260,7 @@ class NpmRecipeHandler(RecipeHandler):
 
     # FIXME this is effectively duplicated from lib/bb/fetch2/npm.py
     # (split out from _getdependencies())
-    def get_node_version(self, pkg, version, d):
+    def get_npm_data(self, pkg, version, d):
         import bb.fetch2
         pkgfullname = pkg
         if version != '*' and not '/' in version:
@@ -261,7 +270,40 @@ class NpmRecipeHandler(RecipeHandler):
         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.get('version', None)
+        return data
+
+    # FIXME this is effectively duplicated from lib/bb/fetch2/npm.py
+    # (split out from _getdependencies())
+    def get_npm_package_dependencies(self, pdata):
+        dependencies = pdata.get('dependencies', {})
+        optionalDependencies = pdata.get('optionalDependencies', {})
+        dependencies.update(optionalDependencies)
+        depsfound = {}
+        optdepsfound = {}
+        for dep in dependencies:
+            if dep in optionalDependencies:
+                optdepsfound[dep] = dependencies[dep]
+            else:
+                depsfound[dep] = dependencies[dep]
+        return depsfound, optdepsfound
+
+    # 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:
+                logger.debug(2, "Skipping %s since it's incompatible with Linux" % pkg)
+                return False
+        return True
+
 
 def register_recipe_handlers(handlers):
     handlers.append((NpmRecipeHandler(), 60))
-- 
2.9.3

[PATCH 3/4] recipetool: create: do not treat numbers in SCM URLs as versions

[Paul Eggleton]

Numbers within SCM (e.g. git) URLs are extremely unlikely to be valid
version numbers - more likely they are just part of the name, thus don't
try to extract them and use them as the version - doing so causes pretty
bad behaviour within devtool:

--------- snip ---------
$ devtool add https://github.com/inhedron/libtr50
NOTE: Fetching git://github.com/inhedron/libtr50;protocol=https...
...
NOTE: Using default source tree path .../build/workspace/sources/libtr
...
RecursionError: maximum recursion depth exceeded while calling a Python object
--------- snip ---------

(This was because ${PV} was being substituted into the URL, but PV's
value was being set to include ${SRCPV}, so there was a circular
reference.)

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 scripts/lib/recipetool/create.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/scripts/lib/recipetool/create.py b/scripts/lib/recipetool/create.py
index 0801223..5bd3853 100644
--- a/scripts/lib/recipetool/create.py
+++ b/scripts/lib/recipetool/create.py
@@ -336,7 +336,7 @@ def determine_from_url(srcuri):
                 pn = res.group(1).strip().replace('_', '-')
                 pv = res.group(2).strip().replace('_', '.')
 
-        if not pn and not pv:
+        if not pn and not pv and parseres.scheme not in ['git', 'gitsm', 'svn', 'hg']:
             srcfile = os.path.basename(parseres.path.rstrip('/'))
             pn, pv = determine_from_filename(srcfile)
 
@@ -567,7 +567,6 @@ def create_recipe(args):
         if name_pv and not realpv:
             realpv = name_pv
 
-
     if not srcuri:
         lines_before.append('# No information for SRC_URI yet (only an external source tree was specified)')
     lines_before.append('SRC_URI = "%s"' % srcuri)
-- 
2.9.3

[PATCH 4/4] devtool: improve parse failure handling

[Paul Eggleton]

With the move to tinfoil2, the behaviour when parsing failed has changed
a bit - exceptions are now raised, so handle these appropriately.
Specifically when if parsing the recipe created when running devtool add
fails, rename it to .bb.parsefailed so that the user can run bitbake
afterwards without parsing being interrupted.

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 scripts/create-pull-request     |  1 +
 scripts/lib/devtool/__init__.py |  7 ++++++-
 scripts/lib/devtool/standard.py | 11 ++++++++++-
 3 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/scripts/create-pull-request b/scripts/create-pull-request
index e82858b..7c6b96d 100755
--- a/scripts/create-pull-request
+++ b/scripts/create-pull-request
@@ -183,6 +183,7 @@ if [ -n "$WEB_URL" ]; then
 		git push $REMOTE $L_BRANCH:$BRANCH
 		echo ""
 	fi
+	echo $WEB_URL
 	wget --no-check-certificate -q $WEB_URL -O /dev/null
 	if [ $? -ne 0 ]; then
 		echo "WARNING: Branch '$BRANCH' was not found on the contrib git tree."
diff --git a/scripts/lib/devtool/__init__.py b/scripts/lib/devtool/__init__.py
index fd2f042..91111e1 100644
--- a/scripts/lib/devtool/__init__.py
+++ b/scripts/lib/devtool/__init__.py
@@ -135,7 +135,12 @@ def parse_recipe(config, tinfoil, pn, appends, filter_workspace=True):
                             not path.startswith(config.workspace_path)]
     else:
         append_files = None
-    return tinfoil.parse_recipe_file(recipefile, appends, append_files)
+    try:
+        rd = tinfoil.parse_recipe_file(recipefile, appends, append_files)
+    except Exception as e:
+        logger.error(str(e))
+        return None
+    return rd
 
 def check_workspace_recipe(workspace, pn, checksrc=True, bbclassextend=False):
     """
diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
index 30b247f..5bd498c 100644
--- a/scripts/lib/devtool/standard.py
+++ b/scripts/lib/devtool/standard.py
@@ -224,8 +224,17 @@ def add(args, config, basepath, workspace):
 
     tinfoil = setup_tinfoil(config_only=True, basepath=basepath)
     try:
-        rd = tinfoil.parse_recipe_file(recipefile, False)
+        try:
+            rd = tinfoil.parse_recipe_file(recipefile, False)
+        except Exception as e:
+            logger.error(str(e))
+            rd = None
         if not rd:
+            # Parsing failed. We just created this recipe and we shouldn't
+            # leave it in the workdir or it'll prevent bitbake from starting
+            movefn = '%s.parsefailed' % recipefile
+            logger.error('Parsing newly created recipe failed, moving recipe to %s for reference. If this looks to be caused by the recipe itself, please report this error.' % movefn)
+            shutil.move(recipefile, movefn)
             return 1
 
         if args.fetchuri and not args.no_git:
-- 
2.9.3