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

15 files changed, 100 insertions, 21 deletions

diff --git a/doc/generated/examples/caching_ex-random_1.xml b/doc/generated/examples/caching_ex-random_1.xml
index 0a432e9..9f59db5 100644
--- a/doc/generated/examples/caching_ex-random_1.xml
+++ b/doc/generated/examples/caching_ex-random_1.xml
@@ -1,9 +1,9 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <screen xmlns="http://www.scons.org/dbxsd/v1.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://www.scons.org/dbxsd/v1.0 http://www.scons.org/dbxsd/v1.0/scons.xsd">% <userinput>scons -Q</userinput>
-cc -o f5.o -c f5.c
-cc -o f3.o -c f3.c
 cc -o f2.o -c f2.c
 cc -o f1.o -c f1.c
+cc -o f5.o -c f5.c
 cc -o f4.o -c f4.c
+cc -o f3.o -c f3.c
 cc -o prog f1.o f2.o f3.o f4.o f5.o
 </screen>
diff --git a/doc/generated/examples/troubleshoot_Dump_1.xml b/doc/generated/examples/troubleshoot_Dump_1.xml
index 99d1399..99518c0 100644
--- a/doc/generated/examples/troubleshoot_Dump_1.xml
+++ b/doc/generated/examples/troubleshoot_Dump_1.xml
@@ -57,6 +57,7 @@ scons: Reading SConscript files ...
   'TARGET_ARCH': None,
   'TARGET_OS': None,
   'TEMPFILE': &lt;class 'SCons.Platform.TempFileMunge'&gt;,
+  'TEMPFILEARGJOIN': ' ',
   'TEMPFILEPREFIX': '@',
   'TOOLS': ['install', 'install'],
   '_CPPDEFFLAGS': '${_defines(CPPDEFPREFIX, CPPDEFINES, CPPDEFSUFFIX, __env__)}',
diff --git a/doc/generated/examples/troubleshoot_Dump_2.xml b/doc/generated/examples/troubleshoot_Dump_2.xml
index e8e0960..8a7979b 100644
--- a/doc/generated/examples/troubleshoot_Dump_2.xml
+++ b/doc/generated/examples/troubleshoot_Dump_2.xml
@@ -90,6 +90,7 @@ scons: Reading SConscript files ...
   'TARGET_ARCH': None,
   'TARGET_OS': None,
   'TEMPFILE': &lt;class 'SCons.Platform.TempFileMunge'&gt;,
+  'TEMPFILEARGJOIN': '\n',
   'TEMPFILEPREFIX': '@',
   'TOOLS': ['msvc', 'install', 'install'],
   '_CCCOMCOM': '$CPPFLAGS $_CPPDEFFLAGS $_CPPINCFLAGS $CCPCHFLAGS $CCPDBFLAGS',
diff --git a/doc/generated/examples/troubleshoot_explain1_3.xml b/doc/generated/examples/troubleshoot_explain1_3.xml
index ebc13f8..a461f60 100644
--- a/doc/generated/examples/troubleshoot_explain1_3.xml
+++ b/doc/generated/examples/troubleshoot_explain1_3.xml
@@ -3,5 +3,5 @@
 cp file.in file.oout
 
 scons: warning: Cannot find target file.out after building
-File "/home/bdeegan/devel/scons/git/as_scons/src/script/scons.py", line 204, in &lt;module&gt;
+File "/home/bdeegan/devel/scons/git/scons/bootstrap/src/script/scons.py", line 204, in &lt;module&gt;
 </screen>
diff --git a/doc/generated/tools.gen b/doc/generated/tools.gen
index be717e3..ecd9c98 100644
--- a/doc/generated/tools.gen
+++ b/doc/generated/tools.gen
@@ -779,19 +779,19 @@ Sets construction variables for the
 </para>
 <para>Sets:  &cv-link-AS;, &cv-link-ASCOM;, &cv-link-ASFLAGS;, &cv-link-ASPPCOM;, &cv-link-ASPPFLAGS;.</para><para>Uses:  &cv-link-ASCOMSTR;, &cv-link-ASPPCOMSTR;.</para></listitem>
   </varlistentry>
