Skip to contentSkip to conversation

devtool/recipetool fixes

PATCH New Paul Eggleton 1 participant 5 messages Feb 6, 2017

lore
Share & tools
lore
#0 lore raw

cover devtool/recipetool fixes

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(-)
Signature · 1 line
2.9.3
#1 lore raw

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

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.

---
 meta/classes/npm.bbclass | 3 +++
 1 file changed, 3 insertions(+)
meta/classes/npm.bbclass +3 -0
index 1c778a7..c538040 100644
@@ -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
Signature · 1 line
2.9.3
#2 lore raw

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

In reply to 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.

---
 scripts/lib/recipetool/create_npm.py | 56 +++++++++++++++++++++++++++++++-----
 1 file changed, 49 insertions(+), 7 deletions(-)
scripts/lib/recipetool/create_npm.py +49 -7
index 888aa2b..3ba6de0 100644
@@ -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))
Signature · 1 line
2.9.3
#3 lore raw

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

In reply to 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.)

---
 scripts/lib/recipetool/create.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
scripts/lib/recipetool/create.py +1 -2
index 0801223..5bd3853 100644
@@ -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)
Signature · 1 line
2.9.3
#4 lore raw

PATCH 4/4 devtool: improve parse failure handling

In reply to 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.

---
 scripts/create-pull-request     |  1 +
 scripts/lib/devtool/__init__.py |  7 ++++++-
 scripts/lib/devtool/standard.py | 11 ++++++++++-
 3 files changed, 17 insertions(+), 2 deletions(-)
scripts/create-pull-request +1 -0
index e82858b..7c6b96d 100755
@@ -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."
scripts/lib/devtool/__init__.py +6 -1
index fd2f042..91111e1 100644
@@ -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):
     """
scripts/lib/devtool/standard.py +10 -1
index 30b247f..5bd498c 100644
@@ -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:
Signature · 1 line
2.9.3