Skip to content
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,11 @@ RELEASE VERSION/DATE TO BE FILLED IN LATER
tar bz2 and xz tests should be skipped on Windows 10 and GitHub
windows-2022. The packaging tar bz2 and xz tests should be run on
Windows 11 and GitHub windows-2025.
- TEMPFILE: Move encoding the tempfile contents before creating the
tempfile due to possible encoding errors. Ensure the tempfile file
handle is closed after writing the tempfile contents. Raise a
TempFileEncodeError exception when the tempfile contents encoding fails
for known exception types.

From William Deegan:
- Fix Issue #4746. TEMPFILE's are written with utf-8 encoding, In case
Expand Down
4 changes: 4 additions & 0 deletions RELEASE.txt
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ CHANGED/ENHANCED EXISTING FUNCTIONALITY
these variables and values are propagated to the user's SCons
environment after running the MSVC batch files.

- TEMPFILE: A TempFileEncodeError exception is raised when encoding
the contents of the tempfile fails due to a limited set of expected
exceptions (e.g., UnicodeError).

FIXES
-----

Expand Down
41 changes: 34 additions & 7 deletions SCons/Platform/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,16 @@
import SCons.Util


TEMPFILE_DEFAULT_ENCODING = "utf-8"


class TempFileEncodeError(SCons.Errors.UserError):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're not really consistent on whether to define Error classes near the point of use, or in Errors.py. That question doesn't have to be resolved here (i.e., consider it a question, not a blocker). The Configure subsystem and the MSVC tool certainly define their own; many are also centralized into Errors.py.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this particular case, it could just as easily be an instance of SCons.Errors.UserError with the message/reason prefixed with something along the lines tempfile encoding error: [UnicodeError] ... instead of a standalone exception. I'm not sure this is an exception a user is interested in catching as there really is no way to to "fix it".


def __init__(self, message):
message = f"{self.__class__.__name__}: {message!s}"
super().__init__(message)


def platform_default():
r"""Return the platform string for our execution environment.

Expand Down Expand Up @@ -246,6 +256,24 @@ def __call__(self, target, source, env, for_signature):
if cmdlist is not None:
return cmdlist

tempfile_esc_func = env.get('TEMPFILEARGESCFUNC', SCons.Subst.quote_spaces)
args = [tempfile_esc_func(arg) for arg in cmd[1:]]
join_char = env.get('TEMPFILEARGJOIN', ' ')
contents = join_char.join(args) + "\n"
encoding = env.get('TEMPFILEENCODING', TEMPFILE_DEFAULT_ENCODING)

try:
tempfile_contents = bytes(contents, encoding=encoding)
except (UnicodeError, LookupError, TypeError):
exc_type, exc_value, _ = sys.exc_info()
if 'TEMPFILEENCODING' in env:
encoding_msg = "env['TEMPFILEENCODING']"
else:
encoding_msg = "default"
err_msg = f"[{exc_type.__name__}] {exc_value!s}"
err_msg += f"\n {type(self).__name__} encoding: {encoding_msg} = {encoding!r}"
raise TempFileEncodeError(err_msg)

# 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.
Expand All @@ -262,6 +290,12 @@ def __call__(self, target, source, env, for_signature):

# default is binary - encode the tempfile contents later
fd, tmp = tempfile.mkstemp(suffix, dir=tempfile_dir)

try:
os.write(fd, tempfile_contents)
finally:
os.close(fd)

native_tmp = SCons.Util.get_native_path(tmp)

# arrange for cleanup on exit:
Expand All @@ -281,13 +315,6 @@ def tmpfile_cleanup(file) -> None:
else:
prefix = "@"

tempfile_esc_func = env.get('TEMPFILEARGESCFUNC', SCons.Subst.quote_spaces)
args = [tempfile_esc_func(arg) for arg in cmd[1:]]
join_char = env.get('TEMPFILEARGJOIN', ' ')
encoding = env.get('TEMPFILEENCODING', 'utf-8')
os.write(fd, bytes(join_char.join(args) + "\n", encoding=encoding))
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
Expand Down
129 changes: 129 additions & 0 deletions test/TEMPFILE/TEMPFILEENCODING.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
#!/usr/bin/env python
#
# MIT License
#
# Copyright The SCons Foundation
#
# 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.

"""
Verify that setting the $TEMPFILEENCODING variable will be used for
encoding the file contents when writing the generated tempfile used
for long command lines.
"""


import TestSCons

test = TestSCons.TestSCons()
Comment thread
bdbaddog marked this conversation as resolved.

SCONSTRUCT_TEMPLATE="""
import SCons.Platform

command = 'xyz ./test€/file.c'
encoding = {encoding!r}

tempfileencoding = {tempfileencoding}
defaultencoding = {defaultencoding}

DefaultEnvironment()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add tools=[] here? It will speed up the test by avoiding tool initializations..

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. That is a bug.


if defaultencoding:
SCons.Platform.TEMPFILE_DEFAULT_ENCODING = encoding
print("SCons.Platform.TEMPFILE_DEFAULT_ENCODING = {encoding!r}")

env = Environment(
tools=[],
MAXLINELENGTH=2,
)

if tempfileencoding:
env['TEMPFILEENCODING'] = encoding

tfm = SCons.Platform.TempFileMunge(command)

try:
tfm(None, None, env, 0)
except SCons.Platform.TempFileEncodeError as e:
print(str(e))
"""

expected_pass = """\
Using tempfile \\S+ for command line:
xyz \\S+
scons\:.*
"""

expected_fail = """\
TempFileEncodeError: \[{exception}\] .+
TempFileMunge encoding\: env\['TEMPFILEENCODING'\] = {encoding!r}
scons\:.*
"""

expected_pass_default = """\
SCons\.Platform\.TEMPFILE_DEFAULT_ENCODING = {encoding!r}
""" + expected_pass

expected_fail_default = """\
SCons\.Platform\.TEMPFILE_DEFAULT_ENCODING = {encoding!r}
TempFileEncodeError: \[{exception}\] .+
TempFileMunge encoding\: default = {encoding!r}
scons\:.*
"""

for test_encoding, test_tempfileencoding, test_defaultencoding, test_expected in [

# expected pass
('', False, False, expected_pass),
('utf-8', True, False, expected_pass),
('utf-8-sig', True, False, expected_pass),
('utf-16', True, False, expected_pass),
('utf-16', False, True, expected_pass_default.format(encoding='utf-16')),

# expected fail
('ascii', True, False, expected_fail.format(exception='UnicodeEncodeError', encoding='ascii')),
('missing', True, False, expected_fail.format(exception='LookupError', encoding='missing')),
('', True, False, expected_fail.format(exception='LookupError', encoding='')),
(None, True, False, expected_fail.format(exception='TypeError', encoding=None)),
('ascii', False, True, expected_fail_default.format(exception='UnicodeEncodeError', encoding='ascii')),

]:

sconstruct = SCONSTRUCT_TEMPLATE.format(
encoding = test_encoding,
tempfileencoding = test_tempfileencoding,
defaultencoding = test_defaultencoding,
)

test.write('SConstruct', sconstruct)

test.run(
arguments='-n -Q .',
stdout=test_expected,
match=TestSCons.match_re,
)

test.pass_test()

# Local Variables:
# tab-width:4
# indent-tabs-mode:nil
# End:
# vim: set expandtab tabstop=4 shiftwidth=4:
Loading