* [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