From b04db00ba0818a1f22c08491b50fc7977cbe8bbd Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Tue, 15 Jan 2019 17:28:02 -0700 Subject: [WIP] customizable tempfile extension (issue #2431) Apply the patch (adjusted) from issue #2341: instead of hardcoding the filename extenstion for the tempfile to help with linking on Windows targets, allow some variability. Current marked WIP because there are some other comments in the issue tracker that can maybe be flushed out by submitting this PR, and there are no tests (should presumably go in test/TEMPFILEPREFIX.py, or a new test TEMPFILEEXTENSION.py) Signed-off-by: Mats Wichmann --- src/CHANGES.txt | 2 ++ src/engine/SCons/Platform/__init__.py | 35 +++++++++++++++++++++++------------ 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/src/CHANGES.txt b/src/CHANGES.txt index 2c0cef6..8059f5d 100644 --- a/src/CHANGES.txt +++ b/src/CHANGES.txt @@ -14,6 +14,8 @@ RELEASE VERSION/DATE TO BE FILLED IN LATER - Improve finding of Microsoft compiler: add a 'products' wildcard in case 2017 Build Tools only is installed as it is considered a separate product from the default Visual Studio + - Add TEMPFILEEXTENSION as in the patch attached to issue #2431, + updated to current codebase. From Daniel Moody: - Improved support for VC14.1 and Visual Studio 2017, as well as arm and arm64 targets. diff --git a/src/engine/SCons/Platform/__init__.py b/src/engine/SCons/Platform/__init__.py index 8117e60..75c4839 100644 --- a/src/engine/SCons/Platform/__init__.py +++ b/src/engine/SCons/Platform/__init__.py @@ -144,15 +144,20 @@ class TempFileMunge(object): line limitation. Example usage: - env["TEMPFILE"] = TempFileMunge - env["LINKCOM"] = "${TEMPFILE('$LINK $TARGET $SOURCES','$LINKCOMSTR')}" + env["TEMPFILE"] = TempFileMunge + env["LINKCOM"] = "${TEMPFILE('$LINK $TARGET $SOURCES','$LINKCOMSTR')}" By default, the name of the temporary file used begins with a - prefix of '@'. This may be configred for other tool chains by - setting '$TEMPFILEPREFIX'. - - env["TEMPFILEPREFIX"] = '-@' # diab compiler - env["TEMPFILEPREFIX"] = '-via' # arm tool chain + prefix of '@'. This may be configured for other tool chains by + setting '$TEMPFILEPREFIX': + env["TEMPFILEPREFIX"] = '-@' # diab compiler + env["TEMPFILEPREFIX"] = '-via' # arm tool chain + env["TEMPFILEPREFIX"] = '' # (the empty string) PC Lint + + You can configure the extension of the temporary file through the + TEMPFILEEXTENSION variable, which defaults to '.lnk' (see comments + in the code below): + env["TEMPFILEEXTENSION"] = '.lnt' # PC Lint """ def __init__(self, cmd, cmdstr = None): self.cmd = cmd @@ -194,16 +199,21 @@ class TempFileMunge(object): # We do a normpath because mktemp() has what appears to be # a bug in Windows that will use a forward slash as a path - # delimiter. Windows's link mistakes that for a command line + # delimiter. Windows' link mistakes that for a command line # switch and barfs. # # We use the .lnk suffix for the benefit of the Phar Lap # linkloc linker, which likes to append an .lnk suffix if # none is given. - (fd, tmp) = tempfile.mkstemp('.lnk', text=True) + if env.has_key('TEMPFILEEXTENSION'): + extension = env.subst('$TEMPFILEEXTENSION') + else: + extension = '.lnk' # TODO: better way to pick default? + (fd, tmp) = tempfile.mkstemp(extension, text=True) + native_tmp = SCons.Util.get_native_path(os.path.normpath(tmp)) - if env.get('SHELL',None) == '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'\\\\') @@ -216,8 +226,9 @@ class TempFileMunge(object): # Windows path names. rm = 'del' - prefix = env.subst('$TEMPFILEPREFIX') - if not prefix: + if env.has_key('TEMPFILEPREFIX'): + prefix = env.subst('$TEMPFILEPREFIX') + else: prefix = '@' args = list(map(SCons.Subst.quote_spaces, cmd[1:])) -- cgit v0.12 From 8c669448e9ef3bcd7c3c16a81beb56770ae7b445 Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Wed, 16 Jan 2019 13:04:23 -0700 Subject: Change TEMPFILEEXTENSION to TEMPFILESUFFIX All the other file extension variables end in SUFFIX, so change this new one to match for consistency. Added doc entry. Signed-off-by: Mats Wichmann --- src/CHANGES.txt | 4 ++-- src/engine/SCons/Platform/__init__.py | 8 ++++---- src/engine/SCons/Platform/__init__.xml | 24 ++++++++++++++++++++---- 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/src/CHANGES.txt b/src/CHANGES.txt index 8059f5d..450052c 100644 --- a/src/CHANGES.txt +++ b/src/CHANGES.txt @@ -14,8 +14,8 @@ RELEASE VERSION/DATE TO BE FILLED IN LATER - Improve finding of Microsoft compiler: add a 'products' wildcard in case 2017 Build Tools only is installed as it is considered a separate product from the default Visual Studio - - Add TEMPFILEEXTENSION as in the patch attached to issue #2431, - updated to current codebase. + - Add TEMPFILESUFFIX to allow a customizable filename extension, as + described in the patch attached to issue #2431. From Daniel Moody: - Improved support for VC14.1 and Visual Studio 2017, as well as arm and arm64 targets. diff --git a/src/engine/SCons/Platform/__init__.py b/src/engine/SCons/Platform/__init__.py index 75c4839..db5b947 100644 --- a/src/engine/SCons/Platform/__init__.py +++ b/src/engine/SCons/Platform/__init__.py @@ -205,11 +205,11 @@ class TempFileMunge(object): # We use the .lnk suffix for the benefit of the Phar Lap # linkloc linker, which likes to append an .lnk suffix if # none is given. - if env.has_key('TEMPFILEEXTENSION'): - extension = env.subst('$TEMPFILEEXTENSION') + if env.has_key('TEMPFILESUFFIX'): + suffix = env.subst('$TEMPFILESUFFIX') else: - extension = '.lnk' # TODO: better way to pick default? - (fd, tmp) = tempfile.mkstemp(extension, text=True) + suffix = '.lnk' # TODO: better way to pick default? + fd, tmp = tempfile.mkstemp(suffix, text=True) native_tmp = SCons.Util.get_native_path(os.path.normpath(tmp)) diff --git a/src/engine/SCons/Platform/__init__.xml b/src/engine/SCons/Platform/__init__.xml index 0802369..f113278 100644 --- a/src/engine/SCons/Platform/__init__.xml +++ b/src/engine/SCons/Platform/__init__.xml @@ -232,13 +232,29 @@ The suffix used for shared object file names. The prefix for a temporary file used -to execute lines longer than $MAXLINELENGTH. -The default is '@'. -This may be set for toolchains that use other values, -such as '-@' for the diab compiler +to store lines lines longer than $MAXLINELENGTH +as operations which call out to a shell will fail +if the line is too long, which particularly +impacts linking. +The default is '@', which works for the Microsoft +and GNU toolchains on Windows. +Set this appropriately for other toolchains, +for example '-@' for the diab compiler or '-via' for ARM toolchain. + + + +The suffix used for the temporary file name +used for long command lines. The name should +include the dot ('.') if one is wanted as +it will not be added automatically. +The default is '.lnk'. + + + + -- cgit v0.12 From cf33e092a699cb72031105a1b23ae09c962beffc Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Wed, 16 Jan 2019 14:53:07 -0700 Subject: Add test for TEMPFILESUFFIX Signed-off-by: Mats Wichmann --- test/TEMPFILEPREFIX.py | 7 +-- test/TEMPFILESUFFIX.py | 126 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 130 insertions(+), 3 deletions(-) create mode 100644 test/TEMPFILESUFFIX.py diff --git a/test/TEMPFILEPREFIX.py b/test/TEMPFILEPREFIX.py index 7f4322b..f5093e1 100644 --- a/test/TEMPFILEPREFIX.py +++ b/test/TEMPFILEPREFIX.py @@ -25,8 +25,9 @@ __revision__ = "__FILE__ __REVISION__ __DATE__ __DEVELOPER__" """ -Verify that setting the $TEMPFILEPREFIX variable will append to the -beginning of the TEMPFILE invocation of a long command line. +Verify that setting the $TEMPFILEPREFIX variable will cause +it to appear at the front of name of the generated tempfile +used for long command lines. """ import os @@ -97,7 +98,7 @@ class TestTempFileMunge(TempFileMunge): def _print_cmd_str(self, target, source, env, cmdstr): super(TestTempFileMunge, self)._print_cmd_str(target, source, None, cmdstr) - + env = Environment( TEMPFILE = TestTempFileMunge, BUILDCOM = '${TEMPFILE("xxx.py $TARGET $SOURCES")}', diff --git a/test/TEMPFILESUFFIX.py b/test/TEMPFILESUFFIX.py new file mode 100644 index 0000000..aee1e2d --- /dev/null +++ b/test/TEMPFILESUFFIX.py @@ -0,0 +1,126 @@ +#!/usr/bin/env python +# +# __COPYRIGHT__ +# +# Permission is hereby granted, free of charge, to any person obtaining +# a copy of this software and associated documentation files (the +# "Software"), to deal in the Software without restriction, including +# without limitation the rights to use, copy, modify, merge, publish, +# distribute, sublicense, and/or sell copies of the Software, and to +# permit persons to whom the Software is furnished to do so, subject to +# the following conditions: +# +# The above copyright notice and this permission notice shall be included +# in all copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY +# KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE +# WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND +# NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE +# LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION +# OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION +# WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. +# + +__revision__ = "__FILE__ __REVISION__ __DATE__ __DEVELOPER__" + +""" +Verify that setting the $TEMPFILESUFFIX variable will cause +it to appear at the end of name of the generated tempfile +used for long command lines. +""" + +import os +import stat + +import TestSCons + +test = TestSCons.TestSCons(match=TestSCons.match_re) + +test.write('echo.py', """\ +from __future__ import print_function +import sys +print(sys.argv) +""") + +echo_py = test.workpath('echo.py') + +st = os.stat(echo_py) +os.chmod(echo_py, st[stat.ST_MODE] | 0o111) + +test.write('SConstruct', """ +import os +env = Environment( + BUILDCOM = '${TEMPFILE("xxx.py $TARGET $SOURCES")}', + MAXLINELENGTH = 16, + TEMPFILESUFFIX = '.foo', +) +env.AppendENVPath('PATH', os.curdir) +env.Command('foo.out', 'foo.in', '$BUILDCOM') +""") + +test.write('foo.in', "foo.in\n") + +test.run(arguments = '-n -Q .', + stdout = """\ +Using tempfile \\S+ for command line: +xxx.py foo.out foo.in +xxx.py \\S+ +""") + +test.write('SConstruct', """ +import os + +def print_cmd_line(s, targets, sources, env): + pass + +env = Environment( + BUILDCOM = '${TEMPFILE("xxx.py $TARGET $SOURCES")}', + MAXLINELENGTH = 16, + TEMPFILESUFFIX = '.foo', + PRINT_CMD_LINE_FUNC=print_cmd_line +) +env.AppendENVPath('PATH', os.curdir) +env.Command('foo.out', 'foo.in', '$BUILDCOM') +""") + +test.run(arguments = '-n -Q .', + stdout = """""") + +test.write('SConstruct', """ +import os +from SCons.Platform import TempFileMunge + +class TestTempFileMunge(TempFileMunge): + + def __init__(self, cmd, cmdstr = None): + super(TestTempFileMunge, self).__init__(cmd, cmdstr) + + def _print_cmd_str(self, target, source, env, cmdstr): + super(TestTempFileMunge, self)._print_cmd_str(target, source, None, cmdstr) + +env = Environment( + TEMPFILE = TestTempFileMunge, + BUILDCOM = '${TEMPFILE("xxx.py $TARGET $SOURCES")}', + MAXLINELENGTH = 16, + TEMPFILESUFFIX = '.foo', + +) +env.AppendENVPath('PATH', os.curdir) +env.Command('foo.out', 'foo.in', '$BUILDCOM') +""") + +test.run(arguments = '-n -Q .', + stdout = """\ +Using tempfile \\S+ for command line: +xxx.py foo.out foo.in +xxx.py \\S+ +""") + +test.pass_test() + +# Local Variables: +# tab-width:4 +# indent-tabs-mode:nil +# End: +# vim: set expandtab tabstop=4 shiftwidth=4: -- cgit v0.12 From 79f4f1bddfc6495fc924e66928b9d06a6128cf1b Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Wed, 16 Jan 2019 16:58:27 -0700 Subject: Set defaults for TEMPFILE* differently Per review comments, subst either return the right thing or None, use this are the way to test for setting default instead of has_key. Signed-off-by: Mats Wichmann --- src/engine/SCons/Platform/__init__.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/engine/SCons/Platform/__init__.py b/src/engine/SCons/Platform/__init__.py index db5b947..ab064ce 100644 --- a/src/engine/SCons/Platform/__init__.py +++ b/src/engine/SCons/Platform/__init__.py @@ -205,12 +205,11 @@ class TempFileMunge(object): # We use the .lnk suffix for the benefit of the Phar Lap # linkloc linker, which likes to append an .lnk suffix if # none is given. - if env.has_key('TEMPFILESUFFIX'): - suffix = env.subst('$TEMPFILESUFFIX') - else: - suffix = '.lnk' # TODO: better way to pick default? - fd, tmp = tempfile.mkstemp(suffix, text=True) + suffix = env.subst('$TEMPFILESUFFIX') + if not suffix: + suffix = '.lnk' + fd, tmp = tempfile.mkstemp(suffix, text=True) native_tmp = SCons.Util.get_native_path(os.path.normpath(tmp)) if env.get('SHELL', None) == 'sh': @@ -226,9 +225,8 @@ class TempFileMunge(object): # Windows path names. rm = 'del' - if env.has_key('TEMPFILEPREFIX'): - prefix = env.subst('$TEMPFILEPREFIX') - else: + prefix = env.subst('$TEMPFILEPREFIX') + if not prefix: prefix = '@' args = list(map(SCons.Subst.quote_spaces, cmd[1:])) -- cgit v0.12 From 0ec7ba737006c4935726f59847add5782c7657b5 Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Thu, 17 Jan 2019 10:23:30 -0700 Subject: TEMPFILEPRFIX test changed back Testing "if not prefix" would return false if an empty string is passed, but that setting should be valid. Signed-off-by: Mats Wichmann --- src/engine/SCons/Platform/__init__.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/engine/SCons/Platform/__init__.py b/src/engine/SCons/Platform/__init__.py index ab064ce..3168378 100644 --- a/src/engine/SCons/Platform/__init__.py +++ b/src/engine/SCons/Platform/__init__.py @@ -194,7 +194,7 @@ class TempFileMunge(object): node = target[0] if SCons.Util.is_List(target) else target cmdlist = getattr(node.attributes, 'tempfile_cmdlist', None) \ if node is not None else None - if cmdlist is not None : + if cmdlist is not None: return cmdlist # We do a normpath because mktemp() has what appears to be @@ -202,11 +202,12 @@ class TempFileMunge(object): # delimiter. Windows' link mistakes that for a command line # switch and barfs. # - # We use the .lnk suffix for the benefit of the Phar Lap + # Default to the .lnk suffix for the benefit of the Phar Lap # linkloc linker, which likes to append an .lnk suffix if # none is given. - suffix = env.subst('$TEMPFILESUFFIX') - if not suffix: + if env.has_key('TEMPFILESUFFIX'): + suffix = env.subst('$TEMPFILESUFFIX') + else: suffix = '.lnk' fd, tmp = tempfile.mkstemp(suffix, text=True) -- cgit v0.12