-  <varlistentry id="t-packaging">
-    <term>packaging</term>
+  <varlistentry id="t-Packaging">
+    <term>Packaging</term>
     <listitem>
 <para xmlns="http://www.scons.org/dbxsd/v1.0">
-A framework for building binary and source packages.
+Sets construction variables for the <function xmlns="http://www.scons.org/dbxsd/v1.0">Package</function> Builder.
 </para>
 </listitem>
   </varlistentry>
-  <varlistentry id="t-Packaging">
-    <term>Packaging</term>
+  <varlistentry id="t-packaging">
+    <term>packaging</term>
     <listitem>
 <para xmlns="http://www.scons.org/dbxsd/v1.0">
-Sets construction variables for the <function xmlns="http://www.scons.org/dbxsd/v1.0">Package</function> Builder.
+A framework for building binary and source packages.
 </para>
 </listitem>
   </varlistentry>
diff --git a/doc/generated/tools.mod b/doc/generated/tools.mod
index f9bc1d7..1209d74 100644
--- a/doc/generated/tools.mod
+++ b/doc/generated/tools.mod
@@ -78,8 +78,8 @@ THIS IS AN AUTOMATICALLY-GENERATED FILE.  DO NOT EDIT.
 <!ENTITY t-mwcc "<literal xmlns='http://www.scons.org/dbxsd/v1.0'>mwcc</literal>">
 <!ENTITY t-mwld "<literal xmlns='http://www.scons.org/dbxsd/v1.0'>mwld</literal>">
 <!ENTITY t-nasm "<literal xmlns='http://www.scons.org/dbxsd/v1.0'>nasm</literal>">
-<!ENTITY t-packaging "<literal xmlns='http://www.scons.org/dbxsd/v1.0'>packaging</literal>">
 <!ENTITY t-Packaging "<literal xmlns='http://www.scons.org/dbxsd/v1.0'>Packaging</literal>">
+<!ENTITY t-packaging "<literal xmlns='http://www.scons.org/dbxsd/v1.0'>packaging</literal>">
 <!ENTITY t-pdf "<literal xmlns='http://www.scons.org/dbxsd/v1.0'>pdf</literal>">
 <!ENTITY t-pdflatex "<literal xmlns='http://www.scons.org/dbxsd/v1.0'>pdflatex</literal>">
 <!ENTITY t-pdftex "<literal xmlns='http://www.scons.org/dbxsd/v1.0'>pdftex</literal>">
@@ -186,8 +186,8 @@ THIS IS AN AUTOMATICALLY-GENERATED FILE.  DO NOT EDIT.
 <!ENTITY t-link-mwcc "<link linkend='t-mwcc' xmlns='http://www.scons.org/dbxsd/v1.0'><literal>mwcc</literal></link>">
 <!ENTITY t-link-mwld "<link linkend='t-mwld' xmlns='http://www.scons.org/dbxsd/v1.0'><literal>mwld</literal></link>">
 <!ENTITY t-link-nasm "<link linkend='t-nasm' xmlns='http://www.scons.org/dbxsd/v1.0'><literal>nasm</literal></link>">
-<!ENTITY t-link-packaging "<link linkend='t-packaging' xmlns='http://www.scons.org/dbxsd/v1.0'><literal>packaging</literal></link>">
 <!ENTITY t-link-Packaging "<link linkend='t-Packaging' xmlns='http://www.scons.org/dbxsd/v1.0'><literal>Packaging</literal></link>">
+<!ENTITY t-link-packaging "<link linkend='t-packaging' xmlns='http://www.scons.org/dbxsd/v1.0'><literal>packaging</literal></link>">
 <!ENTITY t-link-pdf "<link linkend='t-pdf' xmlns='http://www.scons.org/dbxsd/v1.0'><literal>pdf</literal></link>">
 <!ENTITY t-link-pdflatex "<link linkend='t-pdflatex' xmlns='http://www.scons.org/dbxsd/v1.0'><literal>pdflatex</literal></link>">
 <!ENTITY t-link-pdftex "<link linkend='t-pdftex' xmlns='http://www.scons.org/dbxsd/v1.0'><literal>pdftex</literal></link>">
