From f439ae8a8c3aa53219cc0ef7741da106902f778c Mon Sep 17 00:00:00 2001
From: Mats Wichmann <mats@linux.com>
Date: Fri, 29 Mar 2019 11:39:46 -0600
Subject: [PR #333] close files to avoid scons-time races

With runtest now honoring the -j 2 option given to it in
CI setup on Windows, there were some problems where scons-time
tests could try to remove a test directory while some files
in it were still open (these locations were complained about
by Python 3.8 also).

Switch test framework to using mkdtemp also, and to not use
tempfile.template (usage of that and mktemp are long
deprecated)

Signed-off-by: Mats Wichmann <mats@linux.com>
---
 src/CHANGES.txt                                |  4 ++++
 src/script/scons-time.py                       | 28 ++++++++++++++++----------
 test/scons-time/run/config/initial_commands.py |  3 ++-
 testing/framework/TestCmd.py                   | 13 ++++++------
 4 files changed, 29 insertions(+), 19 deletions(-)

diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index fb65386..d44e754 100755
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -7,6 +7,10 @@
 
 RELEASE  VERSION/DATE TO BE FILLED IN LATER
 
+  From Mats Wichmann:
+    - scons-time takes more care closing files and uses safer mkdtemp to avoid
+      possible races on multi-job runs.
+
   From John Doe:
 
     - Whatever John Doe did.
diff --git a/src/script/scons-time.py b/src/script/scons-time.py
index 2ca3070..ff16ac3 100644
--- a/src/script/scons-time.py
+++ b/src/script/scons-time.py
@@ -233,14 +233,16 @@ def unzip(fname):
             os.makedirs(dir)
         except:
             pass
-        open(name, 'wb').write(zf.read(name))
+        with open(name, 'wb') as f:
+            f.write(zf.read(name))
 
 def read_tree(dir):
     for dirpath, dirnames, filenames in os.walk(dir):
         for fn in filenames:
             fn = os.path.join(dirpath, fn)
             if os.path.isfile(fn):
-                open(fn, 'rb').read()
+                with open(fn, 'rb') as f:
+                    f.read()
 
 def redirect_to_file(command, log):
     return '%s > %s 2>&1' % (command, log)
@@ -441,14 +443,14 @@ class SConsTimer(object):
 
     def log_execute(self, command, log):
         command = self.subst(command, self.__dict__)
-        output = os.popen(command).read()
+        with os.popen(command) as p:
+            output = p.read()
         if self.verbose:
             sys.stdout.write(output)
         # TODO: Figure out
         # Not sure we need to write binary here
-        open(log, 'w').write(output)
-
-    #
+        with open(log, 'w') as f:
+            f.write(str(output))
 
     def archive_splitext(self, path):
         """
@@ -613,7 +615,8 @@ class SConsTimer(object):
             search_string = self.time_string_all
         else:
             search_string = time_string
-        contents = open(file).read()
+        with open(file) as f:
+            contents = f.read()
         if not contents:
             sys.stderr.write('file %s has no contents!\n' % repr(file))
             return None
@@ -658,7 +661,8 @@ class SConsTimer(object):
             search_string = self.memory_string_all
         else:
             search_string = memory_string
-        lines = open(file).readlines()
+        with open(file) as f:
+            lines = f.readlines()
         lines = [ l for l in lines if l.startswith(search_string) ][-4:]
         result = [ int(l.split()[-1]) for l in lines[-4:] ]
         if len(result) == 1:
@@ -670,14 +674,14 @@ class SConsTimer(object):
         Returns the counts of the specified object_name.
         """
         object_string = ' ' + object_name + '\n'
-        lines = open(file).readlines()
+        with open(file) as f:
+            lines = f.readlines()
         line = [ l for l in lines if l.endswith(object_string) ][0]
         result = [ int(field) for field in line.split()[:4] ]
         if index is not None:
             result = result[index]
         return result
 
-    #
 
     command_alias = {}
 
@@ -1419,7 +1423,9 @@ class SConsTimer(object):
                 which = a
 
         if self.config_file:
-            HACK_for_exec(open(self.config_file, 'r').read(), self.__dict__)
+            with open(self.config_file, 'r') as f:
+                config = f.read()
+            HACK_for_exec(config, self.__dict__)
 
         if self.chdir:
             os.chdir(self.chdir)
diff --git a/test/scons-time/run/config/initial_commands.py b/test/scons-time/run/config/initial_commands.py
index 167ed49..58d777a 100644
--- a/test/scons-time/run/config/initial_commands.py
+++ b/test/scons-time/run/config/initial_commands.py
@@ -38,7 +38,8 @@ test.write_sample_project('foo.tar.gz')
 
 test.write('config', """\
 def touch(arg):
-    open(arg, 'w')
+    with open(arg, 'w'):
+        pass
 initial_commands = [(touch, 'touch %%%%s', r'%s')]
 """ % test.workpath('INITIAL'))
 
diff --git a/testing/framework/TestCmd.py b/testing/framework/TestCmd.py
index a6a8045..24f2350 100644
--- a/testing/framework/TestCmd.py
+++ b/testing/framework/TestCmd.py
@@ -366,11 +366,9 @@ else:
     def is_String(e):
         return isinstance(e, (str, unicode, UserString))
 
-tempfile.template = 'testcmd.'
+testprefix = 'testcmd.'
 if os.name in ('posix', 'nt'):
-    tempfile.template = 'testcmd.' + str(os.getpid()) + '.'
-else:
-    tempfile.template = 'testcmd.'
+    testprefix += "%s." % str(os.getpid())
 
 re_space = re.compile('\s')
 
@@ -1662,10 +1660,11 @@ class TestCmd(object):
         """
         if path is None:
             try:
-                path = tempfile.mktemp(prefix=tempfile.template)
+                path = tempfile.mkdtemp(prefix=testprefix)
             except TypeError:
-                path = tempfile.mktemp()
-        os.mkdir(path)
+                path = tempfile.mkdtemp()
+        else:
+            os.mkdir(path)
 
         # Symlinks in the path will report things
         # differently from os.getcwd(), so chdir there
-- 
cgit v0.12