fix py2/3. Also fix py2 win32 broken tests

3 files changed, 89 insertions, 60 deletions

diff --git a/src/engine/SCons/Tool/textfile.py b/src/engine/SCons/Tool/textfile.py
index 5ea2344..0e4d943 100644
--- a/src/engine/SCons/Tool/textfile.py
+++ b/src/engine/SCons/Tool/textfile.py
@@ -53,7 +53,9 @@ import re
 
 from SCons.Node import Node
 from SCons.Node.Python import Value
-from SCons.Util import is_String, is_Sequence, is_Dict
+from SCons.Util import is_String, is_Sequence, is_Dict, to_bytes
+
+TEXTFILE_FILE_WRITE_MODE = 'wb'
 
 def _do_subst(node, subs):
     """
@@ -64,12 +66,19 @@ def _do_subst(node, subs):
     1.2345 and so forth.
     """
     contents = node.get_text_contents()
-    if not subs: return contents
-    for (k,v) in subs:
-        contents = re.sub(k, v, contents)
+    if subs:
+        for (k, v) in subs:
+            contents = re.sub(k, v, contents)
+
+    if 'b' in TEXTFILE_FILE_WRITE_MODE:
+        contents = bytearray(contents, 'utf-8')
+
     return contents
 
+
 def _action(target, source, env):
+    # import pdb; pdb.set_trace()
+
     # prepare the line separator
     linesep = env['LINESEPARATOR']
     if linesep is None:
@@ -79,9 +88,11 @@ def _action(target, source, env):
     elif isinstance(linesep, Value):
         linesep = linesep.get_text_contents()
     else:
-        raise SCons.Errors.UserError(
-                           'unexpected type/class for LINESEPARATOR: %s'
-                                         % repr(linesep), None)
+        raise SCons.Errors.UserError('unexpected type/class for LINESEPARATOR: %s'
+                                     % repr(linesep), None)
+
+    if 'b' in TEXTFILE_FILE_WRITE_MODE:
+        linesep = to_bytes(linesep)
 
     # create a dictionary to use for the substitutions
     if 'SUBST_DICT' not in env:
@@ -95,31 +106,36 @@ def _action(target, source, env):
         else:
             raise SCons.Errors.UserError('SUBST_DICT must be dict or sequence')
         subs = []
-        for (k,v) in d:
+        for (k, v) in d:
             if callable(v):
                 v = v()
             if is_String(v):
                 v = env.subst(v)
             else:
                 v = str(v)
-            subs.append((k,v))
+            subs.append((k, v))
 
     # write the file
     try:
-        fd = open(target[0].get_path(), "w")
-    except (OSError,IOError) as e:
+        fd = open(target[0].get_path(), TEXTFILE_FILE_WRITE_MODE)
+    except (OSError, IOError) as e:
         raise SCons.Errors.UserError("Can't write target file %s" % target[0])
+
     # separate lines by 'linesep' only if linesep is not empty
     lsep = None
     for s in source:
-        if lsep: fd.write(lsep)
+        if lsep:
+            fd.write(lsep)
+
         fd.write(_do_subst(s, subs))
         lsep = linesep
     fd.close()
 
+
 def _strfunc(target, source, env):
     return "Creating '%s'" % target[0]
 
+
 def _convert_list_R(newlist, sources):
     for elem in sources:
         if is_Sequence(elem):
@@ -128,6 +144,8 @@ def _convert_list_R(newlist, sources):
             newlist.append(elem)
         else:
             newlist.append(Value(elem))
+
+
 def _convert_list(target, source, env):
     if len(target) != 1:
         raise SCons.Errors.UserError("Only one target file allowed")
@@ -135,26 +153,28 @@ def _convert_list(target, source, env):
     _convert_list_R(newlist, source)
     return target, newlist
 