diff --git a/doc/generated/variables.gen b/doc/generated/variables.gen
index c7048b4..1c2d2f5 100644
--- a/doc/generated/variables.gen
+++ b/doc/generated/variables.gen
@@ -7343,6 +7343,16 @@ The suffix used for tar file names.
 </para>
 </listitem>
   </varlistentry>
+  <varlistentry id="cv-TEMPFILEARGJOIN">
+    <term>TEMPFILEARGJOIN</term>
+    <listitem>
+<para xmlns="http://www.scons.org/dbxsd/v1.0">
+The string (or character) to be used to join the arguments passed to TEMPFILE when command line exceeds the limit set by <envar xmlns="http://www.scons.org/dbxsd/v1.0">$MAXLINELENGTH</envar>.
+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>
+</listitem>
+  </varlistentry>
   <varlistentry id="cv-TEMPFILEPREFIX">
     <term>TEMPFILEPREFIX</term>
     <listitem>
diff --git a/doc/generated/variables.mod b/doc/generated/variables.mod
index 28c08dc..47576f4 100644
--- a/doc/generated/variables.mod
+++ b/doc/generated/variables.mod
@@ -544,6 +544,7 @@ THIS IS AN AUTOMATICALLY-GENERATED FILE.  DO NOT EDIT.
 <!ENTITY cv-TARGET_OS "<envar xmlns='http://www.scons.org/dbxsd/v1.0'>$TARGET_OS</envar>">
 <!ENTITY cv-TARGETS "<envar xmlns='http://www.scons.org/dbxsd/v1.0'>$TARGETS</envar>">
 <!ENTITY cv-TARSUFFIX "<envar xmlns='http://www.scons.org/dbxsd/v1.0'>$TARSUFFIX</envar>">
+<!ENTITY cv-TEMPFILEARGJOIN "<envar xmlns='http://www.scons.org/dbxsd/v1.0'>$TEMPFILEARGJOIN</envar>">
 <!ENTITY cv-TEMPFILEPREFIX "<envar xmlns='http://www.scons.org/dbxsd/v1.0'>$TEMPFILEPREFIX</envar>">
 <!ENTITY cv-TEMPFILESUFFIX "<envar xmlns='http://www.scons.org/dbxsd/v1.0'>$TEMPFILESUFFIX</envar>">
 <!ENTITY cv-TEX "<envar xmlns='http://www.scons.org/dbxsd/v1.0'>$TEX</envar>">
@@ -1183,6 +1184,7 @@ THIS IS AN AUTOMATICALLY-GENERATED FILE.  DO NOT EDIT.
 <!ENTITY cv-link-TARGET_OS "<link linkend='cv-TARGET_OS' xmlns='http://www.scons.org/dbxsd/v1.0'><envar>$TARGET_OS</envar></link>">
 <!ENTITY cv-link-TARGETS "<link linkend='cv-TARGETS' xmlns='http://www.scons.org/dbxsd/v1.0'><envar>$TARGETS</envar></link>">
 <!ENTITY cv-link-TARSUFFIX "<link linkend='cv-TARSUFFIX' xmlns='http://www.scons.org/dbxsd/v1.0'><envar>$TARSUFFIX</envar></link>">
+<!ENTITY cv-link-TEMPFILEARGJOIN "<link linkend='cv-TEMPFILEARGJOIN' xmlns='http://www.scons.org/dbxsd/v1.0'><envar>$TEMPFILEARGJOIN</envar></link>">
 <!ENTITY cv-link-TEMPFILEPREFIX "<link linkend='cv-TEMPFILEPREFIX' xmlns='http://www.scons.org/dbxsd/v1.0'><envar>$TEMPFILEPREFIX</envar></link>">
 <!ENTITY cv-link-TEMPFILESUFFIX "<link linkend='cv-TEMPFILESUFFIX' xmlns='http://www.scons.org/dbxsd/v1.0'><envar>$TEMPFILESUFFIX</envar></link>">
 <!ENTITY cv-link-TEX "<link linkend='cv-TEX' xmlns='http://www.scons.org/dbxsd/v1.0'><envar>$TEX</envar></link>">
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'