All subprocess.Popen calls now use the list variant.

Most (all?) hard coded quotations have been removed in favor of automation
and the pipes or shlex modules.

gitinspector/basedir.py

@@ -33,7 +33,7 @@ def get_basedir_git():
 	global __git_basedir__
 
 	if not __git_basedir__:
-		sp = subprocess.Popen("git rev-parse --is-bare-repository", shell=True, bufsize=1,
+		sp = subprocess.Popen(["git", "rev-parse", "--is-bare-repository"], bufsize=1,
 		                          stdout=subprocess.PIPE, stderr=open(os.devnull, "w"))
 		isbare = sp.stdout.readlines()
 		sp.wait()
@@ -43,10 +43,9 @@ def get_basedir_git():
 		absolute_path = ""
 
 		if isbare:
-			absolute_path = subprocess.Popen("git rev-parse --git-dir", shell=True, bufsize=1,
-		                                 stdout=subprocess.PIPE).stdout
+			absolute_path = subprocess.Popen(["git", "rev-parse", "--git-dir"], bufsize=1, stdout=subprocess.PIPE).stdout
 		else:
-			absolute_path = subprocess.Popen("git rev-parse --show-toplevel", shell=True, bufsize=1,
+			absolute_path = subprocess.Popen(["git", "rev-parse", "--show-toplevel"], bufsize=1,
 			                                 stdout=subprocess.PIPE).stdout
 
 		absolute_path = absolute_path.readlines()

gitinspector/blame.py

@@ -50,13 +50,13 @@ __blame_lock__ = threading.Lock()
 AVG_DAYS_PER_MONTH = 30.4167
 
 class BlameThread(threading.Thread):
-	def __init__(self, useweeks, changes, blame_string, extension, blames, filename):
+	def __init__(self, useweeks, changes, blame_command, extension, blames, filename):
 		__thread_lock__.acquire() # Lock controlling the number of threads running
 		threading.Thread.__init__(self)
 
 		self.useweeks = useweeks
 		self.changes = changes
-		self.blame_string = blame_string
+		self.blame_command = blame_command
 		self.extension = extension
 		self.blames = blames
 		self.filename = filename
@@ -99,7 +99,7 @@ class BlameThread(threading.Thread):
 		__blame_lock__.release() # ...to here.
 
 	def run(self):
-		git_blame_r = subprocess.Popen(self.blame_string, shell=True, bufsize=1, stdout=subprocess.PIPE).stdout
+		git_blame_r = subprocess.Popen(self.blame_command, bufsize=1, stdout=subprocess.PIPE).stdout
 		rows = git_blame_r.readlines()
 		git_blame_r.close()
 
@@ -130,7 +130,7 @@ PROGRESS_TEXT = N_("Checking how many rows belong to each author (Progress): {0:
 class Blame:
 	def __init__(self, hard, useweeks, changes):
 		self.blames = {}
-		ls_tree_r = subprocess.Popen("git ls-tree --name-only -r " + interval.get_ref(), shell=True, bufsize=1,
+		ls_tree_r = subprocess.Popen(["git", "ls-tree", "--name-only", "-r", interval.get_ref()], bufsize=1,
 		                             stdout=subprocess.PIPE).stdout
 		lines = ls_tree_r.readlines()
 
@@ -140,9 +140,10 @@ class Blame:
 			row = row.decode("utf-8", "replace").strip("\"").strip("'").strip()
 
 			if FileDiff.is_valid_extension(row) and not filtering.set_filtered(FileDiff.get_filename(row)):
-				blame_string = "git blame --line-porcelain -w {0} ".format("-C -C -M" if hard else "") + \
-				               interval.get_since() + interval.get_ref() + " -- \"" + row + "\""
-				thread = BlameThread(useweeks, changes, blame_string, FileDiff.get_extension(row), self.blames, row.strip())
+				blame_command = filter(None, ["git", "blame", "--line-porcelain", "-w"] + \
+						(["-C", "-C", "-M"] if hard else []) +
+				                [interval.get_since(), interval.get_ref(), "--", row])
+				thread = BlameThread(useweeks, changes, blame_command, FileDiff.get_extension(row), self.blames, row.strip())
 				thread.daemon = True
 				thread.start()
 

gitinspector/changes.py

@@ -107,10 +107,9 @@ class Changes:
 
 	def __init__(self, hard):
 		self.commits = []
-		git_log_r = subprocess.Popen("git log --reverse --pretty=\"%cd|%H|%aN|%aE\" --stat=100000,8192 --no-merges -w " +
-		                             interval.get_since() + interval.get_until() +
-		                             "{0} --date=short".format("-C -C -M" if hard else ""),
-		                             shell=True, bufsize=1, stdout=subprocess.PIPE).stdout
+		git_log_r = subprocess.Popen(filter(None, ["git", "log", "--reverse", "--pretty=%cd|%H|%aN|%aE", "--stat=100000,8192", "--no-merges", "-w",
+		                             interval.get_since(), interval.get_until(), "--date=short"] + (["-C", "-C", "-M"] if hard else [])),
+		                             bufsize=1, stdout=subprocess.PIPE).stdout
 		commit = None
 		found_valid_extension = False
 		lines = git_log_r.readlines()

gitinspector/clone.py

@@ -32,7 +32,7 @@ def create(url):
 		global __cloned_path__
 
 		location = tempfile.mkdtemp(suffix=".gitinspector")
-		git_clone = subprocess.Popen("git clone {0} {1}".format(url, location), shell=True, bufsize=1, stdout=sys.stderr)
+		git_clone = subprocess.Popen(["git", "clone", url, location], bufsize=1, stdout=sys.stderr)
 		git_clone.wait()
 
 		if git_clone.returncode != 0:

gitinspector/config.py

@@ -29,8 +29,7 @@ import subprocess
 def __read_git_config__(repo, variable):
 	previous_directory = os.getcwd()
 	os.chdir(repo)
-	setting = subprocess.Popen("git config inspector." + variable, shell=True, bufsize=1,
-	                           stdout=subprocess.PIPE).stdout
+	setting = subprocess.Popen(["git", "config", "inspector." + variable], bufsize=1, stdout=subprocess.PIPE).stdout
 	os.chdir(previous_directory)
 
 	try:

gitinspector/interval.py

@@ -19,6 +19,11 @@
 
 from __future__ import unicode_literals
 
+try:
+	from shlex import quote
+except ImportError:
+	from pipes import quote
+
 __since__ = ""
 
 __until__ = ""
@@ -33,14 +38,14 @@ def get_since():
 
 def set_since(since):
 	global __since__
-	__since__ = "--since=\"" + since + "\" "
+	__since__ = "--since=" + quote(since)
 
 def get_until():
 	return __until__
 
 def set_until(until):
 	global __until__
-	__until__ = "--until=\"" + until + "\" "
+	__until__ = "--until=" + quote(until)
 
 def get_ref():
 	return __ref__

gitinspector/metrics.py

@@ -46,7 +46,7 @@ class MetricsLogic:
 		self.cyclomatic_complexity = {}
 		self.cyclomatic_complexity_density = {}
 
-		ls_tree_r = subprocess.Popen("git ls-tree --name-only -r " + interval.get_ref(), shell=True, bufsize=1,
+		ls_tree_r = subprocess.Popen(["git", "ls-tree", "--name-only", "-r", interval.get_ref()], bufsize=1,
 		                             stdout=subprocess.PIPE).stdout
 
 		for i in ls_tree_r.readlines():
@@ -55,8 +55,8 @@ class MetricsLogic:
 			i = i.decode("utf-8", "replace").strip("\"").strip("'").strip()
 
 			if FileDiff.is_valid_extension(i) and not filtering.set_filtered(FileDiff.get_filename(i)):
-				file_r = subprocess.Popen("git show " + interval.get_ref() + ":\"{0}\"".format(i.strip()),
-				                          shell=True, bufsize=1, stdout=subprocess.PIPE).stdout.readlines()
+				file_r = subprocess.Popen(["git", "show", interval.get_ref() + ":{0}".format(i.strip())],
+				                          bufsize=1, stdout=subprocess.PIPE).stdout.readlines()
 
 				extension = FileDiff.get_extension(i)
 				lines = MetricsLogic.get_eloc(file_r, extension)