[gentoo-catalyst] [PATCH 5/5] Remove redundant /bin/bash additions in cmd() calls

[Brian Dolbec]

Remove old spawn_* functions copied from portage.
cmd() now uses subprocess.Popen().
---
 catalyst/support.py                      | 301 +++----------------------------
 catalyst/targets/generic_stage_target.py |  26 +--
 catalyst/targets/grp_target.py           |   2 +-
 catalyst/targets/netboot2_target.py      |   4 +-
 catalyst/targets/netboot_target.py       |   8 +-
 catalyst/targets/tinderbox_target.py     |   2 +-
 6 files changed, 42 insertions(+), 301 deletions(-)

diff --git a/catalyst/support.py b/catalyst/support.py
index e2d64a1..aaacaa9 100644
--- a/catalyst/support.py
+++ b/catalyst/support.py
@@ -4,9 +4,10 @@ import string
 import os
 import types
 import re
-import signal
 import traceback
 import time
+from subprocess import Popen
+
 
 from catalyst.defaults import verbosity, valid_config_file_values
 
@@ -31,34 +32,6 @@ except:
 spawned_pids = []
 
 
-def cleanup(pids,block_exceptions=True):
-	"""function to go through and reap the list of pids passed to it"""
-	global spawned_pids
-	if type(pids) == int:
-		pids = [pids]
-	for x in pids:
-		try:
-			os.kill(x,signal.SIGTERM)
-			if os.waitpid(x,os.WNOHANG)[1] == 0:
-				# feisty bugger, still alive.
-				os.kill(x,signal.SIGKILL)
-				os.waitpid(x,0)
-		except OSError, oe:
-			if block_exceptions:
-				pass
-			if oe.errno not in (10,3):
-				raise oe
-		except SystemExit:
-					raise
-		except Exception:
-			if block_exceptions:
-				pass
-		try:
-			spawned_pids.remove(x)
-		except IndexError:
-			pass
-
-
 # a function to turn a string of non-printable characters
 # into a string of hex characters
 def hexify(str):
@@ -79,8 +52,8 @@ def read_from_clst(file):
 		return -1
 		#raise CatalystError, "Could not open file "+file
 	for line in myf.readlines():
-	    #line = string.replace(line, "\n", "") # drop newline
-	    myline = myline + line
+		#line = string.replace(line, "\n", "") # drop newline
+		myline = myline + line
 	myf.close()
 	return myline
 
@@ -145,259 +118,27 @@ def find_binary(myc):
 	return None
 
 