+
 _common_varlist = ['SUBST_DICT', 'LINESEPARATOR']
 
 _text_varlist = _common_varlist + ['TEXTFILEPREFIX', 'TEXTFILESUFFIX']
 _text_builder = SCons.Builder.Builder(
-    action = SCons.Action.Action(_action, _strfunc, varlist = _text_varlist),
-    source_factory = Value,
-    emitter = _convert_list,
-    prefix = '$TEXTFILEPREFIX',
-    suffix = '$TEXTFILESUFFIX',
-    )
+    action=SCons.Action.Action(_action, _strfunc, varlist=_text_varlist),
+    source_factory=Value,
+    emitter=_convert_list,
+    prefix='$TEXTFILEPREFIX',
+    suffix='$TEXTFILESUFFIX',
+)
 
 _subst_varlist = _common_varlist + ['SUBSTFILEPREFIX', 'TEXTFILESUFFIX']
 _subst_builder = SCons.Builder.Builder(
-    action = SCons.Action.Action(_action, _strfunc, varlist = _subst_varlist),
-    source_factory = SCons.Node.FS.File,
-    emitter = _convert_list,
-    prefix = '$SUBSTFILEPREFIX',
-    suffix = '$SUBSTFILESUFFIX',
-    src_suffix = ['.in'],
-    )
+    action=SCons.Action.Action(_action, _strfunc, varlist=_subst_varlist),
+    source_factory=SCons.Node.FS.File,
+    emitter=_convert_list,
+    prefix='$SUBSTFILEPREFIX',
+    suffix='$SUBSTFILESUFFIX',
+    src_suffix=['.in'],
+)
+
 
 def generate(env):
     env['LINESEPARATOR'] = os.linesep
@@ -165,6 +185,7 @@ def generate(env):
     env['SUBSTFILEPREFIX'] = ''
     env['SUBSTFILESUFFIX'] = ''
 
+
 def exists(env):
     return 1
 
diff --git a/test/redirection.py b/test/redirection.py
index cc8ebf9..ffc76b5 100644
--- a/test/redirection.py
+++ b/test/redirection.py
@@ -33,15 +33,10 @@ test = TestSCons.TestSCons()
 test.write('cat.py', r"""
 import sys
 try:
-    input = open(sys.argv[1], 'rb').read()
-    try:
-        sys.stdout.buffer.write(input)
-    except AttributeError:
-        sys.stdout.write(input)
+    input = open(sys.argv[1], 'r').read()
 except IndexError:
     input = sys.stdin.read()
-    sys.stdout.write(input)
-
+sys.stdout.write(input)
 sys.exit(0)
 """)
 
@@ -57,10 +52,10 @@ env.Command(target='foo4', source='bar4',
             action=r'%(_python_)s cat.py <$SOURCES |%(_python_)s cat.py >$TARGET')
 """ % locals())
 
-test.write('bar1', 'bar1\r\n', mode='w')
-test.write('bar2', 'bar2\r\n', mode='w')
-test.write('bar3', 'bar3\r\n', mode='w')
-test.write('bar4', 'bar4\r\n', mode='w')
+test.write('bar1', 'bar1\r\n')
+test.write('bar2', 'bar2\r\n')
+test.write('bar3', 'bar3\r\n')
+test.write('bar4', 'bar4\r\n')
 
 test.run(arguments='.')
 
diff --git a/test/textfile.py b/test/textfile.py
index f82f50a..91e95e9 100644
--- a/test/textfile.py
+++ b/test/textfile.py
@@ -30,11 +30,13 @@ import os
 
 test = TestSCons.TestSCons()
 
-foo1  = test.workpath('foo1.txt')
+foo1 = test.workpath('foo1.txt')
 #foo2  = test.workpath('foo2.txt')
 #foo1a = test.workpath('foo1a.txt')
 #foo2a = test.workpath('foo2a.txt')
 
+match_mode = 'rb'
+
 test.write('SConstruct', """
 env = Environment(tools=['textfile'])
 data0 = ['Goethe', 'Schiller']
@@ -55,43 +57,48 @@ env.Substfile('bar2', data, LINESEPARATOR='|*')
 data.append(Value(''))
 env.Substfile('bar1a.txt', data)
 env.Substfile('bar2a.txt', data, LINESEPARATOR='|*')
