From 78a8b1cdcfa5d0fb290cf4569e5b18a4d04c73cf Mon Sep 17 00:00:00 2001 From: Dirk Baechle Date: Thu, 21 Aug 2014 23:18:20 +0200 Subject: - fix for issue #2970, false line length calculation in TempFileMunge class --- src/CHANGES.txt | 3 ++ src/engine/SCons/Platform/PlatformTests.py | 50 +++++++++++++++++++++++++++++- src/engine/SCons/Platform/__init__.py | 3 +- 3 files changed, 54 insertions(+), 2 deletions(-) diff --git a/src/CHANGES.txt b/src/CHANGES.txt index cf4264a..0011463 100644 --- a/src/CHANGES.txt +++ b/src/CHANGES.txt @@ -6,6 +6,9 @@ RELEASE 2.3.2.alpha.yyyymmdd - NEW DATE WILL BE INSERTED HERE + From Roland Stark: + - Fixed false line length calculation in the TempFileMunge class (#2970). + From Anatoly Techtonik: - Do not fail on EnsureSConsVersion when running from checkout diff --git a/src/engine/SCons/Platform/PlatformTests.py b/src/engine/SCons/Platform/PlatformTests.py index 515382a..a507e4e 100644 --- a/src/engine/SCons/Platform/PlatformTests.py +++ b/src/engine/SCons/Platform/PlatformTests.py @@ -33,10 +33,13 @@ import TestUnit import SCons.Errors import SCons.Platform +import SCons.Environment +import SCons.Action class Environment(collections.UserDict): def Detect(self, cmd): return cmd + def AppendENVPath(self, key, value): pass @@ -117,9 +120,54 @@ class PlatformTestCase(unittest.TestCase): SCons.Platform.Platform()(env) assert env != {}, env +class TempFileMungeTestCase(unittest.TestCase): + def test_TempFileMunge(self): + """Test the TempFileMunge() class, more specifically the + MAXLINELENGTH setting. + We try setting different maximum line lengths for a + fixed command string and ensure that the tempfile mechanism + kicks in at MAXLINELENGTH+1. + """ + # 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' + expanded_cmd = env.subst(defined_cmd) + # Call the tempfile munger + 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) + 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) + # ...and restoring its setting. + SCons.Action.print_actions = old_actions + assert cmd != defined_cmd, cmd if __name__ == "__main__": - suite = unittest.makeSuite(PlatformTestCase, 'test_') + suite = unittest.TestSuite() + + tclasses = [ PlatformTestCase, + TempFileMungeTestCase ] + for tclass in tclasses: + names = unittest.getTestCaseNames(tclass, 'test_') + suite.addTests(list(map(tclass, names))) + TestUnit.run(suite) # Local Variables: diff --git a/src/engine/SCons/Platform/__init__.py b/src/engine/SCons/Platform/__init__.py index 81a49e7..a9c8076 100644 --- a/src/engine/SCons/Platform/__init__.py +++ b/src/engine/SCons/Platform/__init__.py @@ -173,6 +173,7 @@ class TempFileMunge(object): length = 0 for c in cmd: length += len(c) + length += len(cmd) - 1 if length <= maxline: return self.cmd @@ -187,7 +188,7 @@ class TempFileMunge(object): (fd, tmp) = tempfile.mkstemp('.lnk', text=True) native_tmp = SCons.Util.get_native_path(os.path.normpath(tmp)) - if env['SHELL'] and env['SHELL'] == 'sh': + if env.get('SHELL',None) and env['SHELL'] == 'sh': # The sh shell will try to escape the backslashes in the # path, so unescape them. native_tmp = native_tmp.replace('\\', r'\\\\') -- cgit v0.12 From 1930f1b34740e1b12220c46483d3fb5f9e125aef Mon Sep 17 00:00:00 2001 From: Dirk Baechle Date: Sun, 24 Aug 2014 12:04:48 +0200 Subject: - updated doc string and simplified a comparison, based on PR review comments --- src/engine/SCons/Platform/PlatformTests.py | 11 ++++------- src/engine/SCons/Platform/__init__.py | 2 +- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/engine/SCons/Platform/PlatformTests.py b/src/engine/SCons/Platform/PlatformTests.py index a507e4e..ca3080e 100644 --- a/src/engine/SCons/Platform/PlatformTests.py +++ b/src/engine/SCons/Platform/PlatformTests.py @@ -26,7 +26,6 @@ __revision__ = "__FILE__ __REVISION__ __DATE__ __DEVELOPER__" import SCons.compat import collections -import sys import unittest import TestUnit @@ -121,12 +120,10 @@ class PlatformTestCase(unittest.TestCase): assert env != {}, env class TempFileMungeTestCase(unittest.TestCase): - def test_TempFileMunge(self): - """Test the TempFileMunge() class, more specifically the - MAXLINELENGTH setting. - We try setting different maximum line lengths for a - fixed command string and ensure that the tempfile mechanism - kicks in at MAXLINELENGTH+1. + def test_MAXLINELENGTH(self): + """ Test different values for MAXLINELENGTH with the same + size command string to ensure that the temp file mechanism + kicks in only at MAXLINELENGTH+1, or higher """ # Init class with cmd, such that the fully expanded # string reads "a test command line". diff --git a/src/engine/SCons/Platform/__init__.py b/src/engine/SCons/Platform/__init__.py index a9c8076..fa6df61 100644 --- a/src/engine/SCons/Platform/__init__.py +++ b/src/engine/SCons/Platform/__init__.py @@ -188,7 +188,7 @@ class TempFileMunge(object): (fd, tmp) = tempfile.mkstemp('.lnk', text=True) native_tmp = SCons.Util.get_native_path(os.path.normpath(tmp)) - if env.get('SHELL',None) and env['SHELL'] == 'sh': + if env.get('SHELL',None) == 'sh': # The sh shell will try to escape the backslashes in the # path, so unescape them. native_tmp = native_tmp.replace('\\', r'\\\\') -- cgit v0.12