-def spawn_bash(mycommand,env={},debug=False,opt_name=None,**keywords):
-	"""spawn mycommand as an arguement to bash"""
-	args=[BASH_BINARY]
-	if not opt_name:
-		opt_name=mycommand.split()[0]
-	if "BASH_ENV" not in env:
-		env["BASH_ENV"] = "/etc/spork/is/not/valid/profile.env"
-	if debug:
-		args.append("-x")
-	args.append("-c")
-	args.append(mycommand)
-	return spawn(args,env=env,opt_name=opt_name,**keywords)
-
-
-def spawn_get_output(mycommand,raw_exit_code=False,emulate_gso=True, \
-	collect_fds=[1],fd_pipes=None,**keywords):
-	"""call spawn, collecting the output to fd's specified in collect_fds list
-	emulate_gso is a compatability hack to emulate commands.getstatusoutput's return, minus the
-	requirement it always be a bash call (spawn_type controls the actual spawn call), and minus the
-	'lets let log only stdin and let stderr slide by'.
-
-	emulate_gso was deprecated from the day it was added, so convert your code over.
-	spawn_type is the passed in function to call- typically spawn_bash, spawn, spawn_sandbox, or spawn_fakeroot"""
-	global selinux_capable
-	pr,pw=os.pipe()
-
-	if fd_pipes==None:
-			fd_pipes={}
-			fd_pipes[0] = 0
-
-	for x in collect_fds:
-			fd_pipes[x] = pw
-	keywords["returnpid"]=True
-
-	mypid=spawn_bash(mycommand,fd_pipes=fd_pipes,**keywords)
-	os.close(pw)
-	if type(mypid) != types.ListType:
-			os.close(pr)
-			return [mypid, "%s: No such file or directory" % mycommand.split()[0]]
-
-	fd=os.fdopen(pr,"r")
-	mydata=fd.readlines()
-	fd.close()
-	if emulate_gso:
-			mydata=string.join(mydata)
-			if len(mydata) and mydata[-1] == "\n":
-					mydata=mydata[:-1]
-	retval=os.waitpid(mypid[0],0)[1]
-	cleanup(mypid)
-	if raw_exit_code:
-			return [retval,mydata]
-	retval=process_exit_code(retval)
-	return [retval, mydata]
-
-
-# base spawn function
-def spawn(mycommand,env={},raw_exit_code=False,opt_name=None,fd_pipes=None,returnpid=False,\
-	 uid=None,gid=None,groups=None,umask=None,logfile=None,path_lookup=True,\
-	 selinux_context=None, raise_signals=False, func_call=False):
-	"""base fork/execve function.
-	mycommand is the desired command- if you need a command to execute in a bash/sandbox/fakeroot
-	environment, use the appropriate spawn call.  This is a straight fork/exec code path.
-	Can either have a tuple, or a string passed in.  If uid/gid/groups/umask specified, it changes
-	the forked process to said value.  If path_lookup is on, a non-absolute command will be converted
-	to an absolute command, otherwise it returns None.
-
-	selinux_context is the desired context, dependant on selinux being available.
-	opt_name controls the name the processor goes by.
-	fd_pipes controls which file descriptor numbers are left open in the forked process- it's a dict of
-	current fd's raw fd #, desired #.
-
-	func_call is a boolean for specifying to execute a python function- use spawn_func instead.
-	raise_signals is questionable.  Basically throw an exception if signal'd.  No exception is thrown
-	if raw_input is on.
-
-	logfile overloads the specified fd's to write to a tee process which logs to logfile
-	returnpid returns the relevant pids (a list, including the logging process if logfile is on).
-
-	non-returnpid calls to spawn will block till the process has exited, returning the exitcode/signal
-	raw_exit_code controls whether the actual waitpid result is returned, or intrepretted."""
-
-	myc=''
-	if not func_call:
-		if type(mycommand)==types.StringType:
-			mycommand=mycommand.split()
-		myc = mycommand[0]
-		if not os.access(myc, os.X_OK):
-			if not path_lookup:
-				return None
-			myc = find_binary(myc)
-			if myc == None:
-				return None
-	mypid=[]
-	if logfile:
-		pr,pw=os.pipe()
-		mypid.extend(spawn(('tee','-i','-a',logfile),returnpid=True,fd_pipes={0:pr,1:1,2:2}))
-		retval=os.waitpid(mypid[-1],os.WNOHANG)[1]
-		if retval != 0:
-			# he's dead jim.
-			if raw_exit_code:
-				return retval
-			return process_exit_code(retval)
-
-		if fd_pipes == None:
-			fd_pipes={}
-			fd_pipes[0] = 0
-		fd_pipes[1]=pw
-		fd_pipes[2]=pw
-
-	if not opt_name:
-		opt_name = mycommand[0]
-	myargs=[opt_name]
-	myargs.extend(mycommand[1:])
-	global spawned_pids
-	mypid.append(os.fork())
-	if mypid[-1] != 0:
-		#log the bugger.
-		spawned_pids.extend(mypid)
-
-	if mypid[-1] == 0:
-		if func_call:
-			spawned_pids = []
-
-		# this may look ugly, but basically it moves file descriptors around to ensure no
-		# handles that are needed are accidentally closed during the final dup2 calls.
-		trg_fd=[]
-		if type(fd_pipes)==types.DictType:
-			src_fd=[]
-			k=fd_pipes.keys()
-			k.sort()
-
-			#build list of which fds will be where, and where they are at currently
-			for x in k:
-				trg_fd.append(x)
-				src_fd.append(fd_pipes[x])
-
-			# run through said list dup'ing descriptors so that they won't be waxed
-			# by other dup calls.
-			for x in range(0,len(trg_fd)):
-				if trg_fd[x] == src_fd[x]:
-					continue
-				if trg_fd[x] in src_fd[x+1:]:
-					os.close(trg_fd[x])
-
-			# transfer the fds to their final pre-exec position.
-			for x in range(0,len(trg_fd)):
-				if trg_fd[x] != src_fd[x]:
-					os.dup2(src_fd[x], trg_fd[x])
-		else:
-			trg_fd=[0,1,2]
-
-		# wax all open descriptors that weren't requested be left open.
-		for x in range(0,max_fd_limit):
-			if x not in trg_fd:
-				try:
-					os.close(x)
-				except SystemExit, e:
-					raise
-				except:
-					pass
-
-		# note this order must be preserved- can't change gid/groups if you change uid first.
-		if selinux_capable and selinux_context:
-			import selinux
-			selinux.setexec(selinux_context)
-		if gid:
-			os.setgid(gid)
-		if groups:
-			os.setgroups(groups)
-		if uid:
-			os.setuid(uid)
-		if umask:
-			os.umask(umask)
-		else:
-			os.umask(022)
-
-		try:
-			#print "execing", myc, myargs
-			if func_call:
-				# either use a passed in func for interpretting the results, or return if no exception.
-				# note the passed in list, and dict are expanded.
-				if len(mycommand) == 4:
-					os._exit(mycommand[3](mycommand[0](*mycommand[1],**mycommand[2])))
-				try:
-					mycommand[0](*mycommand[1],**mycommand[2])
-				except Exception,e:
-					print "caught exception",e," in forked func",mycommand[0]
-				sys.exit(0)
-
-			os.execve(myc,myargs,env)
-		except SystemExit, e:
-			raise
-		except Exception, e:
-			if not func_call:
-				raise str(e)+":\n   "+myc+" "+string.join(myargs)
-			print "func call failed"
-
-		# If the execve fails, we need to report it, and exit
-		# *carefully* --- report error here
-		os._exit(1)
-		sys.exit(1)
-		return # should never get reached
-
-	# if we were logging, kill the pipes.
-	if logfile:
-			os.close(pr)
-			os.close(pw)
-
-	if returnpid:
-			return mypid
-
-	# loop through pids (typically one, unless logging), either waiting on their death, or waxing them
-	# if the main pid (mycommand) returned badly.
-	while len(mypid):
-		retval=os.waitpid(mypid[-1],0)[1]
-		if retval != 0:
-			cleanup(mypid[0:-1],block_exceptions=False)
-			# at this point we've killed all other kid pids generated via this call.
-			# return now.
-			if raw_exit_code:
-				return retval
-			return process_exit_code(retval,throw_signals=raise_signals)
-		else:
-			mypid.pop(-1)
-	cleanup(mypid)
-	return 0
-
-
-def cmd(mycmd,myexc="",env={}):
+def cmd(mycmd, myexc="", env={}, debug=False):
 	try:
 		sys.stdout.flush()