-""")
+""", mode='w')
 
-test.run(arguments = '.')
+test.run(arguments='.')
 
 textparts = ['lalala', '42',
              'Goethe', 'Schiller',
              'tanteratei']
-foo1Text  = os.linesep.join(textparts)
-foo2Text  = '|*'.join(textparts)
+foo1Text = os.linesep.join(textparts)
+foo2Text = '|*'.join(textparts)
 foo1aText = foo1Text + os.linesep
 foo2aText = foo2Text + '|*'
 
-test.up_to_date(arguments = '.')
+test.up_to_date(arguments='.')
 
 files = list(map(test.workpath, (
-            'foo1.txt', 'foo2.txt', 'foo1a.txt', 'foo2a.txt',
-            'bar1',     'bar2',     'bar1a.txt', 'bar2a.txt',
-        )))
+    'foo1.txt', 'foo2.txt', 'foo1a.txt', 'foo2a.txt',
+    'bar1',     'bar2',     'bar1a.txt', 'bar2a.txt',
+)))
+
+
 def check_times():
-    # make sure the files didn't get rewritten, because nothing changed:
+    """
+    make sure the files didn't get rewritten, because nothing changed:
+    """
     before = list(map(os.path.getmtime, files))
     # introduce a small delay, to make the test valid
     test.sleep()
     # should still be up-to-date
-    test.up_to_date(arguments = '.')
+    test.up_to_date(arguments='.')
     after = list(map(os.path.getmtime, files))
     test.fail_test(before != after)
 
+
 # make sure that the file content is as expected
-test.must_match('foo1.txt',  foo1Text)
-test.must_match('bar1',      foo1Text)
-test.must_match('foo2.txt',  foo2Text)
-test.must_match('bar2',      foo2Text)
-test.must_match('foo1a.txt', foo1aText)
-test.must_match('bar1a.txt', foo1aText)
-test.must_match('foo2a.txt', foo2aText)
-test.must_match('bar2a.txt', foo2aText)
+test.must_match('foo1.txt',  foo1Text, mode=match_mode)
+test.must_match('bar1',      foo1Text, mode=match_mode)
+test.must_match('foo2.txt',  foo2Text, mode=match_mode)
+test.must_match('bar2',      foo2Text, mode=match_mode)
+test.must_match('foo1a.txt', foo1aText, mode=match_mode)
+test.must_match('bar1a.txt', foo1aText, mode=match_mode)
+test.must_match('foo2a.txt', foo2aText, mode=match_mode)
+test.must_match('bar2a.txt', foo2aText, mode=match_mode)
 check_times()
 
 # write the contents and make sure the files
@@ -132,17 +139,23 @@ s = env.Substfile('sub5', s, SUBST_DICT = sub1)
 s = env.Substfile('sub6', t, SUBST_DICT = sub3)
 """, mode='w')
 
-test.run(arguments = '.')
+test.run(arguments='.')
 
-line1  = 'This line has no substitutions'
+line1 = 'This line has no substitutions'
 line2a = 'This line has @subst@ substitutions'
 line2b = 'This line has most substitutions'
 line3a = 'This line has %subst% substitutions'
 line3b = 'This line has many substitutions'
 
-def matchem(file, lines):
+
+def matchem(match_file, lines):
+    """
+    Join all the lines with correct line separator,
+    then compare
+    """
     lines = os.linesep.join(lines)
-    test.must_match(file, lines, mode='r')
+    test.must_match(match_file, lines, mode=match_mode)
+
 
 matchem('text.txt', [line1, line2a, line3a])
 matchem('sub1', [line1, line2a, line3a])
@@ -152,6 +165,6 @@ matchem('sub4', [line1, line2a, line3b])
 matchem('sub5', [line1, line2b, line3b])
 matchem('sub6', [line1, line2b, line3b])
 
-test.up_to_date(arguments = '.')
+test.up_to_date(arguments='.')
 
 test.pass_test()