public inbox for isar-users@googlegroups.com
 help / color / mirror / Atom feed
* [PATCH v2 0/5] Python refactoring
@ 2019-02-12 16:56 Harald Seiler
  2019-02-12 16:56 ` [PATCH v2 1/5] Remove all uses of subprocess.call(shell=True) Harald Seiler
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Harald Seiler @ 2019-02-12 16:56 UTC (permalink / raw)
  To: isar-users

This series contains a number of improvements to python code in isar:

* All instancess of subprocess.{call,Popen,check_call}() have been changed to
  never use `shell=True`.  This will hopefully prevent bugs in the future.
* The use of the % formatter was replaced by other methods, as % was deprecated
  because it can behave weirdly.  Unfortunately, as we have to support python
  back to 3.5, we can not yet make use of f-Strings, so other options had to be
  used.  I decided to use string concatenation for simple cases where the use
  of .format() would have been too verbose.
* A recursive call in get_image_name was removed in favor of an imperative code
  style.  This should make the respective function easier to read.

Changes in v2:

    * Remove redundant encoding parameter
    * Fix formatting
    * Refactor a task that I previously missed
    * Add a commit for general style fixes

Harald Seiler (5):
  Remove all uses of subprocess.call(shell=True)
  Use modern python formatting
  image: Remove recursion in get_image_name
  wic: Refactor fakeroot script
  Fix python style

 .../example-module/example-module.bb               |  8 ++-
 meta/classes/base.bbclass                          | 70 ++++++++++++++--------
 meta/classes/image.bbclass                         | 22 +++++--
 meta/classes/isar-bootstrap-helper.bbclass         |  8 +--
 meta/classes/isar-events.bbclass                   | 14 ++---
 meta/classes/patch.bbclass                         | 13 ++--
 meta/classes/wic-img.bbclass                       | 21 +++++--
 .../isar-bootstrap/isar-bootstrap-host.bb          |  9 ++-
 .../isar-bootstrap/isar-bootstrap-target.bb        |  5 +-
 .../recipes-core/isar-bootstrap/isar-bootstrap.inc | 12 ++--
 scripts/wic_fakeroot                               |  9 +--
 11 files changed, 120 insertions(+), 71 deletions(-)

-- 
2.14.1


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

* [PATCH v2 1/5] Remove all uses of subprocess.call(shell=True)
  2019-02-12 16:56 [PATCH v2 0/5] Python refactoring Harald Seiler
@ 2019-02-12 16:56 ` Harald Seiler
  2019-02-12 16:56 ` [PATCH v2 2/5] Use modern python formatting Harald Seiler
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Harald Seiler @ 2019-02-12 16:56 UTC (permalink / raw)
  To: isar-users; +Cc: Claudius Heine

Using shell-expansion in subprocess calls has a lot of nasty
side-effects that are hard to debug should one ever surface.
This commit changes all uses of subprocess to

A) not use shell=True by giving a list of args instead of
   a command-line.
B) ensure return-codes are handled appropriately.

Furthermore, in some places the subprocess usage was changed to
be more idiomatic.

Co-authored-by: Claudius Heine <ch@denx.de>
Signed-off-by: Harald Seiler <hws@denx.de>
---
 meta/classes/base.bbclass                  | 10 ++++++----
 meta/classes/image.bbclass                 |  5 +++--
 meta/classes/isar-bootstrap-helper.bbclass |  8 +++-----
 meta/classes/isar-events.bbclass           | 13 +++++++------
 meta/classes/patch.bbclass                 | 13 +++++++++----
 5 files changed, 28 insertions(+), 21 deletions(-)

diff --git a/meta/classes/base.bbclass b/meta/classes/base.bbclass
index 6ca93bf..6fcf452 100644
--- a/meta/classes/base.bbclass
+++ b/meta/classes/base.bbclass
@@ -170,15 +170,17 @@ addtask clean
 do_clean[nostamp] = "1"
 python do_clean() {
     import subprocess
+    import glob
 
     for f in (d.getVar('CLEANFUNCS', True) or '').split():
         bb.build.exec_func(f, d)
 
-    dir = d.expand("${WORKDIR}")
-    subprocess.call('sudo rm -rf ' + dir, shell=True)
+    workdir = d.expand("${WORKDIR}")
+    subprocess.check_call(["sudo", "rm", "-rf", workdir])
 
-    dir = "%s.*" % bb.data.expand(d.getVar('STAMP', False), d)
-    subprocess.call('sudo rm -rf ' + dir, shell=True)
+    stamppath = bb.data.expand(d.getVar('STAMP', False), d)
+    stampdirs = glob.glob(stamppath + ".*")
+    subprocess.check_call(["sudo", "rm", "-rf"] + stampdirs)
 }
 
 # Derived from OpenEmbedded Core: meta/classes/base.bbclass
diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
index be00817..a687662 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -35,8 +35,9 @@ def get_rootfs_size(d):
     import subprocess
     rootfs_extra = int(d.getVar("ROOTFS_EXTRA", True))
 
-    output = subprocess.check_output(['sudo', 'du', '-s', '--block-size=1k',
-                                      d.getVar("IMAGE_ROOTFS", True)])
+    output = subprocess.check_output(
+        ["sudo", "du", "-s", "--block-size=1k", d.getVar("IMAGE_ROOTFS", True)]
+    )
     base_size = int(output.split()[0])
 
     return base_size + rootfs_extra * 1024
diff --git a/meta/classes/isar-bootstrap-helper.bbclass b/meta/classes/isar-bootstrap-helper.bbclass
index d780b85..18081a0 100644
--- a/meta/classes/isar-bootstrap-helper.bbclass
+++ b/meta/classes/isar-bootstrap-helper.bbclass
@@ -9,11 +9,9 @@ IMAGE_TRANSIENT_PACKAGES ??= ""
 
 def get_deb_host_arch():
     import subprocess
-    host_arch = subprocess.Popen("dpkg --print-architecture",
-                                 shell=True,
-                                 env=os.environ,
-                                 stdout=subprocess.PIPE
-                                ).stdout.read().decode('utf-8').strip()
+    host_arch = subprocess.check_output(
+        ["dpkg", "--print-architecture"]
+    ).strip()
     return host_arch
 
 #Debian Distribution for SDK host
diff --git a/meta/classes/isar-events.bbclass b/meta/classes/isar-events.bbclass
index c4a6149..a178d05 100644
--- a/meta/classes/isar-events.bbclass
+++ b/meta/classes/isar-events.bbclass
@@ -16,13 +16,14 @@ python isar_handler() {
 
     basepath = tmpdir + '/work/'
 
-    with open(os.devnull, 'w') as devnull:
-        with open('/proc/mounts', 'rU') as f:
-            lines = f.readlines()
-        for line in lines:
+    with open('/proc/mounts') as f:
+        for line in f.readlines():
             if basepath in line:
-                subprocess.call('sudo umount -l ' + line.split()[1],
-                                stdout=devnull, stderr=devnull, shell=True)
+                subprocess.call(
+                    ["sudo", "umount", "-l", line.split()[1]],
+                    stdout=subprocess.DEVNULL,
+                    stderr=subprocess.DEVNULL,
+                )
 }
 
 isar_handler[eventmask] = "bb.runqueue.runQueueExitWait bb.event.BuildCompleted"
diff --git a/meta/classes/patch.bbclass b/meta/classes/patch.bbclass
index 0bc449f..c19542d 100644
--- a/meta/classes/patch.bbclass
+++ b/meta/classes/patch.bbclass
@@ -21,10 +21,15 @@ python do_patch() {
 
             striplevel = fetcher.ud[src_uri].parm.get("striplevel") or "1"
 
-            cmd = "patch --no-backup-if-mismatch -p " + striplevel + \
-                  " --directory " + src_dir + " --input " + workdir + basename
-            bb.note(cmd)
-            if subprocess.call(cmd, shell=True) != 0:
+            cmd = [
+                "patch",
+                "--no-backup-if-mismatch",
+                "-p", striplevel,
+                "--directory", src_dir,
+                "--input", workdir + basename,
+            ]
+            bb.note(" ".join(cmd))
+            if subprocess.call(cmd) != 0:
                 bb.fatal("patching failed")
         except bb.fetch2.BBFetchException as e:
             raise bb.build.FuncFailed(e)
-- 
2.14.1


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

* [PATCH v2 2/5] Use modern python formatting
  2019-02-12 16:56 [PATCH v2 0/5] Python refactoring Harald Seiler
  2019-02-12 16:56 ` [PATCH v2 1/5] Remove all uses of subprocess.call(shell=True) Harald Seiler