-		retval=spawn_bash(mycmd,env)
-		if retval != 0:
-			raise CatalystError,myexc
+		args=[BASH_BINARY]
+		if "BASH_ENV" not in env:
+			env["BASH_ENV"] = "/etc/spork/is/not/valid/profile.env"
+		if debug:
+			args.append("-x")
+		args.append("-c")
+		args.append(mycmd)
+
+		if debug:
+			print "cmd(); args =", args
+		proc = Popen(args, env=env)
+		if proc.wait() != 0:
+			raise CatalystError("cmd() NON-zero return value from: %s" % myexc,
+				print_traceback=False)
 	except:
 		raise
 
 
-def process_exit_code(retval,throw_signals=False):
-	"""process a waitpid returned exit code, returning exit code if it exit'd, or the
-	signal if it died from signalling
-	if throw_signals is on, it raises a SystemExit if the process was signaled.
-	This is intended for usage with threads, although at the moment you can't signal individual
-	threads in python, only the master thread, so it's a questionable option."""
-	if (retval & 0xff)==0:
-		return retval >> 8 # return exit code
-	else:
-		if throw_signals:
-			#use systemexit, since portage is stupid about exception catching.
-			raise SystemExit()
-		return (retval & 0xff) << 8 # interrupted by signal
-
-
 def file_locate(settings,filelist,expand=1):
 	#if expand=1, non-absolute paths will be accepted and
 	# expanded to os.getcwd()+"/"+localpath if file exists
@@ -459,8 +200,8 @@ def parse_makeconf(mylines):
 			mobj=pat.match(myline)
 			pos += 1
 			if mobj.group(2):
-			    clean_string = re.sub(r"\"",r"",mobj.group(2))
-			    mymakeconf[mobj.group(1)]=clean_string
+				clean_string = re.sub(r"\"",r"",mobj.group(2))
+				mymakeconf[mobj.group(1)]=clean_string
 	return mymakeconf
 
 
diff --git a/catalyst/targets/generic_stage_target.py b/catalyst/targets/generic_stage_target.py
index c6b8dcc..296eee3 100644
--- a/catalyst/targets/generic_stage_target.py
+++ b/catalyst/targets/generic_stage_target.py
@@ -1136,7 +1136,7 @@ class generic_stage_target(TargetBase, ClearBase, GenBase):
 		else:
 			if "fsscript" in self.settings:
 				if os.path.exists(self.settings["controller_file"]):
-					cmd("/bin/bash "+self.settings["controller_file"]+\
+					cmd(self.settings["controller_file"]+\
 						" fsscript","fsscript script failed.",env=self.env)
 					touch(fsscript_resume)
 
@@ -1147,7 +1147,7 @@ class generic_stage_target(TargetBase, ClearBase, GenBase):
 			print "Resume point detected, skipping rcupdate operation..."
 		else:
 			if os.path.exists(self.settings["controller_file"]):
-				cmd("/bin/bash "+self.settings["controller_file"]+" rc-update",\
+				cmd(self.settings["controller_file"]+" rc-update",\
 					"rc-update script failed.",env=self.env)
 				touch(rcupdate_resume)
 
@@ -1183,7 +1183,7 @@ class generic_stage_target(TargetBase, ClearBase, GenBase):
 				"Could not remove stray files in /etc",env=self.env)
 
 		if os.path.exists(self.settings["controller_file"]):
-			cmd("/bin/bash "+self.settings["controller_file"]+" clean",\
+			cmd(self.settings["controller_file"]+" clean",\
 				"clean script failed.",env=self.env)
 			touch(clean_resume)
 
@@ -1232,7 +1232,7 @@ class generic_stage_target(TargetBase, ClearBase, GenBase):
 					os.system("rm -rf "+self.settings["chroot_path"]+x)
 				try:
 					if os.path.exists(self.settings["controller_file"]):
-						cmd("/bin/bash "+self.settings["controller_file"]+\
+						cmd(self.settings["controller_file"]+\
 							" clean","Clean  failed.",env=self.env)
 						touch(remove_resume)
 				except:
@@ -1247,7 +1247,7 @@ class generic_stage_target(TargetBase, ClearBase, GenBase):
 		else:
 			try:
 				if os.path.exists(self.settings["controller_file"]):
-					cmd("/bin/bash "+self.settings["controller_file"]+\
+					cmd(self.settings["controller_file"]+\
 						" preclean","preclean script failed.",env=self.env)
 					touch(preclean_resume)
 
@@ -1289,7 +1289,7 @@ class generic_stage_target(TargetBase, ClearBase, GenBase):
 		else:
 			try:
 				if os.path.exists(self.settings["controller_file"]):
-					cmd("/bin/bash "+self.settings["controller_file"]+" run",\
+					cmd(self.settings["controller_file"]+" run",\
 						"run script failed.",env=self.env)
 					touch(run_local_resume)
 
@@ -1388,7 +1388,7 @@ class generic_stage_target(TargetBase, ClearBase, GenBase):
 
 				""" Before cleaning, unmerge stuff """
 				try:
-					cmd("/bin/bash "+self.settings["controller_file"]+\
+					cmd(self.settings["controller_file"]+\
 						" unmerge "+ myunmerge,"Unmerge script failed.",\
 						env=self.env)
 					print "unmerge shell script"
@@ -1405,7 +1405,7 @@ class generic_stage_target(TargetBase, ClearBase, GenBase):
 			print "Resume point detected, skipping target_setup operation..."
 		else:
 			print "Setting up filesystems per filesystem type"
-			cmd("/bin/bash "+self.settings["controller_file"]+\
+			cmd(self.settings["controller_file"]+\
 				" target_image_setup "+ self.settings["target_path"],\
 				"target_image_setup script failed.",env=self.env)
 			touch(target_setup_resume)
@@ -1434,7 +1434,7 @@ class generic_stage_target(TargetBase, ClearBase, GenBase):
 		else:
 			""" Create the ISO """
 			if "iso" in self.settings:
-				cmd("/bin/bash "+self.settings["controller_file"]+" iso "+\
+				cmd(self.settings["controller_file"]+" iso "+\
 					self.settings["iso"],"ISO creation script failed.",\
 					env=self.env)
 				self.gen_contents_file(self.settings["iso"])
@@ -1461,7 +1461,7 @@ class generic_stage_target(TargetBase, ClearBase, GenBase):
 						list_bashify(self.settings[self.settings["spec_prefix"]\
 						+"/packages"])
 					try:
-						cmd("/bin/bash "+self.settings["controller_file"]+\
+						cmd(self.settings["controller_file"]+\
 							" build_packages "+mypack,\
 							"Error in attempt to build packages",env=self.env)
 						touch(build_packages_resume)
@@ -1486,7 +1486,7 @@ class generic_stage_target(TargetBase, ClearBase, GenBase):
 					"""
 					Execute the script that sets up the kernel build environment
 					"""
-					cmd("/bin/bash "+self.settings["controller_file"]+\
+					cmd(self.settings["controller_file"]+\
 						" pre-kmerge ","Runscript pre-kmerge failed",\
 						env=self.env)
 					for kname in mynames:
@@ -1603,7 +1603,7 @@ class generic_stage_target(TargetBase, ClearBase, GenBase):
 			print "Resume point detected, skipping bootloader operation..."
 		else:
 			try:
-				cmd("/bin/bash "+self.settings["controller_file"]+\
+				cmd(self.settings["controller_file"]+\
 					" bootloader " + self.settings["target_path"],\
 					"Bootloader script failed.",env=self.env)
 				touch(bootloader_resume)
@@ -1619,7 +1619,7 @@ class generic_stage_target(TargetBase, ClearBase, GenBase):
 			print "Resume point detected, skipping build_packages operation..."
 		else:
 			try:
-				cmd("/bin/bash "+self.settings["controller_file"]+\
+				cmd(self.settings["controller_file"]+\
 					" livecd-update","livecd-update failed.",env=self.env)
 				touch(livecd_update_resume)
 
diff --git a/catalyst/targets/grp_target.py b/catalyst/targets/grp_target.py
index a8309a8..033db75 100644
--- a/catalyst/targets/grp_target.py
+++ b/catalyst/targets/grp_target.py
@@ -54,7 +54,7 @@ class grp_target(generic_stage_target):
 			# example call: "grp.sh run pkgset cd1 xmms vim sys-apps/gleep"
 			mypackages=list_bashify(self.settings["grp/"+pkgset+"/packages"])
 			try:
-				cmd("/bin/bash "+self.settings["controller_file"]+" run "+self.settings["grp/"+pkgset+"/type"]\
+				cmd(self.settings["controller_file"]+" run "+self.settings["grp/"+pkgset+"/type"]\
 					+" "+pkgset+" "+mypackages,env=self.env)
 
 			except CatalystError:
diff --git a/catalyst/targets/netboot2_target.py b/catalyst/targets/netboot2_target.py
index 8809dd0..ea07d76 100644
--- a/catalyst/targets/netboot2_target.py
+++ b/catalyst/targets/netboot2_target.py
@@ -87,7 +87,7 @@ class netboot2_target(generic_stage_target):
 					myfiles.append(self.settings["netboot2/extra_files"])
 
 			try:
-				cmd("/bin/bash "+self.settings["controller_file"]+\
+				cmd(self.settings["controller_file"]+\
 					" image " + list_bashify(myfiles),env=self.env)
 			except CatalystError:
 				self.unbind()
@@ -112,7 +112,7 @@ class netboot2_target(generic_stage_target):
 		# no auto resume here as we always want the
 		# freshest images moved
 		try:
-			cmd("/bin/bash "+self.settings["controller_file"]+\
+			cmd(self.settings["controller_file"]+\
 				" final",env=self.env)
 			print ">>> Netboot Build Finished!"
 		except CatalystError:
diff --git a/catalyst/targets/netboot_target.py b/catalyst/targets/netboot_target.py
index 9d01b7e..ae1eb04 100644
--- a/catalyst/targets/netboot_target.py
+++ b/catalyst/targets/netboot_target.py
@@ -59,7 +59,7 @@ class netboot_target(generic_stage_target):
 #		if "netboot/packages" in self.settings:
 #			mypack=list_bashify(self.settings["netboot/packages"])
 #		try:
-#			cmd("/bin/bash "+self.settings["controller_file"]+" packages "+mypack,env=self.env)
+#			cmd(self.settings["controller_file"]+" packages "+mypack,env=self.env)
 #		except CatalystError:
 #			self.unbind()
 #			raise CatalystError,"netboot build aborting due to error."
@@ -71,7 +71,7 @@ class netboot_target(generic_stage_target):
 		else:
 			mycmd = ""
 		try:
-			cmd("/bin/bash "+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."
@@ -99,7 +99,7 @@ class netboot_target(generic_stage_target):
 				myfiles.append(self.settings["netboot/extra_files"])
 
 		try:
-			cmd("/bin/bash "+self.settings["controller_file"]+\
+			cmd(self.settings["controller_file"]+\
 				" image " + list_bashify(myfiles),env=self.env)
 		except CatalystError:
 			self.unbind()
@@ -108,7 +108,7 @@ class netboot_target(generic_stage_target):
 	def create_netboot_files(self):
 		# finish it all up
 		try:
-			cmd("/bin/bash "+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/tinderbox_target.py b/catalyst/targets/tinderbox_target.py
index 1d31989..ea11d3f 100644
--- a/catalyst/targets/tinderbox_target.py
+++ b/catalyst/targets/tinderbox_target.py
@@ -21,7 +21,7 @@ class tinderbox_target(generic_stage_target):
 		# example call: "grp.sh run xmms vim sys-apps/gleep"
 		try:
 			if os.path.exists(self.settings["controller_file"]):
-			    cmd("/bin/bash "+self.settings["controller_file"]+" run "+\
+			    cmd(self.settings["controller_file"]+" run "+\
 				list_bashify(self.settings["tinderbox/packages"]),"run script failed.",env=self.env)
 
 		except CatalystError:
-- 
2.1.0