Merge remote-tracking branch 'origin/fix_3350_mslink_tempfile_join_char' into fix_3350_mslink_tempfile_join_char

7 files changed, 75 insertions, 10 deletions

diff --git a/src/engine/SCons/Defaults.py b/src/engine/SCons/Defaults.py
index e01b8a0..aa7dd2f 100644
--- a/src/engine/SCons/Defaults.py
+++ b/src/engine/SCons/Defaults.py
@@ -582,7 +582,7 @@ ConstructionEnvironment = {
     '__DSHLIBVERSIONFLAGS'   : '${__libversionflags(__env__,"DSHLIBVERSION","_DSHLIBVERSIONFLAGS")}',
 
     'TEMPFILE'      : NullCmdGenerator,
-    'TEMPFILEARGJOINBYTE': bytearray(' '),
+    'TEMPFILEARGJOIN': ' ',
     'Dir'           : Variable_Method_Caller('TARGET', 'Dir'),
     'Dirs'          : Variable_Method_Caller('TARGET', 'Dirs'),
     'File'          : Variable_Method_Caller('TARGET', 'File'),
diff --git a/src/engine/SCons/Platform/PlatformTests.py b/src/engine/SCons/Platform/PlatformTests.py
index d157c3f..ae070f8 100644
--- a/src/engine/SCons/Platform/PlatformTests.py
+++ b/src/engine/SCons/Platform/PlatformTests.py
@@ -27,12 +27,14 @@ import SCons.compat
 
 import collections
 import unittest
+import os
 
 import SCons.Errors
 import SCons.Platform
 import SCons.Environment
 import SCons.Action
 
+
 class Environment(collections.UserDict):
     def Detect(self, cmd):
         return cmd
@@ -40,6 +42,7 @@ class Environment(collections.UserDict):
     def AppendENVPath(self, key, value):
         pass
 
+
 class PlatformTestCase(unittest.TestCase):
     def test_Platform(self):
         """Test the Platform() function"""
@@ -117,6 +120,7 @@ class PlatformTestCase(unittest.TestCase):
         SCons.Platform.Platform()(env)
         assert env != {}, env
 
+
 class TempFileMungeTestCase(unittest.TestCase):
     def test_MAXLINELENGTH(self):
         """ Test different values for MAXLINELENGTH with the same
@@ -138,22 +142,57 @@ class TempFileMungeTestCase(unittest.TestCase):
         env['OVERSIMPLIFIED'] = 'command'
         expanded_cmd = env.subst(defined_cmd)
         # Call the tempfile munger
-        cmd = t(None,None,env,0)
+        cmd = t(None, None, env, 0)
         assert cmd == defined_cmd, cmd
         # Let MAXLINELENGTH equal the string's length
         env['MAXLINELENGTH'] = len(expanded_cmd)
-        cmd = t(None,None,env,0)
+        cmd = t(None, None, env, 0)
         assert cmd == defined_cmd, cmd
         # Finally, let the actual tempfile mechanism kick in
         # Disable printing of actions...
         old_actions = SCons.Action.print_actions
         SCons.Action.print_actions = 0
         env['MAXLINELENGTH'] = len(expanded_cmd)-1
-        cmd = t(None,None,env,0)
+        cmd = t(None, None, env, 0)
         # ...and restoring its setting.
         SCons.Action.print_actions = old_actions
         assert cmd != defined_cmd, cmd
 
+    def test_TEMPFILEARGJOINBYTE(self):
+        """ 
+        Test argument join byte TEMPFILEARGJOINBYTE
+        """
+
+        # Init class with cmd, such that the fully expanded
+        # string reads "a test command line".
+        # Note, how we're using a command string here that is
+        # actually longer than the substituted one. This is to ensure
+        # that the TempFileMunge class internally really takes the
+        # length of the expanded string into account.
+        defined_cmd = "a $VERY $OVERSIMPLIFIED line"
+        t = SCons.Platform.TempFileMunge(defined_cmd)
+        env = SCons.Environment.SubstitutionEnvironment(tools=[])
+        # Setting the line length high enough...
+        env['MAXLINELENGTH'] = 1024
+        env['VERY'] = 'test'
+        env['OVERSIMPLIFIED'] = 'command'
+        env['TEMPFILEARGJOINBYTE'] = os.linesep
+        expanded_cmd = env.subst(defined_cmd)
+
+        # For tempfilemunge to operate.
+        old_actions = SCons.Action.print_actions
+        SCons.Action.print_actions = 0
+        env['MAXLINELENGTH'] = len(expanded_cmd)-1
+        cmd = t(None, None, env, 0)
+        # print("CMD is:%s"%cmd)
+
+        file_content = open(cmd[-1],'rb').read()
+        # print("Content is:[%s]"%file_content)
+        # ...and restoring its setting.
+        SCons.Action.print_actions = old_actions
+        assert file_content != env['TEMPFILEARGJOINBYTE'].join(['test','command','line'])
+
+
     def test_tempfilecreation_once(self):
         # Init class with cmd, such that the fully expanded
         # string reads "a test command line".
@@ -173,9 +212,11 @@ class TempFileMungeTestCase(unittest.TestCase):
         old_actions = SCons.Action.print_actions
         SCons.Action.print_actions = 0
         # Create an instance of object derived class to allow setattrb
-        class Node(object) :
+
+        class Node(object):
             class Attrs(object):
                 pass
+
             def __init__(self):
                 self.attributes = self.Attrs()
         target = [Node()]
@@ -185,6 +226,7 @@ class TempFileMungeTestCase(unittest.TestCase):
         assert cmd != defined_cmd, cmd
         assert cmd == getattr(target[0].attributes, 'tempfile_cmdlist', None)
 
+
 class PlatformEscapeTestCase(unittest.TestCase):
     def test_posix_escape(self):
         """  Check that paths with parens are escaped properly
diff --git a/src/engine/SCons/Platform/__init__.py b/src/engine/SCons/Platform/__init__.py
index 068ab89..f864db8 100644
--- a/src/engine/SCons/Platform/__init__.py
+++ b/src/engine/SCons/Platform/__init__.py
@@ -226,9 +226,10 @@ class TempFileMunge(object):
             prefix = '@'
 
         args = list(map(SCons.Subst.quote_spaces, cmd[1:]))
-        join_char = env.subst('TEMPFILEARGJOINBYTE')
+        join_char = env.get('TEMPFILEARGJOIN',bytearray(' '))
         os.write(fd, bytearray(join_char.join(args) + "\n",'utf-8'))
         os.close(fd)
+
         # XXX Using the SCons.Action.print_actions value directly
         # like this is bogus, but expedient.  This class should
         # really be rewritten as an Action that defines the
diff --git a/src/engine/SCons/Platform/__init__.xml b/src/engine/SCons/Platform/__init__.xml
index f113278..7ea895e 100644
--- a/src/engine/SCons/Platform/__init__.xml
+++ b/src/engine/SCons/Platform/__init__.xml
@@ -257,4 +257,15 @@ The default is '.lnk'.
 </summary>
 </cvar>
 
+<cvar name="TEMPFILEARGJOIN">
+<summary>
+<para>
+The string (or character) to be used to join the arguments passed to TEMPFILE when command line exceeds the limit set by &cv-MAXLINELENGTH;.
+The default value is a space. However for MSVC, MSLINK the default is a line seperator characters as defined by os.linesep.
+Note this value is used literally and not expanded by the subst logic.
+</para>
+</summary>
+</cvar>
+
+
 </sconsdoc>
diff --git a/src/engine/SCons/Tool/mslib.py b/src/engine/SCons/Tool/mslib.py
index c901a75..354f5cf 100644
--- a/src/engine/SCons/Tool/mslib.py
+++ b/src/engine/SCons/Tool/mslib.py
@@ -33,6 +33,8 @@ selection method.
 
 __revision__ = "__FILE__ __REVISION__ __DATE__ __DEVELOPER__"
 
+import os
+
 import SCons.Defaults
 import SCons.Tool
 import SCons.Tool.msvs
@@ -54,6 +56,13 @@ def generate(env):
     env['LIBPREFIX']   = ''
     env['LIBSUFFIX']   = '.lib'
 
+    # Issue #3350
+    # Change tempfile argument joining character from a space to a newline
+    # mslink will fail if any single line is too long, but is fine with many lines
+    # in a tempfile
+    env['TEMPFILEARGJOIN'] = os.linesep
+
+
 def exists(env):
     return msvc_exists(env)
 
diff --git a/src/engine/SCons/Tool/mslink.py b/src/engine/SCons/Tool/mslink.py
index 176eb79..eae2951 100644
--- a/src/engine/SCons/Tool/mslink.py
+++ b/src/engine/SCons/Tool/mslink.py
@@ -34,6 +34,7 @@ from __future__ import print_function
 
 __revision__ = "__FILE__ __REVISION__ __DATE__ __DEVELOPER__"
 
+import os
 import os.path
 
 import SCons.Action
@@ -328,10 +329,10 @@ def generate(env):
     env['LDMODULECOM'] = compositeLdmodAction
 
     # Issue #3350
-    # Change tempfile argument joining character from a bytearray space to a newline
+    # Change tempfile argument joining character from a space to a newline
     # mslink will fail if any single line is too long, but is fine with many lines
     # in a tempfile
-    env['TEMPFILEARGJOINBYTE'] = bytearray('\n')
+    env['TEMPFILEARGJOIN'] = os.linesep
 
 def exists(env):
     return msvc_exists(env)
diff --git a/src/engine/SCons/Tool/msvc.py b/src/engine/SCons/Tool/msvc.py
index 91ffa33..dd7d0ec 100644
--- a/src/engine/SCons/Tool/msvc.py
+++ b/src/engine/SCons/Tool/msvc.py
@@ -34,6 +34,7 @@ selection method.
 __revision__ = "__FILE__ __REVISION__ __DATE__ __DEVELOPER__"
 
 import os.path
+import os
 import re
 import sys
 
@@ -284,10 +285,10 @@ def generate(env):
     msvc_set_PCHPDBFLAGS(env)
 
     # Issue #3350
-    # Change tempfile argument joining character from a bytearray space to a newline
+    # Change tempfile argument joining character from a space to a newline
     # mslink will fail if any single line is too long, but is fine with many lines
     # in a tempfile
-    env['TEMPFILEARGJOINBYTE'] = bytearray('\n')
+    env['TEMPFILEARGJOIN'] = os.linesep
 
 
     env['PCHCOM'] = '$CXX /Fo${TARGETS[1]} $CXXFLAGS $CCFLAGS $CPPFLAGS $_CPPDEFFLAGS $_CPPINCFLAGS /c $SOURCES /Yc$PCHSTOP /Fp${TARGETS[0]} $CCPDBFLAGS $PCHPDBFLAGS'