Skip to content

Add consvar for TEMPFILE encoding - #12

Merged
bdbaddog merged 3 commits into
bdbaddog:fix_4746_TEMPFILE_ENC_ISSUEfrom
mwichmann:fix_4746_TEMPFILE_ENC_ISSUE
Aug 1, 2025
Merged

Add consvar for TEMPFILE encoding#12
bdbaddog merged 3 commits into
bdbaddog:fix_4746_TEMPFILE_ENC_ISSUEfrom
mwichmann:fix_4746_TEMPFILE_ENC_ISSUE

Conversation

@mwichmann

Copy link
Copy Markdown

Alternate approach to avoid decoding problems: don't try to guess how to encode the TEMPFILE. Leave it utf-8 unless TEMPFILEENCODING is set.

Add comment to TEMPFILEDIR doc on the topic as well.

Updates SCons#4749

Signed-off-by: Mats Wichmann mats@linux.com

bdbaddog and others added 2 commits July 20, 2025 20:49
….getpreferredencoding(False) instead of hardcoded utf-8
Alternate approach to avoid decoding problems: don't try to guess how
to encode the TEMPFILE.  Leave it utf-8 unless TEMPFILEENCODING is set.

Add comment to TEMPFILEDIR doc on the topic as well.

Signed-off-by: Mats Wichmann <mats@linux.com>
join_char = env.get('TEMPFILEARGJOIN', ' ')
os.write(fd, bytearray(join_char.join(args) + "\n", encoding="utf-8"))
encoding = env.get('TEMPFILEENCODING', 'utf-8')
os.write(fd, bytes(join_char.join(args) + "\n", encoding=encoding))

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

We're not going to change this - just immediately write it out - so there's no reason for it to be a bytearray.

tempfile_dir = None

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Seems wrong to open the file as text when we're explicitly going to write bytes. Risks unwanted conversion?

@bdbaddog
bdbaddog merged commit d068f13 into bdbaddog:fix_4746_TEMPFILE_ENC_ISSUE Aug 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants