* [PATCH 0/4] Python refactoring
@ 2019-02-12 9:20 Harald Seiler
2019-02-12 9:20 ` [PATCH 1/4] Remove all uses of subprocess.call(shell=True) Harald Seiler
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Harald Seiler @ 2019-02-12 9:20 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.
Harald Seiler (4):
Remove all uses of subprocess.call(shell=True)
Use modern python formatting
image: Remove recursion in get_image_name
wic: Refactor fakeroot script
meta/classes/base.bbclass | 47 +++++++++++++++-------
meta/classes/image.bbclass | 22 +++++++---
meta/classes/isar-bootstrap-helper.bbclass | 9 ++---
meta/classes/isar-events.bbclass | 13 +++---
meta/classes/patch.bbclass | 16 ++++++--
meta/classes/wic-img.bbclass | 21 +++++++---
.../recipes-core/isar-bootstrap/isar-bootstrap.inc | 12 +++---
scripts/wic_fakeroot | 9 ++---
8 files changed, 95 insertions(+), 54 deletions(-)
--
2.14.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/4] Remove all uses of subprocess.call(shell=True)
2019-02-12 9:20 [PATCH 0/4] Python refactoring Harald Seiler
@ 2019-02-12 9:20 ` Harald Seiler
2019-02-12 9:51 ` Claudius Heine
2019-02-12 9:20 ` [PATCH 2/4] Use modern python formatting Harald Seiler
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: Harald Seiler @ 2019-02-12 9:20 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 | 9 +++++----
meta/classes/isar-bootstrap-helper.bbclass | 9 ++++-----
meta/classes/isar-events.bbclass | 13 +++++++------
meta/classes/patch.bbclass | 16 ++++++++++++----
5 files changed, 34 insertions(+), 23 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..930c245 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -27,16 +27,17 @@ def get_image_name(d, name_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 get_image_name(d, os.path.relpath(os.path.realpath(path_link), '/'))
+
return ["", ""]
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..3e11098 100644
--- a/meta/classes/isar-bootstrap-helper.bbclass
+++ b/meta/classes/isar-bootstrap-helper.bbclass
@@ -9,11 +9,10 @@ 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"],
+ encoding="utf-8",
+ ).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..9d4441d 100644
--- a/meta/classes/patch.bbclass
+++ b/meta/classes/patch.bbclass
@@ -21,10 +21,18 @@ 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] 6+ messages in thread
* [PATCH 2/4] Use modern python formatting
2019-02-12 9:20 [PATCH 0/4] Python refactoring Harald Seiler
2019-02-12 9:20 ` [PATCH 1/4] Remove all uses of subprocess.call(shell=True) Harald Seiler
@ 2019-02-12 9:20 ` Harald Seiler
2019-02-12 9:20 ` [PATCH 3/4] image: Remove recursion in get_image_name Harald Seiler
2019-02-12 9:20 ` [PATCH 4/4] wic: Refactor fakeroot script Harald Seiler
3 siblings, 0 replies; 6+ messages in thread
From: Harald Seiler @ 2019-02-12 9:20 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 | 37 +++++++++++++++-------
meta/classes/wic-img.bbclass | 21 ++++++++----
.../recipes-core/isar-bootstrap/isar-bootstrap.inc | 12 +++----
3 files changed, 46 insertions(+), 24 deletions(-)
diff --git a/meta/classes/base.bbclass b/meta/classes/base.bbclass
index 6fcf452..50c4fbf 100644
--- a/meta/classes/base.bbclass
+++ b/meta/classes/base.bbclass
@@ -32,27 +32,40 @@ 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 = sorted(taskdescs.keys())
- for taskname in tasks:
- bb.plain("%s %s" % (taskname.ljust(maxlen), taskdescs[taskname]))
+ tasks[e] = d.getVarFlag(e, 'doc') or ''
+
+ for name, desc in sorted(tasks.items()):
+ bb.plain(
+ "{0:{len}} {1}".format(
+ name,
+ desc,
+ len=maxlen,
+ )
+ )
}
root_cleandirs() {
@@ -81,8 +94,10 @@ python() {
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'):
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] 6+ messages in thread
* [PATCH 3/4] image: Remove recursion in get_image_name
2019-02-12 9:20 [PATCH 0/4] Python refactoring Harald Seiler
2019-02-12 9:20 ` [PATCH 1/4] Remove all uses of subprocess.call(shell=True) Harald Seiler
2019-02-12 9:20 ` [PATCH 2/4] Use modern python formatting Harald Seiler
@ 2019-02-12 9:20 ` Harald Seiler
2019-02-12 9:20 ` [PATCH 4/4] wic: Refactor fakeroot script Harald Seiler
3 siblings, 0 replies; 6+ messages in thread
From: Harald Seiler @ 2019-02-12 9:20 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 | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
index 930c245..8a2c152 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -22,12 +22,23 @@ 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 ["", ""]
--
2.14.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 4/4] wic: Refactor fakeroot script
2019-02-12 9:20 [PATCH 0/4] Python refactoring Harald Seiler
` (2 preceding siblings ...)
2019-02-12 9:20 ` [PATCH 3/4] image: Remove recursion in get_image_name Harald Seiler
@ 2019-02-12 9:20 ` Harald Seiler
3 siblings, 0 replies; 6+ messages in thread
From: Harald Seiler @ 2019-02-12 9:20 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] 6+ messages in thread
* Re: [PATCH 1/4] Remove all uses of subprocess.call(shell=True)
2019-02-12 9:20 ` [PATCH 1/4] Remove all uses of subprocess.call(shell=True) Harald Seiler
@ 2019-02-12 9:51 ` Claudius Heine
0 siblings, 0 replies; 6+ messages in thread
From: Claudius Heine @ 2019-02-12 9:51 UTC (permalink / raw)
To: Harald Seiler, isar-users; +Cc: Claudius Heine
Hi Harald,
On 12/02/2019 10.20, Harald Seiler wrote:
> 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 | 9 +++++----
> meta/classes/isar-bootstrap-helper.bbclass | 9 ++++-----
> meta/classes/isar-events.bbclass | 13 +++++++------
> meta/classes/patch.bbclass | 16 ++++++++++++----
> 5 files changed, 34 insertions(+), 23 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..930c245 100644
> --- a/meta/classes/image.bbclass
> +++ b/meta/classes/image.bbclass
> @@ -27,16 +27,17 @@ def get_image_name(d, name_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 get_image_name(d, os.path.relpath(os.path.realpath(path_link), '/'))
> +
> return ["", ""]
>
> 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..3e11098 100644
> --- a/meta/classes/isar-bootstrap-helper.bbclass
> +++ b/meta/classes/isar-bootstrap-helper.bbclass
> @@ -9,11 +9,10 @@ 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"],
> + encoding="utf-8",
utf-8 should be the default encoding, or is it? I would remove that if
it is. Other check_output calls don't have that.
> + ).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..9d4441d 100644
> --- a/meta/classes/patch.bbclass
> +++ b/meta/classes/patch.bbclass
> @@ -21,10 +21,18 @@ 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,
> + ]
IMO formatting could be improved there. I don't know about you, but I
can read:
cmd = [
"patch",
"--no-backup-if-mismatch",
"-p", striplevel,
"--directory", src_dir,
"--input", workdir + basename,
]
better. :) But that is just opinion, so if you differ I will not insist
on it.
Otherwise great work!
Claudius
> + bb.note(" ".join(cmd))
> + if subprocess.call(cmd) != 0:
> bb.fatal("patching failed")
> except bb.fetch2.BBFetchException as e:
> raise bb.build.FuncFailed(e)
>
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-54 Fax: (+49)-8142-66989-80 Email: ch@denx.de
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-02-12 9:51 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-12 9:20 [PATCH 0/4] Python refactoring Harald Seiler
2019-02-12 9:20 ` [PATCH 1/4] Remove all uses of subprocess.call(shell=True) Harald Seiler
2019-02-12 9:51 ` Claudius Heine
2019-02-12 9:20 ` [PATCH 2/4] Use modern python formatting Harald Seiler
2019-02-12 9:20 ` [PATCH 3/4] image: Remove recursion in get_image_name Harald Seiler
2019-02-12 9:20 ` [PATCH 4/4] wic: Refactor fakeroot script Harald Seiler
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox