10524 wsdiff much slower after move from deprecated commands module

10448 wsdiff explodes on encoding error
10525 wsdiff output is not correct for a binary file
10526 wsdiff tries to spawn 4.8 threads
Reviewed by: Gergő Mihály Doma <domag02@gmail.com>
Reviewed by: Sebastian Wiedenroth <sebastian.wiedenroth@skylime.net>
Approved by: Rich Lowe <richlowe@richlowe.net>

1 files changed, 32 insertions, 86 deletions

diff --git a/usr/src/tools/scripts/wsdiff.py b/usr/src/tools/scripts/wsdiff.py
index 78347ea3b5..aaab371708 100644
--- a/usr/src/tools/scripts/wsdiff.py
+++ b/usr/src/tools/scripts/wsdiff.py
@@ -20,7 +20,7 @@
 # CDDL HEADER END
 #
 # Copyright (c) 2010, Oracle and/or its affiliates. All rights reserved.
-# Copyright 2018 OmniOS Community Edition (OmniOSce) Association.
+# Copyright 2019 OmniOS Community Edition (OmniOSce) Association.
 #
 
 #
@@ -46,8 +46,8 @@
 # and details about why wsdiff(1) thinks they are different will be logged to
 # the results file.
 #
-# By invoking nightly(1) with the -w option to NIGHTLY_FLAGS, nightly(1) will use
-# wsdiff(1) to report on what objects changed since the last build.
+# By invoking nightly(1) with the -w option to NIGHTLY_FLAGS, nightly(1) will
+# use wsdiff(1) to report on what objects changed since the last build.
 #
 # For patch deliverable purposes, it's advised to have nightly do a clobber,
 # non-debug build.
@@ -55,8 +55,8 @@
 # Think about the results. Was something flagged that you don't expect? Go look
 # at the results file to see details about the differences.
 #
-# Use the -i option in conjunction with -v and -V to dive deeper and have wsdiff(1)
-# report with more verbosity.
+# Use the -i option in conjunction with -v and -V to dive deeper and have
+# wsdiff(1) report with more verbosity.
 #
 # Usage: wsdiff [-vVt] [-r results ] [-i filelist ] old new
 #
@@ -75,7 +75,11 @@ import datetime, fnmatch, getopt, os, profile, io, subprocess
 import re, resource, select, shutil, signal, string, struct, sys, tempfile
 import time, threading
 from stat import *
-from subprocess import Popen, PIPE
+
+PY3 = sys.version_info[0] == 3
+
+if not PY3:
+	import commands
 
 # Human readable diffs truncated by default if longer than this
 # Specifying -v on the command line will override
@@ -116,9 +120,10 @@ wsdiff_exceptions = [
 ]
 
 def getoutput(cmd):
-	p = Popen(cmd, shell=True, stdout=PIPE)
-	output, x = p.communicate()
-	return (p.returncode, output.decode(errors='replace'))
+	if PY3:
+		return subprocess.getstatusoutput(cmd)
+	else:
+		return commands.getstatusoutput(cmd)
 
 #####
 # Logging routines
@@ -224,7 +229,7 @@ def log_difference(f, dtype, diffs) :
 #
 
 #
-# Return human readable diffs from two temporary files
+# Return human readable diffs from two files
 #
 def diffFileData(tmpf1, tmpf2) :
 
@@ -267,34 +272,6 @@ def diffFileData(tmpf1, tmpf2) :
 
 	return data
 
-#
-# Return human readable diffs betweeen two datasets
-#
-def diffData(base, ptch, d1, d2) :
-
-	t = threading.currentThread()
-	tmpFile1 = tmpDir1 + os.path.basename(base) + t.getName()
-	tmpFile2 = tmpDir2 + os.path.basename(ptch) + t.getName()
-
-	try:
-		fd1 = io.open(tmpFile1, mode='w', errors='ignore')
-	except:
-		error("failed to open: " + tmpFile1)
-		cleanup(1)
-
-	try:
-		fd2 = io.open(tmpFile2, mode='w', errors='ignore')
-	except:
-		error("failed to open: " + tmpFile2)
-		cleanup(1)
-
-	fd1.write(d1)
-	fd2.write(d2)
-	fd1.close()
-	fd2.close()
-
-	return diffFileData(tmpFile1, tmpFile2)
-
 #####
 # Misc utility functions
 #
