Skip to contentSkip to conversation

devtool/recipetool fixes

PATCH New Paul Eggleton 1 participant 3 messages Jul 2, 2019

lore
Share & tools
lore
#0 lore raw

cover devtool/recipetool fixes

The following changes since commit 1f8a73261c7a134821c40845da27d8edbb0763b6:

Revert "pigz: Add debug for autobuilder errors" (2019-06-30 23:33:12 +0100)

are available in the Git repository at:

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

Paul Eggleton (2):
recipetool: ignore zero-length setup.py files
devtool: upgrade: fix handling of errors parsing upgraded recipe

 scripts/lib/devtool/upgrade.py                | 28 ++++++++++++-------
 .../lib/recipetool/create_buildsys_python.py  |  9 ++++--
 2 files changed, 25 insertions(+), 12 deletions(-)
Signature · 1 line
2.20.1
#1 lore raw

PATCH 1/2 recipetool: ignore zero-length setup.py files

If a setup.py file exists it ought to have something in it before we consider the source tree to be a Python module and treating it as such. (A counter-example is https://www.bro.org/downloads/binpac-0.50.tar.gz - it's not clear why this has a zero-length setup.py in it but we should pay no attention to it.)

Fixes [YOCTO #12923].

---
 scripts/lib/recipetool/create_buildsys_python.py | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
scripts/lib/recipetool/create_buildsys_python.py +7 -2
index ac9bc9237ce..adfa3779561 100644
@@ -154,8 +154,13 @@ class PythonRecipeHandler(RecipeHandler):
         if 'buildsystem' in handled:
             return False
 
-        if not RecipeHandler.checkfiles(srctree, ['setup.py']):
-            return
+        # Check for non-zero size setup.py files
+        setupfiles = RecipeHandler.checkfiles(srctree, ['setup.py'])
+        for fn in setupfiles:
+            if os.path.getsize(fn):
+                break
+        else:
+            return False
 
         # setup.py is always parsed to get at certain required information, such as
         # distutils vs setuptools
Signature · 1 line
2.20.1
#2 lore raw

PATCH 2/2 devtool: upgrade: fix handling of errors parsing upgraded recipe

In reply to Paul Eggleton

As part of upgrading a recipe we create the upgraded recipe file in the workspace and then try to parse it so we can then make further modifications. If for some reason that parsing fails then the failure was not being handled very well - the broken recipe was being left in place, breaking parsing until it was removed by hand. Fix that by adding a call to the cleanup function, and fix the following issues:

* Fix the cleanup function which doesn't look like it has ever worked due to a typo in the function call

* Fix double-printing the error message

* Remove usage of DevtoolError in this case (DevtoolError is for simple usage errors, not this kind of issue which may be the result of a bug).

We're still printing a traceback in this scenario but at least it doesn't break the build system requiring manual cleanup. I also introduced a command-line option to preserve the broken upgraded recipe file(s) for debugging purposes.

(The reproducer for this is "devtool upgrade libnewt-python", however you need to check out revision b82ea144e144671d3f64c0785ba4beafe905cd4f or earlier since that recipe has now been absorbed into the libnewt recipe. The libnewt-python recipe was causing an issue with the upgrade because it actually included the libnewt recipe using ${PV} in the include statement, and of course PV was changing in the upgrade.)

Fixes [YOCTO #13404].

---
 scripts/lib/devtool/upgrade.py | 28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)
scripts/lib/devtool/upgrade.py +18 -10
index 62ec2f94cbc..706f91c9358 100644
@@ -122,18 +122,22 @@ def _cleanup_on_error(rf, srctree):
     rfp = os.path.split(rf)[0] # recipe folder
     rfpp = os.path.split(rfp)[0] # recipes folder
     if os.path.exists(rfp):
-        shutil.rmtree(b)
+        shutil.rmtree(rfp)
     if not len(os.listdir(rfpp)):
         os.rmdir(rfpp)
     srctree = os.path.abspath(srctree)
     if os.path.exists(srctree):
         shutil.rmtree(srctree)
 
-def _upgrade_error(e, rf, srctree):
-    if rf:
-        cleanup_on_error(rf, srctree)
+def _upgrade_error(e, rf, srctree, keep_failure=False, extramsg=None):
+    if rf and not keep_failure:
+        _cleanup_on_error(rf, srctree)
     logger.error(e)
-    raise DevtoolError(e)
+    if extramsg:
+        logger.error(extramsg)
+    if keep_failure:
+        logger.info('Preserving failed upgrade files (--keep-failure)')
+    sys.exit(1)
 
 def _get_uri(rd):
     srcuris = rd.getVar('SRC_URI').split()
@@ -299,7 +303,7 @@ def _add_license_diff_to_recipe(path, diff):
         f.write("\n#\n\n".encode())
         f.write(orig_content)
 
-def _create_new_recipe(newpv, md5, sha256, srcrev, srcbranch, srcsubdir_old, srcsubdir_new, workspace, tinfoil, rd, license_diff, new_licenses):
+def _create_new_recipe(newpv, md5, sha256, srcrev, srcbranch, srcsubdir_old, srcsubdir_new, workspace, tinfoil, rd, license_diff, new_licenses, srctree, keep_failure):
     """Creates the new recipe under workspace"""
 
     bpn = rd.getVar('BPN')
@@ -416,7 +420,10 @@ def _create_new_recipe(newpv, md5, sha256, srcrev, srcbranch, srcsubdir_old, src
         newvalues["LIC_FILES_CHKSUM"] = newlicchksum
         _add_license_diff_to_recipe(fullpath, license_diff)
 
-    rd = tinfoil.parse_recipe_file(fullpath, False)
+    try:
+        rd = tinfoil.parse_recipe_file(fullpath, False)
+    except bb.tinfoil.TinfoilCommandFailed as e:
+        _upgrade_error(e, fullpath, srctree, keep_failure, 'Parsing of upgraded recipe failed')
     oe.recipeutils.patch_recipe(rd, fullpath, newvalues)
 
     return fullpath, copied
@@ -548,11 +555,11 @@ def upgrade(args, config, basepath, workspace):
                                                     tinfoil, rd)
             new_licenses = _extract_licenses(srctree, rd.getVar('LIC_FILES_CHKSUM'))
             license_diff = _generate_license_diff(old_licenses, new_licenses)
-            rf, copied = _create_new_recipe(args.version, md5, sha256, args.srcrev, srcbranch, srcsubdir1, srcsubdir2, config.workspace_path, tinfoil, rd, license_diff, new_licenses)
+            rf, copied = _create_new_recipe(args.version, md5, sha256, args.srcrev, srcbranch, srcsubdir1, srcsubdir2, config.workspace_path, tinfoil, rd, license_diff, new_licenses, srctree, args.keep_failure)
         except bb.process.CmdError as e:
-            _upgrade_error(e, rf, srctree)
+            _upgrade_error(e, rf, srctree, args.keep_failure)
         except DevtoolError as e:
-            _upgrade_error(e, rf, srctree)
+            _upgrade_error(e, rf, srctree, args.keep_failure)
         standard._add_md5(config, pn, os.path.dirname(rf))
 
         af = _write_append(rf, srctree, args.same_dir, args.no_same_dir, rev2,
@@ -623,6 +630,7 @@ 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_upgrade.add_argument('--keep-temp', action="store_true", help='Keep temporary directory (for debugging)')
+    parser_upgrade.add_argument('--keep-failure', action="store_true", help='Keep failed upgrade recipe and associated files  (for debugging)')
     parser_upgrade.set_defaults(func=upgrade, fixed_setup=context.fixed_setup)
 
     parser_latest_version = subparsers.add_parser('latest-version', help='Report the latest version of an existing recipe',
Signature · 1 line
2.20.1