From 70aa9439819c3b659c8aa3c1460b6ed2e46b7350 Mon Sep 17 00:00:00 2001 From: Mike Frysinger Date: Fri, 20 May 2016 01:34:19 -0400 Subject: cmd: add support for running commands directly Very few of the commands we execute actually need to be run through a shell. Extend the cmd helper to take an argv and convert the majority of calls over to that. --- catalyst/base/stagebase.py | 94 +++++++++++++++++++------------------------ catalyst/support.py | 48 +++++++++++----------- catalyst/targets/grp.py | 9 +++-- catalyst/targets/netboot.py | 28 ++++++------- catalyst/targets/netboot2.py | 14 +++---- catalyst/targets/snapshot.py | 14 +++++-- catalyst/targets/tinderbox.py | 6 +-- 7 files changed, 104 insertions(+), 109 deletions(-) diff --git a/catalyst/base/stagebase.py b/catalyst/base/stagebase.py index 0b25516d..78e5b948 100644 --- a/catalyst/base/stagebase.py +++ b/catalyst/base/stagebase.py @@ -13,7 +13,7 @@ from catalyst import log from catalyst.defaults import (SOURCE_MOUNT_DEFAULTS, TARGET_MOUNT_DEFAULTS, PORT_LOGDIR_CLEAN) from catalyst.support import (CatalystError, file_locate, normpath, - cmd, list_bashify, read_makeconf, ismount, file_check) + cmd, read_makeconf, ismount, file_check) from catalyst.base.targetbase import TargetBase from catalyst.base.clearbase import ClearBase from catalyst.base.genbase import GenBase @@ -648,7 +648,7 @@ class StageBase(TargetBase, ClearBase, GenBase): killcmd = normpath(self.settings["sharedir"] + self.settings["shdir"] + "/support/kill-chroot-pids.sh") if os.path.exists(killcmd): - cmd(killcmd, env=self.env) + cmd([killcmd], env=self.env) def mount_safety_check(self): """ @@ -873,10 +873,9 @@ class StageBase(TargetBase, ClearBase, GenBase): clear_path(self.settings['chroot_path'] + self.settings['port_conf'] + '/make.profile') ensure_dirs(self.settings['chroot_path'] + self.settings['port_conf']) - cmd("ln -sf ../.." + self.settings["portdir"] + "/profiles/" + - self.settings["target_profile"] + " " + - self.settings["chroot_path"] + - self.settings["port_conf"] + "/make.profile", + cmd(['ln', '-sf', + '../..' + self.settings['portdir'] + '/profiles/' + self.settings['target_profile'], + self.settings['chroot_path'] + self.settings['port_conf'] + '/make.profile'], env=self.env) self.resume.enable("config_profile_link") @@ -892,7 +891,7 @@ class StageBase(TargetBase, ClearBase, GenBase): # The trailing slashes on both paths are important: # We want to make sure rsync copies the dirs into each # other and not as subdirs. - cmd('rsync -a %s/ %s/' % (self.settings['portage_confdir'], dest), + cmd(['rsync', '-a', self.settings['portage_confdir'] + '/', dest + '/'], env=self.env) self.resume.enable("setup_confdir") @@ -914,7 +913,7 @@ class StageBase(TargetBase, ClearBase, GenBase): "/root_overlay"]: if os.path.exists(x): log.info('Copying root_overlay: %s', x) - cmd('rsync -a ' + x + '/ ' + self.settings['chroot_path'], + cmd(['rsync', '-a', x + '/', self.settings['chroot_path']], env=self.env) def base_dirs(self): @@ -922,7 +921,6 @@ class StageBase(TargetBase, ClearBase, GenBase): def bind(self): for x in self.mounts: - _cmd = '' log.debug('bind(); x = %s', x) target = normpath(self.settings["chroot_path"] + self.target_mounts[x]) ensure_dirs(target, mode=0o755) @@ -935,23 +933,25 @@ class StageBase(TargetBase, ClearBase, GenBase): log.debug('bind(); src = %s', src) if "snapcache" in self.settings["options"] and x == "portdir": self.snapcache_lock.read_lock() + _cmd = None if os.uname()[0] == "FreeBSD": if src == "/dev": - _cmd = "mount -t devfs none " + target + _cmd = ['mount', '-t', 'devfs', 'none', target] else: - _cmd = "mount_nullfs " + src + " " + target + _cmd = ['mount_nullfs', src, target] else: if src == "tmpfs": if "var_tmpfs_portage" in self.settings: - _cmd = "mount -t tmpfs -o size=" + \ - self.settings["var_tmpfs_portage"] + "G " + \ - src + " " + target + _cmd = ['mount', '-t', 'tmpfs', + '-o', 'size=' + self.settings['var_tmpfs_portage'] + 'G', + src, target] elif src == "shmfs": - _cmd = "mount -t tmpfs -o noexec,nosuid,nodev shm " + target + _cmd = ['mount', '-t', 'tmpfs', '-o', 'noexec,nosuid,nodev', 'shm', target] else: - _cmd = "mount --bind " + src + " " + target - log.debug('bind(); _cmd = %s', _cmd) - cmd(_cmd, env=self.env, fail_func=self.unbind) + _cmd = ['mount', '--bind', src, target] + if _cmd: + log.debug('bind(); _cmd = %s', _cmd) + cmd(_cmd, env=self.env, fail_func=self.unbind) log.debug('bind(); finished :D') def unbind(self): @@ -1134,7 +1134,7 @@ class StageBase(TargetBase, ClearBase, GenBase): else: if "fsscript" in self.settings: if os.path.exists(self.settings["controller_file"]): - cmd(self.settings['controller_file'] + ' fsscript', + cmd([self.settings['controller_file'], 'fsscript'], env=self.env) self.resume.enable("fsscript") @@ -1144,7 +1144,7 @@ class StageBase(TargetBase, ClearBase, GenBase): log.notice('Resume point detected, skipping rcupdate operation...') else: if os.path.exists(self.settings["controller_file"]): - cmd(self.settings["controller_file"]+" rc-update",\ + cmd([self.settings['controller_file'], 'rc-update'], env=self.env) self.resume.enable("rcupdate") @@ -1178,12 +1178,12 @@ class StageBase(TargetBase, ClearBase, GenBase): # Clean up old and obsoleted files in /etc if os.path.exists(self.settings["stage_path"]+"/etc"): - cmd("find "+self.settings["stage_path"]+\ - "/etc -maxdepth 1 -name \"*-\" | xargs rm -f",\ + cmd(['find', self.settings['stage_path'] + '/etc', + '-maxdepth', '1', '-name', '*-', '-delete'], env=self.env) if os.path.exists(self.settings["controller_file"]): - cmd(self.settings['controller_file'] + ' clean', env=self.env) + cmd([self.settings['controller_file'], 'clean'], env=self.env) self.resume.enable("clean") def empty(self): @@ -1224,7 +1224,7 @@ class StageBase(TargetBase, ClearBase, GenBase): clear_path(self.settings["chroot_path"] + x) try: if os.path.exists(self.settings["controller_file"]): - cmd(self.settings['controller_file'] + ' clean', + cmd([self.settings['controller_file'], 'clean'], env=self.env) self.resume.enable("remove") except: @@ -1238,7 +1238,7 @@ class StageBase(TargetBase, ClearBase, GenBase): else: try: if os.path.exists(self.settings["controller_file"]): - cmd(self.settings['controller_file'] + ' preclean', + cmd([self.settings['controller_file'], 'preclean'], env=self.env) self.resume.enable("preclean") @@ -1295,7 +1295,7 @@ class StageBase(TargetBase, ClearBase, GenBase): try: if os.path.exists(self.settings["controller_file"]): log.info('run_local() starting controller script...') - cmd(self.settings["controller_file"]+" run",\ + cmd([self.settings['controller_file'], 'run'], env=self.env) self.resume.enable("run_local") else: @@ -1422,19 +1422,12 @@ class StageBase(TargetBase, ClearBase, GenBase): if isinstance(self.settings[self.settings['spec_prefix']+'/unmerge'], str): self.settings[self.settings["spec_prefix"]+"/unmerge"]=\ [self.settings[self.settings["spec_prefix"]+"/unmerge"]] - myunmerge=\ - self.settings[self.settings["spec_prefix"]+"/unmerge"][:] - - for x in range(0,len(myunmerge)): - # Surround args with quotes for passing to bash, allows - # things like "<" to remain intact - myunmerge[x]="'"+myunmerge[x]+"'" - myunmerge = ' '.join(myunmerge) # Before cleaning, unmerge stuff try: - cmd(self.settings['controller_file'] + - ' unmerge ' + myunmerge, env=self.env) + cmd([self.settings['controller_file'], 'unmerge'] + + self.settings[self.settings['spec_prefix'] + '/unmerge'], + env=self.env) log.info('unmerge shell script') except CatalystError: self.unbind() @@ -1447,9 +1440,8 @@ class StageBase(TargetBase, ClearBase, GenBase): log.notice('Resume point detected, skipping target_setup operation...') else: log.notice('Setting up filesystems per filesystem type') - cmd(self.settings["controller_file"]+\ - " target_image_setup "+ self.settings["target_path"],\ - env=self.env) + cmd([self.settings['controller_file'], 'target_image_setup', + self.settings['target_path']], env=self.env) self.resume.enable("target_setup") def setup_overlay(self): @@ -1460,7 +1452,7 @@ class StageBase(TargetBase, ClearBase, GenBase): if self.settings["spec_prefix"]+"/overlay" in self.settings: for x in self.settings[self.settings["spec_prefix"]+"/overlay"]: if os.path.exists(x): - cmd('rsync -a ' + x + '/ ' + self.settings['target_path'], + cmd(['rsync', '-a', x + '/', self.settings['target_path']], env=self.env) self.resume.enable("setup_overlay") @@ -1471,8 +1463,7 @@ class StageBase(TargetBase, ClearBase, GenBase): else: # Create the ISO if "iso" in self.settings: - cmd(self.settings["controller_file"]+" iso "+\ - self.settings['iso'], + cmd([self.settings['controller_file'], 'iso', self.settings['iso']], env=self.env) self.gen_contents_file(self.settings["iso"]) self.gen_digest_file(self.settings["iso"]) @@ -1492,12 +1483,9 @@ class StageBase(TargetBase, ClearBase, GenBase): and self.resume.is_enabled("build_packages"): log.notice('Resume point detected, skipping build_packages operation...') else: - mypack=\ - list_bashify(self.settings[self.settings["spec_prefix"]\ - +"/packages"]) try: - cmd(self.settings["controller_file"]+\ - " build_packages "+mypack,\ + cmd([self.settings['controller_file'], 'build_packages'] + + self.settings[self.settings["spec_prefix"] + '/packages'], env=self.env) fileutils.touch(build_packages_resume) self.resume.enable("build_packages") @@ -1518,7 +1506,7 @@ class StageBase(TargetBase, ClearBase, GenBase): if isinstance(mynames, str): mynames=[mynames] # Execute the script that sets up the kernel build environment - cmd(self.settings['controller_file'] + ' pre-kmerge', + cmd([self.settings['controller_file'], 'pre-kmerge'], env=self.env) for kname in mynames: self._build_kernel(kname=kname) @@ -1556,7 +1544,7 @@ class StageBase(TargetBase, ClearBase, GenBase): self._copy_initramfs_overlay(kname=kname) # Execute the script that builds the kernel - cmd(self.settings['controller_file'] + ' kernel ' + kname, + cmd([self.settings['controller_file'], 'kernel', kname], env=self.env) if "boot/kernel/"+kname+"/initramfs_overlay" in self.settings: @@ -1566,7 +1554,7 @@ class StageBase(TargetBase, ClearBase, GenBase): self.resume.is_enabled("build_kernel_"+kname) # Execute the script that cleans up the kernel build environment - cmd(self.settings['controller_file'] + ' post-kmerge', + cmd([self.settings['controller_file'], 'post-kmerge'], env=self.env) def _copy_kernel_config(self, kname): @@ -1604,8 +1592,8 @@ class StageBase(TargetBase, ClearBase, GenBase): log.notice('Resume point detected, skipping bootloader operation...') else: try: - cmd(self.settings["controller_file"]+\ - " bootloader " + self.settings["target_path"].rstrip('/'),\ + cmd([self.settings['controller_file'], 'bootloader', + self.settings['target_path'].rstrip('/')], env=self.env) self.resume.enable("bootloader") except CatalystError: @@ -1618,7 +1606,7 @@ class StageBase(TargetBase, ClearBase, GenBase): log.notice('Resume point detected, skipping build_packages operation...') else: try: - cmd(self.settings['controller_file'] + ' livecd-update', + cmd([self.settings['controller_file'], 'livecd-update'], env=self.env) self.resume.enable("livecd_update") diff --git a/catalyst/support.py b/catalyst/support.py index 0ce49d2d..f2ae5bb6 100644 --- a/catalyst/support.py +++ b/catalyst/support.py @@ -12,18 +12,6 @@ from catalyst.defaults import valid_config_file_values BASH_BINARY = "/bin/bash" -def list_bashify(mylist): - if isinstance(mylist, str): - mypack=[mylist] - else: - mypack=mylist[:] - for x in range(0,len(mypack)): - # surround args with quotes for passing to bash, - # allows things like "<" to remain intact - mypack[x]="'"+mypack[x]+"'" - return ' '.join(mypack) - - class CatalystError(Exception): def __init__(self, message, print_traceback=False): if message: @@ -31,26 +19,38 @@ class CatalystError(Exception): def cmd(mycmd, env=None, debug=False, fail_func=None): - if env is None: - env = {} + """Run the external |mycmd|. + + If |mycmd| is a string, then it's assumed to be a bash snippet and will + be run through bash. Otherwise, it's a standalone command line and will + be run directly. + """ log.debug('cmd: %r', mycmd) sys.stdout.flush() - args=[BASH_BINARY] - if "BASH_ENV" not in env: + + if env is None: + env = {} + if 'BASH_ENV' not in env: env = env.copy() - env["BASH_ENV"] = "/etc/spork/is/not/valid/profile.env" - if debug: - args.append("-x") - args.append("-c") - args.append(mycmd) + env['BASH_ENV'] = '/etc/spork/is/not/valid/profile.env' + + args = [] + if isinstance(mycmd, str): + args.append(BASH_BINARY) + if debug: + args.append('-x') + args.extend(['-c', mycmd]) + else: + args.extend(mycmd) log.debug('args: %r', args) proc = Popen(args, env=env) - if proc.wait() != 0: + ret = proc.wait() + if ret: if fail_func: - log.error('CMD(), NON-Zero command return. Running fail_func().') + log.error('cmd(%r) exited %s; running fail_func().', args, ret) fail_func() - raise CatalystError("cmd() NON-zero return value from: %s" % args, + raise CatalystError('cmd(%r) exited %s' % (args, ret), print_traceback=False) diff --git a/catalyst/targets/grp.py b/catalyst/targets/grp.py index c1bd2dd2..049bc55b 100644 --- a/catalyst/targets/grp.py +++ b/catalyst/targets/grp.py @@ -7,7 +7,7 @@ import os import glob from catalyst import log -from catalyst.support import (CatalystError, normpath, cmd, list_bashify) +from catalyst.support import (CatalystError, normpath, cmd) from catalyst.base.stagebase import StageBase @@ -41,10 +41,11 @@ class grp(StageBase): def run_local(self): for pkgset in self.settings["grp"]: # example call: "grp.sh run pkgset cd1 xmms vim sys-apps/gleep" - mypackages=list_bashify(self.settings["grp/"+pkgset+"/packages"]) try: - cmd(self.settings["controller_file"]+" run "+self.settings["grp/"+pkgset+"/type"]\ - +" "+pkgset+" "+mypackages,env=self.env) + cmd([self.settings['controller_file'], 'run', + self.settings['grp/' + pkgset + '/type'], + pkgset] + self.settings['grp/' + pkgset + '/packages'], + env=self.env) except CatalystError: self.unbind() diff --git a/catalyst/targets/netboot.py b/catalyst/targets/netboot.py index 5285c056..161300db 100644 --- a/catalyst/targets/netboot.py +++ b/catalyst/targets/netboot.py @@ -7,7 +7,7 @@ import os from catalyst import log from catalyst.support import (CatalystError, normpath, - cmd, list_bashify, file_locate) + cmd, file_locate) from catalyst.base.stagebase import StageBase @@ -62,22 +62,22 @@ class netboot(StageBase): # def build_packages(self): # # build packages # if "netboot/packages" in self.settings: -# mypack=list_bashify(self.settings["netboot/packages"]) -# try: -# cmd(self.settings["controller_file"]+" packages "+mypack,env=self.env) -# except CatalystError: -# self.unbind() -# raise CatalystError("netboot build aborting due to error.", -# print_traceback=True) +# try: +# cmd([self.settings['controller_file'], 'packages'] + +# self.settings['netboot/packages'], env=self.env) +# except CatalystError: +# self.unbind() +# raise CatalystError('netboot build aborting due to error.', +# print_traceback=True) def build_busybox(self): # build busybox if "netboot/busybox_config" in self.settings: - mycmd = self.settings["netboot/busybox_config"] + mycmd = [self.settings['netboot/busybox_config']] else: - mycmd = "" + mycmd = [] try: - cmd(self.settings["controller_file"]+" busybox "+ mycmd,env=self.env) + cmd([self.settings['controller_file'], 'busybox'] + mycmd, env=self.env) except CatalystError: self.unbind() raise CatalystError("netboot build aborting due to error.", @@ -106,8 +106,8 @@ class netboot(StageBase): myfiles.append(self.settings["netboot/extra_files"]) try: - cmd(self.settings["controller_file"]+\ - " image " + list_bashify(myfiles),env=self.env) + cmd([self.settings['controller_file'], 'image'] + myfiles, + env=self.env) except CatalystError: self.unbind() raise CatalystError("netboot build aborting due to error.", @@ -116,7 +116,7 @@ class netboot(StageBase): def create_netboot_files(self): # finish it all up try: - cmd(self.settings["controller_file"]+" finish",env=self.env) + cmd([self.settings['controller_file'], 'finish'], env=self.env) except CatalystError: self.unbind() raise CatalystError("netboot build aborting due to error.", diff --git a/catalyst/targets/netboot2.py b/catalyst/targets/netboot2.py index 568b791f..da856ba9 100644 --- a/catalyst/targets/netboot2.py +++ b/catalyst/targets/netboot2.py @@ -8,7 +8,7 @@ import shutil from stat import ST_UID, ST_GID, ST_MODE from catalyst import log -from catalyst.support import (CatalystError, normpath, cmd, list_bashify) +from catalyst.support import (CatalystError, normpath, cmd) from catalyst.fileops import ensure_dirs, clear_path from catalyst.base.stagebase import StageBase @@ -89,8 +89,8 @@ class netboot2(StageBase): myfiles.append(self.settings["netboot2/extra_files"]) try: - cmd(self.settings["controller_file"]+\ - " image " + list_bashify(myfiles),env=self.env) + cmd([self.settings['controller_file'], 'image'] + + myfiles, env=self.env) except CatalystError: self.unbind() raise CatalystError("Failed to copy files to image!", @@ -106,8 +106,9 @@ class netboot2(StageBase): if "netboot2/overlay" in self.settings: for x in self.settings["netboot2/overlay"]: if os.path.exists(x): - cmd("rsync -a "+x+"/ "+\ - self.settings["chroot_path"] + self.settings["merge_path"], env=self.env) + cmd(['rsync', '-a', x + '/', + self.settings['chroot_path'] + self.settings['merge_path']], + env=self.env) self.resume.enable("setup_overlay") def move_kernels(self): @@ -115,8 +116,7 @@ class netboot2(StageBase): # no auto resume here as we always want the # freshest images moved try: - cmd(self.settings["controller_file"]+\ - " final",env=self.env) + cmd([self.settings['controller_file'], 'final'], env=self.env) log.notice('Netboot Build Finished!') except CatalystError: self.unbind() diff --git a/catalyst/targets/snapshot.py b/catalyst/targets/snapshot.py index 196166a8..7ba94b29 100644 --- a/catalyst/targets/snapshot.py +++ b/catalyst/targets/snapshot.py @@ -56,10 +56,16 @@ class snapshot(TargetBase, GenBase): mytmp=self.settings["tmp_path"] ensure_dirs(mytmp) - target_snapshot = self.settings["portdir"] + "/ " + mytmp + "/%s/" % self.settings["repo_name"] - cmd("rsync -a --no-o --no-g --delete --exclude /packages/ --exclude /distfiles/ " + - "--exclude /local/ --exclude CVS/ --exclude .svn --filter=H_**/files/digest-* " + - target_snapshot, env=self.env) + cmd(['rsync', '-a', '--no-o', '--no-g', '--delete', + '--exclude=/packages/', + '--exclude=/distfiles/', + '--exclude=/local/', + '--exclude=CVS/', + '--exclude=.svn', + '--filter=H_**/files/digest-*', + self.settings['portdir'] + '/', + mytmp + '/' + self.settings['repo_name'] + '/'], + env=self.env) log.notice('Compressing Portage snapshot tarball ...') compressor = CompressMap(self.settings["compress_definitions"], diff --git a/catalyst/targets/tinderbox.py b/catalyst/targets/tinderbox.py index f7895de6..85a939f4 100644 --- a/catalyst/targets/tinderbox.py +++ b/catalyst/targets/tinderbox.py @@ -5,7 +5,7 @@ Tinderbox target import os -from catalyst.support import cmd, list_bashify, CatalystError +from catalyst.support import cmd, CatalystError from catalyst.base.stagebase import StageBase @@ -24,8 +24,8 @@ class tinderbox(StageBase): # example call: "grp.sh run xmms vim sys-apps/gleep" try: if os.path.exists(self.settings["controller_file"]): - cmd(self.settings["controller_file"]+" run "+\ - list_bashify(self.settings["tinderbox/packages"]),env=self.env) + cmd([self.settings['controller_file'], 'run'] + + self.settings['tinderbox/packages'], env=self.env) except CatalystError: self.unbind() -- cgit v1.2.3