@@ -456,7 +433,7 @@ def getTheFileType(f) :
 elfmagic = b'\177ELF'
 def isELF(f) :
 	try:
-		with io.open(f, mode='rb') as fd:
+		with open(f, mode='rb') as fd:
 			magic = fd.read(len(elfmagic))
 
 		if magic == elfmagic :
@@ -471,7 +448,7 @@ def isELF(f) :
 #
 def isBinary(f) :
 	try:
-		with io.open(f, mode='rb') as fd:
+		with open(f, mode='rb') as fd:
 			s = fd.read()
 
 		if s.find(b'\0') == -1 :
@@ -1056,8 +1033,6 @@ def compareArchives(base, ptch, fileType) :
 #####
 # (Basic) file comparison
 #
-# There's some special case code here for Javadoc HTML files
-#
 # Returns 1 if difference detected
 #         0 if no difference detected
 #        -1 on error
@@ -1070,44 +1045,25 @@ def compareBasic(base, ptch, quiet, fileType) :
 		return 1
 
 	try:
-		baseFile = io.open(base, errors='replace')
+		with open(base, 'rb') as fh:
+			baseData = fh.read()
 	except:
 		error("could not open " + base)
 		return -1
+
 	try:
-		ptchFile = io.open(ptch, errors='replace')
+		with open(ptch, 'rb') as fh:
+			ptchData = fh.read()
 	except:
 		error("could not open " + ptch)
 		return -1
 
-	baseData = baseFile.read()
-	ptchData = ptchFile.read()
-
-	baseFile.close()
-	ptchFile.close()
-
-	needToSnip = False
-	if fileType == "HTML" :
-		needToSnip = True
-		toSnipBeginStr = "<!-- Generated by javadoc"
-		toSnipEndStr = "-->\n"
-
-	if needToSnip :
-		toSnipBegin = baseData.find(toSnipBeginStr)
-		if toSnipBegin != -1 :
-			toSnipEnd = (baseData[toSnipBegin:].find(toSnipEndStr) +
-						len(toSnipEndStr))
-			baseData = (baseData[:toSnipBegin] +
-				   baseData[toSnipBegin + toSnipEnd:])
-			ptchData = (ptchData[:toSnipBegin] +
-				   ptchData[toSnipBegin + toSnipEnd:])
-
 	if quiet :
 		if baseData != ptchData :
 			return 1
 	else :
 		if len(baseData) != len(ptchData) or baseData != ptchData :
-			diffs = diffData(base, ptch, baseData, ptchData)
+			diffs = diffFileData(base, ptch)
 			difference(fileName, fileType, diffs)
 			return 1
 	return 0
@@ -1147,35 +1103,25 @@ def compareByDumping(base, ptch, quiet, fileType) :
 	os.system(ptchCmd)
 
 	try:
-		baseFile = open(tmpFile1)
+		with open(tmpFile1, 'rb') as fh:
+			baseData = fh.read()
 	except:
 		error("could not open: " + tmpFile1)
 		return
+
 	try:
-		ptchFile = open(tmpFile2)
+		with open(tmpFile2, 'rb') as fh:
+			ptchData = fh.read()
 	except:
 		error("could not open: " + tmpFile2)
 		return
 
-	baseData = baseFile.read()
-	ptchData = ptchFile.read()
-
-	baseFile.close()
-	ptchFile.close()
+	ret = 0
 
 	if len(baseData) != len(ptchData) or baseData != ptchData :
 		if not quiet :
 			data = diffFileData(tmpFile1, tmpFile2);
-			try:
-				os.unlink(tmpFile1)
-			except OSError as e:
-				error("compareByDumping: unlink failed %s" % e)
-			try:
-				os.unlink(tmpFile2)
-			except OSError as e:
-				error("compareByDumping: unlink failed %s" % e)
-			difference(fileName, fileType, data)
-		return 1
+		ret = 1
 
 	# Remove the temporary files now.
 	try:
@@ -1187,7 +1133,7 @@ def compareByDumping(base, ptch, quiet, fileType) :
 	except OSError as e:
 		error("compareByDumping: unlink failed %s" % e)
 
-	return 0
+	return ret
 
 #####
 #
@@ -1487,7 +1433,7 @@ def main() :
 		if max_threads == -1 :
 			max_threads = 1
 		else :
-			max_threads += max_threads/5
+			max_threads += int(max_threads/5)
 
 	# Set signal handler to attempt graceful exit
 	debug("Setting signal handler")