@ 2019-02-12 16:56 ` Harald Seiler
  2019-02-12 16:56 ` [PATCH v2 3/5] image: Remove recursion in get_image_name Harald Seiler
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Harald Seiler @ 2019-02-12 16:56 UTC (permalink / raw)
  To: isar-users

The use of % for formatting is discouraged because it has side-
effects that are not immediately obvious.  This commit refactors
all uses of % to a better formatting style.

As f-Strings are not availible in in some python versions we support
(<3.6) the formatter of choice is .format() or manual concatenation
in cases where it is more concise.

This commit additionally refactors the showdata and listtasks tasks
to make better use of the new formatter code.

Signed-off-by: Harald Seiler <hws@denx.de>
---
 meta/classes/base.bbclass                          | 58 ++++++++++++++--------
 meta/classes/wic-img.bbclass                       | 21 +++++---
 .../recipes-core/isar-bootstrap/isar-bootstrap.inc | 12 ++---
 3 files changed, 58 insertions(+), 33 deletions(-)

diff --git a/meta/classes/base.bbclass b/meta/classes/base.bbclass
index 6fcf452..4dea51b 100644
--- a/meta/classes/base.bbclass
+++ b/meta/classes/base.bbclass
@@ -32,27 +32,35 @@ do_showdata[nostamp] = "1"
 python do_showdata() {
     for e in d.keys():
         if d.getVarFlag(e, 'python'):
-            bb.plain("\npython %s () {\n%s}\n" % (e, d.getVar(e, True)))
+            code = d.getVar(e, True)
+            if code.startswith("def"):
+                bb.plain("\n" + code + "\n")
+            else:
+                bb.plain(
+                    "\npython {name} () {{\n{code}}}\n".format(
+                        name=e, code=code
+                    )
+                )
 }
 
 # Derived from Open Embedded: openembedded-core/meta/classes/utility-tasks.bbclass
 addtask listtasks
 do_listtasks[nostamp] = "1"
 python do_listtasks() {
-    taskdescs = {}
+    tasks = {}
     maxlen = 0
     for e in d.keys():
         if d.getVarFlag(e, 'task'):
             maxlen = max(maxlen, len(e))
             if e.endswith('_setscene'):
-                desc = "%s (setscene version)" % (d.getVarFlag(e[:-9], 'doc') or '')
+                tasks[e] = (
+                    d.getVarFlag(e[:-9], 'doc') or ''
+                ) + " (setscene version)"
             else:
-                desc = d.getVarFlag(e, 'doc') or ''
-            taskdescs[e] = desc
+                tasks[e] = d.getVarFlag(e, 'doc') or ''
 
-    tasks = sorted(taskdescs.keys())
-    for taskname in tasks:
-        bb.plain("%s  %s" % (taskname.ljust(maxlen), taskdescs[taskname]))
+    for name, desc in sorted(tasks.items()):
+        bb.plain("{0:{len}}  {1}".format(name, desc, len=maxlen))
 }
 
 root_cleandirs() {
@@ -70,30 +78,40 @@ root_cleandirs() {
 
 python() {
     import re
+
     for e in d.keys():
         flags = d.getVarFlags(e)
         if flags and flags.get('task'):
             rcleandirs = flags.get('root_cleandirs')
             if rcleandirs:
                 tmpdir = os.path.normpath(d.getVar("TMPDIR", True))
-                rcleandirs = list(os.path.normpath(d.expand(i))
-                                  for i in rcleandirs.split())
+                rcleandirs = list(
+                    os.path.normpath(d.expand(i)) for i in rcleandirs.split()
+                )
 
                 for i in rcleandirs:
                     if not i.startswith(tmpdir):
-                        bb.fatal("root_cleandirs entry %s is not contained in "
-                                 "TMPDIR %s" % (i, tmpdir))
+                        bb.fatal(
+                            "root_cleandirs entry {} is not contained in TMPDIR {}".format(
+                                i, tmpdir
+                            )
+                        )
 
-                ws = re.match("^\s*", d.getVar(e, False)).group()
                 if flags.get('python'):
-                    d.prependVar(e, ws + "d.setVar('ROOT_CLEANDIRS_DIRS', '"
-                                       + " ".join(rcleandirs) + "')\n"
-                                  + ws + "bb.build.exec_func("
-                                       + "'root_cleandirs', d)\n")
+                    cleandir_code = (
+                        "{ws}d.setVar('ROOT_CLEANDIRS_DIRS', '{dirlist}')\n"
+                        "{ws}bb.build.exec_func('root_cleandirs', d)\n"
+                    )
                 else:
-                    d.prependVar(e, ws + "ROOT_CLEANDIRS_DIRS='"
-                                       + " ".join(rcleandirs) + "'\n"
-                                  + ws + "root_cleandirs\n")
+                    cleandir_code = (
+                        "{ws}ROOT_CLEANDIRS_DIRS='{dirlist}'\n"
+                        "{ws}root_cleandirs\n"
+                    )
+
+                ws = re.match(r"^\s*", d.getVar(e, False)).group()
+                d.prependVar(
+                    e, cleandir_code.format(ws=ws, dirlist=" ".join(rcleandirs))
+                )
 }
 
 # filter out all "apt://" URIs out of SRC_URI and stick them into SRC_APT
diff --git a/meta/classes/wic-img.bbclass b/meta/classes/wic-img.bbclass
index 76602d8..9cb869c 100644
--- a/meta/classes/wic-img.bbclass
+++ b/meta/classes/wic-img.bbclass
@@ -5,6 +5,8 @@
 #
 
 python () {
+    import itertools
+
     wks_full_path = None
 
     wks_file = d.getVar('WKS_FILE', True)
@@ -18,14 +20,21 @@ python () {
             wks_full_path = wks_file
     else:
         bbpaths = d.getVar('BBPATH', True).split(':')
+        corebase_paths = bbpaths
+
         corebase = d.getVar('COREBASE', True)
-        search_path = ':'.join('%s/wic' % p for p in bbpaths) + ':' + \
-            ':'.join('%s/scripts/lib/wic/canned-wks' % l \
-                     for l in (bbpaths + [corebase]))
+        if corebase is not None:
+            corebase_paths.append(corebase)
+
+        search_path = ":".join(itertools.chain(
+            (p + "/wic" for p in bbpaths),
+            (l + "/scripts/lib/wic/canned-wks"
+             for l in (corebase_paths)),
+        ))
         wks_full_path = bb.utils.which(search_path, wks_file)
 
     if not wks_full_path:
-        bb.fatal("WKS_FILE '%s' not found" % wks_file)
+        bb.fatal("WKS_FILE '{}' not found".format(wks_file))
 
     d.setVar('WKS_FULL_PATH', wks_full_path)
 }
@@ -68,12 +77,12 @@ python do_rootfs_wicenv () {
         for var in wicvars.split():
             value = d.getVar(var, True)
             if value:
-                envf.write('%s="%s"\n' % (var, value.strip()))
+                envf.write('{}="{}"\n'.format(var, value.strip()))
 
     # this part is stolen from OE ./meta/recipes-core/meta/wic-tools.bb
     with open(os.path.join(outdir, "wic-tools.env"), 'w') as envf:
         for var in ('RECIPE_SYSROOT_NATIVE', 'STAGING_DATADIR', 'STAGING_LIBDIR'):
-            envf.write('%s="%s"\n' % (var, d.getVar(var, True).strip()))
+            envf.write('{}="{}"\n'.format(var, d.getVar(var, True).strip()))
 
 }
 
diff --git a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
index 234d339..5114714 100644
--- a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
+++ b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
@@ -40,18 +40,16 @@ python () {
         d.setVar("DEBOOTSTRAP_KEYRING", "--keyring ${APTKEYRING}")
         for key in distro_apt_keys.split():
             url = urlparse(key)
-            filename = ''.join([wd, url.path])
-            d.appendVar("SRC_URI", " %s" % key)
-            d.appendVar("APTKEYFILES", " %s" % filename)
+            d.appendVar("SRC_URI", " " + key)
+            d.appendVar("APTKEYFILES", " " + wd + url.path)
     if bb.utils.to_boolean(d.getVar('ISAR_USE_CACHED_BASE_REPO')):
         own_pub_key = d.getVar("BASE_REPO_KEY", False)
         if own_pub_key:
             d.setVar("DEBOOTSTRAP_KEYRING", "--keyring ${APTKEYRING}")
             for key in own_pub_key.split():
                 url = urlparse(key)
-                filename = ''.join([wd, url.path])
-                d.appendVar("SRC_URI", " %s" % key)
-                d.appendVar("APTKEYFILES", " %s" % filename)
+                d.appendVar("SRC_URI", " " + key)
+                d.appendVar("APTKEYFILES", " " + wd + url.path)
 }
 
 def aggregate_files(d, file_list, file_out):
@@ -170,7 +168,7 @@ def get_distro_suite(d, is_host):
 def get_distro_components_argument(d, is_host):
     components = get_distro_primary_source_entry(d, is_host)[2]
     if components and components.strip():
-        return "--components=%s" % ",".join(components.split())
+        return "--components=" + ",".join(components.split())
     else:
         return ""
 
-- 
2.14.1


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

* [PATCH v2 3/5] image: Remove recursion in get_image_name
  2019-02-12 16:56 [PATCH v2 0/5] Python refactoring Harald Seiler
  2019-02-12 16:56 ` [PATCH v2 1/5] Remove all uses of subprocess.call(shell=True) Harald Seiler
  2019-02-12 16:56 ` [PATCH v2 2/5] Use modern python formatting Harald Seiler
@ 2019-02-12 16:56 ` Harald Seiler
  2019-02-12 16:56 ` [PATCH v2 4/5] wic: Refactor fakeroot script Harald Seiler
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Harald Seiler @ 2019-02-12 16:56 UTC (permalink / raw)
  To: isar-users

The use of recursion in this function is not immediately
obvious.  This commit refactors the recursion into a more
visible imperative code style.

Signed-off-by: Harald Seiler <hws@denx.de>
---
 meta/classes/image.bbclass | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
index a687662..3e3bf0c 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -22,13 +22,24 @@ ROOTFS_EXTRA ?= "64"
 def get_image_name(d, name_link):
     S = d.getVar("IMAGE_ROOTFS", True)
     path_link = os.path.join(S, name_link)
+
+    # If path_link does not exist, it might be a symlink
+    # in the target rootfs.  This block attempts to resolve
+    # it relative to the rootfs location.
+    if not os.path.exists(path_link):
+        path_link = os.path.join(
+            S,
+            os.path.relpath(
+                os.path.realpath(path_link),
+                "/",
+            ),
+        )
+
     if os.path.exists(path_link):
         base = os.path.basename(os.path.realpath(path_link))
         full = d.getVar("IMAGE_FULLNAME", True) + "." + base
         return [base, full]
-    if os.path.islink(path_link):
-        return get_image_name(d, os.path.relpath(os.path.realpath(path_link),
-                                                 '/'))
+
     return ["", ""]
 
 def get_rootfs_size(d):
-- 
2.14.1


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

* [PATCH v2 4/5] wic: Refactor fakeroot script
  2019-02-12 16:56 [PATCH v2 0/5] Python refactoring Harald Seiler
                   ` (2 preceding siblings ...)
  2019-02-12 16:56 ` [PATCH v2 3/5] image: Remove recursion in get_image_name Harald Seiler
@ 2019-02-12 16:56 ` Harald Seiler
  2019-02-12 16:56 ` [PATCH v2 5/5] Fix python style Harald Seiler
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Harald Seiler @ 2019-02-12 16:56 UTC (permalink / raw)
  To: isar-users

Signed-off-by: Harald Seiler <hws@denx.de>
---
 scripts/wic_fakeroot | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/scripts/wic_fakeroot b/scripts/wic_fakeroot
index 9e01c38..88a03fa 100755
--- a/scripts/wic_fakeroot
+++ b/scripts/wic_fakeroot
@@ -11,7 +11,6 @@
 #
 import os
 import sys
-import shutil
 import subprocess
 
 args = sys.argv
@@ -24,14 +23,12 @@ cmd = args[0]
 #  i.e. in wics partition.py the "du -ks" fails on
 #    var/cache/apt/archives/partial
 #    rootfs/root ...
-assert 'root' == os.environ["USER"]
+assert os.geteuid() == 0, "wic_fakeroot must be run as root!"
 
 # e2fsck <= 1.43.5 returns 1 on non-errors (stretch and before affected)
 # treat 1 as safe ... the filesystem was successfully repaired and is OK
 if cmd.startswith('fsck.'):
     ret = subprocess.call(args)
-    if ret == 0 or ret == 1:
-        sys.exit(0)
-    sys.exit(ret)
+    sys.exit(0 if ret == 1 else ret)
 
-os.execv(shutil.which(cmd), args)
+os.execvp(cmd, args)
-- 
2.14.1


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

* [PATCH v2 5/5] Fix python style
  2019-02-12 16:56 [PATCH v2 0/5] Python refactoring Harald Seiler
                   ` (3 preceding siblings ...)
  2019-02-12 16:56 ` [PATCH v2 4/5] wic: Refactor fakeroot script Harald Seiler
@ 2019-02-12 16:56 ` Harald Seiler
  2019-02-12 17:14 ` [PATCH v2 0/5] Python refactoring Henning Schild
  2019-02-18 13:29 ` Maxim Yu. Osipov
  6 siblings, 0 replies; 8+ messages in thread
From: Harald Seiler @ 2019-02-12 16:56 UTC (permalink / raw)
  To: isar-users

Signed-off-by: Harald Seiler <hws@denx.de>
---
 meta-isar/recipes-kernel/example-module/example-module.bb | 8 +++++++-
 meta/classes/base.bbclass                                 | 2 +-
 meta/classes/isar-events.bbclass                          | 1 -
 meta/recipes-core/isar-bootstrap/isar-bootstrap-host.bb   | 9 ++++++---
 meta/recipes-core/isar-bootstrap/isar-bootstrap-target.bb | 5 +++--
 5 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/meta-isar/recipes-kernel/example-module/example-module.bb b/meta-isar/recipes-kernel/example-module/example-module.bb
index b725416..a08a6c5 100644
--- a/meta-isar/recipes-kernel/example-module/example-module.bb
+++ b/meta-isar/recipes-kernel/example-module/example-module.bb
@@ -11,7 +11,13 @@
 # has hard dependencies from linux-compiler-gcc-4.8-arm, what
 # conflicts with the host binaries.
 python() {
-    if d.getVar('KERNEL_NAME') in ['armmp', 'arm64', 'rpi-rpfv', 'amd64', '686-pae']:
+    if d.getVar('KERNEL_NAME') in [
+        'armmp',
+        'arm64',
+        'rpi-rpfv',
+        'amd64',
+        '686-pae',
+    ]:
         d.setVar('ISAR_CROSS_COMPILE', '0')
 }
 
diff --git a/meta/classes/base.bbclass b/meta/classes/base.bbclass
index 4dea51b..eebcfe3 100644
--- a/meta/classes/base.bbclass
+++ b/meta/classes/base.bbclass
@@ -125,7 +125,7 @@ python() {
     src_apt = []
     for u in src_uri:
         if u.startswith(prefix):
-            src_apt.append(u[len(prefix):])
+            src_apt.append(u[len(prefix) :])
         else:
             new_src_uri.append(u)
 
diff --git a/meta/classes/isar-events.bbclass b/meta/classes/isar-events.bbclass
index a178d05..7dd1ed5 100644
--- a/meta/classes/isar-events.bbclass
+++ b/meta/classes/isar-events.bbclass
@@ -8,7 +8,6 @@ addhandler isar_handler
 
 python isar_handler() {
     import subprocess
-    import bb.runqueue
 
     tmpdir = d.getVar('TMPDIR', True)
     if not tmpdir:
diff --git a/meta/recipes-core/isar-bootstrap/isar-bootstrap-host.bb b/meta/recipes-core/isar-bootstrap/isar-bootstrap-host.bb
index a793585..08b068f 100644
--- a/meta/recipes-core/isar-bootstrap/isar-bootstrap-host.bb
+++ b/meta/recipes-core/isar-bootstrap/isar-bootstrap-host.bb
@@ -29,13 +29,16 @@ do_apt_config_prepare[vardeps] += "\
 python do_apt_config_prepare() {
     if not os.path.islink(d.getVar("DEPLOY_ISAR_BOOTSTRAP", True)):
         apt_preferences_out = d.getVar("APTPREFS", True)
-        apt_preferences_list = (d.getVar("HOST_DISTRO_APT_PREFERENCES", True) or ""
-                             ).split()
+        apt_preferences_list = (
+            d.getVar("HOST_DISTRO_APT_PREFERENCES", True) or ""
+        ).split()
         aggregate_files(d, apt_preferences_list, apt_preferences_out)
 
         apt_sources_out = d.getVar("APTSRCS", True)
         apt_sources_init_out = d.getVar("APTSRCS_INIT", True)
-        apt_sources_list = (d.getVar("HOST_DISTRO_APT_SOURCES", True) or "").split()
+        apt_sources_list = (
+            d.getVar("HOST_DISTRO_APT_SOURCES", True) or ""
+        ).split()
 
         aggregate_files(d, apt_sources_list, apt_sources_init_out)
         aggregate_aptsources_list(d, apt_sources_list, apt_sources_out)
diff --git a/meta/recipes-core/isar-bootstrap/isar-bootstrap-target.bb b/meta/recipes-core/isar-bootstrap/isar-bootstrap-target.bb
index bec6fa8..79f3e34 100644
--- a/meta/recipes-core/isar-bootstrap/isar-bootstrap-target.bb
+++ b/meta/recipes-core/isar-bootstrap/isar-bootstrap-target.bb
@@ -28,8 +28,9 @@ do_apt_config_prepare[vardeps] += "\
 python do_apt_config_prepare() {
     if not os.path.islink(d.getVar("DEPLOY_ISAR_BOOTSTRAP", True)):
         apt_preferences_out = d.getVar("APTPREFS", True)
-        apt_preferences_list = (d.getVar("DISTRO_APT_PREFERENCES", True) or ""
-                             ).split()
+        apt_preferences_list = (
+            d.getVar("DISTRO_APT_PREFERENCES", True) or ""
+        ).split()
         aggregate_files(d, apt_preferences_list, apt_preferences_out)
 
         apt_sources_out = d.getVar("APTSRCS", True)
-- 
2.14.1


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

* Re: [PATCH v2 0/5] Python refactoring
  2019-02-12 16:56 [PATCH v2 0/5] Python refactoring Harald Seiler
                   ` (4 preceding siblings ...)
  2019-02-12 16:56 ` [PATCH v2 5/5] Fix python style Harald Seiler
@ 2019-02-12 17:14 ` Henning Schild
  2019-02-18 13:29 ` Maxim Yu. Osipov
  6 siblings, 0 replies; 8+ messages in thread
From: Henning Schild @ 2019-02-12 17:14 UTC (permalink / raw)
  To: Harald Seiler; +Cc: isar-users

Nice cleanups!

Henning

On Tue, 12 Feb 2019 17:56:32 +0100
Harald Seiler <hws@denx.de> wrote:

> This series contains a number of improvements to python code in isar:
> 
> * All instancess of subprocess.{call,Popen,check_call}() have been
> changed to never use `shell=True`.  This will hopefully prevent bugs
> in the future.
> * The use of the % formatter was replaced by other methods, as % was
> deprecated because it can behave weirdly.  Unfortunately, as we have
> to support python back to 3.5, we can not yet make use of f-Strings,
> so other options had to be used.  I decided to use string
> concatenation for simple cases where the use of .format() would have
> been too verbose.
> * A recursive call in get_image_name was removed in favor of an
> imperative code style.  This should make the respective function
> easier to read.
> 
> Changes in v2:
> 
>     * Remove redundant encoding parameter
>     * Fix formatting
>     * Refactor a task that I previously missed
>     * Add a commit for general style fixes
> 
> Harald Seiler (5):
>   Remove all uses of subprocess.call(shell=True)
>   Use modern python formatting
>   image: Remove recursion in get_image_name
>   wic: Refactor fakeroot script
>   Fix python style
> 
>  .../example-module/example-module.bb               |  8 ++-
>  meta/classes/base.bbclass                          | 70
> ++++++++++++++--------
> meta/classes/image.bbclass                         | 22 +++++--
> meta/classes/isar-bootstrap-helper.bbclass         |  8 +--
> meta/classes/isar-events.bbclass                   | 14 ++---
> meta/classes/patch.bbclass                         | 13 ++--
> meta/classes/wic-img.bbclass                       | 21
> +++++-- .../isar-bootstrap/isar-bootstrap-host.bb          |  9
> ++- .../isar-bootstrap/isar-bootstrap-target.bb        |  5
> +- .../recipes-core/isar-bootstrap/isar-bootstrap.inc | 12 ++--
> scripts/wic_fakeroot                               |  9 +-- 11 files
> changed, 120 insertions(+), 71 deletions(-)
> 


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

* Re: [PATCH v2 0/5] Python refactoring
  2019-02-12 16:56 [PATCH v2 0/5] Python refactoring Harald Seiler
                   ` (5 preceding siblings ...)
  2019-02-12 17:14 ` [PATCH v2 0/5] Python refactoring Henning Schild
@ 2019-02-18 13:29 ` Maxim Yu. Osipov
  6 siblings, 0 replies; 8+ messages in thread
From: Maxim Yu. Osipov @ 2019-02-18 13:29 UTC (permalink / raw)
  To: Harald Seiler, isar-users

Hi Harald,

I've applied your series (patch #2 has to be rebased) and ran "fast" CI:
'./scripts/ci_build.sh -q -f'.

Build has been failed:

See for detail:
http://isar-build.org:8080/job/isar_mosipov_develop/43/console

Thanks,
Maxim.

P.S. I'll send a rebased patch in separate email.

On 2/12/19 5:56 PM, Harald Seiler wrote:
> This series contains a number of improvements to python code in isar:
> 
> * All instancess of subprocess.{call,Popen,check_call}() have been changed to
>    never use `shell=True`.  This will hopefully prevent bugs in the future.
> * The use of the % formatter was replaced by other methods, as % was deprecated
>    because it can behave weirdly.  Unfortunately, as we have to support python
>    back to 3.5, we can not yet make use of f-Strings, so other options had to be
>    used.  I decided to use string concatenation for simple cases where the use
>    of .format() would have been too verbose.
> * A recursive call in get_image_name was removed in favor of an imperative code
>    style.  This should make the respective function easier to read.
> 
> Changes in v2:
> 
>      * Remove redundant encoding parameter
>      * Fix formatting
>      * Refactor a task that I previously missed
>      * Add a commit for general style fixes
> 
> Harald Seiler (5):
>    Remove all uses of subprocess.call(shell=True)
>    Use modern python formatting
>    image: Remove recursion in get_image_name
>    wic: Refactor fakeroot script
>    Fix python style
> 
>   .../example-module/example-module.bb               |  8 ++-
>   meta/classes/base.bbclass                          | 70 ++++++++++++++--------
>   meta/classes/image.bbclass                         | 22 +++++--
>   meta/classes/isar-bootstrap-helper.bbclass         |  8 +--
>   meta/classes/isar-events.bbclass                   | 14 ++---
>   meta/classes/patch.bbclass                         | 13 ++--
>   meta/classes/wic-img.bbclass                       | 21 +++++--
>   .../isar-bootstrap/isar-bootstrap-host.bb          |  9 ++-
>   .../isar-bootstrap/isar-bootstrap-target.bb        |  5 +-
>   .../recipes-core/isar-bootstrap/isar-bootstrap.inc | 12 ++--
>   scripts/wic_fakeroot                               |  9 +--
>   11 files changed, 120 insertions(+), 71 deletions(-)
> 


-- 
Maxim Osipov
ilbers GmbH
Maria-Merian-Str. 8
85521 Ottobrunn
Germany
+49 (151) 6517 6917
mosipov@ilbers.de
http://ilbers.de/
Commercial register Munich, HRB 214197
General Manager: Baurzhan Ismagulov

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

end of thread, other threads:[~2019-02-18 13:29 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-12 16:56 [PATCH v2 0/5] Python refactoring Harald Seiler
2019-02-12 16:56 ` [PATCH v2 1/5] Remove all uses of subprocess.call(shell=True) Harald Seiler
2019-02-12 16:56 ` [PATCH v2 2/5] Use modern python formatting Harald Seiler
2019-02-12 16:56 ` [PATCH v2 3/5] image: Remove recursion in get_image_name Harald Seiler
2019-02-12 16:56 ` [PATCH v2 4/5] wic: Refactor fakeroot script Harald Seiler
2019-02-12 16:56 ` [PATCH v2 5/5] Fix python style Harald Seiler
2019-02-12 17:14 ` [PATCH v2 0/5] Python refactoring Henning Schild
2019-02-18 13:29 ` Maxim Yu. Osipov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox